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.

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

Widen dependency ranges for related packages for instrumentation #4975

Open
mydea opened this issue Sep 4, 2024 · 2 comments · May be fixed by #5165
Open

Widen dependency ranges for related packages for instrumentation #4975

mydea opened this issue Sep 4, 2024 · 2 comments · May be fixed by #5165

Comments

@mydea
Copy link
Contributor

mydea commented Sep 4, 2024

Related to #4815

I noticed that e.g. @opentelemetry/instrumentation-http or @opentelemetry/instrumentation-fetch all hard-depend on specific versions of e.g. @opentelemetry/instrumentation, core, semantic-conventions etc. Is this really desired?

Based on https://github.com/open-telemetry/opentelemetry-specification/blob/a03382ada8afa9415266a84dafac0510ec8c160f/specification/upgrading.md?plain=1#L97-L122, if I understand correctly, instrumentation should continue to work for following minor releases of core packages. By relaxing e.g. @opentelemetry/core dependencies to e.g. ^1.26.0 etc. we could make compatibility much easier. As of now, it can be pretty hard to update instrumentation.

FWIW all the instrumentation from opentelemetry-js-contrib seems to already do it that way.

If that seems OK (cc @pichlermarc who explained these concepts to me previously) and I am not misunderstanding something, I'd be happy to open a PR updating the version ranges accordingly.

TLDR: I would propose to change e.g. these dependencies in @opentelemetry/instrumentation-http:

"dependencies": {
    "@opentelemetry/core": "1.26.0",
    "@opentelemetry/instrumentation": "0.53.0",
    "@opentelemetry/semantic-conventions": "1.27.0",
    "semver": "^7.5.2"
  },

to

"dependencies": {
    "@opentelemetry/core": "^1.26.0",
    "@opentelemetry/instrumentation": "^0.53.0",
    "@opentelemetry/semantic-conventions": "^1.27.0",
    "semver": "^7.5.2"
  },

and do the same for all instrumentation packages in this monorepo.

mydea added a commit to getsentry/sentry-javascript that referenced this issue Sep 5, 2024
…13587)

This bumps all our internal OTEL instrumentation to their latest
version.

Mainly, this fixes two things:

* Mongoose now supports v7 & v8
open-telemetry/opentelemetry-js-contrib#2353
* A variety of bug fixes, including a problem with http.get in ESM mode
open-telemetry/opentelemetry-js#4857 - See:
https://github.com/open-telemetry/opentelemetry-js/releases/tag/experimental%2Fv0.53.0

Related:

* open-telemetry/opentelemetry-js#4975 Issue
about relaxing deps in "core" instrumentation packages
* PR to bump deps in `@prisma/instrumentation`:
prisma/prisma#25160
* PR to bump deps in `opentelemetry-instrument-remix`:
justindsmith/opentelemetry-instrumentations-js#52
* PR to bump deps in `opentelemetry-instrumentation-fetch-node`:
gas-buddy/opentelemetry-instrumentation-fetch-node#17
(which will also be superseded by
#13485 eventually)

* Closes #11499
Copy link

github-actions bot commented Nov 4, 2024

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the stale label Nov 4, 2024
@mydea
Copy link
Contributor Author

mydea commented Nov 15, 2024

This is still relevant I believe, I can also just open a PR for this.

mydea added a commit to mydea/opentelemetry-js that referenced this issue Nov 15, 2024
@github-actions github-actions bot removed the stale label Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant