Skip to content
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(core): Add updateSpanName helper function [v8] #14736

Merged
merged 2 commits into from
Dec 17, 2024
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
@@ -1,5 +1,5 @@
import { expect } from '@playwright/test';
import type { Event } from '@sentry/core';
import { type Event, SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME } from '@sentry/core';

import {
SEMANTIC_ATTRIBUTE_SENTRY_OP,
Expand All @@ -10,27 +10,34 @@ import {
import { sentryTest } from '../../../../utils/fixtures';
import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers';

sentryTest('sets the source to custom when updating the transaction name', async ({ getLocalTestUrl, page }) => {
if (shouldSkipTracingTest()) {
sentryTest.skip();
}
sentryTest(
'sets the source to custom when updating the transaction name with `span.updateName`',
async ({ getLocalTestUrl, page }) => {
if (shouldSkipTracingTest()) {
sentryTest.skip();
}

const url = await getLocalTestUrl({ testDir: __dirname });
const url = await getLocalTestUrl({ testDir: __dirname });

const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);

const traceContextData = eventData.contexts?.trace?.data;
const traceContextData = eventData.contexts?.trace?.data;

expect(traceContextData).toMatchObject({
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser',
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload',
});
expect(traceContextData).toBeDefined();

expect(eventData.transaction).toBe('new name');
expect(eventData.transaction).toBe('new name');

expect(eventData.contexts?.trace?.op).toBe('pageload');
expect(eventData.spans?.length).toBeGreaterThan(0);
expect(eventData.transaction_info?.source).toEqual('custom');
});
expect(traceContextData).toMatchObject({
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser',
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload',
});

expect(traceContextData![SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME]).toBeUndefined();

expect(eventData.contexts?.trace?.op).toBe('pageload');
expect(eventData.spans?.length).toBeGreaterThan(0);
expect(eventData.transaction_info?.source).toEqual('custom');
},
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import * as Sentry from '@sentry/browser';

window.Sentry = Sentry;
window._testBaseTimestamp = performance.timeOrigin / 1000;

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
integrations: [Sentry.browserTracingIntegration()],
tracesSampleRate: 1,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
const activeSpan = Sentry.getActiveSpan();
const rootSpan = activeSpan && Sentry.getRootSpan(activeSpan);

Sentry.updateSpanName(rootSpan, 'new name');
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { expect } from '@playwright/test';
import { type Event, SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME } from '@sentry/core';

import {
SEMANTIC_ATTRIBUTE_SENTRY_OP,
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE,
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
} from '@sentry/browser';
import { sentryTest } from '../../../../utils/fixtures';
import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers';

sentryTest(
'sets the source to custom when updating the transaction name with Sentry.updateSpanName',
async ({ getLocalTestUrl, page }) => {
if (shouldSkipTracingTest()) {
sentryTest.skip();
}

const url = await getLocalTestUrl({ testDir: __dirname });

const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);

const traceContextData = eventData.contexts?.trace?.data;

expect(traceContextData).toBeDefined();

expect(traceContextData).toMatchObject({
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser',
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload',
});

expect(traceContextData![SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME]).toBeUndefined();

expect(eventData.transaction).toBe('new name');

expect(eventData.contexts?.trace?.op).toBe('pageload');
expect(eventData.spans?.length).toBeGreaterThan(0);
expect(eventData.transaction_info?.source).toEqual('custom');
},
);
Original file line number Diff line number Diff line change
Expand Up @@ -181,5 +181,9 @@ async function captureErrorAndGetEnvelopeTraceHeader(page: Page): Promise<Partia
await page.locator('#btnCaptureError').click();

const [, errorEnvelopeTraceHeader] = (await errorEventPromise)[0];

// @ts-expect-error - EventEnvelopeHeaders type in (types/envelope.ts) suggests that trace_id is optional,
// which the DynamicSamplingContext type does not permit.
// TODO(v9): We should adjust the EventEnvelopeHeaders type because the trace header always needs to have a trace_id
return errorEnvelopeTraceHeader;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
const { loggingTransport } = require('@sentry-internal/node-integration-tests');
const Sentry = require('@sentry/node');

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
release: '1.0',
// disable attaching headers to /test/* endpoints
tracePropagationTargets: [/^(?!.*test).*$/],
tracesSampleRate: 1.0,
transport: loggingTransport,
});

// express must be required after Sentry is initialized
const express = require('express');
const cors = require('cors');
const bodyParser = require('body-parser');
const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests');

const app = express();

app.use(cors());
app.use(bodyParser.json());
app.use(bodyParser.text());
app.use(bodyParser.raw());

app.get('/test/:id/span-updateName', (_req, res) => {
const span = Sentry.getActiveSpan();
const rootSpan = Sentry.getRootSpan(span);
rootSpan.updateName('new-name');
res.send({ response: 'response 1' });
});

app.get('/test/:id/span-updateName-source', (_req, res) => {
const span = Sentry.getActiveSpan();
const rootSpan = Sentry.getRootSpan(span);
rootSpan.updateName('new-name');
rootSpan.setAttribute(Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'custom');
res.send({ response: 'response 2' });
});

app.get('/test/:id/updateSpanName', (_req, res) => {
const span = Sentry.getActiveSpan();
const rootSpan = Sentry.getRootSpan(span);
Sentry.updateSpanName(rootSpan, 'new-name');
res.send({ response: 'response 3' });
});

