-
Notifications
You must be signed in to change notification settings - Fork 20
Introduce ForUserAgent #790
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
Conversation
tracing-api/src/main/java/com/palantir/tracing/api/TraceHttpHeaders.java
Outdated
Show resolved
Hide resolved
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.
This should also set for-user-agent in the undertow tracing handlers which are more common in our apps than jersey these days.
tracing-jersey/src/main/java/com/palantir/tracing/jersey/TraceEnrichingFilter.java
Outdated
Show resolved
Hide resolved
tracing-jersey/src/main/java/com/palantir/tracing/jersey/TraceEnrichingFilter.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Carter Kozak <[email protected]>
tracing-api/src/main/java/com/palantir/tracing/api/TracingHeadersEnrichingFunction.java
Show resolved
Hide resolved
Co-authored-by: Carter Kozak <[email protected]>
tracing-okhttp3/src/main/java/com/palantir/tracing/OkhttpTraceInterceptor2.java
Show resolved
Hide resolved
tracing-okhttp3/src/main/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptor.java
Outdated
Show resolved
Hide resolved
/** | ||
* Internal header names meant for consumption only inside of the tracing codebase. | ||
*/ | ||
public interface InternalTraceHttpHeaders { | ||
String FETCH_USER_AGENT = "Fetch-User-Agent"; | ||
} |
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.
This shouldn't be public if we can avoid it
tracing-api/src/main/java/com/palantir/tracing/api/TracingHeadersEnrichingFunction.java
Show resolved
Hide resolved
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.
lgtm!
🍿🤖 |
Ah, we'll need another +1 here since the autorelease RC has my name on it... |
👍 |
Released 6.5.0 |
Before this PR
When looking at the trace users needed to take a look at the whole trace to find out which service/FE-app started the request chain. With this PR the tracing library will propagate this information if set through the
ForUserAgent
header.After this PR
==COMMIT_MSG==
User agent specified inside
ForUserAgent
http header will be propagated to requests made in the same trace.==COMMIT_MSG==
Possible downsides?
Possibly slight perf degradation as there is one more value to send/receive for every http request.
Concerns
ForUserAgent
based on standard user agent or should it only be propagated only if explicitly set? Something like this