Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
feat(core): Add updateSpanName helper function
  • Loading branch information
Lms24 committed Dec 13, 2024
commit 1be63e28f8f46aed32f981ce02be774ea1834722
27 changes: 26 additions & 1 deletion packages/core/src/utils/spanUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import { getMainCarrier } from '../carrier';
import { getCurrentScope } from '../currentScopes';
import { getMetricSummaryJsonForSpan, updateMetricSummaryOnSpan } from '../metrics/metric-summary';
import type { MetricType } from '../metrics/types';
import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '../semanticAttributes';
import {
SEMANTIC_ATTRIBUTE_SENTRY_OP,
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
} from '../semanticAttributes';
import type { SentrySpan } from '../tracing/sentrySpan';
import { SPAN_STATUS_OK, SPAN_STATUS_UNSET } from '../tracing/spanstatus';
import type {
Expand Down Expand Up @@ -310,3 +314,24 @@ export function showSpanDropWarning(): void {
hasShownSpanDropWarning = true;
}
}

/**
* Updates the name of the given span and ensures that the span name is not
* overwritten by the Sentry SDK.
*
* Use this function instead of `span.updateName()` if you want to make sure that
* your name is kept. For some spans, for example root `http.server` spans the
* Sentry SDK would otherwise overwrite the span name with a high-quality name
* it infers when the span ends.
*
* Use this function in server code or when your span is started on the server
* and on the client (browser). If you only update a span name on the client,
* you can also use `span.updateName()` the SDK does not overwrite the name.
*
* @param span - The span to update the name of.
* @param name - The name to set on the span.
*/
export function updateSpanName(span: Span, name: string): void {
span.updateName(name);
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'custom');
}
21 changes: 19 additions & 2 deletions packages/core/test/lib/utils/spanUtils.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
SEMANTIC_ATTRIBUTE_SENTRY_OP,
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
SPAN_STATUS_ERROR,
SPAN_STATUS_OK,
SPAN_STATUS_UNSET,
Expand All @@ -14,8 +15,14 @@ import {
} from '../../../src';
import type { Span, SpanAttributes, SpanStatus, SpanTimeInput } from '../../../src/types-hoist';
import type { OpenTelemetrySdkTraceBaseSpan } from '../../../src/utils/spanUtils';
import { spanToTraceContext } from '../../../src/utils/spanUtils';
import { getRootSpan, spanIsSampled, spanTimeInputToSeconds, spanToJSON } from '../../../src/utils/spanUtils';
import {
getRootSpan,
spanIsSampled,
spanTimeInputToSeconds,
spanToJSON,
spanToTraceContext,
updateSpanName,
} from '../../../src/utils/spanUtils';
import { TestClient, getDefaultTestClientOptions } from '../../mocks/client';

function createMockedOtelSpan({
Expand Down Expand Up @@ -332,3 +339,13 @@ describe('getRootSpan', () => {
});
});
});

