Skip to content

Commit

Permalink
fix(opentelemetry): Ignore invalid parent spans (#14512)
Browse files Browse the repository at this point in the history
We do not align with how OTEL traces and the default samplers handle
invalid span contexts today.
If a span context is invalid, e.g. the span ID or trace ID are invalid,
then the span is ignored for new spans as a parent, and the new span
will start a new trace. This change aligns our own sampling strategy
with this, to ensure that invalid parent span contexts are ignored.
  • Loading branch information
mydea authored Nov 29, 2024
1 parent 1f08b3b commit e21c3bc
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 1 deletion.
11 changes: 10 additions & 1 deletion packages/opentelemetry/src/sampler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export class SentrySampler implements Sampler {
): SamplingResult {
const options = this._client.getOptions();

const parentSpan = trace.getSpan(context);
const parentSpan = getValidSpan(context);
const parentContext = parentSpan?.spanContext();

if (!hasTracingEnabled(options)) {
Expand Down Expand Up @@ -210,3 +210,12 @@ function getBaseTraceState(context: Context, spanAttributes: SpanAttributes): Tr

return traceState;
}

/**
* If the active span is invalid, we want to ignore it as parent.
* This aligns with how otel tracers and default samplers handle these cases.
*/
function getValidSpan(context: Context): Span | undefined {
const span = trace.getSpan(context);
return span && isSpanContextValid(span.spanContext()) ? span : undefined;
}
21 changes: 21 additions & 0 deletions packages/opentelemetry/test/trace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1450,6 +1450,27 @@ describe('trace (sampling)', () => {
},
});
});

it('ignores parent span context if it is invalid', () => {
mockSdkInit({ tracesSampleRate: 1 });
const traceId = 'd4cda95b652f4a1592b449d5929fda1b';

const spanContext = {
traceId,
spanId: 'INVALID',
traceFlags: TraceFlags.SAMPLED,
};

context.with(trace.setSpanContext(ROOT_CONTEXT, spanContext), () => {
startSpan({ name: 'outer' }, span => {
expect(span.isRecording()).toBe(true);
expect(span.spanContext().spanId).not.toBe('INVALID');
expect(span.spanContext().spanId).toMatch(/[a-f0-9]{16}/);
expect(span.spanContext().traceId).not.toBe(traceId);
expect(span.spanContext().traceId).toMatch(/[a-f0-9]{32}/);
});
});
});
});

describe('HTTP methods (sampling)', () => {
Expand Down

0 comments on commit e21c3bc

Please sign in to comment.