Skip to content

Commit bc57430

Browse files
AG0708codexisaacsRulaKhaledclaude
authored
fix(deno)!: Enable sessions for HTTP requests and align option names (#23313)
Deno's `node:http` server integration disabled release-health sessions even<br>though the shared HTTP instrumentation enables them by default. Keeping the<br>shared default aligns Deno with the other server runtimes and restores session<br>aggregates for incoming requests when a release is configured. ## Root cause The Deno integration passed `sessions: false` when it subscribed to shared HTTP<br>server instrumentation. There is no Deno-specific duplicate instrumentation<br>that requires that override. Fixes #22888 --------- Signed-off-by: Abhinav Gorrepati <[email protected]> Co-authored-by: OpenAI Codex <[email protected]> Co-authored-by: isaacs <[email protected]> Co-authored-by: RulaKhaled <[email protected]> Co-authored-by: Claude Opus 5 <[email protected]>
1 parent 8966cda commit bc57430

5 files changed

Lines changed: 308 additions & 17 deletions

File tree

‎docs/migration/v11-end-state.md‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,21 @@ Sentry.httpIntegration({
542542
543543
`httpIntegration`'s `instrumentation` option is still honored for **outgoing** request spans.
544544
545+
### Deno `node:http` server requests are tracked as sessions
546+
547+
Affected SDKs: `@sentry/deno`.
548+
549+
`denoHttpIntegration` now creates [Sessions](https://docs.sentry.io/product/releases/health/#sessions) for incoming `node:http` requests, matching the other server SDKs. In v10 it disabled them unconditionally, so release health reported no session data for Deno servers. If you have a `release` configured, you will start seeing session aggregates for incoming requests. Pass `sessions: false` to restore the previous behavior:
550+
551+
```js
552+
Sentry.init({
553+
dsn: '__DSN__',
554+
integrations: [Sentry.denoHttpIntegration({ sessions: false })],
555+
});
556+
```
557+
558+
`sessionFlushingDelayMS` is also configurable now, and defaults to `60000` (60s) as in the other SDKs.
559+
545560
### Node HTTP transport `keepAlive` defaults to `true`
546561
547562
Affected SDKs: `@sentry/node` and dependents.
@@ -1268,6 +1283,33 @@ Several default integrations were renamed to match the names used by the other S
12681283
- `DenoMysql` => `Mysql`
12691284
- `DenoPostgres` => `Postgres`
12701285
1286+
### `denoHttpIntegration` incoming span hooks renamed
1287+
1288+
Affected SDKs: `@sentry/deno`.
1289+
1290+
The incoming-span hooks on `denoHttpIntegration` were renamed to match `httpIntegration` in the other server SDKs. Their arguments are typed as `HttpIncomingMessage` / `HttpServerResponse` now, instead of `unknown`.
1291+
1292+
| Removed option | Replacement |
1293+
| ----------------------- | --------------- |
1294+
| `onIncomingSpanCreated` | `onSpanCreated` |
1295+
| `onIncomingSpanEnd` | `onSpanEnd` |
1296+
1297+
```js
1298+
// before
1299+
Sentry.denoHttpIntegration({
1300+
onIncomingSpanCreated: (span, req, res) => {
1301+
span.setAttribute('custom', true);
1302+
},
1303+
});
1304+
1305+
// after
1306+
Sentry.denoHttpIntegration({
1307+
onSpanCreated: (span, req, res) => {
1308+
span.setAttribute('custom', true);
1309+
},
1310+
});
1311+
```
1312+
12711313
### `OtlpIntegration` integration renamed to `Otlp`
12721314
12731315
Affected SDKs: Server-side SDKs (`@sentry/node` and all dependents).

‎packages/deno/src/integrations/http.ts‎

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { subscribe } from 'node:diagnostics_channel';
22
import { errorMonitor } from 'node:events';
33
import type { RequestOptions } from 'node:http';
4-
import type { HttpIncomingMessage, Integration, IntegrationFn, Span } from '@sentry/core';
4+
import type { HttpIncomingMessage, HttpServerResponse, Integration, IntegrationFn, Span } from '@sentry/core';
55
import {
66
defineIntegration,
77
getHttpClientSubscriptions,
@@ -27,6 +27,21 @@ export interface DenoHttpIntegrationOptions {
2727
*/
2828
spans?: boolean;
2929

30+
/**
31+
* Whether the integration should create [Sessions](https://docs.sentry.io/product/releases/health/#sessions) for
32+
* incoming requests to track the health and crash-free rate of your releases in Sentry.
33+
*
34+
* @default `true`
35+
*/
36+
sessions?: boolean;
37+
38+
/**
39+
* Number of milliseconds until sessions are flushed as a session aggregate.
40+
*
41+
* @default `60000` (60s)
42+
*/
43+
sessionFlushingDelayMS?: number;
44+
3045
/**
3146
* Whether to inject trace propagation headers (sentry-trace, baggage) into outgoing HTTP requests.
3247
*
@@ -77,14 +92,15 @@ export interface DenoHttpIntegrationOptions {
7792
ignoreOutgoingRequests?: (url: string, request: RequestOptions) => boolean;
7893

7994
/**
80-
* Hook invoked after the server span is created but before the request is handled.
95+
* A hook that can be used to mutate the span for incoming requests.
96+
* This is triggered after the span is created, but before it is recorded.
8197
*/
82-
onIncomingSpanCreated?: (span: Span, request: unknown, response: unknown) => void;
98+
onSpanCreated?: (span: Span, request: HttpIncomingMessage, response: HttpServerResponse) => void;
8399

84100
/**
85-
* Hook invoked when the server span ends, before it is recorded.
101+
* A hook that can be used to mutate the span one last time when the response is finished.
86102
*/
87-
onIncomingSpanEnd?: (span: Span, request: unknown, response: unknown) => void;
103+
onSpanEnd?: (span: Span, request: HttpIncomingMessage, response: HttpServerResponse) => void;
88104
}
89105

90106
const _denoHttpIntegration = ((options: DenoHttpIntegrationOptions = {}) => {
@@ -95,21 +111,13 @@ const _denoHttpIntegration = ((options: DenoHttpIntegrationOptions = {}) => {
95111
name: INTEGRATION_NAME,
96112
setupOnce() {
97113
const { [HTTP_ON_SERVER_REQUEST]: onHttpServerRequest } = getHttpServerSubscriptions({
98-
// `spans` falls through to the client's tracing config when unset.
99-
spans: options.spans,
100-
ignoreStaticAssets: options.ignoreStaticAssets,
101-
ignoreIncomingRequests: options.ignoreIncomingRequests,
102-
maxRequestBodySize: options.maxRequestBodySize,
103-
ignoreRequestBody: options.ignoreRequestBody,
104-
onSpanCreated: options.onIncomingSpanCreated,
105-
onSpanEnd: options.onIncomingSpanEnd,
114+
...options,
106115
errorMonitor,
107-
sessions: false,
108116
});
109117
subscribe(HTTP_ON_SERVER_REQUEST, onHttpServerRequest);
110118

111119
const { [HTTP_ON_CLIENT_REQUEST]: onHttpClientRequest } = getHttpClientSubscriptions({
112-
spans: options.spans,
120+
...options,
113121
breadcrumbs,
114122
propagateTrace: tracePropagation,
115123
ignoreOutgoingRequests: options.ignoreOutgoingRequests
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
// <reference lib="deno.ns" />
2+
3+
/**
4+
* Lives in its own file because `setupOnce` runs once per process
5+
* (`installedIntegrations` guards it) and the diagnostics channel
6+
* subscription is global. Deno gives each test file a fresh module graph,
7+
* so this is the only way to install `denoHttpIntegration` with
8+
* non-default options after `deno-http.test.ts` has installed it with
9+
* the defaults.
10+
*/
11+
12+
import * as http from 'node:http';
13+
import type { Envelope } from '@sentry/core';
14+
import { forEachEnvelopeItem, getIsolationScope, getMainCarrier } from '@sentry/core';
15+
import { assert } from 'https://deno.land/[email protected]/assert/assert.ts';
16+
import { assertEquals } from 'https://deno.land/[email protected]/assert/assert_equals.ts';
17+
import { denoHttpIntegration, init } from '../build/esm/index.js';
18+
import { makeTestTransport } from './transport.ts';
19+
20+
Deno.test({
21+
name: 'denoHttpIntegration: node:http incoming request records no session when sessions: false',
22+
async fn() {
23+
getMainCarrier().__SENTRY__ = undefined;
24+
25+
const envelopes: Envelope[] = [];
26+
const client = init({
27+
dsn: 'https://username@domain/123',
28+
release: '1.0.0',
29+
integrations: [denoHttpIntegration({ sessions: false })],
30+
transport: makeTestTransport(envelope => {
31+
envelopes.push(envelope);
32+
}),
33+
});
34+
35+
// Captured inside the handler so we can tell "sessions were disabled"
36+
// apart from "the request was never instrumented at all".
37+
let isolatedTransactionName: string | undefined;
38+
const server = http.createServer((_req, res) => {
39+
isolatedTransactionName = getIsolationScope().getScopeData().transactionName;
40+
res.end('ok');
41+
});
42+
const port: number = await new Promise(resolve => {
43+
server.listen(0, '127.0.0.1', () => {
44+
resolve((server.address() as { port: number }).port);
45+
});
46+
});
47+
48+
const response = await fetch(`http://127.0.0.1:${port}/health`);
49+
assertEquals(await response.text(), 'ok');
50+
await new Promise<void>(resolve => server.close(() => resolve()));
51+
await client.flush(2_000);
52+
53+
const itemTypes: string[] = [];
54+
for (const envelope of envelopes) {
55+
forEachEnvelopeItem(envelope, ([headers]) => {
56+
itemTypes.push(headers.type);
57+
});
58+
}
59+
60+
assertEquals(isolatedTransactionName, 'GET /health');
61+
assert(!itemTypes.includes('sessions'), `expected no session envelope item, got: ${itemTypes.join(', ')}`);
62+
assert(!itemTypes.includes('session'), `expected no session envelope item, got: ${itemTypes.join(', ')}`);
63+
},
64+
});
Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
// <reference lib="deno.ns" />
2+
3+
/**
4+
* Lives in its own file because `setupOnce` runs once per process
5+
* (`installedIntegrations` guards it) and the diagnostics channel
6+
* subscription is global. Deno gives each test file a fresh module graph,
7+
* so this is the only way to install `denoHttpIntegration` with
8+
* `spans: false` after another file has installed it with the defaults.
9+
*
10+
* Both tests below share that single `spans: false` subscription.
11+
*/
12+
13+
import * as http from 'node:http';
14+
import type { TransactionEvent } from '@sentry/core';
15+
import { getIsolationScope, getMainCarrier } from '@sentry/core';
16+
import { assert } from 'https://deno.land/[email protected]/assert/assert.ts';
17+
import { assertEquals } from 'https://deno.land/[email protected]/assert/assert_equals.ts';
18+
import type { DenoClient } from '../build/esm/index.js';
19+
import { denoHttpIntegration, init, startSpan } from '../build/esm/index.js';
20+
21+
/**
22+
* `spans: false` must win over `tracesSampleRate: 1`, so tracing is on
23+
* everywhere except the HTTP integration. Without it the option would be
24+
* indistinguishable from tracing being off.
25+
*/
26+
function initWithSpansDisabled(transactions: TransactionEvent[]): DenoClient {
27+
getMainCarrier().__SENTRY__ = undefined;
28+
return init({
29+
dsn: 'https://username@domain/123',
30+
tracesSampleRate: 1,
31+
traceLifecycle: 'static',
32+
integrations: [denoHttpIntegration({ spans: false })],
33+
beforeSendTransaction: (event: TransactionEvent) => {
34+
transactions.push(event);
35+
return null;
36+
},
37+
}) as DenoClient;
38+
}
39+
40+
Deno.test({
41+
name: 'denoHttpIntegration: node:http outgoing request creates no http.client span when spans: false',
42+
async fn() {
43+
const transactions: TransactionEvent[] = [];
44+
const client = initWithSpansDisabled(transactions);
45+
46+
// Deno.serve for the target so this does not depend on the node:http
47+
// server instrumentation.
48+
const abortController = new AbortController();
49+
let onListen: ((_: unknown) => void) | undefined;
50+
const listening = new Promise(resolve => (onListen = resolve));
51+
// Captured so we can tell "spans were disabled" apart from "the client
52+
// was never instrumented at all" -- header injection survives spans: false.
53+
let sentryTraceHeader: string | null = null;
54+
const target = Deno.serve(
55+
{ port: 0, signal: abortController.signal, onListen, hostname: '127.0.0.1' },
56+
(request: Request) => {
57+
sentryTraceHeader = request.headers.get('sentry-trace');
58+
return new Response('pong');
59+
},
60+
);
61+
await listening;
62+
63+
await startSpan({ name: 'parent', op: 'test' }, async () => {
64+
await new Promise<void>((resolve, reject) => {
65+
const req = http.request({ host: '127.0.0.1', port: target.addr.port, path: '/ping', method: 'GET' }, res => {
66+
res.on('data', () => {});
67+
res.on('end', () => resolve());
68+
res.on('error', reject);
69+
});
70+
req.on('error', reject);
71+
req.end();
72+
});
73+
});
74+
75+
abortController.abort();
76+
await target.finished;
77+
78+
// Event capture runs through the client's async processing queue, so
79+
// drain it before reading the sink -- otherwise these assertions race.
80+
await client.flush(5_000);
81+
82+
// The parent span proves tracing itself is live, so an absent
83+
// http.client span is the option working rather than tracing being off.
84+
assert(sentryTraceHeader, 'expected an injected sentry-trace header, so the client was instrumented');
85+
const parent = transactions.find(t => t.transaction === 'parent');
86+
assert(parent, `expected the 'parent' transaction, got: ${transactions.map(t => t.transaction).join(', ')}`);
87+
const childOps = parent!.spans?.map(s => s.op) ?? [];
88+
assertEquals(
89+
childOps.includes('http.client'),
90+
false,
91+
`expected no http.client span, got ops: ${childOps.join(', ')}`,
92+
);
93+
},
94+
});
95+
96+
Deno.test({
97+
name: 'denoHttpIntegration: node:http incoming request creates no http.server transaction when spans: false',
98+
async fn() {
99+
const transactions: TransactionEvent[] = [];
100+
const client = initWithSpansDisabled(transactions);
101+
102+
// Captured inside the handler so we can tell "spans were disabled" apart
103+
// from "the request was never instrumented at all".
104+
let isolatedTransactionName: string | undefined;
105+
const server = http.createServer((_req, res) => {
106+
isolatedTransactionName = getIsolationScope().getScopeData().transactionName;
107+
res.end('ok');
108+
});
109+
const port: number = await new Promise(resolve => {
110+
server.listen(0, '127.0.0.1', () => {
111+
resolve((server.address() as { port: number }).port);
112+
});
113+
});
114+
115+
const response = await fetch(`http://127.0.0.1:${port}/users/42`);
116+
assertEquals(await response.text(), 'ok');
117+
await new Promise<void>(resolve => server.close(() => resolve()));
118+
119+
// Drain the async processing queue first. Without this, "no http.server
120+
// transaction yet" and "no http.server transaction at all" look alike,
121+
// so the assertion below could pass while spans were still enabled.
122+
await client.flush(5_000);
123+
124+
// Request isolation still runs with spans off, so this proves the
125+
// instrumentation saw the request.
126+
assertEquals(isolatedTransactionName, 'GET /users/42');
127+
const ops = transactions.map(t => t.contexts?.trace?.op);
128+
assertEquals(ops.includes('http.server'), false, `expected no http.server transaction, got ops: ${ops.join(', ')}`);
129+
},
130+
});

‎packages/deno/test/deno-http.test.ts‎

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
// <reference lib="deno.ns" />
22

33
import * as http from 'node:http';
4-
import type { TransactionEvent } from '@sentry/core';
5-
import { getMainCarrier } from '@sentry/core';
4+
import type { Envelope, SessionAggregates, TransactionEvent } from '@sentry/core';
5+
import { forEachEnvelopeItem, getMainCarrier } from '@sentry/core';
66
import { assert } from 'https://deno.land/[email protected]/assert/assert.ts';
77
import { assertEquals } from 'https://deno.land/[email protected]/assert/assert_equals.ts';
88
import { assertExists } from 'https://deno.land/[email protected]/assert/assert_exists.ts';
99
import type { DenoClient } from '../build/esm/index.js';
1010
import { init, startSpan } from '../build/esm/index.js';
11+
import { makeTestTransport } from './transport.ts';
1112

1213
function resetGlobals(): void {
1314
getMainCarrier().__SENTRY__ = undefined;
@@ -110,6 +111,52 @@ Deno.test({
110111
},
111112
});
112113

114+
Deno.test({
115+
name: 'denoHttpIntegration: node:http incoming request records a release-health session by default',
116+
async fn() {
117+
resetGlobals();
118+
const envelopes: Envelope[] = [];
119+
const client = init({
120+
dsn: 'https://username@domain/123',
121+
release: '1.0.0',
122+
transport: makeTestTransport(envelope => {
123+
envelopes.push(envelope);
124+
}),
125+
});
126+
127+
const server = http.createServer((_req, res) => {
128+
res.end('ok');
129+
});
130+
const port: number = await new Promise(resolve => {
131+
server.listen(0, '127.0.0.1', () => {
132+
resolve((server.address() as { port: number }).port);
133+
});
134+
});
135+
136+
const response = await fetch(`http://127.0.0.1:${port}/health`);
137+
assertEquals(await response.text(), 'ok');
138+
await new Promise<void>(resolve => server.close(() => resolve()));
139+
await client.flush(2_000);
140+
141+
let sessionAggregates: SessionAggregates | undefined;
142+
for (const envelope of envelopes) {
143+
forEachEnvelopeItem(envelope, item => {
144+
const [headers, body] = item;
145+
if (headers.type === 'sessions') {
146+
sessionAggregates = body as SessionAggregates;
147+
}
148+
});
149+
}
150+
151+
assertExists(sessionAggregates);
152+
assertEquals(sessionAggregates.attrs?.release, '1.0.0');
153+
assertEquals(sessionAggregates.aggregates.length, 1);
154+
assertEquals(sessionAggregates.aggregates[0]?.exited, 1);
155+
assertEquals(sessionAggregates.aggregates[0]?.errored, 0);
156+
assertEquals(sessionAggregates.aggregates[0]?.crashed, 0);
157+
},
158+
});
159+
113160
Deno.test({
114161
name: 'denoHttpIntegration: node:http outgoing request creates a child http.client span',
115162
async fn() {

0 commit comments

Comments
 (0)