Skip to content

Commit

Permalink
ref(node): Avoid using propagator for node-fetch TWP instrumentation (#…
Browse files Browse the repository at this point in the history
…14509)

Instead, use the new and improved `getTraceData` directly, which avoids
URL issues etc. and just gets the trace data.

This also deprecates `generateSpanContextForPropagationContext` and
inlines it to the one place we still use this for, I'll probably adapt
this in a follow up PR.
  • Loading branch information
mydea authored Nov 28, 2024
1 parent 0ac1e10 commit e2883df
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 49 deletions.
4 changes: 4 additions & 0 deletions docs/migration/draft-v9-migration-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@

- Deprecated `autoInstrumentRemix: false`. The next major version will default to behaving as if this option were `true` and the option itself will be removed.

## `@sentry/opentelemetry`

- Deprecated `generateSpanContextForPropagationContext` in favor of doing this manually - we do not need this export anymore.

## Server-side SDKs (`@sentry/node` and all dependents)

- Deprecated `processThreadBreadcrumbIntegration` in favor of `childProcessIntegration`. Functionally they are the same.
Expand Down
38 changes: 9 additions & 29 deletions packages/node/src/integrations/node-fetch.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,9 @@
import { context, propagation, trace } from '@opentelemetry/api';
import type { UndiciRequest, UndiciResponse } from '@opentelemetry/instrumentation-undici';
import { UndiciInstrumentation } from '@opentelemetry/instrumentation-undici';
import {
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
addBreadcrumb,
defineIntegration,
getCurrentScope,
hasTracingEnabled,
} from '@sentry/core';
import { LRUMap, getClient, getTraceData } from '@sentry/core';
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, addBreadcrumb, defineIntegration, hasTracingEnabled } from '@sentry/core';
import { getBreadcrumbLogLevelFromHttpStatusCode, getSanitizedUrlString, parseUrl } from '@sentry/core';
import {
addOpenTelemetryInstrumentation,
generateSpanContextForPropagationContext,
getPropagationContextFromSpan,
} from '@sentry/opentelemetry';
import { addOpenTelemetryInstrumentation, shouldPropagateTraceForUrl } from '@sentry/opentelemetry';
import type { IntegrationFn, SanitizedRequestData } from '@sentry/types';

