fixes

Author: mydea

dev-packages/e2e-tests/test-applications/node-express-esm-preload/tests/server.test.ts

@@ -117,9 +117,7 @@ test('Should record a transaction for route with parameters', async ({ request }
   });
 });
 
-// This fails https://github.com/getsentry/sentry-javascript/pull/12587#issuecomment-2181019422
-// Skipping this for now so we don't block releases
-test.skip('Should record spans from http instrumentation', async ({ request }) => {
+test('Should record spans from http instrumentation', async ({ request }) => {
   const transactionEventPromise = waitForTransaction('node-express-esm-preload', transactionEvent => {
     return transactionEvent.contexts?.trace?.data?.['http.target'] === '/http-req';
   });

packages/node-core/src/integrations/http/incoming-requests.ts

@@ -11,7 +11,7 @@ import {
   SEMATTRS_NET_HOST_PORT,
   SEMATTRS_NET_PEER_IP,
 } from '@opentelemetry/semantic-conventions';
-import type { AggregationCounts, Client, Scope, SpanAttributes } from '@sentry/core';
+import type { AggregationCounts, Client, Scope, SpanAttributes, SpanStatus } from '@sentry/core';
 import {
   debug,
   generateSpanId,
@@ -251,31 +251,33 @@ export function instrumentServer(
 
             // After 'error', no further events other than 'close' should be emitted.
             let isEnded = false;
-            response.on('close', () => {
+            function endSpan(status: SpanStatus): void {
               if (isEnded) {
                 return;
               }
 
               isEnded = true;
-              const newAttributes = getIncomingRequestAttributesOnResponse(request, response);
-              span.setAttributes(newAttributes);
-              span.setStatus(getSpanStatusFromHttpCode(response.statusCode));
 
-              span.end();
-            });
-            response.on(errorMonitor, () => {
-              if (isEnded) {
-                return;
-              }
-
-              isEnded = true;
               const newAttributes = getIncomingRequestAttributesOnResponse(request, response);
               span.setAttributes(newAttributes);
+              span.setStatus(status);
+              span.end();
 
-              const status = getSpanStatusFromHttpCode(response.statusCode);
+              // Update the transaction name if the route has changed
+              if (newAttributes['http.route']) {
+                getIsolationScope().setTransactionName(
+                  `${request.method?.toUpperCase() || 'GET'} ${newAttributes['http.route']}`,
+                );
+              }
+            }
 
-              span.setStatus(status.code === SPAN_STATUS_ERROR ? status : { code: SPAN_STATUS_ERROR });
-              span.end();
+            response.on('close', () => {
+              endSpan(getSpanStatusFromHttpCode(response.statusCode));
+            });
+            response.on(errorMonitor, () => {
+              const httpStatus = getSpanStatusFromHttpCode(response.statusCode);
+              // Ensure we def. have an error status here
+              endSpan(httpStatus.code === SPAN_STATUS_ERROR ? httpStatus : { code: SPAN_STATUS_ERROR });
             });
 
             return target.apply(thisArg, args);
@@ -544,7 +546,8 @@ function getIncomingRequestAttributesOnResponse(request: IncomingMessage, respon
   newAttributes['http.status_text'] = (statusMessage || '').toUpperCase();
 
   if (rpcMetadata?.type === RPCType.HTTP && rpcMetadata.route !== undefined) {
-    newAttributes[ATTR_HTTP_ROUTE] = rpcMetadata.route;
+    const routeName = rpcMetadata.route;
+    newAttributes[ATTR_HTTP_ROUTE] = routeName;
   }
 
   return newAttributes;

packages/node/src/integrations/http.ts

@@ -144,7 +144,11 @@ const instrumentSentryHttp = generateInstrumentOnce<SentryHttpInstrumentationOpt
 );
 
 export const instrumentOtelHttp = generateInstrumentOnce<HttpInstrumentationConfig>(INTEGRATION_NAME, config => {
-  const instrumentation = new HttpInstrumentation(config);
+  const instrumentation = new HttpInstrumentation({
+    ...config,
+    // This is hard-coded and can never be overridden by the user
+    disableIncomingRequestInstrumentation: true,
+  });
 
   // We want to update the logger namespace so we can better identify what is happening here
   try {
@@ -243,8 +247,6 @@ export const httpIntegration = defineIntegration((options: HttpOptions = {}) =>
 
 function getConfigWithDefaults(options: Partial<HttpOptions> = {}): HttpInstrumentationConfig {
   const instrumentationConfig = {
-    disableIncomingRequestInstrumentation: true,
-
     ignoreOutgoingRequestHook: request => {
       const url = getRequestUrl(request);