Skip to content

ref(node): Refactor some otel internals in http integrations #22292

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.

Sign up for GitHub

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
mydea merged 2 commits into from
Jul 15, 2026
+73 −50
Merged

ref(node): Refactor some otel internals in http integrations #22292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6ca2a3b
ref(node): Refactor some otel internals in http integrations
mydea Jul 15, 2026
2e9ba2e
ref
mydea Jul 15, 2026
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
2 changes: 2 additions & 0 deletions packages/core/src/integrations/http/client-subscriptions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import type { SpanStatus } from '../../types/spanStatus';
import { addOutgoingRequestBreadcrumb } from './add-outgoing-request-breadcrumb';
import {
bindScopeToEmitter,
getSpanStatusFromHttpCode,
SPAN_STATUS_ERROR,
SPAN_STATUS_UNSET,
Expand Down Expand Up @@ -156,6 +157,7 @@ export function getHttpClientSubscriptions(options: HttpInstrumentationOptions):
response.resume();
}
setIncomingResponseSpanData(response, span);
bindScopeToEmitter(response);

@cursor cursor Bot Jul 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Response binding omits active span

Medium Severity

Outgoing HTTP responses call bindScopeToEmitter without first activating the outgoing http.client span. bindScopeToEmitter freezes whatever scope is current at bind time, but that span is created via startInactiveSpan, so listeners on the response stream later run without that span active and child spans can attach to the wrong parent.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 2e9ba2e. Configure here.

@mydea mydea Jul 15, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this is fine and also how it was before, the emitter should not have the span as parent as that could break parent-child relationships 🤔

options.outgoingResponseHook?.(span, response);

