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

fix: bump otel #763

Merged
merged 9 commits into from
Jan 8, 2025
Merged

fix: bump otel #763

merged 9 commits into from
Jan 8, 2025

Conversation

Netail
Copy link
Contributor

@Netail Netail commented Dec 27, 2024

Why

Bump OTEL packages

What

Bump otel packages

  • Set target to ES2016, else .includes wasn't supported yet
  • Add node to tsconfig types, for global type

Links

Checklist

  • Tests added
  • Changelog updated
  • Documentation updated

@CLAassistant
Copy link

CLAassistant commented Dec 27, 2024

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@Netail Netail changed the title bump: otel fix: bump otel Dec 27, 2024
@codecapitano
Copy link
Collaborator

Hey @Netail thank you so much for the contribution.
Can give a quick update on what OTEL bug is fixed due to that update?

@Netail
Copy link
Contributor Author

Netail commented Jan 6, 2025

Hey @Netail thank you so much for the contribution. Can give a quick update on what OTEL bug is fixed due to that update?

Oops, not necessarily a bug in here actually. But nextjs' bundling seems to have an issue (vercel/next.js#74331), where it picks the latest version. The rest of our apps uses 0.57.0, while faro uses 0.53.0, between these versions a function got removed which is now throwing an error in faro

@codecapitano
Copy link
Collaborator

Aaaah got it @Netail thanks that's fine. 🙏

@Netail
Copy link
Contributor Author

Netail commented Jan 6, 2025

Could have reverted our versions ofc, but the dependencies here had to updated at some point, so why not xd

@@ -2,6 +2,6 @@
"extends": "./tsconfig.base.json",
"compilerOptions": {
"module": "CommonJS",
"target": "ES5"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any specific reason to change the target here?

Note:
We have users (at least back in the days) who need ES5 support.

Copy link
Contributor Author

@Netail Netail Jan 6, 2025

Choose a reason for hiding this comment

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

The build failed locally, because .includes() wasn't available yet

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Might be able to revert it actually

@codecapitano
Copy link
Collaborator

Could have reverted our versions ofc, but the dependencies here had to updated at some point, so why not xd

It's already time to update OTEL deps so you had perfect timing :)
Thanks a lot for that 🙏

@codecapitano codecapitano added the dependencies Pull requests that update a dependency file label Jan 6, 2025
@codecapitano codecapitano enabled auto-merge (squash) January 7, 2025 10:56
auto-merge was automatically disabled January 7, 2025 13:02

Head branch was pushed to by a user without write access

@Netail Netail requested a review from codecapitano January 8, 2025 12:52
@Netail
Copy link
Contributor Author

Netail commented Jan 8, 2025

After some ci/cd fixes, it should be good now xd

Copy link
Collaborator

@codecapitano codecapitano left a comment

Choose a reason for hiding this comment

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

Great work and thanks for your patience in fixing the sometimes annoying linter issues.
Again many thanks for your contribution 🥳

@codecapitano
Copy link
Collaborator

@Netail I can't promise that we release a new version this week so it might take till Monday.

@codecapitano codecapitano merged commit 95a8dbe into grafana:main Jan 8, 2025
5 checks passed
@Netail
Copy link
Contributor Author

Netail commented Jan 8, 2025

No worries, thanks :)

@Netail Netail deleted the bump/otel branch January 8, 2025 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants