-
Notifications
You must be signed in to change notification settings - Fork 182
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
Make AsyncContext
visible from TransportObserver#onNewConnection()
#1303
Make AsyncContext
visible from TransportObserver#onNewConnection()
#1303
Conversation
Motivation: Users who want to correlate `TransportObserver#onNewConnection()` events with the request object may use `AsyncContext` to propagate correlation key. However, `AsyncContext` is not visible from `TransportObserver#onNewConnection()` because it runs from IO-thread. Modifications: - Trigger `TransportObserver#onNewConnection()` as soon as someone subscribes to `TcpConnector#connect` single on the client-side to get access to the caller's thread `AsyncContext`; - Trigger `TransportObserver#onNewConnection()` at the beginning of `initChannel` on the server-side for consistency with the client-side; - Update internal classes and tests to use new API; - Add tests to verify `AsyncContext` is visible for expected callbacks; Result: `AsyncContext` is visible from `TransportObserver#onNewConnection()` on the client-side.
|
||
@Override | ||
public SecurityHandshakeObserver onSecurityHandshake() { | ||
// AsyncContext is unknown at this point because this event is triggered by network |
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.
In some cases, ConnectionObserver
events are generated by network events when the request-related AsyncContext
is unknown. For those cases, users should capture the AsyncContext
state from other callbacks, keep it in local variable(s), and propagate when necessary. Examples:
ConnectionObserver#onSecurityHandshake()
and allSecurityHandshakeObserver
callbacks should use the state captured inTransportObserver#onNewConnection()
.ReadObserver
/WriteObserver
events should use the state captured inDataObserver
callbacks.StreamObserver#streamClosed
events should use the state captured inStreamObserver#streamEstablished()
orMultiplexedObserver#onNewStream()
.connectionClosed
events can be correlated with request(s) usingrequestId<->connectionId
correlation at the higher levels.
@Scottmitch we can cover more cases (for example, onSecurityHandshake
) using another technique if necessary. Because we have access to subscriber
at the time we invoke this callback, we can check if it's an instance of ContextPreservingSubscriber
and extract the AsyncContextMap
from it. However, it will require some movements for ContextPreserving*
classes. I will defer that until we have a use-case.
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.
agreed lets defer until necessary.
...ttp-netty/src/test/java/io/servicetalk/http/netty/HttpTransportObserverAsyncContextTest.java
Show resolved
Hide resolved
...ttp-netty/src/test/java/io/servicetalk/http/netty/HttpTransportObserverAsyncContextTest.java
Outdated
Show resolved
Hide resolved
|
||
@Override | ||
public SecurityHandshakeObserver onSecurityHandshake() { | ||
// AsyncContext is unknown at this point because this event is triggered by network |
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.
agreed lets defer until necessary.
Motivation:
Users who want to correlate
TransportObserver#onNewConnection()
events withthe request object may use
AsyncContext
to propagate correlation key.However,
AsyncContext
is not visible fromTransportObserver#onNewConnection()
because it runs from IO-thread. It's also important to invoke
TransportObserver#onNewConnection()
callback earlier on the client-side.Modifications:
TransportObserver#onNewConnection()
as soon as someone subscribes toTcpConnector#connect
single on the client-side to get access to the caller'sthread
AsyncContext
;TransportObserver#onNewConnection()
at the beginning ofinitChannel
on the server-side for consistency with the client-side;
AsyncContext
is visible for expected callbacks;Result:
AsyncContext
is visible fromTransportObserver#onNewConnection()
on theclient-side.