Skip to content
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

Support hapi v21 #1383

Open
blumamir opened this issue Feb 8, 2023 · 5 comments
Open

Support hapi v21 #1383

blumamir opened this issue Feb 8, 2023 · 5 comments
Assignees
@toddtarsi
Labels
never-stale

Comments

@blumamir
Copy link
Member

blumamir commented Feb 8, 2023

hapi instrumentation has [support for version range '>=17 <21', as fixed in #1382

The package released a new major version 21.0.0 at 11/07/2022, and we should update the instrumentation accordingly:

supported version for patch
README
unit tests
test-all-versions (currently missing).

Relevant renovate PR, with the output errors of trying to bump the instrumentation version to 22: #1280

Looks like v21 now publishes it's own types and they are not compatible with previous versions of hapi. Not sure how to handle this in instrumentation
@blumamir blumamir mentioned this issue Feb 8, 2023
Closed
1 task
@blumamir blumamir added up-for-grabs Good for taking. Extra help will be provided by maintainers never-stale labels Feb 8, 2023
@toddtarsi
Copy link
Contributor

Yes, I'd like to help here if possible. This installs the legacy definitely typed package, which overrides my built in types and breaks all sorts of other places in monorepos because those type definitions override the modern builtins.

@blumamir
Copy link
Member Author

Sure, thanks 👍

@blumamir blumamir removed the up-for-grabs Good for taking. Extra help will be provided by maintainers label Feb 20, 2023
@Towerism
Copy link

Towerism commented May 2, 2023

My team is waiting on this. Do you need help @toddtarsi?

@toddtarsi
Copy link
Contributor

@blumamir @Towerism - Uh oh. I'm sorry, I just saw this was assigned to me. Sometimes my github notifications get a bit overwhelming. I'd be happy to start taking a look tonight. I believe we'll just change the dependency in this file to just drop the types package and bring up hapi to v21, and glue up wherever the types complain. If not me, thats fine. Sorry about that all!

https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-hapi/package.json

@toddtarsi toddtarsi mentioned this issue May 9, 2023
Closed
@toddtarsi
Copy link
Contributor

@Towerism @blumamir - Okay, I made a PR for this, but I will need help testing it and such.

@nlochschmidt nlochschmidt mentioned this issue Mar 2, 2024
Merged
iwalt pushed a commit to DEFRA/waste-tracking-service that referenced this issue May 1, 2024
Merged PR 28496: Azure Monitor tracing for waste-tracking-gateway
21c6fed 
Azure Monitor tracing for waste-tracking-gateway

Added Application Insights tracing to _waste-tracking-gateway_ via [OpenTelemetry](https://opentelemetry.io/).

 - Downgraded Hapi from v21 to v20 because unfortunately instrumentation is [not yet provided for v21](open-telemetry/opentelemetry-js-contrib#1383). If this becomes a problem, we can forgo the Hapi layer and rely on just HTTP instrumentation.
 - Traces will be written to console in development.
 - Setting `APPINSIGHTS_CONNECTION_STRING` emits traces to Azure Monitor.
 - This required an additional _tracing.js_ entry point as instrumentation must be loaded first.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@toddtarsi toddtarsi

Labels
never-stale
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@Towerism @toddtarsi @blumamir