Tracing uniquely identifies requests #574
Conversation
Generate changelog in
|
|
needs more test coverage |
| public static final String TRACE_SAMPLED_KEY = "_sampled"; | ||
|
|
||
| /** The key under which tracing request ids are inserted into SLF4J {@link org.slf4j.MDC MDCs}. */ | ||
| public static final String REQUEST_ID_KEY = "_requestId"; |
There was a problem hiding this comment.
This will appear in every log line. Do we want to abbreviate it in some way? As written it's self-explanatory.
| /** | ||
| * Initializes the current thread's trace, erasing any previously accrued open spans. | ||
| * | ||
| * @deprecated Use {@link #initTraceWithSpan(Observability, String, String, SpanType)} |
There was a problem hiding this comment.
there seems to be relatively little usage of this method, with the majority of it being in tests
There was a problem hiding this comment.
Unfortunately many tests attempt to peel back layers of abstraction rather than testing results. These methods are only really used for server-incoming requests where we have prior knowledge of a request identifier.
There was a problem hiding this comment.
Sorry my comment was unclear, I was looking at internal usage across the fleet
There was a problem hiding this comment.
Ah, that makes sense 👍
| if (!requestId.isPresent()) { | ||
| throw new SafeIllegalStateException("No requestId is set", SafeArg.of("span", detachedSpan)); | ||
| } | ||
| exchange.putAttachment(TracingAttachments.REQUEST_ID, requestId.get()); |
There was a problem hiding this comment.
I think we should add a response header as well for easier debugging
There was a problem hiding this comment.
I'm not sure I'm ready to modify the wire api at this point, the traceid is more actionable in the majority of cases as an api consumer while the requestId gives us the ability to do stronger structured analysis.
There was a problem hiding this comment.
ok, we can add it later if needed
|
👍 looks great! |
|
Released 4.6.0 |
Before this PR
Many requests may share a single traceId, there was no identifying information to distinguish between requests.
After this PR
==COMMIT_MSG==
Tracing uniquely identifies requests
==COMMIT_MSG==
Possible downsides?