-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Receiver and SenderContext #3293
Conversation
} | ||
Object requestTag = request.tag(); | ||
if (requestTag instanceof Tags) { | ||
return tagsToKeyValues(((Tags) requestTag).stream()); | ||
} | ||
else if (requestTag instanceof KeyValues) { | ||
return (Iterable<KeyValue>) requestTag; |
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.
unchecked: unchecked cast
Reply with "@sonatype-lift help" for info about LiftBot commands.
Reply with "@sonatype-lift ignore" to tell LiftBot to leave out the above finding from this PR.
Reply with "@sonatype-lift ignoreall" to tell LiftBot to leave out all the findings from this PR and from the status bar in Github.
When talking to LiftBot, you need to refresh the page to see its response. Click here to get to know more about LiftBot commands.
Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
e1eba27
to
76a1fdd
Compare
cc @bclozel (that will be the context that you requested for - if you extend from it then propagation will happen out of the box) |
76a1fdd
to
0f34b04
Compare
micrometer-observation/src/main/java/io/micrometer/observation/transport/ReceiverContext.java
Show resolved
Hide resolved
...servation/src/main/java/io/micrometer/observation/transport/RequestReplyReceiverContext.java
Show resolved
Hide resolved
micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpContext.java
Show resolved
Hide resolved
With this change we are spiking complete removal of any http / messaging / rpc abstractions and leaving a ReceiverContext and RequestReply or FireAndForget SenderContext. That way we are able to propagate any content over the wire and we are not forcing any default tagging mechanism (other than the one to be backward compatible). It turned out that we're not properly supporting OkHttp instrumentation so I had to rewrite it to use the Interceptor approach not the EventListener one (which simplified a lot of things too). Thing to consider: do we even need the transport.http package - should we remove it too? Those extend the Observation.KeyValuesConvention and could be provided by external libraries.
e23e263
to
ff61e52
Compare
With this change we are spiking complete removal of any http / messaging / rpc abstractions and leaving a ReceiverContext and RequestReply or FireAndForget SenderContext. That way we are able to propagate any content over the wire and we are not forcing any default tagging mechanism (other than the one to be backward compatible).
It turned out that we're not properly supporting OkHttp instrumentation so I had to rewrite it to use the Interceptor approach not the EventListener one (which simplified a lot of things too).
Migration guide:
HttpRequest
&HttpResponse
abstraction + theHttpClientContext
andHttpServerContext
you'll need to use theRequestReplySenderContext
(for the client side) andRequestReplyReceiverContext
(for the server side)