fix(webapp): stop api inheriting inbound sampled traceparents so trace sampling applies (#4532)
## What The internal tracing `ParentBasedSampler` in `tracer.server.ts` left `remoteParentSampled` at its default of `AlwaysOn`. Any request arriving with a `traceparent` whose sampled flag was set got recorded in full, bypassing `INTERNAL_OTEL_TRACE_SAMPLING_RATE` entirely. Because the SDK propagates its (always-sampled) trace context on calls back to the platform from inside running tasks, the large majority of API server spans inherited a sampled parent and ignored the divisor. The sampling knob was effectively inert on the busiest service. This registers a custom propagator (`NonInheritingTraceContextPropagator`) that stops adopting the inbound trace as the parent: - `inject` still delegates to the standard W3C trace + baggage propagators, so outbound propagation is unchanged. - `extract` drops the parent span (`trace.deleteSpan`) while preserving baggage, so every incoming request roots its own trace and the ratio sampler applies uniformly. `remoteParentSampled` is also set to the ratio sampler as a belt-and-suspenders fallback, in case an inbound sampled parent ever reaches the sampler another way. Two effects: the divisor becomes effective on the API server, and the API no longer stitches onto (and inflates) the propagated task-run traces, which is where the very large, un-thinnable trace chains came from. Rooting each request removes those chains rather than only diluting them. Only the internal APM trace pipeline (`INTERNAL_OTEL_TRACE_EXPORTER_URL`) is affected. The user-facing run-trace pipeline (`otel.v1.traces` -> ClickHouse) is a separate path and is untouched. The only consumer of the global propagator's `extract` is the OTel HTTP/Express auto-instrumentation, so the blast radius is inbound-request trace shape. ## Evidence (local full-stack red/green, divisor 10) A local OTLP/JSON sink counting spans; a driver fires N requests at a real endpoint, each carrying a distinct sampled `traceparent`, then counts how many spans/traces carry that run's marker. | run | code | sent | kept traces | kept fraction | | --- | --- | --- | --- | --- | | before | unmodified | 500 | 500 | 1.00 | | after | this PR | 500 | 67 | 0.134 | | after | this PR | 2000 | 213 | 0.1065 | Before: 100% of inherited-sampled requests kept, divisor ignored. After: ~10% kept (the divisor), converging on it at larger N. In every after-run each kept request is a single self-rooted trace (kept spans == kept distinct traces), confirming the inherited chains are gone, not just thinned. `typecheck` passes. ## Rollout / rollback No flag. Behavior stays governed by the existing `INTERNAL_OTEL_TRACE_SAMPLING_RATE`. Rollback is a straight revert with no data migration. ## Notes Internal dashboards that count raw span or request volume from this pipeline will read lower once this ships. That is expected: those counts were inflated by the bypass, not a real drop in traffic. Latency/percentile monitors retain plenty of samples at the current divisor. refs TRI-13031
This commit is contained in:
@@ -0,0 +1,6 @@
|
||||
---
|
||||
area: webapp
|
||||
type: improvement
|
||||
---
|
||||
|
||||
Reduced internal overhead on the API under high load.
|
||||
@@ -14,7 +14,15 @@ import {
|
||||
trace,
|
||||
metrics,
|
||||
type Meter,
|
||||
type TextMapPropagator,
|
||||
type TextMapGetter,
|
||||
type TextMapSetter,
|
||||
} from "@opentelemetry/api";
|
||||
import {
|
||||
CompositePropagator,
|
||||
W3CBaggagePropagator,
|
||||
W3CTraceContextPropagator,
|
||||
} from "@opentelemetry/core";
|
||||
import sentryRemix from "@sentry/remix";
|
||||
import { logs, SeverityNumber } from "@opentelemetry/api-logs";
|
||||
import { OTLPTraceExporter } from "@opentelemetry/exporter-trace-otlp-http";
|
||||
@@ -125,6 +133,24 @@ class CustomWebappSampler implements Sampler {
|
||||
}
|
||||
}
|
||||
|
||||
class NonInheritingTraceContextPropagator implements TextMapPropagator {
|
||||
private readonly _delegate = new CompositePropagator({
|
||||
propagators: [new W3CTraceContextPropagator(), new W3CBaggagePropagator()],
|
||||
});
|
||||
|
||||
inject(context: Context, carrier: unknown, setter: TextMapSetter): void {
|
||||
this._delegate.inject(context, carrier, setter);
|
||||
}
|
||||
|
||||
extract(context: Context, carrier: unknown, getter: TextMapGetter): Context {
|
||||
return trace.deleteSpan(this._delegate.extract(context, carrier, getter));
|
||||
}
|
||||
|
||||
fields(): string[] {
|
||||
return this._delegate.fields();
|
||||
}
|
||||
}
|
||||
|
||||
export const {
|
||||
tracer,
|
||||
logger: otelLogger,
|
||||
@@ -281,11 +307,14 @@ function setupTelemetry() {
|
||||
}
|
||||
}
|
||||
|
||||
const ratioSampler = new TraceIdRatioBasedSampler(samplingRate);
|
||||
|
||||
const provider = new NodeTracerProvider({
|
||||
forceFlushTimeoutMillis: 15_000,
|
||||
resource: getResource(),
|
||||
sampler: new ParentBasedSampler({
|
||||
root: new CustomWebappSampler(new TraceIdRatioBasedSampler(samplingRate)),
|
||||
root: new CustomWebappSampler(ratioSampler),
|
||||
remoteParentSampled: ratioSampler,
|
||||
}),
|
||||
spanLimits: {
|
||||
attributeCountLimit: 1024,
|
||||
@@ -324,7 +353,10 @@ function setupTelemetry() {
|
||||
);
|
||||
}
|
||||
|
||||
provider.register({ contextManager: createContextManager() });
|
||||
provider.register({
|
||||
contextManager: createContextManager(),
|
||||
propagator: new NonInheritingTraceContextPropagator(),
|
||||
});
|
||||
|
||||
let instrumentations: Instrumentation[] = [
|
||||
new AwsSdkInstrumentation({
|
||||
|
||||
Reference in New Issue
Block a user