Skip to content

Commit 4b72b41

Browse files
isaacsisaacs
authored
feat(core, node): portable Express integration (#19928)
This extracts the functionality from the OTel Express intstrumentation, replacing it with a portable standalone integration in `@sentry/core`, which can be extended and applied to patch any Express module import in whatever way makes sense for the platform in question. Currently in node, that is still an OpenTelemetry intstrumentation, but just handling the automatic module load instrumentation, not the entire tracing integration. This is somewhat a proof of concept, to see what it takes to port a fairly invovled OTel integration into a state where it can support all of the platforms that we care about, but it does impose a bit less of a translation layer between OTel and Sentry semantics (for example, no need to use the no-op `span.recordException()`). User-visible changes (beyond the added export in `@sentry/core`): * Express router spans have an origin of `auto.http.express` rather than `auto.http.otel.express`, since it's no longer technically an otel integration. * The empty `measurements: {}` object is no longer attached to span data, as that was an artifact of otel's `span.recordError`, which is a no-op anyway, and no longer executed. Obviously this is not a full clean-room reimplementation, and relies on the fact that the opentelemetry-js-contrib project is Apache 2.0 licensed. I included the link to the upstream license in the index file for the Express integration, but there may be a more appropriate way to ensure that the license is respected properly. It was arguably a derivative work already, but simple redistribution is somewhat different than re-implementation with subtly different context. This reduces the node overhead and makes the Express instrumentation portable to other SDKs, but it of course *increases* the bundle size of `@sentry/core` considerably. It would be a good idea to explore splitting out integrations from core, so that they're bundled and analyzed separately, rather than shipping to all SDKs that extend core. Closes [JS-1982](https://linear.app/getsentry/issue/JS-1982/featcore-node-portable-express-integration)
1 parent 354ac78 commit 4b72b41

File tree

37 files changed

+2857
-280
lines changed

37 files changed

+2857
-280
lines changed

‎dev-packages/e2e-tests/test-applications/nestjs-11/tests/transactions.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ test('Sends an API route transaction', async ({ baseURL }) => {
6161
'express.name': '/test-transaction',
6262
'express.type': 'request_handler',
6363
'http.route': '/test-transaction',
64-
'sentry.origin': 'auto.http.otel.express',
64+
'sentry.origin': 'auto.http.express',
6565
'sentry.op': 'request_handler.express',
6666
},
6767
op: 'request_handler.express',
@@ -72,7 +72,7 @@ test('Sends an API route transaction', async ({ baseURL }) => {
7272
status: 'ok',
7373
timestamp: expect.any(Number),
7474
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
75-
origin: 'auto.http.otel.express',
75+
origin: 'auto.http.express',
7676
},
7777
{
7878
data: {

‎dev-packages/e2e-tests/test-applications/nestjs-8/tests/transactions.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ test('Sends an API route transaction', async ({ baseURL }) => {
6565
'express.name': '/test-transaction',
6666
'express.type': 'request_handler',
6767
'http.route': '/test-transaction',
68-
'sentry.origin': 'auto.http.otel.express',
68+
'sentry.origin': 'auto.http.express',
6969
'sentry.op': 'request_handler.express',
7070
},
7171
op: 'request_handler.express',
@@ -76,7 +76,7 @@ test('Sends an API route transaction', async ({ baseURL }) => {
7676
status: 'ok',
7777
timestamp: expect.any(Number),
7878
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
79-
origin: 'auto.http.otel.express',
79+
origin: 'auto.http.express',
8080
},
8181
{
8282
data: {

‎dev-packages/e2e-tests/test-applications/nestjs-basic/tests/transactions.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ test('Sends an API route transaction', async ({ baseURL }) => {
6161
'express.name': '/test-transaction',
6262
'express.type': 'request_handler',
6363
'http.route': '/test-transaction',
64-
'sentry.origin': 'auto.http.otel.express',
64+
'sentry.origin': 'auto.http.express',
6565
'sentry.op': 'request_handler.express',
6666
},
6767
op: 'request_handler.express',
@@ -72,7 +72,7 @@ test('Sends an API route transaction', async ({ baseURL }) => {
7272
status: 'ok',
7373
timestamp: expect.any(Number),
7474
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
75-
origin: 'auto.http.otel.express',
75+
origin: 'auto.http.express',
7676
},
7777
{
7878
data: {

‎dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/transactions.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ test('Sends an API route transaction from module', async ({ baseURL }) => {
6161
'express.name': '/example-module/transaction',
6262
'express.type': 'request_handler',
6363
'http.route': '/example-module/transaction',
64-
'sentry.origin': 'auto.http.otel.express',
64+
'sentry.origin': 'auto.http.express',
6565
'sentry.op': 'request_handler.express',
6666
},
6767
op: 'request_handler.express',
@@ -72,7 +72,7 @@ test('Sends an API route transaction from module', async ({ baseURL }) => {
7272
status: 'ok',
7373
timestamp: expect.any(Number),
7474
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
75-
origin: 'auto.http.otel.express',
75+
origin: 'auto.http.express',
7676
},
7777
{
7878
data: {

‎dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ test('Sends an API route transaction from module', async ({ baseURL }) => {
6161
'express.name': '/example-module/transaction',
6262
'express.type': 'request_handler',
6363
'http.route': '/example-module/transaction',
64-
'sentry.origin': 'auto.http.otel.express',
64+
'sentry.origin': 'auto.http.express',
6565
'sentry.op': 'request_handler.express',
6666
},
6767
op: 'request_handler.express',
@@ -72,7 +72,7 @@ test('Sends an API route transaction from module', async ({ baseURL }) => {
7272
status: 'ok',
7373
timestamp: expect.any(Number),
7474
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
75-
origin: 'auto.http.otel.express',
75+
origin: 'auto.http.express',
7676
},
7777
{
7878
data: {

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,12 @@ test('Should record a transaction for route with parameters', async ({ request }
6565
data: {
6666
'express.name': 'query',
6767
'express.type': 'middleware',
68-
'sentry.origin': 'auto.http.otel.express',
68+
'sentry.origin': 'auto.http.express',
6969
'sentry.op': 'middleware.express',
7070
},
7171
op: 'middleware.express',
7272
description: 'query',
73-
origin: 'auto.http.otel.express',
73+
origin: 'auto.http.express',
7474
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
7575
span_id: expect.stringMatching(/[a-f0-9]{16}/),
7676
start_timestamp: expect.any(Number),
@@ -83,12 +83,12 @@ test('Should record a transaction for route with parameters', async ({ request }
8383
data: {
8484
'express.name': 'expressInit',
8585
'express.type': 'middleware',
86-
'sentry.origin': 'auto.http.otel.express',
86+
'sentry.origin': 'auto.http.express',
8787
'sentry.op': 'middleware.express',
8888
},
8989
op: 'middleware.express',
9090
description: 'expressInit',
91-
origin: 'auto.http.otel.express',
91+
origin: 'auto.http.express',
9292
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
9393
span_id: expect.stringMatching(/[a-f0-9]{16}/),
9494
start_timestamp: expect.any(Number),
@@ -102,12 +102,12 @@ test('Should record a transaction for route with parameters', async ({ request }
102102
'express.name': '/test-transaction/:param',
103103
'express.type': 'request_handler',
104104
'http.route': '/test-transaction/:param',
105-
'sentry.origin': 'auto.http.otel.express',
105+
'sentry.origin': 'auto.http.express',
106106
'sentry.op': 'request_handler.express',
107107
},
108108
op: 'request_handler.express',
109109
description: '/test-transaction/:param',
110-
origin: 'auto.http.otel.express',
110+
origin: 'auto.http.express',
111111
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
112112
span_id: expect.stringMatching(/[a-f0-9]{16}/),
113113
start_timestamp: expect.any(Number),

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,12 @@ test('Should record a transaction for route with parameters', async ({ request }
6565
data: {
6666
'express.name': 'query',
6767
'express.type': 'middleware',
68-
'sentry.origin': 'auto.http.otel.express',
68+
'sentry.origin': 'auto.http.express',
6969
'sentry.op': 'middleware.express',
7070
},
7171
op: 'middleware.express',
7272
description: 'query',
73-
origin: 'auto.http.otel.express',
73+
origin: 'auto.http.express',
7474
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
7575
span_id: expect.stringMatching(/[a-f0-9]{16}/),
7676
start_timestamp: expect.any(Number),
@@ -83,12 +83,12 @@ test('Should record a transaction for route with parameters', async ({ request }
8383
data: {
8484
'express.name': 'expressInit',
8585
'express.type': 'middleware',
86-
'sentry.origin': 'auto.http.otel.express',
86+
'sentry.origin': 'auto.http.express',
8787
'sentry.op': 'middleware.express',
8888
},
8989
op: 'middleware.express',
9090
description: 'expressInit',
91-
origin: 'auto.http.otel.express',
91+
origin: 'auto.http.express',
9292
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
9393
span_id: expect.stringMatching(/[a-f0-9]{16}/),
9494
start_timestamp: expect.any(Number),
@@ -102,12 +102,12 @@ test('Should record a transaction for route with parameters', async ({ request }
102102
'express.name': '/test-transaction/:param',
103103
'express.type': 'request_handler',
104104
'http.route': '/test-transaction/:param',
105-
'sentry.origin': 'auto.http.otel.express',
105+
'sentry.origin': 'auto.http.express',
106106
'sentry.op': 'request_handler.express',
107107
},
108108
op: 'request_handler.express',
109109
description: '/test-transaction/:param',
110-
origin: 'auto.http.otel.express',
110+
origin: 'auto.http.express',
111111
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
112112
span_id: expect.stringMatching(/[a-f0-9]{16}/),
113113
start_timestamp: expect.any(Number),

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,12 @@ test('Should record a transaction for route with parameters', async ({ request }
6565
data: {
6666
'express.name': 'query',
6767
'express.type': 'middleware',
68-
'sentry.origin': 'auto.http.otel.express',
68+
'sentry.origin': 'auto.http.express',
6969
'sentry.op': 'middleware.express',
7070
},
7171
op: 'middleware.express',
7272
description: 'query',
73-
origin: 'auto.http.otel.express',
73+
origin: 'auto.http.express',
7474
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
7575
span_id: expect.stringMatching(/[a-f0-9]{16}/),
7676
start_timestamp: expect.any(Number),
@@ -83,12 +83,12 @@ test('Should record a transaction for route with parameters', async ({ request }
8383
data: {
8484
'express.name': 'expressInit',
8585
'express.type': 'middleware',
86-
'sentry.origin': 'auto.http.otel.express',
86+
'sentry.origin': 'auto.http.express',
8787
'sentry.op': 'middleware.express',
8888
},
8989
op: 'middleware.express',
9090
description: 'expressInit',
91-
origin: 'auto.http.otel.express',
91+
origin: 'auto.http.express',
9292
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
9393
span_id: expect.stringMatching(/[a-f0-9]{16}/),
9494
start_timestamp: expect.any(Number),
@@ -102,12 +102,12 @@ test('Should record a transaction for route with parameters', async ({ request }
102102
'express.name': '/test-transaction/:param',
103103
'express.type': 'request_handler',
104104
'http.route': '/test-transaction/:param',
105-
'sentry.origin': 'auto.http.otel.express',
105+
'sentry.origin': 'auto.http.express',
106106
'sentry.op': 'request_handler.express',
107107
},
108108
op: 'request_handler.express',
109109
description: '/test-transaction/:param',
110-
origin: 'auto.http.otel.express',
110+
origin: 'auto.http.express',
111111
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
112112
span_id: expect.stringMatching(/[a-f0-9]{16}/),
113113
start_timestamp: expect.any(Number),

‎dev-packages/e2e-tests/test-applications/node-express-v5/tests/transactions.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,15 +85,15 @@ test('Sends an API route transaction', async ({ baseURL }) => {
8585
// auto instrumented span
8686
expect(spans).toContainEqual({
8787
data: {
88-
'sentry.origin': 'auto.http.otel.express',
88+
'sentry.origin': 'auto.http.express',
8989
'sentry.op': 'request_handler.express',
9090
'http.route': '/test-transaction',
9191
'express.name': '/test-transaction',
9292
'express.type': 'request_handler',
9393
},
9494
description: '/test-transaction',
9595
op: 'request_handler.express',
96-
origin: 'auto.http.otel.express',
96+
origin: 'auto.http.express',
9797
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
9898
span_id: expect.stringMatching(/[a-f0-9]{16}/),
9999
start_timestamp: expect.any(Number),

‎dev-packages/e2e-tests/test-applications/node-express/tests/transactions.test.ts‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -85,14 +85,14 @@ test('Sends an API route transaction', async ({ baseURL }) => {
8585
// auto instrumented spans
8686
expect(spans).toContainEqual({
8787
data: {
88-
'sentry.origin': 'auto.http.otel.express',
88+
'sentry.origin': 'auto.http.express',
8989
'sentry.op': 'middleware.express',
9090
'express.name': 'query',
9191
'express.type': 'middleware',
9292
},
9393
description: 'query',
9494
op: 'middleware.express',
95-
origin: 'auto.http.otel.express',
95+
origin: 'auto.http.express',
9696
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
9797
span_id: expect.stringMatching(/[a-f0-9]{16}/),
9898
start_timestamp: expect.any(Number),
@@ -103,14 +103,14 @@ test('Sends an API route transaction', async ({ baseURL }) => {
103103

104104
expect(spans).toContainEqual({
105105
data: {
106-
'sentry.origin': 'auto.http.otel.express',
106+
'sentry.origin': 'auto.http.express',
107107
'sentry.op': 'middleware.express',
108108
'express.name': 'expressInit',
109109
'express.type': 'middleware',
110110
},
111111
description: 'expressInit',
112112
op: 'middleware.express',
113-
origin: 'auto.http.otel.express',
113+
origin: 'auto.http.express',
114114
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
115115
span_id: expect.stringMatching(/[a-f0-9]{16}/),
116116
start_timestamp: expect.any(Number),
@@ -121,15 +121,15 @@ test('Sends an API route transaction', async ({ baseURL }) => {
121121

122122
expect(spans).toContainEqual({
123123
data: {
124-
'sentry.origin': 'auto.http.otel.express',
124+
'sentry.origin': 'auto.http.express',
125125
'sentry.op': 'request_handler.express',
126126
'http.route': '/test-transaction',
127127
'express.name': '/test-transaction',
128128
'express.type': 'request_handler',
129129
},
130130
description: '/test-transaction',
131131
op: 'request_handler.express',
132-
origin: 'auto.http.otel.express',
132+
origin: 'auto.http.express',
133133
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
134134
span_id: expect.stringMatching(/[a-f0-9]{16}/),
135135
start_timestamp: expect.any(Number),
@@ -161,14 +161,14 @@ test('Sends an API route transaction for an errored route', async ({ baseURL })
161161

162162
expect(spans).toContainEqual({
163163
data: {
164-
'sentry.origin': 'auto.http.otel.express',
164+
'sentry.origin': 'auto.http.express',
165165
'sentry.op': 'middleware.express',
166166
'express.name': 'query',
167167
'express.type': 'middleware',
168168
},
169169
description: 'query',
170170
op: 'middleware.express',
171-
origin: 'auto.http.otel.express',
171+
origin: 'auto.http.express',
172172
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
173173
span_id: expect.stringMatching(/[a-f0-9]{16}/),
174174
start_timestamp: expect.any(Number),
@@ -179,14 +179,14 @@ test('Sends an API route transaction for an errored route', async ({ baseURL })
179179

180180
expect(spans).toContainEqual({
181181
data: {
182-
'sentry.origin': 'auto.http.otel.express',
182+
'sentry.origin': 'auto.http.express',
183183
'sentry.op': 'middleware.express',
184184
'express.name': 'expressInit',
185185
'express.type': 'middleware',
186186
},
187187
description: 'expressInit',
188188
op: 'middleware.express',
189-
origin: 'auto.http.otel.express',
189+
origin: 'auto.http.express',
190190
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
191191
span_id: expect.stringMatching(/[a-f0-9]{16}/),
192192
start_timestamp: expect.any(Number),
@@ -197,22 +197,21 @@ test('Sends an API route transaction for an errored route', async ({ baseURL })
197197

198198
expect(spans).toContainEqual({
199199
data: {
200-
'sentry.origin': 'auto.http.otel.express',
200+
'sentry.origin': 'auto.http.express',
201201
'sentry.op': 'request_handler.express',
202202
'http.route': '/test-exception/:id',
203203
'express.name': '/test-exception/:id',
204204
'express.type': 'request_handler',
205205
},
206206
description: '/test-exception/:id',
207207
op: 'request_handler.express',
208-
origin: 'auto.http.otel.express',
208+
origin: 'auto.http.express',
209209
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
210210
span_id: expect.stringMatching(/[a-f0-9]{16}/),
211211
start_timestamp: expect.any(Number),
212212
status: 'internal_error',
213213
timestamp: expect.any(Number),
214214
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
215-
measurements: {},
216215
});
217216
});
218217

0 commit comments

Comments
 (0)