From b55dd072b7912003b07eddb65b8ba98ca1aa72ac Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 2 Nov 2022 16:50:44 +0100 Subject: [PATCH] feat(otel): Ignore outgoing Sentry HTTP requests from otel integration (#6116) --- packages/opentelemetry-node/package.json | 3 +- .../opentelemetry-node/src/spanprocessor.ts | 14 ++- .../src/utils/is-sentry-request.ts | 29 +++++ .../test/spanprocessor.test.ts | 114 +++++++++++++++++- 4 files changed, 154 insertions(+), 6 deletions(-) create mode 100644 packages/opentelemetry-node/src/utils/is-sentry-request.ts diff --git a/packages/opentelemetry-node/package.json b/packages/opentelemetry-node/package.json index d604e407900e..1ea02961c743 100644 --- a/packages/opentelemetry-node/package.json +++ b/packages/opentelemetry-node/package.json @@ -33,7 +33,8 @@ "@opentelemetry/core": "^1.7.0", "@opentelemetry/sdk-trace-base": "^1.7.0", "@opentelemetry/sdk-trace-node": "^1.7.0", - "@opentelemetry/semantic-conventions": "^1.7.0" + "@opentelemetry/semantic-conventions": "^1.7.0", + "@sentry/node": "7.17.3" }, "scripts": { "build": "run-p build:rollup build:types", diff --git a/packages/opentelemetry-node/src/spanprocessor.ts b/packages/opentelemetry-node/src/spanprocessor.ts index 10cd0d957eea..2bbae33e58a5 100644 --- a/packages/opentelemetry-node/src/spanprocessor.ts +++ b/packages/opentelemetry-node/src/spanprocessor.ts @@ -5,6 +5,7 @@ import { Transaction } from '@sentry/tracing'; import { Span as SentrySpan, TransactionContext } from '@sentry/types'; import { logger } from '@sentry/utils'; +import { isSentryRequestSpan } from './utils/is-sentry-request'; import { mapOtelStatus } from './utils/map-otel-status'; import { parseSpanDescription } from './utils/parse-otel-span-description'; @@ -33,9 +34,6 @@ export class SentrySpanProcessor implements OtelSpanProcessor { return; } - // TODO: handle sentry requests - // if isSentryRequest(otelSpan) return; - const otelSpanId = otelSpan.spanContext().spanId; const otelParentSpanId = otelSpan.parentSpanId; @@ -79,6 +77,16 @@ export class SentrySpanProcessor implements OtelSpanProcessor { return; } + // Auto-instrumentation often captures outgoing HTTP requests + // This means that Sentry HTTP requests created by this integration can, in turn, be captured by OTEL auto instrumentation, + // leading to an infinite loop. + // In this case, we do not want to finish the span, in order to avoid sending it to Sentry + if (isSentryRequestSpan(otelSpan)) { + // Make sure to remove any references, so this can be GCed + this._map.delete(otelSpanId); + return; + } + if (sentrySpan instanceof Transaction) { updateTransactionWithOtelData(sentrySpan, otelSpan); finishTransactionWithContextFromOtelData(sentrySpan, otelSpan); diff --git a/packages/opentelemetry-node/src/utils/is-sentry-request.ts b/packages/opentelemetry-node/src/utils/is-sentry-request.ts new file mode 100644 index 000000000000..1a504f3c3820 --- /dev/null +++ b/packages/opentelemetry-node/src/utils/is-sentry-request.ts @@ -0,0 +1,29 @@ +import { Span as OtelSpan } from '@opentelemetry/sdk-trace-base'; +import { SemanticAttributes } from '@opentelemetry/semantic-conventions'; +import { getCurrentHub } from '@sentry/core'; + +/** + * + * @param otelSpan Checks wheter a given OTEL Span is an http request to sentry. + * @returns boolean + */ +export function isSentryRequestSpan(otelSpan: OtelSpan): boolean { + const { attributes } = otelSpan; + + const httpUrl = attributes[SemanticAttributes.HTTP_URL]; + + if (!httpUrl) { + return false; + } + + return isSentryRequestUrl(httpUrl.toString()); +} + +/** + * Checks whether given url points to Sentry server + * @param url url to verify + */ +function isSentryRequestUrl(url: string): boolean { + const dsn = getCurrentHub().getClient()?.getDsn(); + return dsn ? url.includes(dsn.host) : false; +} diff --git a/packages/opentelemetry-node/test/spanprocessor.test.ts b/packages/opentelemetry-node/test/spanprocessor.test.ts index 1dddf135e1dd..306396d49376 100644 --- a/packages/opentelemetry-node/test/spanprocessor.test.ts +++ b/packages/opentelemetry-node/test/spanprocessor.test.ts @@ -4,12 +4,16 @@ import { Resource } from '@opentelemetry/resources'; import { Span as OtelSpan } from '@opentelemetry/sdk-trace-base'; import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node'; import { SemanticAttributes, SemanticResourceAttributes } from '@opentelemetry/semantic-conventions'; -import { Hub, makeMain } from '@sentry/core'; +import { createTransport, Hub, makeMain } from '@sentry/core'; +import { NodeClient } from '@sentry/node'; import { addExtensionMethods, Span as SentrySpan, SpanStatusType, Transaction } from '@sentry/tracing'; import { Contexts, Scope } from '@sentry/types'; +import { resolvedSyncPromise } from '@sentry/utils'; import { SENTRY_SPAN_PROCESSOR_MAP, SentrySpanProcessor } from '../src/spanprocessor'; +const SENTRY_DSN = 'https://0@0.ingest.sentry.io/0'; + // Integration Test of SentrySpanProcessor beforeAll(() => { @@ -22,7 +26,13 @@ describe('SentrySpanProcessor', () => { let spanProcessor: SentrySpanProcessor; beforeEach(() => { - hub = new Hub(); + const client = new NodeClient({ + dsn: SENTRY_DSN, + integrations: [], + transport: () => createTransport({ recordDroppedEvent: () => undefined }, _ => resolvedSyncPromise({})), + stackParser: () => [], + }); + hub = new Hub(client); makeMain(hub); spanProcessor = new SentrySpanProcessor(); @@ -561,6 +571,106 @@ describe('SentrySpanProcessor', () => { }); }); }); + + describe('skip sentry requests', () => { + it('does not finish transaction for Sentry request', async () => { + const otelSpan = provider.getTracer('default').startSpan('POST to sentry', { + attributes: { + [SemanticAttributes.HTTP_METHOD]: 'POST', + [SemanticAttributes.HTTP_URL]: `${SENTRY_DSN}/sub/route`, + }, + }) as OtelSpan; + + const sentrySpanTransaction = getSpanForOtelSpan(otelSpan) as Transaction | undefined; + expect(sentrySpanTransaction).toBeDefined(); + + otelSpan.end(); + + expect(sentrySpanTransaction?.endTimestamp).toBeUndefined(); + + // Ensure it is still removed from map! + expect(getSpanForOtelSpan(otelSpan)).toBeUndefined(); + }); + + it('finishes transaction for non-Sentry request', async () => { + const otelSpan = provider.getTracer('default').startSpan('POST to sentry', { + attributes: { + [SemanticAttributes.HTTP_METHOD]: 'POST', + [SemanticAttributes.HTTP_URL]: 'https://other.sentry.io/sub/route', + }, + }) as OtelSpan; + + const sentrySpanTransaction = getSpanForOtelSpan(otelSpan) as Transaction | undefined; + expect(sentrySpanTransaction).toBeDefined(); + + otelSpan.end(); + + expect(sentrySpanTransaction?.endTimestamp).toBeDefined(); + }); + + it('does not finish spans for Sentry request', async () => { + const tracer = provider.getTracer('default'); + + tracer.startActiveSpan('GET /users', () => { + tracer.startActiveSpan( + 'SELECT * FROM users;', + { + attributes: { + [SemanticAttributes.HTTP_METHOD]: 'POST', + [SemanticAttributes.HTTP_URL]: `${SENTRY_DSN}/sub/route`, + }, + }, + child => { + const childOtelSpan = child as OtelSpan; + + const sentrySpan = getSpanForOtelSpan(childOtelSpan); + expect(sentrySpan).toBeDefined(); + + childOtelSpan.end(); + + expect(sentrySpan?.endTimestamp).toBeUndefined(); + + // Ensure it is still removed from map! + expect(getSpanForOtelSpan(childOtelSpan)).toBeUndefined(); + }, + ); + }); + }); + + it('handles child spans of Sentry requests normally', async () => { + const tracer = provider.getTracer('default'); + + tracer.startActiveSpan('GET /users', () => { + tracer.startActiveSpan( + 'SELECT * FROM users;', + { + attributes: { + [SemanticAttributes.HTTP_METHOD]: 'POST', + [SemanticAttributes.HTTP_URL]: `${SENTRY_DSN}/sub/route`, + }, + }, + child => { + const childOtelSpan = child as OtelSpan; + + const grandchildSpan = tracer.startSpan('child 1'); + + const sentrySpan = getSpanForOtelSpan(childOtelSpan); + expect(sentrySpan).toBeDefined(); + + const sentryGrandchildSpan = getSpanForOtelSpan(grandchildSpan); + expect(sentryGrandchildSpan).toBeDefined(); + + grandchildSpan.end(); + + childOtelSpan.end(); + + expect(sentryGrandchildSpan?.endTimestamp).toBeDefined(); + expect(sentrySpan?.endTimestamp).toBeUndefined(); + }, + ); + }); + }); + }); }); // OTEL expects a custom date format