From 71ac3be55335dc12140bce2f94ee13b1bec49732 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 20 Jun 2023 10:14:01 +0200 Subject: [PATCH 1/2] Add `http.response.status_code` to `span.data` --- packages/core/src/tracing/span.ts | 1 + .../test/server/cjsApiEndpoints.test.ts | 9 ++++++++ .../test/server/errorApiEndpoint.test.ts | 3 +++ .../test/server/tracing200.test.ts | 3 +++ .../test/server/tracing500.test.ts | 3 +++ .../test/server/tracingHttp.test.ts | 6 ++++++ .../suites/express/tracing/test.ts | 4 ++++ packages/node/test/handlers.test.ts | 1 + .../integration/test/server/action.test.ts | 21 +++++++++++++++++++ .../integration/test/server/loader.test.ts | 9 ++++++++ packages/replay/test/fixtures/transaction.ts | 1 + packages/tracing/test/span.test.ts | 1 + 12 files changed, 62 insertions(+) diff --git a/packages/core/src/tracing/span.ts b/packages/core/src/tracing/span.ts index fe9e7aba017c..59a8ed11d68a 100644 --- a/packages/core/src/tracing/span.ts +++ b/packages/core/src/tracing/span.ts @@ -227,6 +227,7 @@ export class Span implements SpanInterface { */ public setHttpStatus(httpStatus: number): this { this.setTag('http.status_code', String(httpStatus)); + this.setData('http.response.status_code', httpStatus); const spanStatus = spanStatusfromHttpCode(httpStatus); if (spanStatus !== 'unknown_error') { this.setStatus(spanStatus); diff --git a/packages/nextjs/test/integration/test/server/cjsApiEndpoints.test.ts b/packages/nextjs/test/integration/test/server/cjsApiEndpoints.test.ts index fc0ae186f64b..dfbf3b620aa8 100644 --- a/packages/nextjs/test/integration/test/server/cjsApiEndpoints.test.ts +++ b/packages/nextjs/test/integration/test/server/cjsApiEndpoints.test.ts @@ -18,6 +18,9 @@ describe('CommonJS API Endpoints', () => { op: 'http.server', status: 'ok', tags: { 'http.status_code': '200' }, + data: { + 'http.response.status_code': 200, + }, }, }, transaction: `GET ${unwrappedRoute}`, @@ -51,6 +54,9 @@ describe('CommonJS API Endpoints', () => { op: 'http.server', status: 'ok', tags: { 'http.status_code': '200' }, + data: { + 'http.response.status_code': 200, + }, }, }, transaction: `GET ${wrappedRoute}`, @@ -84,6 +90,9 @@ describe('CommonJS API Endpoints', () => { op: 'http.server', status: 'ok', tags: { 'http.status_code': '200' }, + data: { + 'http.response.status_code': 200, + }, }, }, transaction: `GET ${route}`, diff --git a/packages/nextjs/test/integration/test/server/errorApiEndpoint.test.ts b/packages/nextjs/test/integration/test/server/errorApiEndpoint.test.ts index 45168eeeee33..d259db3f2801 100644 --- a/packages/nextjs/test/integration/test/server/errorApiEndpoint.test.ts +++ b/packages/nextjs/test/integration/test/server/errorApiEndpoint.test.ts @@ -45,6 +45,9 @@ describe('Error API Endpoints', () => { op: 'http.server', status: 'internal_error', tags: { 'http.status_code': '500' }, + data: { + 'http.response.status_code': 500, + }, }, }, transaction: 'GET /api/error', diff --git a/packages/nextjs/test/integration/test/server/tracing200.test.ts b/packages/nextjs/test/integration/test/server/tracing200.test.ts index f68279138558..ac6b2db163aa 100644 --- a/packages/nextjs/test/integration/test/server/tracing200.test.ts +++ b/packages/nextjs/test/integration/test/server/tracing200.test.ts @@ -16,6 +16,9 @@ describe('Tracing 200', () => { op: 'http.server', status: 'ok', tags: { 'http.status_code': '200' }, + data: { + 'http.response.status_code': 200, + }, }, }, transaction: 'GET /api/users', diff --git a/packages/nextjs/test/integration/test/server/tracing500.test.ts b/packages/nextjs/test/integration/test/server/tracing500.test.ts index 79b23dcfb786..b94fe781e2d2 100644 --- a/packages/nextjs/test/integration/test/server/tracing500.test.ts +++ b/packages/nextjs/test/integration/test/server/tracing500.test.ts @@ -16,6 +16,9 @@ describe('Tracing 500', () => { op: 'http.server', status: 'internal_error', tags: { 'http.status_code': '500' }, + data: { + 'http.response.status_code': 500, + }, }, }, transaction: 'GET /api/broken', diff --git a/packages/nextjs/test/integration/test/server/tracingHttp.test.ts b/packages/nextjs/test/integration/test/server/tracingHttp.test.ts index 912c54e0996b..0f8615c4b7f0 100644 --- a/packages/nextjs/test/integration/test/server/tracingHttp.test.ts +++ b/packages/nextjs/test/integration/test/server/tracingHttp.test.ts @@ -20,6 +20,9 @@ describe('Tracing HTTP', () => { op: 'http.server', status: 'ok', tags: { 'http.status_code': '200' }, + data: { + 'http.response.status_code': 200, + }, }, }, spans: [ @@ -28,6 +31,9 @@ describe('Tracing HTTP', () => { op: 'http.client', status: 'ok', tags: { 'http.status_code': '200' }, + data: { + 'http.response.status_code': 200, + }, }, ], transaction: 'GET /api/http', diff --git a/packages/node-integration-tests/suites/express/tracing/test.ts b/packages/node-integration-tests/suites/express/tracing/test.ts index ae9d619c48cc..835c2938ae5b 100644 --- a/packages/node-integration-tests/suites/express/tracing/test.ts +++ b/packages/node-integration-tests/suites/express/tracing/test.ts @@ -11,6 +11,7 @@ test('should create and send transactions for Express routes and spans for middl trace: { data: { url: '/test/express', + 'http.response.status_code': 200, }, op: 'http.server', status: 'ok', @@ -43,6 +44,7 @@ test('should set a correct transaction name for routes specified in RegEx', asyn trace: { data: { url: '/test/regex', + 'http.response.status_code': 200, }, op: 'http.server', status: 'ok', @@ -71,6 +73,7 @@ test.each([['array1'], ['array5']])( trace: { data: { url: `/test/${segment}`, + 'http.response.status_code': 200, }, op: 'http.server', status: 'ok', @@ -107,6 +110,7 @@ test.each([ trace: { data: { url: `/test/${segment}`, + 'http.response.status_code': 200, }, op: 'http.server', status: 'ok', diff --git a/packages/node/test/handlers.test.ts b/packages/node/test/handlers.test.ts index 3cf5127ce848..1ccadc7fe41d 100644 --- a/packages/node/test/handlers.test.ts +++ b/packages/node/test/handlers.test.ts @@ -351,6 +351,7 @@ describe('tracingHandler', () => { expect(finishTransaction).toHaveBeenCalled(); expect(transaction.status).toBe('ok'); expect(transaction.tags).toEqual(expect.objectContaining({ 'http.status_code': '200' })); + expect(transaction.data).toEqual(expect.objectContaining({ 'http.response.status_code': 200 })); done(); }); }); diff --git a/packages/remix/test/integration/test/server/action.test.ts b/packages/remix/test/integration/test/server/action.test.ts index 6bb4b74d3540..98d208c932c1 100644 --- a/packages/remix/test/integration/test/server/action.test.ts +++ b/packages/remix/test/integration/test/server/action.test.ts @@ -63,6 +63,9 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada tags: { 'http.status_code': '500', }, + data: { + 'http.response.status_code': 500, + }, }, }, }); @@ -158,6 +161,9 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada method: 'POST', 'http.status_code': '302', }, + data: { + 'http.response.status_code': 302, + }, }, }, tags: { @@ -174,6 +180,9 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada method: 'GET', 'http.status_code': '500', }, + data: { + 'http.response.status_code': 500, + }, }, }, tags: { @@ -224,6 +233,9 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada method: 'POST', 'http.status_code': '500', }, + data: { + 'http.response.status_code': 500, + }, }, }, tags: { @@ -274,6 +286,9 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada method: 'POST', 'http.status_code': '500', }, + data: { + 'http.response.status_code': 500, + }, }, }, tags: { @@ -324,6 +339,9 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada method: 'POST', 'http.status_code': '500', }, + data: { + 'http.response.status_code': 500, + }, }, }, tags: { @@ -374,6 +392,9 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada method: 'POST', 'http.status_code': '500', }, + data: { + 'http.response.status_code': 500, + }, }, }, tags: { diff --git a/packages/remix/test/integration/test/server/loader.test.ts b/packages/remix/test/integration/test/server/loader.test.ts index 2545f63d6e92..61b3a2287256 100644 --- a/packages/remix/test/integration/test/server/loader.test.ts +++ b/packages/remix/test/integration/test/server/loader.test.ts @@ -21,6 +21,9 @@ describe.each(['builtin', 'express'])('Remix API Loaders with adapter = %s', ada tags: { 'http.status_code': '500', }, + data: { + 'http.response.status_code': 500, + }, }, }, }); @@ -95,6 +98,9 @@ describe.each(['builtin', 'express'])('Remix API Loaders with adapter = %s', ada method: 'GET', 'http.status_code': '302', }, + data: { + 'http.response.status_code': 302, + }, }, }, tags: { @@ -111,6 +117,9 @@ describe.each(['builtin', 'express'])('Remix API Loaders with adapter = %s', ada method: 'GET', 'http.status_code': '500', }, + data: { + 'http.response.status_code': 500, + }, }, }, tags: { diff --git a/packages/replay/test/fixtures/transaction.ts b/packages/replay/test/fixtures/transaction.ts index dfe0be4cfd4b..b9d6fa309fa1 100644 --- a/packages/replay/test/fixtures/transaction.ts +++ b/packages/replay/test/fixtures/transaction.ts @@ -59,6 +59,7 @@ export function Transaction(traceId?: string, obj?: Partial): any { method: 'GET', url: '/api/0/projects/sentry-emerging-tech/billy-test/replays/c11bd625b0e14081a0827a22a0a9be4e/', type: 'fetch', + 'http.response.status_code': 200, }, description: 'GET /api/0/projects/sentry-emerging-tech/billy-test/replays/c11bd625b0e14081a0827a22a0a9be4e/', op: 'http.client', diff --git a/packages/tracing/test/span.test.ts b/packages/tracing/test/span.test.ts index e642f7490589..1720dd4e6ec4 100644 --- a/packages/tracing/test/span.test.ts +++ b/packages/tracing/test/span.test.ts @@ -96,6 +96,7 @@ describe('Span', () => { span.setHttpStatus(404); expect((span.getTraceContext() as any).status).toBe('not_found'); expect(span.tags['http.status_code']).toBe('404'); + expect(span.data['http.response.status_code']).toBe(404); }); test('isSuccess', () => { From fe7330ccd8c6628ae3f45e0860900c2de00625f2 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Tue, 27 Jun 2023 15:00:02 +0200 Subject: [PATCH 2/2] Fix tests failing on unexpected http response status code --- .../integration/test/client/tracingFetch.test.ts | 1 + packages/node/test/integrations/undici.test.ts | 16 ++++++++-------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/packages/nextjs/test/integration/test/client/tracingFetch.test.ts b/packages/nextjs/test/integration/test/client/tracingFetch.test.ts index 88634c92012e..b1eb8a5f1bb8 100644 --- a/packages/nextjs/test/integration/test/client/tracingFetch.test.ts +++ b/packages/nextjs/test/integration/test/client/tracingFetch.test.ts @@ -36,6 +36,7 @@ test('should correctly instrument `fetch` for performance tracing', async ({ pag url: 'http://example.com', type: 'fetch', 'http.response_content_length': expect.any(Number), + 'http.response.status_code': 200, }, description: 'GET http://example.com', op: 'http.client', diff --git a/packages/node/test/integrations/undici.test.ts b/packages/node/test/integrations/undici.test.ts index f4925f7046a2..46756cbe88cd 100644 --- a/packages/node/test/integrations/undici.test.ts +++ b/packages/node/test/integrations/undici.test.ts @@ -56,9 +56,9 @@ conditionalTest({ min: 16 })('Undici integration', () => { { description: 'GET http://localhost:18099/', op: 'http.client', - data: { + data: expect.objectContaining({ 'http.method': 'GET', - }, + }), }, ], [ @@ -68,10 +68,10 @@ conditionalTest({ min: 16 })('Undici integration', () => { { description: 'GET http://localhost:18099/', op: 'http.client', - data: { + data: expect.objectContaining({ 'http.method': 'GET', 'http.query': '?foo=bar', - }, + }), }, ], [ @@ -80,9 +80,9 @@ conditionalTest({ min: 16 })('Undici integration', () => { { method: 'POST' }, { description: 'POST http://localhost:18099/', - data: { + data: expect.objectContaining({ 'http.method': 'POST', - }, + }), }, ], [ @@ -91,9 +91,9 @@ conditionalTest({ min: 16 })('Undici integration', () => { { method: 'POST' }, { description: 'POST http://localhost:18099/', - data: { + data: expect.objectContaining({ 'http.method': 'POST', - }, + }), }, ], [