Skip to content

Commit a8efc9a

Browse files
committed
actually fix it with a hook
1 parent f8751c8 commit a8efc9a

File tree

5 files changed

+180
-194
lines changed

5 files changed

+180
-194
lines changed

packages/core/src/client.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import type { Integration } from './types-hoist/integration';
2626
import type { Log } from './types-hoist/log';
2727
import type { ClientOptions } from './types-hoist/options';
2828
import type { ParameterizedString } from './types-hoist/parameterize';
29+
import type { RequestEventData } from './types-hoist/request';
2930
import type { SdkMetadata } from './types-hoist/sdkmetadata';
3031
import type { Session, SessionAggregates } from './types-hoist/session';
3132
import type { SeverityLevel } from './types-hoist/severity';
@@ -687,6 +688,17 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
687688
*/
688689
public on(hook: 'flushLogs', callback: () => void): () => void;
689690

691+
/**
692+
* A hook that is called when a http server request is started.
693+
* This hook is called after request isolation, but before the request is processed.
694+
*
695+
* @returns {() => void} A function that, when executed, removes the registered callback.
696+
*/
697+
public on(
698+
hook: 'httpServerRequest',
699+
callback: (request: unknown, response: unknown, normalizedRequest: RequestEventData) => void,
700+
): () => void;
701+
690702
/**
691703
* Register a hook on this client.
692704
*/
@@ -875,6 +887,17 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
875887
*/
876888
public emit(hook: 'flushLogs'): void;
877889

890+
/**
891+
* Emit a hook event for client when a http server request is started.
892+
* This hook is called after request isolation, but before the request is processed.
893+
*/
894+
public emit(
895+
hook: 'httpServerRequest',
896+
request: unknown,
897+
response: unknown,
898+
normalizedRequest: RequestEventData,
899+
): void;
900+
878901
/**
879902
* Emit a hook that was previously registered via `on()`.
880903
*/

packages/node-core/src/integrations/http/httpServerIntegration.ts