app.get('/test/:id/updateSpanNameAndSource', (_req, res) => {
const span = Sentry.getActiveSpan();
const rootSpan = Sentry.getRootSpan(span);
Sentry.updateSpanName(rootSpan, 'new-name');
rootSpan.setAttribute(Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'component');
res.send({ response: 'response 4' });
});

Sentry.setupExpressErrorHandler(app);

startExpressServerAndSendPortToRunner(app);
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import { SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME } from '@sentry/core';
import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/node';
import { cleanupChildProcesses, createRunner } from '../../../../utils/runner';

describe('express tracing', () => {
afterAll(() => {
cleanupChildProcesses();
});

describe('CJS', () => {
// This test documents the unfortunate behaviour of using `span.updateName` on the server-side.
// For http.server root spans (which is the root span on the server 99% of the time), Otel's http instrumentation
// calls `span.updateName` and overwrites whatever the name was set to before (by us or by users).
test("calling just `span.updateName` doesn't update the final name in express (missing source)", done => {
createRunner(__dirname, 'server.js')
.expect({
transaction: {
transaction: 'GET /test/:id/span-updateName',
transaction_info: {
source: 'route',
},
},
})
.start(done)
.makeRequest('get', '/test/123/span-updateName');
});

// Also calling `updateName` AND setting a source doesn't change anything - Otel has no concept of source, this is sentry-internal.
// Therefore, only the source is updated but the name is still overwritten by Otel.
test("calling `span.updateName` and setting attribute source doesn't update the final name in express but it updates the source", done => {
createRunner(__dirname, 'server.js')
.expect({
transaction: {
transaction: 'GET /test/:id/span-updateName-source',
transaction_info: {
source: 'custom',
},
},
})
.start(done)
.makeRequest('get', '/test/123/span-updateName-source');
});

// This test documents the correct way to update the span name (and implicitly the source) in Node:
test('calling `Sentry.updateSpanName` updates the final name and source in express', done => {
createRunner(__dirname, 'server.js')
.expect({
transaction: txnEvent => {
expect(txnEvent).toMatchObject({
transaction: 'new-name',
transaction_info: {
source: 'custom',
},
contexts: {
trace: {
op: 'http.server',
data: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom' },
},
},
});
// ensure we delete the internal attribute once we're done with it
expect(txnEvent.contexts?.trace?.data?.[SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME]).toBeUndefined();
},
})
.start(done)
.makeRequest('get', '/test/123/updateSpanName');
});
});

// This test documents the correct way to update the span name (and implicitly the source) in Node:
test('calling `Sentry.updateSpanName` and setting source subsequently updates the final name and sets correct source', done => {
createRunner(__dirname, 'server.js')
.expect({
transaction: txnEvent => {
expect(txnEvent).toMatchObject({
transaction: 'new-name',
transaction_info: {
source: 'component',
},
contexts: {
trace: {
op: 'http.server',
data: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component' },
},
},
});
// ensure we delete the internal attribute once we're done with it
expect(txnEvent.contexts?.trace?.data?.[SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME]).toBeUndefined();
},
})
.start(done)
.makeRequest('get', '/test/123/updateSpanNameAndSource');
});
});
Original file line number Diff line number Diff line change
@@ -1,11 +1,42 @@
import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/node';
import { cleanupChildProcesses, createRunner } from '../../../../utils/runner';

afterAll(() => {
cleanupChildProcesses();
});

test('should send a manually started root span', done => {
test('sends a manually started root span with source custom', done => {
createRunner(__dirname, 'scenario.ts')
.expect({ transaction: { transaction: 'test_span' } })
.expect({
transaction: {
transaction: 'test_span',
transaction_info: { source: 'custom' },
contexts: {
trace: {
span_id: expect.any(String),
trace_id: expect.any(String),
data: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom' },
},
},
},
})
.start(done);
});

test("doesn't change the name for manually started spans even if attributes triggering inference are set", done => {
createRunner(__dirname, 'scenario.ts')
.expect({
transaction: {
transaction: 'test_span',
transaction_info: { source: 'custom' },
contexts: {
trace: {
span_id: expect.any(String),
trace_id: expect.any(String),
data: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom' },
},
},
},
})
.start(done);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { loggingTransport } from '@sentry-internal/node-integration-tests';
import * as Sentry from '@sentry/node';

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
release: '1.0',
tracesSampleRate: 1.0,
transport: loggingTransport,
});

Sentry.startSpan(
{ name: 'test_span', attributes: { [Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url' } },
(span: Sentry.Span) => {
span.updateName('new name');
},
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/node';
import { cleanupChildProcesses, createRunner } from '../../../../utils/runner';

afterAll(() => {
cleanupChildProcesses();
});

test('updates the span name when calling `span.updateName`', done => {
createRunner(__dirname, 'scenario.ts')
.expect({
transaction: {
transaction: 'new name',
transaction_info: { source: 'url' },
contexts: {
trace: {
span_id: expect.any(String),
trace_id: expect.any(String),
data: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url' },
},
},
},
})
.start(done);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { loggingTransport } from '@sentry-internal/node-integration-tests';
import * as Sentry from '@sentry/node';

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
release: '1.0',
tracesSampleRate: 1.0,
transport: loggingTransport,
});

Sentry.startSpan(
{ name: 'test_span', attributes: { [Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url' } },
(span: Sentry.Span) => {
Sentry.updateSpanName(span, 'new name');
},
);
Loading
Loading