-
-
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(node): Use @opentelemetry/instrumentation-undici
for fetch tracing
#13485
Changes from all commits
dbd4ed8
eb0a361
15b856c
f94e183
7342e61
5d2d0f2
4fa38f3
4368628
6b3adb3
2e621b2
0db9814
df01350
cc873d8
033a154
7545513
88728a1
0cb3322
4a73857
e8e2198
8bb6c75
049d328
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 |
---|---|---|
@@ -1,32 +1,9 @@ | ||
import type { Span } from '@opentelemetry/api'; | ||
import { trace } from '@opentelemetry/api'; | ||
import { context, propagation } from '@opentelemetry/api'; | ||
import { addBreadcrumb, defineIntegration, getCurrentScope, hasTracingEnabled } from '@sentry/core'; | ||
import { | ||
addOpenTelemetryInstrumentation, | ||
generateSpanContextForPropagationContext, | ||
getPropagationContextFromSpan, | ||
} from '@sentry/opentelemetry'; | ||
import type { UndiciRequest, UndiciResponse } from '@opentelemetry/instrumentation-undici'; | ||
import { UndiciInstrumentation } from '@opentelemetry/instrumentation-undici'; | ||
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, addBreadcrumb, defineIntegration } from '@sentry/core'; | ||
import { addOpenTelemetryInstrumentation } from '@sentry/opentelemetry'; | ||
import type { IntegrationFn, SanitizedRequestData } from '@sentry/types'; | ||
import { getSanitizedUrlString, logger, parseUrl } from '@sentry/utils'; | ||
import { DEBUG_BUILD } from '../debug-build'; | ||
import { NODE_MAJOR } from '../nodeVersion'; | ||
|
||
import type { FetchInstrumentation } from 'opentelemetry-instrumentation-fetch-node'; | ||
|
||
import { addOriginToSpan } from '../utils/addOriginToSpan'; | ||
|
||
interface FetchRequest { | ||
method: string; | ||
origin: string; | ||
path: string; | ||
headers: string | string[]; | ||
} | ||
|
||
interface FetchResponse { | ||
headers: Buffer[]; | ||
statusCode: number; | ||
} | ||
import { getSanitizedUrlString, parseUrl } from '@sentry/utils'; | ||
|
||
interface NodeFetchOptions { | ||
/** | ||
|
@@ -46,106 +23,38 @@ const _nativeNodeFetchIntegration = ((options: NodeFetchOptions = {}) => { | |
const _breadcrumbs = typeof options.breadcrumbs === 'undefined' ? true : options.breadcrumbs; | ||
const _ignoreOutgoingRequests = options.ignoreOutgoingRequests; | ||
|
||
async function getInstrumentation(): Promise<FetchInstrumentation | void> { | ||
// Only add NodeFetch if Node >= 18, as previous versions do not support it | ||
if (NODE_MAJOR < 18) { | ||
DEBUG_BUILD && logger.log('NodeFetch is not supported on Node < 18, skipping instrumentation...'); | ||
return; | ||
timfish marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
try { | ||
const pkg = await import('opentelemetry-instrumentation-fetch-node'); | ||
const { FetchInstrumentation } = pkg; | ||
|
||
class SentryNodeFetchInstrumentation extends FetchInstrumentation { | ||
// We extend this method so we have access to request _and_ response for the breadcrumb | ||
public onHeaders({ request, response }: { request: FetchRequest; response: FetchResponse }): void { | ||
if (_breadcrumbs) { | ||
_addRequestBreadcrumb(request, response); | ||
} | ||
|
||
return super.onHeaders({ request, response }); | ||
} | ||
} | ||
|
||
return new SentryNodeFetchInstrumentation({ | ||
ignoreRequestHook: (request: FetchRequest) => { | ||
return { | ||
name: 'NodeFetch', | ||
setupOnce() { | ||
const instrumentation = new UndiciInstrumentation({ | ||
requireParentforSpans: false, | ||
ignoreRequestHook: request => { | ||
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. is this a typo in our config or upstream, seems like it should be 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, thought the same but this is incorrect/different upstream! |
||
const url = getAbsoluteUrl(request.origin, request.path); | ||
const tracingDisabled = !hasTracingEnabled(); | ||
const shouldIgnore = _ignoreOutgoingRequests && url && _ignoreOutgoingRequests(url); | ||
|
||
if (shouldIgnore) { | ||
return true; | ||
} | ||
|
||
// If tracing is disabled, we still want to propagate traces | ||
// So we do that manually here, matching what the instrumentation does otherwise | ||
if (tracingDisabled) { | ||
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 requestHeaders = request.headers; | ||
if (Array.isArray(requestHeaders)) { | ||
Object.entries(addedHeaders).forEach(headers => requestHeaders.push(...headers)); | ||
} else { | ||
request.headers += Object.entries(addedHeaders) | ||
.map(([k, v]) => `${k}: ${v}\r\n`) | ||
.join(''); | ||
} | ||
|
||
// Prevent starting a span for this request | ||
return true; | ||
} | ||
|
||
return false; | ||
return !!shouldIgnore; | ||
}, | ||
onRequest: ({ span }: { span: Span }) => { | ||
_updateSpan(span); | ||
startSpanHook: () => { | ||
return { | ||
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.node_fetch', | ||
}; | ||
}, | ||
responseHook: (_, { request, response }) => { | ||
if (_breadcrumbs) { | ||
addRequestBreadcrumb(request, response); | ||
} | ||
}, | ||
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
} as any); | ||
} catch (error) { | ||
// Could not load instrumentation | ||
DEBUG_BUILD && logger.log('Error while loading NodeFetch instrumentation: \n', error); | ||
} | ||
} | ||
|
||
return { | ||
name: 'NodeFetch', | ||
setupOnce() { | ||
// eslint-disable-next-line @typescript-eslint/no-floating-promises | ||
getInstrumentation().then(instrumentation => { | ||
if (instrumentation) { | ||
addOpenTelemetryInstrumentation(instrumentation); | ||
} | ||
}); | ||
|
||
addOpenTelemetryInstrumentation(instrumentation); | ||
}, | ||
}; | ||
}) satisfies IntegrationFn; | ||
|
||
export const nativeNodeFetchIntegration = defineIntegration(_nativeNodeFetchIntegration); | ||
|
||
/** Update the span with data we need. */ | ||
function _updateSpan(span: Span): void { | ||
addOriginToSpan(span, 'auto.http.otel.node_fetch'); | ||
} | ||
|
||
/** Add a breadcrumb for outgoing requests. */ | ||
function _addRequestBreadcrumb(request: FetchRequest, response: FetchResponse): void { | ||
function addRequestBreadcrumb(request: UndiciRequest, response: UndiciResponse): void { | ||
const data = getBreadcrumbData(request); | ||
|
||
addBreadcrumb( | ||
|
@@ -165,7 +74,7 @@ function _addRequestBreadcrumb(request: FetchRequest, response: FetchResponse): | |
); | ||
} | ||
|
||
function getBreadcrumbData(request: FetchRequest): Partial<SanitizedRequestData> { | ||
function getBreadcrumbData(request: UndiciRequest): Partial<SanitizedRequestData> { | ||
try { | ||
const url = new URL(request.path, request.origin); | ||
const parsedUrl = parseUrl(url.toString()); | ||
|
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.
I replaced these in a couple of places since I just added temporary constants for them