Lines changed: 23 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
/* eslint-disable max-lines */
21
import type { ChannelListener } from 'node:diagnostics_channel';
32
import { subscribe } from 'node:diagnostics_channel';
43
import type { EventEmitter } from 'node:events';
54
import type { IncomingMessage, RequestOptions, Server, ServerResponse } from 'node:http';
65
import type { Socket } from 'node:net';
76
import { context, createContextKey, propagation } from '@opentelemetry/api';
8-
import type { AggregationCounts, Client, Integration, IntegrationFn, RequestEventData, Scope } from '@sentry/core';
7+
import type { AggregationCounts, Client, Integration, IntegrationFn, Scope } from '@sentry/core';
98
import {
9+
addNonEnumerableProperty,
1010
debug,
1111
generateSpanId,
1212
getClient,
@@ -22,17 +22,14 @@ import { MAX_BODY_BYTE_LENGTH } from './constants';
2222

2323
type ServerEmit = typeof Server.prototype.emit;
2424

25+
type StartSpanCallback = (next: () => boolean) => boolean;
26+
type RequestWithOptionalStartSpanCallback = IncomingMessage & {
27+
_startSpanCallback?: StartSpanCallback;
28+
};
29+
2530
const HTTP_SERVER_INSTRUMENTED_KEY = createContextKey('sentry_http_server_instrumented');
2631
const INTEGRATION_NAME = 'Http.Server';
2732

28-
interface ServerCallbackOptions {
29-
request: IncomingMessage;
30-
response: ServerResponse;
31-
normalizedRequest: RequestEventData;
32-
}
33-
34-
type ServerCallback = (fn: () => boolean, options: ServerCallbackOptions) => boolean;
35-
3633
const clientToRequestSessionAggregatesMap = new Map<
3734
Client,
3835
{ [timestampRoundedToSeconds: string]: { exited: number; crashed: number; errored: number } }
@@ -86,6 +83,14 @@ export interface HttpServerIntegrationOptions {
8683
maxRequestBodySize?: 'none' | 'small' | 'medium' | 'always';
8784
}
8885

86+
/**
87+
* Add a callback to the request object that will be called when the request is started.
88+
* The callback will receive the next function to continue processing the request.
89+
*/
90+
export function addStartSpanCallback(request: RequestWithOptionalStartSpanCallback, callback: StartSpanCallback): void {
91+
addNonEnumerableProperty(request, '_startSpanCallback', callback);
92+
}
93+
8994
const _httpServerIntegration = ((options: HttpServerIntegrationOptions = {}) => {
9095
const _options = {
9196
sessions: options.sessions ?? true,
@@ -94,22 +99,17 @@ const _httpServerIntegration = ((options: HttpServerIntegrationOptions = {}) =>
9499
ignoreRequestBody: options.ignoreRequestBody,
95100
};
96101

97-
const serverCallbacks: ServerCallback[] = [];
98-
99102
return {
100103
name: INTEGRATION_NAME,
101104
setupOnce() {
102105
const onHttpServerRequestStart = ((_data: unknown) => {
103106
const data = _data as { server: Server };
104107

105-
instrumentServer(data.server, _options, serverCallbacks);
108+
instrumentServer(data.server, _options);
106109
}) satisfies ChannelListener;
107110

108111
subscribe('http.server.request.start', onHttpServerRequestStart);
109112
},
110-
addServerCallback(callback: ServerCallback) {
111-
serverCallbacks.push(callback);
112-
},
113113
afterAllSetup(client) {
114114
if (DEBUG_BUILD && client.getIntegrationByName('Http')) {
115115
debug.warn(
@@ -129,7 +129,6 @@ export const httpServerIntegration = _httpServerIntegration as (
129129
) => Integration & {
130130
name: 'HttpServer';
131131
setupOnce: () => void;
132-
addServerCallback: (callback: ServerCallback) => void;
133132
};
134133

135134
/**
@@ -149,7 +148,6 @@ function instrumentServer(
149148
sessions: boolean;
150149
sessionFlushingDelayMS: number;
151150
},
152-
serverCallbacks: ServerCallback[],
153151
): void {
154152
// eslint-disable-next-line @typescript-eslint/unbound-method
155153
const originalEmit: ServerEmit = server.emit;
@@ -221,15 +219,14 @@ function instrumentServer(
221219
.setValue(HTTP_SERVER_INSTRUMENTED_KEY, true);
222220

223221
return context.with(ctx, () => {
224-
if (serverCallbacks?.length) {
225-
return wrapInCallbacks(
226-
() => target.apply(thisArg, args),
227-
{ request, response, normalizedRequest },
228-
serverCallbacks.slice(),
229-
);
230-
} else {
231-
return target.apply(thisArg, args);
222+
// This is used (optionally) by the httpServerSpansIntegration to attach _startSpanCallback to the request object
223+
client.emit('httpServerRequest', request, response, normalizedRequest);
224+
225+
const callback = (request as RequestWithOptionalStartSpanCallback)._startSpanCallback;
226+
if (callback) {
227+
return callback(() => target.apply(thisArg, args));
232228
}
229+
return target.apply(thisArg, args);
233230
});
234231
});
235232
},
@@ -432,18 +429,3 @@ function patchRequestToCaptureBody(
432429
}
433430
}
434431
}
435-
436-
// Wrap a fn in one or multiple callbacks
437-
function wrapInCallbacks(
438-
fn: () => boolean,
439-
options: ServerCallbackOptions,
440-
callbacks: ((fn: () => boolean, options: ServerCallbackOptions) => boolean)[],
441-
): boolean {
442-
const callback = callbacks.shift();
443-
444-
if (!callback) {
445-
return fn();
446-
}
447-
448-
return callback(() => wrapInCallbacks(fn, options, callbacks), options);
449-
}

0 commit comments

Comments
 (0)