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

chore(deps): Migrate from OpenCensus to OpenTelemetry #2169

Merged
merged 11 commits into from
Feb 17, 2025

Conversation

jsoriano
Copy link
Contributor

@jsoriano jsoriano commented Feb 13, 2025

Description

Migrate from OpenCensus to OpenTelemetry, doing some adjustements where there is no 1:1 migration path:

  • zpages handler explicitly added to http.DefaultServeMux, as zpages.Handle() is not available anymore.
  • Annotations replaced by Attributes.

I am also adjusting some uses of spans in compactions, so their annotations are not overwritten on every call to runCompactDef in cases where it is called multiple times.

Not using latest version of OpenTelemetry because it requires Go 1.22, and badger still supports 1.21.

Fix #2155.

Checklist

  • Code compiles correctly and linting passes locally
  • For all code changes, an entry added to the CHANGELOG.md file describing and linking to
    this PR
  • Tests added for new functionality, or regression tests for bug fixes added as applicable
  • For public APIs, new features, etc., PR on docs repo staged and linked here

@jsoriano jsoriano requested a review from a team as a code owner February 13, 2025 13:35
@jsoriano
Copy link
Contributor Author

Not sure how to address the issue with protobuf, could it be that protoc has a different version in CI? 🤔

@mauri870
Copy link

mauri870 commented Feb 13, 2025

About the Go version, I see #2125 downgraded it from 1.23 to 1.21. Even if the repo goes with the last N-2 versions as opposed to the Go release policy of N-1 that would still be valid to bump it to 1.22 now. Let's wait for a maintainer's input on this.

@mangalaman93
Copy link
Member

We have upgraded the Min Go version to 1.22 here #2171. Please upgrade the opentelemetry to the latest and we would be happy to merge the PR. Thank you for creating the PR.

@jsoriano
Copy link
Contributor Author

OTel dependencies upgraded.

Copy link
Member

@mangalaman93 mangalaman93 left a comment

Choose a reason for hiding this comment

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

Minor spelling mistake! LGTM otherwise. @harshil-goel for final review before merge.

Copy link
Member

@mangalaman93 mangalaman93 left a comment

Choose a reason for hiding this comment

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

two more comments, we are good to merge otherwise! And resolve the conflicts as well. Thank you.

@mangalaman93 mangalaman93 enabled auto-merge (squash) February 17, 2025 17:35
@ryanfoxtyler ryanfoxtyler merged commit 3568af4 into hypermodeinc:main Feb 17, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE]: Change opencensus to opentelemetry
4 participants