-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(core): Implement strictTraceContinuation
#16313
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
base: develop
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,8 @@ import { hasSpansEnabled } from '../utils/hasSpansEnabled'; | |
import { parseSampleRate } from '../utils/parseSampleRate'; | ||
import { _getSpanForScope, _setSpanForScope } from '../utils/spanOnScope'; | ||
import { addChildSpanToSpan, getRootSpan, spanIsSampled, spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils'; | ||
import { baggageHeaderToDynamicSamplingContext } from '../utils-hoist/baggage'; | ||
import { extractOrgIdFromDsnHost } from '../utils-hoist/dsn'; | ||
import { logger } from '../utils-hoist/logger'; | ||
import { generateTraceId } from '../utils-hoist/propagationContext'; | ||
import { propagationContextFromHeaders } from '../utils-hoist/tracing'; | ||
|
@@ -215,6 +217,47 @@ export const continueTrace = <V>( | |
} | ||
|
||
const { sentryTrace, baggage } = options; | ||
const client = getClient(); | ||
|
||
const clientOptions = client?.getOptions(); | ||
const strictTraceContinuation = clientOptions?.strictTraceContinuation || false; // default for `strictTraceContinuation` is `false` todo(v10): set default to `true` | ||
|
||
const incomingDsc = baggageHeaderToDynamicSamplingContext(baggage); | ||
const baggageOrgId = incomingDsc?.org_id; | ||
|
||
let sdkOrgId: string | undefined; | ||
const dsn = client?.getDsn(); | ||
if (dsn?.host) { | ||
sdkOrgId = extractOrgIdFromDsnHost(dsn.host); | ||
} | ||
|
||
const shouldStartNewTrace = (): boolean => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. l: Let's maybe make this a separate function that takes arguments instead of inlining this function? No strong feelings, but this is how we usually do this I suppose 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah no strong opinions about this. Just thought it's not a lot of logic and when inlining this it does not disrupt you from the code-reading flow but it still keeps this part of the code separated :D |
||
// Case: baggage org ID and SDK org ID don't match - always start new trace | ||
if (baggageOrgId && sdkOrgId && baggageOrgId !== sdkOrgId) { | ||
DEBUG_BUILD && | ||
logger.info(`Starting a new trace because org IDs don't match (incoming: ${baggageOrgId}, sdk: ${sdkOrgId})`); | ||
return true; | ||
} | ||
|
||
if (strictTraceContinuation) { | ||
// With strict continuation enabled, start new trace if: | ||
// - Baggage has org ID but SDK doesn't have one | ||
// - SDK has org ID but baggage doesn't have one | ||
if ((baggageOrgId && !sdkOrgId) || (!baggageOrgId && sdkOrgId)) { | ||
DEBUG_BUILD && | ||
logger.info( | ||
`Starting a new trace because strict trace continuation is enabled and one org ID is missing (incoming: ${baggageOrgId}, sdk: ${sdkOrgId})`, | ||
); | ||
return true; | ||
} | ||
} | ||
|
||
return false; | ||
}; | ||
|
||
if (shouldStartNewTrace()) { | ||
return startNewTrace(callback); | ||
} | ||
|
||
return withScope(scope => { | ||
const propagationContext = propagationContextFromHeaders(sentryTrace, baggage); | ||
|
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.
m: I think we should use the same logic here as for injecting the org id, which is use the specified one if defined, else extract it from dsn? We may also put this into a reusable utility, I suppose!