feat(otel): Ignore outgoing Sentry HTTP requests from otel integration (#6116)

Author: mydea

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",

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);

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;
+}

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://[email protected]/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