From 51bbf327f02d2d2e4bb391f49f8285518c25d43e Mon Sep 17 00:00:00 2001
From: Francesco Novy <francesco.novy@sentry.io>
Date: Mon, 12 Aug 2024 13:05:08 +0200
Subject: [PATCH] fix(opentelemetry): Do not overwrite http span name if kind
 is internal (#13282)

If the kind of a http span is neither client nor server, it implies it
is most likely being started with `startSpan()` manually, in which case
we rather not want to overwrite the name.
---
 .../suites/tracing/nestjs/scenario.ts         |  5 ++++-
 .../suites/tracing/nestjs/test.ts             |  7 ++++++-
 .../src/utils/parseSpanDescription.ts         | 18 +++++++++++++++---
 .../test/utils/parseSpanDescription.test.ts   | 19 +++++++++++++++++++
 4 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/dev-packages/node-integration-tests/suites/tracing/nestjs/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/nestjs/scenario.ts
index b75ff4d8a9ef..953619d8d437 100644
--- a/dev-packages/node-integration-tests/suites/tracing/nestjs/scenario.ts
+++ b/dev-packages/node-integration-tests/suites/tracing/nestjs/scenario.ts
@@ -13,7 +13,7 @@ Sentry.init({
 });
 
 import { Controller, Get, Injectable, Module } from '@nestjs/common';
-import { NestFactory } from '@nestjs/core';
+import { BaseExceptionFilter, HttpAdapterHost, NestFactory } from '@nestjs/core';
 
 const port = 3450;
 
@@ -48,6 +48,9 @@ class AppModule {}
 async function run(): Promise<void> {
   const app = await NestFactory.create(AppModule);
   await app.listen(port);
+
+  const { httpAdapter } = app.get(HttpAdapterHost);
+  Sentry.setupNestErrorHandler(app, new BaseExceptionFilter(httpAdapter));
   sendPortToRunner(port);
 }
 
diff --git a/dev-packages/node-integration-tests/suites/tracing/nestjs/test.ts b/dev-packages/node-integration-tests/suites/tracing/nestjs/test.ts
index 686c93e1cad6..80570044d64d 100644
--- a/dev-packages/node-integration-tests/suites/tracing/nestjs/test.ts
+++ b/dev-packages/node-integration-tests/suites/tracing/nestjs/test.ts
@@ -25,7 +25,12 @@ conditionalTest({ min: 16 })('nestjs auto instrumentation', () => {
           'nestjs.callback': 'getHello',
           'nestjs.controller': 'AppController',
           'nestjs.type': 'request_context',
-          'sentry.op': 'http',
+          'sentry.op': 'request_context.nestjs',
+          'sentry.origin': 'auto.http.otel.nestjs',
+          component: '@nestjs/core',
+          'http.method': 'GET',
+          'http.route': '/',
+          'http.url': '/',
         }),
       }),
     ]),
diff --git a/packages/opentelemetry/src/utils/parseSpanDescription.ts b/packages/opentelemetry/src/utils/parseSpanDescription.ts
index 6d1c9936899b..b600b81f8aec 100644
--- a/packages/opentelemetry/src/utils/parseSpanDescription.ts
+++ b/packages/opentelemetry/src/utils/parseSpanDescription.ts
@@ -14,7 +14,7 @@ import {
 import type { SpanAttributes, TransactionSource } from '@sentry/types';
 import { getSanitizedUrlString, parseUrl, stripUrlQueryAndFragment } from '@sentry/utils';
 
-import { SEMANTIC_ATTRIBUTE_SENTRY_OP } from '@sentry/core';
+import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core';
 import { SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION } from '../semanticAttributes';
 import type { AbstractSpan } from '../types';
 import { getSpanKind } from './getSpanKind';
@@ -163,10 +163,22 @@ export function descriptionForHttpMethod(
     data['http.fragment'] = fragment;
   }
 
+  // If the span kind is neither client nor server, we use the original name
+  // this infers that somebody manually started this span, in which case we don't want to overwrite the name
+  const isClientOrServerKind = kind === SpanKind.CLIENT || kind === SpanKind.SERVER;
+
+  // If the span is an auto-span (=it comes from one of our instrumentations),
+  // we always want to infer the name
+  // this is necessary because some of the auto-instrumentation we use uses kind=INTERNAL
+  const origin = attributes[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN] || 'manual';
+  const isManualSpan = !`${origin}`.startsWith('auto');
+
+  const useInferredDescription = isClientOrServerKind || !isManualSpan;
+
   return {
     op: opParts.join('.'),
-    description,
-    source,
+    description: useInferredDescription ? description : name,
+    source: useInferredDescription ? source : 'custom',
     data,
   };
 }
diff --git a/packages/opentelemetry/test/utils/parseSpanDescription.test.ts b/packages/opentelemetry/test/utils/parseSpanDescription.test.ts
index cfa1a43094c4..2b1d25dbacff 100644
--- a/packages/opentelemetry/test/utils/parseSpanDescription.test.ts
+++ b/packages/opentelemetry/test/utils/parseSpanDescription.test.ts
@@ -231,6 +231,25 @@ describe('descriptionForHttpMethod', () => {
         source: 'route',
       },
     ],
+    [
+      'works with basic client GET with SpanKind.INTERNAL',
+      'GET',
+      {
+        [SEMATTRS_HTTP_METHOD]: 'GET',
+        [SEMATTRS_HTTP_URL]: 'https://www.example.com/my-path',
+        [SEMATTRS_HTTP_TARGET]: '/my-path',
+      },
+      'test name',
+      SpanKind.INTERNAL,
+      {
+        op: 'http',
+        description: 'test name',
+        data: {
+          url: 'https://www.example.com/my-path',
+        },
+        source: 'custom',
+      },
+    ],
   ])('%s', (_, httpMethod, attributes, name, kind, expected) => {
     const actual = descriptionForHttpMethod({ attributes, kind, name }, httpMethod);
     expect(actual).toEqual(expected);