fix!: always assign a trace ID to each request#1033
Conversation
|
Actually, marking this as WIP (don't review) while I figure out what to do when we call |
Codecov Report
@@ Coverage Diff @@
## master #1033 +/- ##
==========================================
- Coverage 94.87% 94.85% -0.03%
==========================================
Files 96 96
Lines 6288 6311 +23
Branches 494 496 +2
==========================================
+ Hits 5966 5986 +20
- Misses 165 166 +1
- Partials 157 159 +2
Continue to review full report at Codecov.
|
| * created because the Trace Agent was disabled. | ||
| */ | ||
| export const UNTRACED_CHILD_SPAN = createPhantomSpanData(SpanType.UNTRACED); | ||
| export const DISABLED_CHILD_SPAN = createPhantomSpanData(SpanType.UNTRACED); |
There was a problem hiding this comment.
This is going to sound nit-picky...
Is there a reason why the concept was renamed on the left hand side but left the same on the right hand side?
Based on your decision to rename the export, it seems that you prefer external users to conceptually think about this concept as disabled. If that is the right conceptualization, why is not the right conceptualization for the internals in this file?
There was a problem hiding this comment.
The variable formerly named UNTRACED_CHILD_SPAN is now only used when the Trace Agent is disabled which is why the LHS changed (where as before it is used for a disabled agent or for unsampled traces). As for the RHS, there continues to be multiple reasons for a span object to represent "untraced" work -- either unsampled, or disabled.
I could get behind the idea that we could just introduce UNSAMPLED and DISABLED in place of UNTRACED, though.
There was a problem hiding this comment.
+1 on the rename. I won't block this PR, but conceptual clarity is important.
| * created because the Trace Agent was disabled. | ||
| */ | ||
| export const UNTRACED_CHILD_SPAN = createPhantomSpanData(SpanType.UNTRACED); | ||
| export const DISABLED_CHILD_SPAN = createPhantomSpanData(SpanType.UNTRACED); |
There was a problem hiding this comment.
+1 on the rename. I won't block this PR, but conceptual clarity is important.
2023925 to
bedc655
Compare
Fixes #1025
We already store untraced requests (via a single object representing ALL untraced requests) in CLS, this changes it so that these objects also have a trace ID (and span ID, though not sure if it is useful).