-
Notifications
You must be signed in to change notification settings - Fork 650
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add BaggageContext property to ChannelHandlerContext #1574
base: main
Are you sure you want to change the base?
Conversation
Can one of the admins verify this patch? |
10 similar comments
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
@ktoso & @weissi This should now be a working version of what we discussed. I left out documentation until now if we want to move things around a bit first. For the tests I noticed that it'd be the first using the |
Sources/NIO/DeadChannel.swift
Outdated
/// A `DeadChannelCore` is a `ChannelCore` for a `DeadChannel`. A `DeadChannel` is used as a replacement `Channel` when | ||
/// the original `Channel` is closed. Given that the original `Channel` is closed the `DeadChannelCore` should fail | ||
/// all operations. | ||
private final class DeadChannelCore: ChannelCore { | ||
var baggage = BaggageContext() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure about usage of DeadChannelCore, but sounds like it may be safer to never store baggage values in here, there's no real purpose to propagate using a dead channel it feels like?
@swift-server-bot add to whitelist // yeah seems i dont have the superpowers here 😉 thought maybe it's CI-wide |
884697b
to
f604a10
Compare
f604a10
to
7a1fcb8
Compare
7a1fcb8
to
ad1bc15
Compare
BaggageContext
has matured and was accepted by the SSWG.Publicly expose a
BaggageContext
through theChannelHandlerContext
.Motivation:
As already discussed in slashmo/gsoc-swift-tracing#48, in order to share a
BaggageContext
between multiple handlers on the same channel, we want to store aBaggageContext
on aChannel
, which is publicly accessible through theChannelHandlerContext
.Modifications:
Result:
Users of
BaggageContext
and related APIs will be able to access aBaggageContext
through theChannelHandlerContext
. This has the side-effect of NIO now depending on slashmo/gsoc-swift-baggage-context.