let finished = false;
Expand Down
1 change: 0 additions & 1 deletion packages/node-core/src/integrations/http/SentryHttpInstrumentation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ export function instrumentHttpOutgoingRequests(
},
outgoingResponseHook(span, response) {
options.outgoingResponseHook?.(span, response);
context.bind(context.active(), response);
},
errorMonitor,
// Pass these in to detect OTel double-wrapping if we're enabling spans
Expand Down
120 changes: 71 additions & 49 deletions packages/node-core/src/integrations/http/httpServerSpansIntegration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,28 @@
/* eslint-disable max-lines */
import { errorMonitor } from 'node:events';
import type { IncomingHttpHeaders } from 'node:http';
import { context, SpanKind, trace } from '@opentelemetry/api';
import { context } from '@opentelemetry/api';
import type { RPCMetadata } from '@opentelemetry/core';
import { RPCType, setRPCMetadata } from '@opentelemetry/core';
import {
HTTP_CLIENT_IP,
HTTP_FLAVOR,
HTTP_HOST,
HTTP_METHOD,
HTTP_RESPONSE_STATUS_CODE,
HTTP_ROUTE,
HTTP_SCHEME,
HTTP_STATUS_CODE,
HTTP_TARGET,
HTTP_URL,
HTTP_USER_AGENT,
NET_HOST_IP,
NET_HOST_NAME,
NET_HOST_PORT,
NET_PEER_IP,
NET_PEER_PORT,
NET_TRANSPORT,
SENTRY_HTTP_PREFETCH,
} from '@sentry/conventions/attributes';
import type {
Event,
Expand All @@ -33,6 +46,10 @@ import {
SPAN_STATUS_ERROR,
stripUrlQueryAndFragment,
isTracingSuppressed,
bindScopeToEmitter,
startInactiveSpan,
withActiveSpan,
SPAN_KIND,
} from '@sentry/core';
import { DEBUG_BUILD } from '../../debug-build';
import type { NodeClient } from '../../sdk/client';
Expand Down Expand Up @@ -141,32 +158,33 @@ const _httpServerSpansIntegration = ((options: HttpServerSpansIntegrationOptions
const host = headers.host as string | undefined;
const hostname = host?.replace(/^(.*)(:[0-9]{1,5})/, '$1') || 'localhost';

const tracer = client.tracer;
const scheme = fullUrl.startsWith('https') ? 'https' : 'http';

const method = normalizedRequest.method || request.method?.toUpperCase() || 'GET';
const httpTargetWithoutQueryFragment = urlObj ? urlObj.pathname : stripUrlQueryAndFragment(fullUrl);
const bestEffortTransactionName = `${method} ${httpTargetWithoutQueryFragment}`;

// We use the plain tracer.startSpan here so we can pass the span kind
const span = tracer.startSpan(bestEffortTransactionName, {
kind: SpanKind.SERVER,
const span = startInactiveSpan({
name: bestEffortTransactionName,
kind: SPAN_KIND.SERVER,
attributes: {
// Sentry specific attributes
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.http',
'sentry.http.prefetch': isKnownPrefetchRequest(request) || undefined,
[SENTRY_HTTP_PREFETCH]: isKnownPrefetchRequest(request) || undefined,
// Old Semantic Conventions attributes - added for compatibility with what `@opentelemetry/instrumentation-http` output before
'http.url': fullUrl,
'http.method': normalizedRequest.method,
'http.target': urlObj ? `${urlObj.pathname}${urlObj.search}` : httpTargetWithoutQueryFragment,
'http.host': host,
'net.host.name': hostname,
'http.client_ip': typeof ips === 'string' ? ips.split(',')[0] : undefined,
'http.user_agent': userAgent,
'http.scheme': scheme,
'http.flavor': httpVersion,
'net.transport': httpVersion?.toUpperCase() === 'QUIC' ? 'ip_udp' : 'ip_tcp',
/* eslint-disable typescript/no-deprecated */
[HTTP_URL]: fullUrl,
[HTTP_METHOD]: normalizedRequest.method,
[HTTP_TARGET]: urlObj ? `${urlObj.pathname}${urlObj.search}` : httpTargetWithoutQueryFragment,
[HTTP_HOST]: host,
[NET_HOST_NAME]: hostname,
[HTTP_CLIENT_IP]: typeof ips === 'string' ? ips.split(',')[0] : undefined,
[HTTP_USER_AGENT]: userAgent,
[HTTP_SCHEME]: scheme,
[HTTP_FLAVOR]: httpVersion,
[NET_TRANSPORT]: httpVersion?.toUpperCase() === 'QUIC' ? 'ip_udp' : 'ip_tcp',
/* eslint-enable typescript/no-deprecated */
...getRequestContentLengthAttribute(request),
...httpHeadersToSpanAttributes(normalizedRequest.headers || {}, client.getDataCollectionOptions()),
},
Expand All @@ -180,42 +198,45 @@ const _httpServerSpansIntegration = ((options: HttpServerSpansIntegrationOptions

const rpcMetadata: RPCMetadata = { type: RPCType.HTTP, span };

return context.with(setRPCMetadata(trace.setSpan(context.active(), span), rpcMetadata), () => {
context.bind(context.active(), request);
context.bind(context.active(), response);

// Ensure we only end the span once
// E.g. error can be emitted before close is emitted
let isEnded = false;
function endSpan(status: SpanStatus): void {
if (isEnded) {
return;
return withActiveSpan(span, () => {
// TODO(v11): Get rid of RPC metadata here
return context.with(setRPCMetadata(context.active(), rpcMetadata), () => {
bindScopeToEmitter(request);
bindScopeToEmitter(response);

// Ensure we only end the span once
// E.g. error can be emitted before close is emitted
let isEnded = false;
function endSpan(status: SpanStatus): void {
if (isEnded) {
return;
}

isEnded = true;

const newAttributes = getIncomingRequestAttributesOnResponse(request, response, rpcMetadata);
span.setAttributes(newAttributes);
span.setStatus(status);
span.end();

// Update the transaction name if the route has changed
const route = newAttributes['http.route'];
if (route) {
getIsolationScope().setTransactionName(`${request.method?.toUpperCase() || 'GET'} ${route}`);
}
}

isEnded = true;

const newAttributes = getIncomingRequestAttributesOnResponse(request, response, rpcMetadata);
span.setAttributes(newAttributes);
span.setStatus(status);
span.end();

// Update the transaction name if the route has changed
const route = newAttributes['http.route'];
if (route) {
getIsolationScope().setTransactionName(`${request.method?.toUpperCase() || 'GET'} ${route}`);
}
}
response.on('close', () => {
endSpan(getSpanStatusFromHttpCode(response.statusCode));
});
response.on(errorMonitor, () => {
const httpStatus = getSpanStatusFromHttpCode(response.statusCode);
// Ensure we def. have an error status here
endSpan(httpStatus.code === SPAN_STATUS_ERROR ? httpStatus : { code: SPAN_STATUS_ERROR });
});

response.on('close', () => {
endSpan(getSpanStatusFromHttpCode(response.statusCode));
return next();
});
response.on(errorMonitor, () => {
const httpStatus = getSpanStatusFromHttpCode(response.statusCode);
// Ensure we def. have an error status here
endSpan(httpStatus.code === SPAN_STATUS_ERROR ? httpStatus : { code: SPAN_STATUS_ERROR });
});

return next();
});
};

Expand Down Expand Up @@ -398,7 +419,8 @@ function getIncomingRequestAttributesOnResponse(
newAttributes[NET_HOST_PORT] = localPort;
// eslint-disable-next-line typescript/no-deprecated
newAttributes[NET_PEER_IP] = remoteAddress;
newAttributes['net.peer.port'] = remotePort;
// oxlint-disable-next-line typescript/no-deprecated
newAttributes[NET_PEER_PORT] = remotePort;
}
// eslint-disable-next-line typescript/no-deprecated
newAttributes[HTTP_STATUS_CODE] = statusCode;
Expand Down
Loading