-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Proposal to possibly fix @microlabs/otel-cf-workers conflics #17399
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
base: develop
Are you sure you want to change the base?
Changes from all commits
ddab6e0
53f3af1
7d7d3b9
0d67dbc
b2ac226
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,4 +1,4 @@ | ||
import type { Context, Span, SpanOptions, Tracer, TracerProvider } from '@opentelemetry/api'; | ||
import type { Context, ProxyTracerProvider, Span, SpanOptions, Tracer, TracerProvider } from '@opentelemetry/api'; | ||
import { trace } from '@opentelemetry/api'; | ||
import { startInactiveSpan, startSpanManual } from '@sentry/core'; | ||
|
||
|
@@ -7,16 +7,24 @@ import { startInactiveSpan, startSpanManual } from '@sentry/core'; | |
* This is not perfect but handles easy/common use cases. | ||
*/ | ||
export function setupOpenTelemetryTracer(): void { | ||
trace.setGlobalTracerProvider(new SentryCloudflareTraceProvider()); | ||
const result = trace.setGlobalTracerProvider(new SentryCloudflareTraceProvider()); | ||
if (result) { | ||
return; | ||
} | ||
const current = trace.getTracerProvider() as ProxyTracerProvider; | ||
current.setDelegate(new SentryCloudflareTraceProvider(current.getDelegate())); | ||
} | ||
|
||
class SentryCloudflareTraceProvider implements TracerProvider { | ||
private readonly _tracers: Map<string, Tracer> = new Map(); | ||
|
||
public constructor(private readonly _provider?: TracerProvider) {} | ||
|
||
public getTracer(name: string, version?: string, options?: { schemaUrl?: string }): Tracer { | ||
const key = `${name}@${version || ''}:${options?.schemaUrl || ''}`; | ||
if (!this._tracers.has(key)) { | ||
this._tracers.set(key, new SentryCloudflareTracer()); | ||
const tracer = this._provider?.getTracer?.(key, version, options); | ||
this._tracers.set(key, new SentryCloudflareTracer(tracer)); | ||
} | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion | ||
|
@@ -25,15 +33,56 @@ class SentryCloudflareTraceProvider implements TracerProvider { | |
} | ||
|
||
class SentryCloudflareTracer implements Tracer { | ||
public constructor(private readonly _tracer?: Tracer) {} | ||
public startSpan(name: string, options?: SpanOptions): Span { | ||
return startInactiveSpan({ | ||
const topSpan = this._tracer?.startSpan?.(name, options); | ||
const sentrySpan = startInactiveSpan({ | ||
name, | ||
...options, | ||
attributes: { | ||
...options?.attributes, | ||
'sentry.cloudflare_tracer': true, | ||
}, | ||
}); | ||
if (!topSpan) { | ||
return sentrySpan; | ||
} | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const _proxied = new WeakMap<CallableFunction, CallableFunction>(); | ||
return new Proxy(sentrySpan, { | ||
set: (target, p, newValue, receiver) => { | ||
try { | ||
Reflect.set(topSpan, p, newValue); | ||
} catch { | ||
// | ||
} | ||
return Reflect.set(target, p, newValue, receiver); | ||
}, | ||
get: (target, p) => { | ||
const propertyValue = Reflect.get(target, p); | ||
if (typeof propertyValue !== 'function') { | ||
return propertyValue; | ||
} | ||
const proxyTo = Reflect.get(topSpan, p); | ||
if (typeof proxyTo !== 'function') { | ||
return propertyValue; | ||
} | ||
if (_proxied.has(propertyValue)) { | ||
return _proxied.get(propertyValue); | ||
} | ||
const proxy = new Proxy(propertyValue, { | ||
apply: (target, thisArg, argArray) => { | ||
try { | ||
Reflect.apply(proxyTo, topSpan, argArray); | ||
} catch { | ||
// | ||
} | ||
return Reflect.apply(target, thisArg, argArray); | ||
}, | ||
}); | ||
_proxied.set(propertyValue, proxy); | ||
return proxy; | ||
}, | ||
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. Bug: Proxy Issues Cause Telemetry and Memory ProblemsThe |
||
}); | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
/** | ||
|
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.
Bug: Tracer Provider Verification Missing
The code assumes the existing global tracer provider is a
ProxyTracerProvider
and callssetDelegate
without verification. If the current provider doesn't implementsetDelegate
, this will cause a runtime error.