describe('updateSpanName', () => {
it('updates the span name and source', () => {
const span = new SentrySpan({ name: 'old-name', attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url' } });
updateSpanName(span, 'new-name');
const spanJSON = spanToJSON(span);
expect(spanJSON.description).toBe('new-name');
expect(spanJSON.data?.[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]).toBe('custom');
});
});
35 changes: 25 additions & 10 deletions packages/opentelemetry/src/utils/parseSpanDescription.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type { SpanAttributes, TransactionSource } from '@sentry/core';
import {
SEMANTIC_ATTRIBUTE_SENTRY_OP,
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
getSanitizedUrlString,
parseUrl,
stripUrlQueryAndFragment,
Expand All @@ -36,12 +37,12 @@ interface SpanDescription {
/**
* Infer the op & description for a set of name, attributes and kind of a span.
*/
export function inferSpanData(name: string, attributes: SpanAttributes, kind: SpanKind): SpanDescription {
export function inferSpanData(originalName: string, attributes: SpanAttributes, kind: SpanKind): SpanDescription {
// if http.method exists, this is an http request span
// eslint-disable-next-line deprecation/deprecation
const httpMethod = attributes[ATTR_HTTP_REQUEST_METHOD] || attributes[SEMATTRS_HTTP_METHOD];
if (httpMethod) {
return descriptionForHttpMethod({ attributes, name, kind }, httpMethod);
return descriptionForHttpMethod({ attributes, name: originalName, kind }, httpMethod);
}

// eslint-disable-next-line deprecation/deprecation
Expand All @@ -53,17 +54,19 @@ export function inferSpanData(name: string, attributes: SpanAttributes, kind: Sp
// If db.type exists then this is a database call span
// If the Redis DB is used as a cache, the span description should not be changed
if (dbSystem && !opIsCache) {
return descriptionForDbSystem({ attributes, name });
return descriptionForDbSystem({ attributes, name: originalName });
}

const customSourceOrRoute = attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] === 'custom' ? 'custom' : 'route';

// If rpc.service exists then this is a rpc call span.
// eslint-disable-next-line deprecation/deprecation
const rpcService = attributes[SEMATTRS_RPC_SERVICE];
if (rpcService) {
return {
op: 'rpc',
description: name,
source: 'route',
description: originalName,
source: customSourceOrRoute,
};
}

Expand All @@ -73,24 +76,28 @@ export function inferSpanData(name: string, attributes: SpanAttributes, kind: Sp
if (messagingSystem) {
return {
op: 'message',
description: name,
source: 'route',
description: originalName,
source: customSourceOrRoute,
};
}

// If faas.trigger exists then this is a function as a service span.
// eslint-disable-next-line deprecation/deprecation
const faasTrigger = attributes[SEMATTRS_FAAS_TRIGGER];
if (faasTrigger) {
return { op: faasTrigger.toString(), description: name, source: 'route' };
return { op: faasTrigger.toString(), description: originalName, source: customSourceOrRoute };
}

return { op: undefined, description: name, source: 'custom' };
return { op: undefined, description: originalName, source: 'custom' };
}

/**
* Extract better op/description from an otel span.
*
* Does not overwrite the span name if the source is already set to custom to ensure
* that user-updated span names are preserved. In this case, we only adjust the op but
* leave span description and source unchanged.
*
* Based on https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/7422ce2a06337f68a59b552b8c5a2ac125d6bae5/exporter/sentryexporter/sentry_exporter.go#L306
*/
export function parseSpanDescription(span: AbstractSpan): SpanDescription {
Expand All @@ -102,6 +109,11 @@ export function parseSpanDescription(span: AbstractSpan): SpanDescription {
}

function descriptionForDbSystem({ attributes, name }: { attributes: Attributes; name: string }): SpanDescription {
// if we already set the source to custom, we don't overwrite the span description but just adjust the op
if (attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] === 'custom') {
return { op: 'db', description: name, source: 'custom' };
}

// Use DB statement (Ex "SELECT * FROM table") if possible as description.
// eslint-disable-next-line deprecation/deprecation
const statement = attributes[SEMATTRS_DB_STATEMENT];
Expand Down Expand Up @@ -174,7 +186,10 @@ export function descriptionForHttpMethod(
const origin = attributes[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN] || 'manual';
const isManualSpan = !`${origin}`.startsWith('auto');

const useInferredDescription = isClientOrServerKind || !isManualSpan;
// If users (or in very rare occasions we) set the source to custom, we don't overwrite it
const alreadyHasCustomSource = attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] === 'custom';

const useInferredDescription = !alreadyHasCustomSource && (isClientOrServerKind || !isManualSpan);

return {
op: opParts.join('.'),
Expand Down
99 changes: 99 additions & 0 deletions packages/opentelemetry/test/utils/parseSpanDescription.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
SEMATTRS_RPC_SERVICE,
} from '@opentelemetry/semantic-conventions';

import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
import { descriptionForHttpMethod, getSanitizedUrl, parseSpanDescription } from '../../src/utils/parseSpanDescription';

describe('parseSpanDescription', () => {
Expand Down Expand Up @@ -81,6 +82,21 @@ describe('parseSpanDescription', () => {
source: 'task',
},
],
[
'works with db system and custom source',
{
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom',
[SEMATTRS_DB_SYSTEM]: 'mysql',
[SEMATTRS_DB_STATEMENT]: 'SELECT * from users',
},
'test name',
SpanKind.CLIENT,
{
description: 'test name',
op: 'db',
source: 'custom',
},
],
[
'works with db system without statement',
{
Expand All @@ -107,6 +123,20 @@ describe('parseSpanDescription', () => {
source: 'route',
},
],
[
'works with rpc service and custom source',
{
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom',
[SEMATTRS_RPC_SERVICE]: 'rpc-test-service',
},
'test name',
undefined,
{
description: 'test name',
op: 'rpc',
source: 'custom',
},
],
[
'works with messaging system',
{
Expand All @@ -120,6 +150,20 @@ describe('parseSpanDescription', () => {
source: 'route',
},
],
[
'works with messaging system and custom source',
{
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom',
[SEMATTRS_MESSAGING_SYSTEM]: 'test-messaging-system',
},
'test name',
undefined,
{
description: 'test name',
op: 'message',
source: 'custom',
},
],
[
'works with faas trigger',
{
Expand All @@ -133,6 +177,20 @@ describe('parseSpanDescription', () => {
source: 'route',
},
],
[
'works with faas trigger and custom source',
{
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom',
[SEMATTRS_FAAS_TRIGGER]: 'test-faas-trigger',
},
'test name',
undefined,
{
description: 'test name',
op: 'test-faas-trigger',
source: 'custom',
},
],
])('%s', (_, attributes, name, kind, expected) => {
const actual = parseSpanDescription({ attributes, kind, name } as unknown as Span);
expect(actual).toEqual(expected);
Expand Down Expand Up @@ -172,6 +230,26 @@ describe('descriptionForHttpMethod', () => {
source: 'url',
},
],
[
'works with prefetch request',
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 is just an additional test I added because we didn't test the condition to add .prefetch to the op.

'GET',
{
[SEMATTRS_HTTP_METHOD]: 'GET',
[SEMATTRS_HTTP_URL]: 'https://www.example.com/my-path',
[SEMATTRS_HTTP_TARGET]: '/my-path',
'sentry.http.prefetch': true,
},
'test name',
SpanKind.CLIENT,
{
op: 'http.client.prefetch',
description: 'GET https://www.example.com/my-path',
data: {
url: 'https://www.example.com/my-path',
},
source: 'url',
},
],
[
'works with basic server POST',
'POST',
Expand Down Expand Up @@ -230,6 +308,27 @@ describe('descriptionForHttpMethod', () => {
source: 'custom',
},
],
[
"doesn't overwrite name with source custom",
'GET',
{
[SEMATTRS_HTTP_METHOD]: 'GET',
[SEMATTRS_HTTP_URL]: 'https://www.example.com/my-path/123',
[SEMATTRS_HTTP_TARGET]: '/my-path/123',
[ATTR_HTTP_ROUTE]: '/my-path/:id',
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom',
},
'test name',
SpanKind.CLIENT,
{
op: 'http.client',
description: 'test name',
data: {
url: 'https://www.example.com/my-path/123',
},
source: 'custom',
},
],
])('%s', (_, httpMethod, attributes, name, kind, expected) => {
const actual = descriptionForHttpMethod({ attributes, kind, name }, httpMethod);
expect(actual).toEqual(expected);
Expand Down