Skip to content

fix(node): Ensure incoming traces are propagated without HttpInstrumentation #15732

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

Merged
merged 1 commit into from
Mar 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 29 additions & 10 deletions packages/node/src/integrations/http/SentryHttpInstrumentation.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
/* eslint-disable max-lines */
import type * as http from 'node:http';
import type { IncomingMessage, RequestOptions } from 'node:http';
import type * as https from 'node:https';
import type { EventEmitter } from 'node:stream';
import { context, propagation } from '@opentelemetry/api';
import { VERSION } from '@opentelemetry/core';
import type { InstrumentationConfig } from '@opentelemetry/instrumentation';
import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation';
Expand All @@ -12,31 +9,44 @@ import {
generateSpanId,
getBreadcrumbLogLevelFromHttpStatusCode,
getClient,
getCurrentScope,
getIsolationScope,
getSanitizedUrlString,
httpRequestToRequestData,
logger,
parseUrl,
stripUrlQueryAndFragment,
withIsolationScope,
withScope,
} from '@sentry/core';
import type * as http from 'node:http';
import type { IncomingMessage, RequestOptions } from 'node:http';
import type * as https from 'node:https';
import type { EventEmitter } from 'node:stream';
import { DEBUG_BUILD } from '../../debug-build';
import { getRequestUrl } from '../../utils/getRequestUrl';
import { getRequestInfo } from './vendor/getRequestInfo';
import { stealthWrap } from './utils';
import { getRequestInfo } from './vendor/getRequestInfo';

type Http = typeof http;
type Https = typeof https;

type SentryHttpInstrumentationOptions = InstrumentationConfig & {
export type SentryHttpInstrumentationOptions = InstrumentationConfig & {
/**
* Whether breadcrumbs should be recorded for requests.
*
* @default `true`
*/
breadcrumbs?: boolean;

/**
* Whether to extract the trace ID from the `sentry-trace` header for incoming requests.
* By default this is done by the HttpInstrumentation, but if that is not added (e.g. because tracing is disabled, ...)
* then this instrumentation can take over.
*
* @default `false`
*/
extractIncomingTraceFromHeader?: boolean;

/**
* Do not capture breadcrumbs for outgoing HTTP requests to URLs where the given callback returns `true`.
* For the scope of this instrumentation, this callback only controls breadcrumb creation.
Expand Down Expand Up @@ -185,9 +195,18 @@ export class SentryHttpInstrumentation extends InstrumentationBase<SentryHttpIns
}

return withIsolationScope(isolationScope, () => {
return withScope(scope => {
// Set a new propagationSpanId for this request
scope.getPropagationContext().propagationSpanId = generateSpanId();
// Set a new propagationSpanId for this request
// We rely on the fact that `withIsolationScope()` will implicitly also fork the current scope
// This way we can save an "unnecessary" `withScope()` invocation
getCurrentScope().getPropagationContext().propagationSpanId = generateSpanId();

// If we don't want to extract the trace from the header, we can skip this
if (!instrumentation.getConfig().extractIncomingTraceFromHeader) {
return original.apply(this, [event, ...args]);
}

const ctx = propagation.extract(context.active(), normalizedRequest.headers);
return context.with(ctx, () => {
return original.apply(this, [event, ...args]);
});
});
Expand Down
27 changes: 13 additions & 14 deletions packages/node/src/integrations/http/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import type { HTTPModuleRequestIncomingMessage } from '../../transports/http-mod
import type { NodeClientOptions } from '../../types';
import { addOriginToSpan } from '../../utils/addOriginToSpan';
import { getRequestUrl } from '../../utils/getRequestUrl';
import type { SentryHttpInstrumentationOptions } from './SentryHttpInstrumentation';
import { SentryHttpInstrumentation } from './SentryHttpInstrumentation';
import { SentryHttpInstrumentationBeforeOtel } from './SentryHttpInstrumentationBeforeOtel';

Expand Down Expand Up @@ -102,19 +103,12 @@ const instrumentSentryHttpBeforeOtel = generateInstrumentOnce(`${INTEGRATION_NAM
return new SentryHttpInstrumentationBeforeOtel();
});

const instrumentSentryHttp = generateInstrumentOnce<{
breadcrumbs?: HttpOptions['breadcrumbs'];
ignoreOutgoingRequests?: HttpOptions['ignoreOutgoingRequests'];
trackIncomingRequestsAsSessions?: HttpOptions['trackIncomingRequestsAsSessions'];
sessionFlushingDelayMS?: HttpOptions['sessionFlushingDelayMS'];
}>(`${INTEGRATION_NAME}.sentry`, options => {
return new SentryHttpInstrumentation({
breadcrumbs: options?.breadcrumbs,
ignoreOutgoingRequests: options?.ignoreOutgoingRequests,
trackIncomingRequestsAsSessions: options?.trackIncomingRequestsAsSessions,
sessionFlushingDelayMS: options?.sessionFlushingDelayMS,
});
});
const instrumentSentryHttp = generateInstrumentOnce<SentryHttpInstrumentationOptions>(
`${INTEGRATION_NAME}.sentry`,
options => {
return new SentryHttpInstrumentation(options);
},
);

export const instrumentOtelHttp = generateInstrumentOnce<HttpInstrumentationConfig>(INTEGRATION_NAME, config => {
const instrumentation = new HttpInstrumentation(config);
Expand Down Expand Up @@ -161,7 +155,12 @@ export const httpIntegration = defineIntegration((options: HttpOptions = {}) =>
// This is the Sentry-specific instrumentation that isolates requests & creates breadcrumbs
// Note that this _has_ to be wrapped after the OTEL instrumentation,
// otherwise the isolation will not work correctly
instrumentSentryHttp(options);
instrumentSentryHttp({
...options,
// If spans are not instrumented, it means the HttpInstrumentation has not been added
// In that case, we want to handle incoming trace extraction ourselves
extractIncomingTraceFromHeader: !instrumentSpans,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

didn't think of that 🤯

});
},
};
});
Expand Down
Loading