interface NodeFetchOptions {
Expand All @@ -37,6 +27,8 @@ const _nativeNodeFetchIntegration = ((options: NodeFetchOptions = {}) => {
return {
name: 'NodeFetch',
setupOnce() {
const propagationDecisionMap = new LRUMap<string, boolean>(100);

const instrumentation = new UndiciInstrumentation({
requireParentforSpans: false,
ignoreRequestHook: request => {
Expand All @@ -50,22 +42,10 @@ const _nativeNodeFetchIntegration = ((options: NodeFetchOptions = {}) => {
// If tracing is disabled, we still want to propagate traces
// So we do that manually here, matching what the instrumentation does otherwise
if (!hasTracingEnabled()) {
const ctx = context.active();
const addedHeaders: Record<string, string> = {};

// We generate a virtual span context from the active one,
// Where we attach the URL to the trace state, so the propagator can pick it up
const activeSpan = trace.getSpan(ctx);
const propagationContext = activeSpan
? getPropagationContextFromSpan(activeSpan)
: getCurrentScope().getPropagationContext();

const spanContext = generateSpanContextForPropagationContext(propagationContext);
// We know that in practice we'll _always_ haven a traceState here
spanContext.traceState = spanContext.traceState?.set('sentry.url', url);
const ctxWithUrlTraceState = trace.setSpanContext(ctx, spanContext);

propagation.inject(ctxWithUrlTraceState, addedHeaders);
const tracePropagationTargets = getClient()?.getOptions().tracePropagationTargets;
const addedHeaders = shouldPropagateTraceForUrl(url, tracePropagationTargets, propagationDecisionMap)
? getTraceData()
: {};

const requestHeaders = request.headers;
if (Array.isArray(requestHeaders)) {
Expand Down
7 changes: 6 additions & 1 deletion packages/opentelemetry/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export { getDynamicSamplingContextFromSpan } from '@sentry/core';
export { isSentryRequestSpan } from './utils/isSentryRequest';

export { enhanceDscWithOpenTelemetryRootSpanName } from './utils/enhanceDscWithOpenTelemetryRootSpanName';
// eslint-disable-next-line deprecation/deprecation
export { generateSpanContextForPropagationContext } from './utils/generateSpanContextForPropagationContext';

export { getActiveSpan } from './utils/getActiveSpan';
Expand All @@ -44,7 +45,11 @@ export { setupEventContextTrace } from './setupEventContextTrace';

export { setOpenTelemetryContextAsyncContextStrategy } from './asyncContextStrategy';
export { wrapContextManagerClass } from './contextManager';
export { SentryPropagator, getPropagationContextFromSpan } from './propagator';
export {
SentryPropagator,
getPropagationContextFromSpan,
shouldPropagateTraceForUrl,
} from './propagator';
export { SentrySpanProcessor } from './spanProcessor';
export {
SentrySampler,
Expand Down
69 changes: 50 additions & 19 deletions packages/opentelemetry/src/propagator.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { Baggage, Context, Span, TextMapGetter, TextMapSetter } from '@opentelemetry/api';
import type { Baggage, Context, Span, SpanContext, TextMapGetter, TextMapSetter } from '@opentelemetry/api';
import { TraceFlags } from '@opentelemetry/api';
import { INVALID_TRACEID } from '@opentelemetry/api';
import { context } from '@opentelemetry/api';
import { propagation, trace } from '@opentelemetry/api';
Expand Down Expand Up @@ -30,8 +31,8 @@ import {
} from './constants';
import { DEBUG_BUILD } from './debug-build';
import { getScopesFromContext, setScopesOnContext } from './utils/contextData';
import { generateSpanContextForPropagationContext } from './utils/generateSpanContextForPropagationContext';
import { getSamplingDecision } from './utils/getSamplingDecision';
import { makeTraceState } from './utils/makeTraceState';
import { setIsSetup } from './utils/setupCheck';

/** Get the Sentry propagation context from a span context. */
Expand Down Expand Up @@ -88,11 +89,7 @@ export class SentryPropagator extends W3CBaggagePropagator {
const url = activeSpan && getCurrentURL(activeSpan);

const tracePropagationTargets = getClient()?.getOptions()?.tracePropagationTargets;
if (
typeof url === 'string' &&
tracePropagationTargets &&
!this._shouldInjectTraceData(tracePropagationTargets, url)
) {
if (!shouldPropagateTraceForUrl(url, tracePropagationTargets, this._urlMatchesTargetsMap)) {
DEBUG_BUILD &&
logger.log(
'[Tracing] Not injecting trace data for url because it does not match tracePropagationTargets:',
Expand Down Expand Up @@ -168,22 +165,36 @@ export class SentryPropagator extends W3CBaggagePropagator {
public fields(): string[] {
return [SENTRY_TRACE_HEADER, SENTRY_BAGGAGE_HEADER];
}
}

/** If we want to inject trace data for a given URL. */
private _shouldInjectTraceData(tracePropagationTargets: Options['tracePropagationTargets'], url: string): boolean {
if (tracePropagationTargets === undefined) {
return true;
}
const NOT_PROPAGATED_MESSAGE =
'[Tracing] Not injecting trace data for url because it does not match tracePropagationTargets:';

const cachedDecision = this._urlMatchesTargetsMap.get(url);
if (cachedDecision !== undefined) {
return cachedDecision;
}
/**
* Check if a given URL should be propagated to or not.
* If no url is defined, or no trace propagation targets are defined, this will always return `true`.
* You can also optionally provide a decision map, to cache decisions and avoid repeated regex lookups.
*/
export function shouldPropagateTraceForUrl(
url: string | undefined,
tracePropagationTargets: Options['tracePropagationTargets'],
decisionMap?: LRUMap<string, boolean>,
): boolean {
if (typeof url !== 'string' || !tracePropagationTargets) {
return true;
}

const decision = stringMatchesSomePattern(url, tracePropagationTargets);
this._urlMatchesTargetsMap.set(url, decision);
return decision;
const cachedDecision = decisionMap?.get(url);
if (cachedDecision !== undefined) {
DEBUG_BUILD && !cachedDecision && logger.log(NOT_PROPAGATED_MESSAGE, url);
return cachedDecision;
}

const decision = stringMatchesSomePattern(url, tracePropagationTargets);
decisionMap?.set(url, decision);

DEBUG_BUILD && !decision && logger.log(NOT_PROPAGATED_MESSAGE, url);
return decision;
}

/**
Expand Down Expand Up @@ -287,3 +298,23 @@ function getCurrentURL(span: Span): string | undefined {

return undefined;
}

// TODO: Adjust this behavior to avoid invalid spans
function generateSpanContextForPropagationContext(propagationContext: PropagationContext): SpanContext {
// We store the DSC as OTEL trace state on the span context
const traceState = makeTraceState({
parentSpanId: propagationContext.parentSpanId,
dsc: propagationContext.dsc,
sampled: propagationContext.sampled,
});

const spanContext: SpanContext = {
traceId: propagationContext.traceId,
spanId: propagationContext.parentSpanId || '',
isRemote: true,
traceFlags: propagationContext.sampled ? TraceFlags.SAMPLED : TraceFlags.NONE,
traceState,
};

return spanContext;
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import { makeTraceState } from './makeTraceState';
/**
* Generates a SpanContext that represents a PropagationContext.
* This can be set on a `context` to make this a (virtual) active span.
*
* @deprecated This function is deprecated and will be removed in the next major version.
*/
export function generateSpanContextForPropagationContext(propagationContext: PropagationContext): SpanContext {
// We store the DSC as OTEL trace state on the span context
Expand Down

0 comments on commit e2883df

Please sign in to comment.