Skip to content

ref(node): Remove unused @opentelemetry/instrumentation-http dependency#21113

Merged
nicohrubec merged 1 commit into
developfrom
ref/remove-unused-instrumentation-http-dep
May 22, 2026
Merged

ref(node): Remove unused @opentelemetry/instrumentation-http dependency#21113
nicohrubec merged 1 commit into
developfrom
ref/remove-unused-instrumentation-http-dep

Conversation

@nicohrubec
Copy link
Copy Markdown
Member

@nicohrubec nicohrubec commented May 22, 2026

Removes @opentelemetry/instrumentation-http from @sentry/node dependencies. The otel instrumentation was essentially already removed in #20393 and the dependency seems to just be a leftover.

…ency

The SDK fully replaced this with its own `SentryHttpInstrumentation` and
`httpServerSpansIntegration`. The package had zero imports in production
source code and was shipping unnecessary bytes to users.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@nicohrubec nicohrubec marked this pull request as ready for review May 22, 2026 07:38
@nicohrubec nicohrubec requested a review from a team as a code owner May 22, 2026 07:38
@nicohrubec nicohrubec requested review from JPeer264, andreiborza and mydea and removed request for a team May 22, 2026 07:38
Copy link
Copy Markdown
Member

@andreiborza andreiborza left a comment

Choose a reason for hiding this comment

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

No lockfile changes? 🤔

@nicohrubec
Copy link
Copy Markdown
Member Author

@andreiborza lol I was also Sus about that but yarn install doesn't do anything

@mydea
Copy link
Copy Markdown
Member

mydea commented May 22, 2026

it also seems that the check lockfile job is not really working,

No lockfile changes? 🤔

I also noticed this is not really being checked, we have a step for this but this seems to not work properly - opened a PR here to fix this: #21115

@mydea
Copy link
Copy Markdown
Member

mydea commented May 22, 2026

@andreiborza lol I was also Sus about that but yarn install doesn't do anything

I think we have another dependency on this, in node-core-integration-tests. do we still need this? I guess we are testing that something interops with this, so likely yes...?

@nicohrubec
Copy link
Copy Markdown
Member Author

@mydea yes it looks like we still use this for some integration tests that check that users can use the otel instrumentation and nothing breaks so I think that's still useful

@andreiborza
Copy link
Copy Markdown
Member

Ah right, yea for node-core I added some tests for that. I think we can drop those tests tbh, we're walking away from being that interop with otel so no need for that scenario?

Copy link
Copy Markdown
Member Author

there is actually a bunch of tests for interop, both in node and node core integration tests and multiple e2e test apps. I am also fine with removing if we no longer want/need to support this

@nicohrubec
Copy link
Copy Markdown
Member Author

nicohrubec commented May 22, 2026

if we are not supporting interop anymore, do we then still have a need for https://github.com/getsentry/sentry-javascript/blob/develop/packages/node-core/src/types.ts#L12 or can we remove that in v11?

@andreiborza
Copy link
Copy Markdown
Member

if we are not supporting interop anymore, do we then still have a need for develop/packages/node-core/src/types.ts#L12 or can we remove that in v11?

Those are going away too.

@JPeer264
Copy link
Copy Markdown
Member

JPeer264 commented May 22, 2026

If this line gets removed then we also get rid of the yarn.lock entry:

"@opentelemetry/instrumentation-http": "0.214.0",

Seems like I'm too late to the party, everything got answered :D

@mydea
Copy link
Copy Markdown
Member

mydea commented May 22, 2026

I think it's OK to leave these tests and the dependency for now, we can remove this on v11 branch then.

@nicohrubec
Copy link
Copy Markdown
Member Author

nicohrubec commented May 22, 2026

Shouldn't hurt to remove the node dependency already? I'll create follow up tickets for the tests and deprecation

Copy link
Copy Markdown
Member

@JPeer264 JPeer264 left a comment

Choose a reason for hiding this comment

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

I'm ok with this removal.

@nicohrubec nicohrubec merged commit 683bee2 into develop May 22, 2026
174 of 175 checks passed
@nicohrubec nicohrubec deleted the ref/remove-unused-instrumentation-http-dep branch May 22, 2026 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants