fix(core, node, bun, deno): Align server span client address with event IP

dev-packages/e2e-tests/test-applications/deno/tests/streamed/transactions.test.ts

@@ -146,6 +146,10 @@ const SEGMENT_SPAN = {
       type: 'string',
       value: 'http:',
     },
+    'user.ip_address': {
+      type: 'string',
+      value: expect.any(String),
+    },
     'user_agent.original': {
       type: 'string',
       value: 'node',

dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts

@@ -171,6 +171,27 @@ describe('httpIntegration', () => {
         runner.makeRequest('post', '/test?a=1&b=2#hash', { data: 'test body' });
         await runner.completed();
       });
+
+      test('prefers the forwarded client address, without the socket port', async () => {
+        const runner = createRunner()
+          .expect({
+            transaction: transaction => {
+              const data = transaction.contexts?.trace?.data;
+              expect(data).toEqual(
+                expect.objectContaining({
+                  'client.address': '203.0.113.7',
+                  'network.peer.address': '::1',
+                  'network.peer.port': expect.any(Number),
+                }),
+              );
+              expect(data).not.toHaveProperty('client.port');
+            },
+          })
+          .start();
+
+        runner.makeRequest('get', '/test', { headers: { 'X-Forwarded-For': '203.0.113.7, 10.0.0.1' } });
+        await runner.completed();
+      });
     });
 
     describe('custom server.emit', () => {

packages/bun/src/integrations/bunserver.ts

@@ -21,6 +21,7 @@ import {
   filterCollectedUrl,
   filterCollectedUrlQuery,
 } from '@sentry/core';
+import { getClientIPAddress } from '@sentry/core/server';
 import type { Server, ServeOptions } from 'bun';
 import {
   CLIENT_ADDRESS,
@@ -245,30 +246,35 @@ function wrapRequestHandler<T extends RouteHandler = RouteHandler>(
     const client = getClient();
     const dataCollection = client?.getDataCollectionOptions();
 
-    if (dataCollection?.userInfo) {
-      // `client.address` is the originating client, so a forwarding header wins over the socket, which
-      // behind a proxy holds the proxy's address.
-      const forwardedFor = request.headers.get('x-forwarded-for')?.split(',')[0]?.trim();
+    let socketAddress: { address: string; port: number } | undefined;
+    if (dataCollection) {
+      const headers = request.headers.toJSON();
       // Bun passes the `Server` as the second argument to both `fetch` and route handlers, except
       // when the handler runs through `server.fetch()`.
-      const socketAddress = getRequestIP(args[1], request);
-      if (forwardedFor || socketAddress?.address) {
-        attributes[CLIENT_ADDRESS] = forwardedFor || socketAddress?.address;
-      }
-      if (socketAddress?.port) {
-        attributes[CLIENT_PORT] = socketAddress.port;
+      socketAddress = getRequestIP(args[1], request);
+
+      if (dataCollection.userInfo) {
+        // `client.address` is the originating client, so a forwarding header wins over the socket, which
+        // behind a proxy holds the proxy's address. The socket port is the proxy's too, so `client.port`
+        // stays unset then.
+        const forwardedAddress = getClientIPAddress(headers);
+        if (forwardedAddress) {
+          attributes[CLIENT_ADDRESS] = forwardedAddress;
+        } else if (socketAddress) {
+          attributes[CLIENT_ADDRESS] = socketAddress.address;
+          attributes[CLIENT_PORT] = socketAddress.port;
+        }
       }
+
+      Object.assign(attributes, httpHeadersToSpanAttributes(headers, dataCollection));
     }
 
     // describes the OSI application-layer protocol (http), not the scheme (might be https)
     attributes[NETWORK_PROTOCOL_NAME] = 'http';
 
-    if (dataCollection) {
-      Object.assign(attributes, httpHeadersToSpanAttributes(request.headers.toJSON(), dataCollection));
-    }
-
     isolationScope.setSDKProcessingMetadata({
       normalizedRequest: winterCGRequestToRequestData(request),
+      ipAddress: socketAddress?.address,
     });
 
     if (client && dataCollection) {
@@ -329,12 +335,15 @@ function wrapRequestHandler<T extends RouteHandler = RouteHandler>(
   });
 }
 
-function getRequestIP(server: unknown, request: Request): { address: string; port: number } | undefined {
-  if (typeof (server as Partial<Server> | undefined)?.requestIP !== 'function') {
+function getRequestIP(
+  server: Partial<Pick<Server, 'requestIP'>> | undefined,
+  request: Request,
+): { address: string; port: number } | undefined {
+  if (typeof server?.requestIP !== 'function') {
     return undefined;
   }
   try {
-    return (server as Server).requestIP(request) ?? undefined;
+    return server.requestIP(request) ?? undefined;
   } catch {
     // Defensive: never let a failed lookup break the user's handler.
     return undefined;

packages/bun/test/integrations/bunserver.test.ts

@@ -570,7 +570,7 @@ describe('Bun Serve Integration', () => {
       );
     });
 
-    test('prefers the first x-forwarded-for address over the socket address', async () => {
+    test('prefers the first x-forwarded-for address over the socket address, without the socket port', async () => {
       const server = Bun.serve({
         async fetch(_req) {
           return new Response('Bun!');
@@ -593,6 +593,97 @@ describe('Bun Serve Integration', () => {
         }),
         expect.any(Function),
       );
+      expect(startSpanSpy).toHaveBeenCalledWith(
+        expect.objectContaining({
+          attributes: expect.not.objectContaining({
+            'client.port': expect.anything(),
+          }),
+        }),
+        expect.any(Function),
+      );
+    });
+
+    test('reads the client address from other forwarding headers', async () => {
+      const server = Bun.serve({
+        async fetch(_req) {
+          return new Response('Bun!');
+        },
+        port,
+      });
+
+      await fetch(`http://localhost:${port}/`, {
+        headers: { 'X-Real-IP': '203.0.113.8' },
+      });
+
+      await server.stop();
+
+      expect(startSpanSpy).toHaveBeenCalledTimes(1);
+      expect(startSpanSpy).toHaveBeenCalledWith(
+        expect.objectContaining({
+          attributes: expect.objectContaining({
+            'client.address': '203.0.113.8',
+          }),
+        }),
+        expect.any(Function),
+      );
+    });
+
+    test('falls back to the socket address when the forwarding header is not an IP', async () => {
+      const server = Bun.serve({
+        async fetch(_req) {
+          return new Response('Bun!');
+        },
+        port,
+      });
+
+      await fetch(`http://localhost:${port}/`, {
+        headers: { 'X-Forwarded-For': 'unknown' },
+      });
+
+      await server.stop();
+
+      expect(startSpanSpy).toHaveBeenCalledTimes(1);
+      expect(startSpanSpy).toHaveBeenCalledWith(
+        expect.objectContaining({
+          attributes: expect.objectContaining({
+            'client.address': expect.stringMatching(/^(127\.0\.0\.1|::1|::ffff:127\.0\.0\.1)$/),
+            'client.port': expect.any(Number),
+          }),
+        }),
+        expect.any(Function),
+      );
+    });
+
+    test('sets the socket address as the user IP on error events', async () => {
+      const events: SentryCore.Event[] = [];
+      setupClient({
+        integrations: [SentryCore.requestDataIntegration()],
+        beforeSend: event => {
+          events.push(event);
+          return null;
+        },
+      });
+
+      const server = Bun.serve({
+        async fetch(_req) {
+          SentryCore.captureException(new Error('Boom'));
+          return new Response('Bun!');
+        },
+        port,
+      });
+
+      await fetch(`http://localhost:${port}/`);
+
+      await server.stop();
+      await SentryCore.flush();
+
+      expect(events).toEqual([
+        expect.objectContaining({
+          user: expect.objectContaining({
+            ip_address: expect.stringMatching(/^(127\.0\.0\.1|::1|::ffff:127\.0\.0\.1)$/),
+          }),
+        }),
+      ]);
     });
 
     test('does not capture client address when userInfo collection is disabled', async () => {
@@ -680,8 +771,14 @@ describe('Bun Serve Integration', () => {
       await server.stop();
 
       expect(startSpanSpy).toHaveBeenCalledTimes(1);
-      const attributes = startSpanSpy.mock.calls[0]?.[0]?.attributes;
-      expect(attributes?.['http.request.header.x-forwarded-for']).toEqual(['203.0.113.7']);
+      expect(startSpanSpy).toHaveBeenCalledWith(
+        expect.objectContaining({
+          attributes: expect.objectContaining({
+            'http.request.header.x-forwarded-for': ['203.0.113.7'],
+          }),
+        }),
+        expect.any(Function),
+      );
     });
 
     test('filters request headers according to the dataCollection deny list', async () => {
@@ -703,9 +800,15 @@ describe('Bun Serve Integration', () => {
       await server.stop();
 
       expect(startSpanSpy).toHaveBeenCalledTimes(1);
-      const attributes = startSpanSpy.mock.calls[0]?.[0]?.attributes;
-      expect(attributes?.['http.request.header.x-internal']).toEqual(['[Filtered]']);
-      expect(attributes?.['http.request.header.x-public']).toEqual(['public-value']);
+      expect(startSpanSpy).toHaveBeenCalledWith(
+        expect.objectContaining({
+          attributes: expect.objectContaining({
+            'http.request.header.x-internal': ['[Filtered]'],
+            'http.request.header.x-public': ['public-value'],
+          }),
+        }),
+        expect.any(Function),
+      );
     });
 
     test('filters always-sensitive request headers even when collection is permissive', async () => {
@@ -725,8 +828,14 @@ describe('Bun Serve Integration', () => {
       await server.stop();
 
       expect(startSpanSpy).toHaveBeenCalledTimes(1);
-      const attributes = startSpanSpy.mock.calls[0]?.[0]?.attributes;
-      expect(attributes?.['http.request.header.authorization']).toEqual(['[Filtered]']);
+      expect(startSpanSpy).toHaveBeenCalledWith(
+        expect.objectContaining({
+          attributes: expect.objectContaining({
+            'http.request.header.authorization': ['[Filtered]'],
+          }),
+        }),
+        expect.any(Function),
+      );
     });
 
     test('applies the dataCollection response header collection behavior', async () => {
@@ -746,9 +855,12 @@ describe('Bun Serve Integration', () => {
       await server.stop();
 
       expect(setAttributesSpy).toHaveBeenCalledTimes(1);
-      const responseAttributes = setAttributesSpy.mock.calls[0]?.[0];
-      expect(responseAttributes?.['http.response.header.x-internal']).toEqual(['[Filtered]']);
-      expect(responseAttributes?.['http.response.header.x-public']).toEqual(['public-value']);
+      expect(setAttributesSpy).toHaveBeenCalledWith(
+        expect.objectContaining({
+          'http.response.header.x-internal': ['[Filtered]'],
+          'http.response.header.x-public': ['public-value'],
+        }),
+      );
     });
   });
 

packages/core/src/integrations/http/server-subscription.ts

@@ -69,6 +69,7 @@ import {
 } from '@sentry/conventions/attributes';
 import { HTTP_SERVER } from '@sentry/conventions/op';
 import { filterCollectedUrl, filterCollectedUrlQuery } from '../../utils/data-collection/filterCollectedUrl';
+import { getClientIPAddress } from '../../utils/clientIPAddress';
 
 // Tree-shakable guard to remove all code related to tracing
 declare const __SENTRY_TRACING__: boolean;
@@ -306,7 +307,6 @@ function buildServerSpanWrap(
         : `${method} ${httpTargetWithoutQueryFragment}`;
       const headers = request.headers;
       const userAgent = headers['user-agent'];
-      const ips = headers['x-forwarded-for'];
       const httpVersion = request.httpVersion;
       const host = headers.host as undefined | string;
       const hostname = host?.replace(/^(.*)(:[0-9]{1,5})/, '$1') || 'localhost';
@@ -315,8 +315,10 @@ function buildServerSpanWrap(
       const { localAddress, localPort, remoteAddress, remotePort } = socket ?? {};
       const collectClientAddress = client.getDataCollectionOptions().userInfo;
       // `client.address` is the originating client, so a forwarding header wins over the socket, which
-      // behind a proxy holds the proxy's address. `network.peer.address` keeps the socket value.
-      const clientAddress = getForwardedClientAddress(ips) ?? remoteAddress;
+      // behind a proxy holds the proxy's address. The socket port is the proxy's too, so `client.port`
+      // stays unset then. `network.peer.*` keeps the socket values.
+      const forwardedAddress = getClientIPAddress(headers);
+      const clientAddress = forwardedAddress || remoteAddress;
 
       return startSpanManual(
         {
@@ -333,7 +335,7 @@ function buildServerSpanWrap(
             [NETWORK_LOCAL_ADDRESS]: localAddress,
             [NETWORK_LOCAL_PORT]: localPort,
             [CLIENT_ADDRESS]: collectClientAddress ? clientAddress : undefined,
-            [CLIENT_PORT]: remotePort,
+            [CLIENT_PORT]: forwardedAddress ? undefined : remotePort,
             [NETWORK_PEER_ADDRESS]: collectClientAddress ? remoteAddress : undefined,
             [NETWORK_PEER_PORT]: remotePort,
             [SENTRY_HTTP_PREFETCH]: isKnownPrefetchRequest(request) || undefined,
@@ -392,14 +394,6 @@ function buildServerSpanWrap(
   };
 }
 
-/**
- * First entry of `X-Forwarded-For`: the client as seen by the outermost proxy.
- * https://opentelemetry.io/docs/specs/semconv/registry/attributes/client/#client-address
- */
-function getForwardedClientAddress(forwardedFor: string | string[] | undefined): string | undefined {
-  return typeof forwardedFor === 'string' ? forwardedFor.split(',')[0]?.trim() || undefined : undefined;
-}
-
 function shouldIgnoreSpansForIncomingRequest(
   request: HttpIncomingMessage,
   {

packages/core/src/integrations/requestdata.ts

@@ -16,7 +16,8 @@ import { filterQueryParams } from '../utils/data-collection/filterQueryParams';
 import { filterUrlQuery } from '../utils/data-collection/filterUrlQuery';
 import { filterCookiePairs, httpHeadersToSpanAttributes } from '../utils/request';
 import { getUrlQuery } from '../utils/url';
-import { getClientIPAddress, ipHeaderNames } from '../vendor/getIpAddress';
+import { getClientIPAddress } from '../utils/clientIPAddress';
+import { ipHeaderNames } from '../vendor/getIpAddress';
 import { safeSetSpanJSONAttributes } from '../tracing/spans/captureSpan';
 import { SENTRY_IS_LOCALHOST, URL_FULL, URL_QUERY } from '@sentry/conventions/attributes';
 

packages/core/src/server.ts

@@ -23,6 +23,7 @@ export {
   processHttpServerTransactionEvent,
 } from './integrations/http/server-transaction-event';
 export { recordRequestSession } from './integrations/http/record-request-session';
+export { getClientIPAddress } from './utils/clientIPAddress';
 export { addOutgoingRequestBreadcrumb } from './integrations/http/add-outgoing-request-breadcrumb';
 export {
   getRequestUrl,