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

feat(node): Update httpIntegration handling of incoming requests #17371

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mydea merged 16 commits into develop from fn/ref-otel-incoming-request
Sep 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .size-limit.js
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ module.exports = [
import: createImport('init'),
ignore: [...builtinModules, ...nodePrefixedBuiltinModules],
gzip: true,
limit: '116 KB',
limit: '51 KB',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we just forgot to adjust this? Would be pretty sweet otherwise xD

Copy link
Member

@andreiborza andreiborza Aug 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just counted this wrongly 😅

},
// Node SDK (ESM)
{
Expand All @@ -233,14 +233,14 @@ module.exports = [
import: createImport('init'),
ignore: [...builtinModules, ...nodePrefixedBuiltinModules],
gzip: true,
limit: '150 KB',
limit: '152 KB',
},
{
name: '@sentry/node - without tracing',
path: 'packages/node/build/esm/index.js',
import: createImport('initWithoutDefaultIntegrations', 'getDefaultIntegrationsWithoutPerformance'),
gzip: true,
limit: '110 KB',
limit: '95 KB',
ignore: [...builtinModules, ...nodePrefixedBuiltinModules],
modifyWebpackConfig: function (config) {
const webpack = require('webpack');
Expand All @@ -263,7 +263,7 @@ module.exports = [
import: createImport('init'),
ignore: [...builtinModules, ...nodePrefixedBuiltinModules],
gzip: true,
limit: '135 KB',
limit: '107 KB',
},
];

Expand Down
9 changes: 9 additions & 0 deletions CHANGELOG.md
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@

- "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott

### Important Changes

- **feat(node)**: Update `httpIntegration` handling of incoming requests

This version updates the handling of the Node SDK of incoming requests. Instead of relying on @opentelemetry/instrumentation-http, we now handle incoming request instrumentation internally, ensuring that we can optimize performance as much as possible and avoid interop problems.

This change should not affect users, unless they are relying on very in-depth implementation details. Importantly, this also drops the `_experimentalConfig` option of the integration - this will no longer do anything.
Finally, you can still pass `instrumentation.{requestHook,responseHook,applyCustomAttributesOnSpan}` options, but they are deprecated and will be removed in v11. Instead, you can use the new `incomingRequestSpanHook` configuration option if you want to adjust the incoming request span.

## 10.8.0

### Important Changes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,7 @@ test('Should record a transaction for route with parameters', async ({ request }
});
});

// This fails https://github.com/getsentry/sentry-javascript/pull/12587#issuecomment-2181019422
// Skipping this for now so we don't block releases
test.skip('Should record spans from http instrumentation', async ({ request }) => {
test('Should record spans from http instrumentation', async ({ request }) => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if this is fixed by this specifically or something else, but this seems to work again now.

Lms24 and andreiborza reacted with thumbs up emoji
const transactionEventPromise = waitForTransaction('node-express-esm-preload', transactionEvent => {
return transactionEvent.contexts?.trace?.data?.['http.target'] === '/http-req';
});
Expand Down
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ describe('express tracing', () => {

// Also calling `updateName` AND setting a source doesn't change anything - Otel has no concept of source, this is sentry-internal.
// Therefore, only the source is updated but the name is still overwritten by Otel.
test("calling `span.updateName` and setting attribute source doesn't update the final name in express but it updates the source", async () => {
test('calling `span.updateName` and setting attribute source updates the final name in express', async () => {
const runner = createRunner(__dirname, 'server.js')
.expect({
transaction: {
transaction: 'GET /test/:id/span-updateName-source',
transaction: 'new-name',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is actually... fixed here now? Not quite sure why/how, but apparently some timing stuff is better now?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this indicates that the retroactive span name extraction is no longer use. Which makes sense to me if we now set a good name right from the start!

(I think this is fine btw. Actually leaves less room for user error in case they don't use the Sentry.updateSpanName helper)

transaction_info: {
source: 'custom',
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,20 @@ import { loggingTransport } from '@sentry-internal/node-integration-tests';
Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
release: '1.0',
// disable attaching headers to /test/* endpoints
tracePropagationTargets: [/^(?!.*test).*$/],
tracesSampleRate: 1.0,
transport: loggingTransport,

integrations: [
Sentry.httpIntegration({
incomingRequestSpanHook: (span, req, res) => {
span.setAttribute('incomingRequestSpanHook', 'yes');
Sentry.setExtra('incomingRequestSpanHookCalled', {
reqUrl: req.url,
reqMethod: req.method,
resUrl: res.req.url,
resMethod: res.req.method,
});
},
instrumentation: {
requestHook: (span, req) => {
span.setAttribute('attr1', 'yes');
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import * as Sentry from '@sentry/node';
import { loggingTransport } from '@sentry-internal/node-integration-tests';

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
release: '1.0',
tracesSampleRate: 1.0,
transport: loggingTransport,
// We deliberately disable dedupe here, to make sure we do not double wrap the emit function
// Otherwise, duplicate spans (that we want to test against) may be dropped by dedupe detection
integrations: integrations => integrations.filter(integration => integration.name !== 'Dedupe'),
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { sendPortToRunner } from '@sentry-internal/node-integration-tests';
import http from 'http';

const server = http.createServer(function (request, response) {
const useProxy = request.url.includes('proxy');
const patchOriginalEmit = request.url.includes('original');

// Monkey patch it again to ensure it still works later
// We cover multiple possible scenarios here:
// 1. Use proxy to overwrite server.emit
// 2. Use proxy to overwrite server.emit, using initial server.emit
// 3. Use classic monkey patching to overwrite server.emit
// 4. Use classic monkey patching to overwrite server.emit, using initial server.emit
monkeyPatchEmit(server, { useProxy, patchOriginalEmit });

response.end('Hello Node.js Server!');
});

const initialServerEmit = server.emit;

server.listen(0, () => {
sendPortToRunner(server.address().port);
});

function monkeyPatchEmit(server, { useProxy = false, patchOriginalEmit = false }) {
const originalEmit = patchOriginalEmit ? initialServerEmit : server.emit;

if (useProxy) {
server.emit = new Proxy(originalEmit, {
apply(target, thisArg, args) {
return target.apply(thisArg, args);
},
});
} else {
server.emit = function () {
return originalEmit.apply(server, arguments);
};
}
}
View file Open in desktop

This file was deleted.

Loading
Loading

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