Skip to content

Add support for Express v5 #2435

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.

Sign up for GitHub

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

Jump to bottom
Closed
timfish opened this issue Sep 17, 2024 · 7 comments · Fixed by #2801
Closed

Add support for Express v5 #2435

timfish opened this issue Sep 17, 2024 · 7 comments · Fixed by #2801
Assignees
@timfish
Labels
feature-request pkg:instrumentation-express

Comments

@timfish
Copy link
Contributor

timfish commented Sep 17, 2024

Express v5 has been released and it's not supported:

opentelemetry-js-contrib/plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts

Lines 61 to 62 in 80c2f1a

'express',
['>=4.0.0 <5'],

I'm looking into the changes required so you can assign this issue to me!
@pichlermarc
Copy link
Member

@timfish It's yours, thanks for looking into it 🙂

@pichlermarc
Copy link
Member

cc @JamieDanielson @pkanal (component owners)

@timfish

This comment was marked as outdated.

Sign in to view
@timfish timfish mentioned this issue Sep 18, 2024
Closed
@Lms24 Lms24 mentioned this issue Jan 24, 2025
Closed
3 tasks
@hitsthings
Copy link

Hi just curious about the future of this. Movement on the PR looks stalled, but it seems like Express team is looking to emit events that would allow for a simpler implementation. Are you waiting for those events or shipping first as-is? Is there any blocker or is it just one of those "need to find the time" things?

Cheers!

@timfish
Copy link
Contributor Author

timfish commented Jan 31, 2025

Movement on the PR looks stalled, but it seems like Express team is looking to emit events that would allow for a simpler implementation. Are you waiting for those events or shipping first as-is? Is there any blocker or is it just one of those "need to find the time" things?

Yep, that's about it. The plan is to use Node diagnostics_channels to send events from the router package that can be consumed by Open Telemetry. There's an old open PR to do this but it's years old and wasn't specifically created with Otel in mind. I'm looking into getting a new PR working but I don't have any time scale yet.

@hitsthings
Copy link

Awesome, thanks for that update! Much appreciated!

@CreativeBrainInc
Copy link

Wondering any update on the new PR ??
We are using Express 5 and wanted to implement OpenTelemetry.

trentm added a commit to trentm/opentelemetry-js-contrib that referenced this issue Apr 25, 2025
@trentm
feat(instrumentation-express): add support for Express v5, take 2
29d61a9 
This PR starts with @timfish's open-telemetry#2437 (Tim did all the hard work) and
makes it a smaller patch -- mostly by avoiding having separate
test/v4 and test/v5 trees.

Now that we are on JS SDK v2 and hence only support Node.js v18 as
a minimum (the same as express@5) the testing story is now simpler:
no need to avoid running some tests with older Node.js versions.
(See open-telemetry#2722 for a general discussion of the pain when having to do that.)

Obsoletes: open-telemetry#2437
Closes: open-telemetry#2435
@trentm trentm mentioned this issue Apr 25, 2025
Merged
@pichlermarc pichlermarc mentioned this issue Apr 30, 2025
Closed
@trentm trentm closed this as completed in b3a70d7 May 1, 2025
@dyladan dyladan mentioned this issue Apr 30, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@timfish timfish

Labels
feature-request pkg:instrumentation-express
Projects
None yet
Milestone
No milestone
4 participants
@hitsthings @CreativeBrainInc @timfish @pichlermarc