-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
feat: Streamline sentry-trace
, baggage
and DSC handling
#14364
Changes from all commits
464d725
b31203e
07c8fcf
81d87ff
8bd3ef8
5201fc8
1608c4c
f6208e0
04227f2
a5f8c1e
12c9f45
e2de55d
d0c92b8
112ca89
c47f0ac
58bb2ac
ec4aa58
ad67471
7b423e8
7a557f4
e026314
7d5865e
0d5c32d
f8a5d82
a2e4c8f
d43ceae
b8432f9
5ae3b58
aa3846a
ae0e8ec
9f0b1ea
d5f08b6
a20fc43
c3963be
57bfd0e
80ecf6b
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 |
---|---|---|
|
@@ -9,24 +9,17 @@ import { | |
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, | ||
SentryNonRecordingSpan, | ||
getActiveSpan, | ||
getClient, | ||
getCurrentScope, | ||
getDynamicSamplingContextFromClient, | ||
getDynamicSamplingContextFromSpan, | ||
getIsolationScope, | ||
getTraceData, | ||
hasTracingEnabled, | ||
instrumentFetchRequest, | ||
setHttpStatus, | ||
spanToJSON, | ||
spanToTraceHeader, | ||
startInactiveSpan, | ||
} from '@sentry/core'; | ||
import { | ||
addFetchEndInstrumentationHandler, | ||
addFetchInstrumentationHandler, | ||
browserPerformanceTimeOrigin, | ||
dynamicSamplingContextToSentryBaggageHeader, | ||
generateSentryTraceHeader, | ||
parseUrl, | ||
stringMatchesSomePattern, | ||
} from '@sentry/core'; | ||
|
@@ -76,15 +69,17 @@ export interface RequestInstrumentationOptions { | |
* | ||
* Default: true | ||
*/ | ||
traceXHR: boolean /** | ||
traceXHR: boolean; | ||
|
||
/** | ||
* Flag to disable tracking of long-lived streams, like server-sent events (SSE) via fetch. | ||
* Do not enable this in case you have live streams or very long running streams. | ||
* | ||
* Disabled by default since it can lead to issues with streams using the `cancel()` api | ||
* (https://github.com/getsentry/sentry-javascript/issues/13950) | ||
* | ||
* Default: false | ||
*/; | ||
*/ | ||
trackFetchStreamPerformance: boolean; | ||
|
||
/** | ||
|
@@ -401,12 +396,9 @@ export function xhrCallback( | |
xhr.__sentry_xhr_span_id__ = span.spanContext().spanId; | ||
spans[xhr.__sentry_xhr_span_id__] = span; | ||
|
||
const client = getClient(); | ||
|
||
if (xhr.setRequestHeader && shouldAttachHeaders(sentryXhrData.url) && client) { | ||
if (shouldAttachHeaders(sentryXhrData.url)) { | ||
addTracingHeadersToXhrRequest( | ||
xhr, | ||
client, | ||
// If performance is disabled (TWP) or there's no active root span (pageload/navigation/interaction), | ||
// we do not want to use the span as base for the trace headers, | ||
// which means that the headers will be generated from the scope and the sampling decision is deferred | ||
|
@@ -417,22 +409,12 @@ export function xhrCallback( | |
return span; | ||
} | ||
|
||
function addTracingHeadersToXhrRequest(xhr: SentryWrappedXMLHttpRequest, client: Client, span?: Span): void { | ||
const scope = getCurrentScope(); | ||
const isolationScope = getIsolationScope(); | ||
const { traceId, spanId, sampled, dsc } = { | ||
...isolationScope.getPropagationContext(), | ||
...scope.getPropagationContext(), | ||
}; | ||
function addTracingHeadersToXhrRequest(xhr: SentryWrappedXMLHttpRequest, span?: Span): void { | ||
const { 'sentry-trace': sentryTrace, baggage } = getTraceData({ span }); | ||
|
||
const sentryTraceHeader = | ||
span && hasTracingEnabled() ? spanToTraceHeader(span) : generateSentryTraceHeader(traceId, spanId, sampled); | ||
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. Here, we have checked for 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. For browser, I think that's fine! |
||
|
||
const sentryBaggageHeader = dynamicSamplingContextToSentryBaggageHeader( | ||
dsc || (span ? getDynamicSamplingContextFromSpan(span) : getDynamicSamplingContextFromClient(traceId, client)), | ||
); | ||
|
||
setHeaderOnXhr(xhr, sentryTraceHeader, sentryBaggageHeader); | ||
if (sentryTrace) { | ||
setHeaderOnXhr(xhr, sentryTrace, baggage); | ||
} | ||
} | ||
|
||
function setHeaderOnXhr( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,23 +32,22 @@ import type { | |
} from '@sentry/types'; | ||
|
||
import { getEnvelopeEndpointWithUrlEncodedAuth } from './api'; | ||
import { getIsolationScope } from './currentScopes'; | ||
import { getCurrentScope, getIsolationScope, getTraceContextFromScope } from './currentScopes'; | ||
import { DEBUG_BUILD } from './debug-build'; | ||
import { createEventEnvelope, createSessionEnvelope } from './envelope'; | ||
import type { IntegrationIndex } from './integration'; | ||
import { afterSetupIntegrations } from './integration'; | ||
import { setupIntegration, setupIntegrations } from './integration'; | ||
import type { Scope } from './scope'; | ||
import { updateSession } from './session'; | ||
import { getDynamicSamplingContextFromClient } from './tracing/dynamicSamplingContext'; | ||
import { getDynamicSamplingContextFromScope } from './tracing/dynamicSamplingContext'; | ||
import { createClientReportEnvelope } from './utils-hoist/clientreport'; | ||
import { dsnToString, makeDsn } from './utils-hoist/dsn'; | ||
import { addItemToEnvelope, createAttachmentEnvelopeItem } from './utils-hoist/envelope'; | ||
import { SentryError } from './utils-hoist/error'; | ||
import { isParameterizedString, isPlainObject, isPrimitive, isThenable } from './utils-hoist/is'; | ||
import { consoleSandbox, logger } from './utils-hoist/logger'; | ||
import { checkOrSetAlreadyCaught, uuid4 } from './utils-hoist/misc'; | ||
import { dropUndefinedKeys } from './utils-hoist/object'; | ||
import { SyncPromise, rejectedSyncPromise, resolvedSyncPromise } from './utils-hoist/syncpromise'; | ||
import { parseSampleRate } from './utils/parseSampleRate'; | ||
import { prepareEvent } from './utils/prepareEvent'; | ||
|
@@ -672,7 +671,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> { | |
protected _prepareEvent( | ||
event: Event, | ||
hint: EventHint, | ||
currentScope?: Scope, | ||
currentScope = getCurrentScope(), | ||
isolationScope = getIsolationScope(), | ||
): PromiseLike<Event | null> { | ||
const options = this.getOptions(); | ||
|
@@ -692,30 +691,18 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> { | |
return evt; | ||
} | ||
|
||
const propagationContext = { | ||
...isolationScope.getPropagationContext(), | ||
...(currentScope ? currentScope.getPropagationContext() : undefined), | ||
evt.contexts = { | ||
trace: getTraceContextFromScope(currentScope), | ||
...evt.contexts, | ||
}; | ||
|
||
const trace = evt.contexts && evt.contexts.trace; | ||
if (!trace && propagationContext) { | ||
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. These checks were not really necessary, I believe - first, |
||
const { traceId: trace_id, spanId, parentSpanId, dsc } = propagationContext; | ||
evt.contexts = { | ||
trace: dropUndefinedKeys({ | ||
trace_id, | ||
span_id: spanId, | ||
parent_span_id: parentSpanId, | ||
}), | ||
...evt.contexts, | ||
}; | ||
const dynamicSamplingContext = getDynamicSamplingContextFromScope(this, currentScope); | ||
|
||
const dynamicSamplingContext = dsc ? dsc : getDynamicSamplingContextFromClient(trace_id, this); | ||
evt.sdkProcessingMetadata = { | ||
dynamicSamplingContext, | ||
...evt.sdkProcessingMetadata, | ||
}; | ||
|
||
evt.sdkProcessingMetadata = { | ||
dynamicSamplingContext, | ||
...evt.sdkProcessingMetadata, | ||
}; | ||
} | ||
return evt; | ||
}); | ||
} | ||
|
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.
kind of unrelated but this kept flaking every now and then, just fixing this here...