Skip to content

fix(opentelemetry): Ensure only orphaned spans of sent spans are sent #16590

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

Merged
merged 3 commits into from
Jun 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,15 @@ import { waitForTransaction } from '@sentry-internal/test-utils';

test('should create AI spans with correct attributes', async ({ page }) => {
const aiTransactionPromise = waitForTransaction('nextjs-15', async transactionEvent => {
return transactionEvent?.transaction === 'ai-test';
return transactionEvent.transaction === 'GET /ai-test';
Copy link
Member Author

Choose a reason for hiding this comment

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

this actually also shows the incorrect behavior we had before - the ai-test span was sent as a transaction instead of as part of the GET /ai-test span, which is what it should have been.

});

await page.goto('/ai-test');

const aiTransaction = await aiTransactionPromise;

expect(aiTransaction).toBeDefined();
expect(aiTransaction.contexts?.trace?.op).toBe('function');
expect(aiTransaction.transaction).toBe('ai-test');
expect(aiTransaction.transaction).toBe('GET /ai-test');

const spans = aiTransaction.spans || [];

Expand Down
6 changes: 3 additions & 3 deletions packages/opentelemetry/src/spanExporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,9 +180,6 @@ export class SentrySpanExporter {

this.flushSentSpanCache();
const sentSpans = this._maybeSend(finishedSpans);
for (const span of finishedSpans) {
this._sentSpans.set(span.spanContext().spanId, Date.now() + DEFAULT_TIMEOUT * 1000);
}

const sentSpanCount = sentSpans.size;
const remainingOpenSpanCount = finishedSpans.length - sentSpanCount;
Expand All @@ -191,7 +188,10 @@ export class SentrySpanExporter {
`SpanExporter exported ${sentSpanCount} spans, ${remainingOpenSpanCount} spans are waiting for their parent spans to finish`,
);

const expirationDate = Date.now() + DEFAULT_TIMEOUT * 1000;

for (const span of sentSpans) {
this._sentSpans.set(span.spanContext().spanId, expirationDate);
const bucketEntry = this._spansToBucketEntry.get(span);
if (bucketEntry) {
bucketEntry.spans.delete(span);
Expand Down
74 changes: 74 additions & 0 deletions packages/opentelemetry/test/integration/transactions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -680,6 +680,80 @@ describe('Integration | Transactions', () => {
expect(finishedSpans[0]?.name).toBe('inner span 2');
});

it('only considers sent spans, not finished spans, for flushing orphaned spans of sent spans', async () => {
const timeout = 5 * 60 * 1000;
const now = Date.now();
vi.useFakeTimers();
vi.setSystemTime(now);

const logs: unknown[] = [];
vi.spyOn(logger, 'log').mockImplementation(msg => logs.push(msg));

const transactions: Event[] = [];

mockSdkInit({
tracesSampleRate: 1,
beforeSendTransaction: event => {
transactions.push(event);
return null;
},
});

const provider = getProvider();
const multiSpanProcessor = provider?.activeSpanProcessor as
| (SpanProcessor & { _spanProcessors?: SpanProcessor[] })
| undefined;
const spanProcessor = multiSpanProcessor?.['_spanProcessors']?.find(
spanProcessor => spanProcessor instanceof SentrySpanProcessor,
) as SentrySpanProcessor | undefined;

const exporter = spanProcessor ? spanProcessor['_exporter'] : undefined;

if (!exporter) {
throw new Error('No exporter found, aborting test...');
}

/**
* This is our span structure:
* span 1 --------
* span 2 ---
* span 3 -
*
* Where span 2 is finished before span 3 & span 1
*/

const [span1, span3] = startSpanManual({ name: 'span 1' }, span1 => {
const [span2, span3] = startSpanManual({ name: 'span 2' }, span2 => {
const span3 = startInactiveSpan({ name: 'span 3' });
return [span2, span3];
});

// End span 2 before span 3
span2.end();

return [span1, span3];
});

vi.advanceTimersByTime(1);

// nothing should be sent yet, as span1 is not yet done
expect(transactions).toHaveLength(0);

// Now finish span1, should be sent with only span2 but without span3, as that is not yet finished
span1.end();
vi.advanceTimersByTime(1);

expect(transactions).toHaveLength(1);
expect(transactions[0]?.spans).toHaveLength(1);

// now finish span3, which should be sent as transaction too
span3.end();
vi.advanceTimersByTime(timeout);

expect(transactions).toHaveLength(2);
expect(transactions[1]?.spans).toHaveLength(0);
});

it('uses & inherits DSC on span trace state', async () => {
const transactionEvents: Event[] = [];
const beforeSendTransaction = vi.fn(event => {
Expand Down
Loading