Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 3478a85

Browse files
committed
fixes
1 parent 4486294 commit 3478a85

File tree

3 files changed

+26
-23
lines changed

3 files changed

+26
-23
lines changed

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,7 @@ test('Should record a transaction for route with parameters', async ({ request }
117117
});
118118
});
119119

120-
// This fails https://github.com/getsentry/sentry-javascript/pull/12587#issuecomment-2181019422
121-
// Skipping this for now so we don't block releases
122-
test.skip('Should record spans from http instrumentation', async ({ request }) => {
120+
test('Should record spans from http instrumentation', async ({ request }) => {
123121
const transactionEventPromise = waitForTransaction('node-express-esm-preload', transactionEvent => {
124122
return transactionEvent.contexts?.trace?.data?.['http.target'] === '/http-req';
125123
});

‎packages/node-core/src/integrations/http/incoming-requests.ts

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
SEMATTRS_NET_HOST_PORT,
1212
SEMATTRS_NET_PEER_IP,
1313
} from '@opentelemetry/semantic-conventions';
14-
import type { AggregationCounts, Client, Scope, SpanAttributes } from '@sentry/core';
14+
import type { AggregationCounts, Client, Scope, SpanAttributes,SpanStatus } from '@sentry/core';
1515
import {
1616
debug,
1717
generateSpanId,
@@ -251,31 +251,33 @@ export function instrumentServer(
251251

252252
// After 'error', no further events other than 'close' should be emitted.
253253
let isEnded = false;
254-
response.on('close',()=> {
254+
functionendSpan(status: SpanStatus): void {
255255
if (isEnded) {
256256
return;
257257
}
258258

259259
isEnded = true;
260-
const newAttributes = getIncomingRequestAttributesOnResponse(request, response);
261-
span.setAttributes(newAttributes);
262-
span.setStatus(getSpanStatusFromHttpCode(response.statusCode));
263260

264-
span.end();
265-
});
266-
response.on(errorMonitor, () => {
267-
if (isEnded) {
268-
return;
269-
}
270-
271-
isEnded = true;
272261
const newAttributes = getIncomingRequestAttributesOnResponse(request, response);
273262
span.setAttributes(newAttributes);
263+
span.setStatus(status);
264+
span.end();
274265

275-
const status = getSpanStatusFromHttpCode(response.statusCode);
266+
// Update the transaction name if the route has changed
267+
if (newAttributes['http.route']) {
268+
getIsolationScope().setTransactionName(
269+
`${request.method?.toUpperCase() || 'GET'} ${newAttributes['http.route']}`,
270+
);
271+
}
272+
}
276273

277-
span.setStatus(status.code === SPAN_STATUS_ERROR ? status : { code: SPAN_STATUS_ERROR });
278-
span.end();
274+
response.on('close', () => {
275+
endSpan(getSpanStatusFromHttpCode(response.statusCode));
276+
});
277+
response.on(errorMonitor, () => {
278+
const httpStatus = getSpanStatusFromHttpCode(response.statusCode);
279+
// Ensure we def. have an error status here
280+
endSpan(httpStatus.code === SPAN_STATUS_ERROR ? httpStatus : { code: SPAN_STATUS_ERROR });
279281
});
280282

281283
return target.apply(thisArg, args);
@@ -544,7 +546,8 @@ function getIncomingRequestAttributesOnResponse(request: IncomingMessage, respon
544546
newAttributes['http.status_text'] = (statusMessage || '').toUpperCase();
545547

546548
if (rpcMetadata?.type === RPCType.HTTP && rpcMetadata.route !== undefined) {
547-
newAttributes[ATTR_HTTP_ROUTE] = rpcMetadata.route;
549+
const routeName = rpcMetadata.route;
550+
newAttributes[ATTR_HTTP_ROUTE] = routeName;
548551
}
549552

550553
return newAttributes;

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,11 @@ const instrumentSentryHttp = generateInstrumentOnce<SentryHttpInstrumentationOpt
144144
);
145145

146146
export const instrumentOtelHttp = generateInstrumentOnce<HttpInstrumentationConfig>(INTEGRATION_NAME, config => {
147-
const instrumentation = new HttpInstrumentation(config);
147+
const instrumentation = new HttpInstrumentation({
148+
...config,
149+
// This is hard-coded and can never be overridden by the user
150+
disableIncomingRequestInstrumentation: true,
151+
});
148152

149153
// We want to update the logger namespace so we can better identify what is happening here
150154
try {
@@ -243,8 +247,6 @@ export const httpIntegration = defineIntegration((options: HttpOptions = {}) =>
243247

244248
function getConfigWithDefaults(options: Partial<HttpOptions> = {}): HttpInstrumentationConfig {
245249
const instrumentationConfig = {
246-
disableIncomingRequestInstrumentation: true,
247-
248250
ignoreOutgoingRequestHook: request => {
249251
const url = getRequestUrl(request);
250252

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /