Skip to content

fix(otel): Add transaction source logic to otel spans #6160

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.

Sign up for GitHub

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 1 commit into from
Nov 9, 2022
Merged

fix(otel): Add transaction source logic to otel spans #6160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions packages/opentelemetry-node/src/spanprocessor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,9 @@ function updateSpanWithOtelData(sentrySpan: SentrySpan, otelSpan: OtelSpan): voi
function updateTransactionWithOtelData(transaction: Transaction, otelSpan: OtelSpan): void {
transaction.setStatus(mapOtelStatus(otelSpan));

const { op, description } = parseSpanDescription(otelSpan);
const { op, description, source } = parseSpanDescription(otelSpan);
transaction.op = op;
transaction.name = description;
transaction.setName(description, source);
}

function convertOtelTimeToSeconds([seconds, nano]: [number, number]): number {
Expand Down
20 changes: 14 additions & 6 deletions packages/opentelemetry-node/src/utils/parse-otel-span-description.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import { AttributeValue, SpanKind } from '@opentelemetry/api';
import { Span as OtelSpan } from '@opentelemetry/sdk-trace-base';
import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
import { TransactionSource } from '@sentry/types';

interface SpanDescription {
op: string | undefined;
description: string;
source: TransactionSource;
}

/**
Expand Down Expand Up @@ -36,6 +38,7 @@ export function parseSpanDescription(otelSpan: OtelSpan): SpanDescription {
return {
op: 'rpc',
description: name,
source: 'route',
};
}

Expand All @@ -45,16 +48,17 @@ export function parseSpanDescription(otelSpan: OtelSpan): SpanDescription {
return {
op: 'message',
description: name,
source: 'route',
};
}

// If faas.trigger exists then this is a function as a service span.
const faasTrigger = attributes[SemanticAttributes.FAAS_TRIGGER];
if (faasTrigger) {
return { op: faasTrigger.toString(), description: name };
return { op: faasTrigger.toString(), description: name, source: 'route' };
}

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

function descriptionForDbSystem(otelSpan: OtelSpan, _dbSystem: AttributeValue): SpanDescription {
Expand All @@ -65,7 +69,7 @@ function descriptionForDbSystem(otelSpan: OtelSpan, _dbSystem: AttributeValue):

const description = statement ? statement.toString() : name;

return { op: 'db', description };
return { op: 'db', description, source: 'task' };
}

function descriptionForHttpMethod(otelSpan: OtelSpan, httpMethod: AttributeValue): SpanDescription {
Expand All @@ -82,15 +86,19 @@ function descriptionForHttpMethod(otelSpan: OtelSpan, httpMethod: AttributeValue
break;
}

const httpTarget = attributes[SemanticAttributes.HTTP_TARGET];
const httpRoute = attributes[SemanticAttributes.HTTP_ROUTE];

// Ex. /api/users
const httpPath = attributes[SemanticAttributes.HTTP_ROUTE] || attributes[SemanticAttributes.HTTP_TARGET];
const httpPath = httpRoute || httpTarget;

if (!httpPath) {
return { op: opParts.join('.'), description: name };
return { op: opParts.join('.'), description: name, source: 'custom' };
}

// Ex. description="GET /api/users".
const description = `${httpMethod} ${httpPath}`;
const source: TransactionSource = httpRoute ? 'route' : 'url';

return { op: opParts.join('.'), description };
return { op: opParts.join('.'), description, source };
}
30 changes: 30 additions & 0 deletions packages/opentelemetry-node/test/spanprocessor.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,36 @@ describe('SentrySpanProcessor', () => {
});
});

it('adds transaction source `url` for HTTP_TARGET', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

l: Instead of adding these tests, maybe it is better to adjust all the tests starting here:

sentry-javascript/packages/opentelemetry-node/test/spanprocessor.test.ts

Line 408 in af9d0f7

it('updates op/description based on attributes for HTTP_METHOD without HTTP_ROUTE', async () => {
to simply also check the source? There are already tests for the different otel attribute variations, IMHO we could just add an expect(sentrySpan?.transaction?.metadata.source).toBe('XXX'); check to all of them?

Copy link
Member Author

Choose a reason for hiding this comment

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

I did this because source only applies for transactions - so I wanted an explicit scenario where we were checking the transaction (span.transaction when it's a transaction is a self reference) for this test. Gonna keep it like this for now, we can always update later on if need be.

const tracer = provider.getTracer('default');

tracer.startActiveSpan('GET /users', otelSpan => {
const sentrySpan = getSpanForOtelSpan(otelSpan);

otelSpan.setAttribute(SemanticAttributes.HTTP_METHOD, 'GET');
otelSpan.setAttribute(SemanticAttributes.HTTP_TARGET, '/my/route/123');

otelSpan.end();

expect(sentrySpan?.transaction?.metadata.source).toBe('url');
});
});

it('adds transaction source `url` for HTTP_ROUTE', async () => {
const tracer = provider.getTracer('default');

tracer.startActiveSpan('GET /users', otelSpan => {
const sentrySpan = getSpanForOtelSpan(otelSpan);

otelSpan.setAttribute(SemanticAttributes.HTTP_METHOD, 'GET');
otelSpan.setAttribute(SemanticAttributes.HTTP_ROUTE, '/my/route/:id');

otelSpan.end();

expect(sentrySpan?.transaction?.metadata.source).toBe('route');
});
});

it('updates based on attributes for DB_SYSTEM', async () => {
const tracer = provider.getTracer('default');

Expand Down