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

Bump opentelemetry to latest version #33

Merged
merged 1 commit into from
Aug 19, 2024
Merged

Bump opentelemetry to latest version #33

merged 1 commit into from
Aug 19, 2024

Conversation

sfc-gh-bdrutu
Copy link
Collaborator

No description provided.

@sfc-gh-bdrutu sfc-gh-bdrutu requested a review from a team as a code owner July 18, 2024 03:28
@sfc-gh-tmonk sfc-gh-tmonk self-requested a review July 18, 2024 04:06
@snowflakedb snowflakedb deleted a comment from sfc-gh-tmonk Jul 18, 2024
@sfc-gh-bdrutu sfc-gh-bdrutu force-pushed the bump-otel-to-latest branch 6 times, most recently from d7d7f1e to 0f6dafe Compare August 19, 2024 20:02
@sfc-gh-tmonk
Copy link
Collaborator

@sfc-gh-bdrutu Regarding the failed tests, I can give some additional context.

The test_*_encoder.py tests come from https://github.com/open-telemetry/opentelemetry-python/tree/main/exporter/opentelemetry-exporter-otlp-proto-common/tests

I added these tests when we were going:

  • from our own custom implementation of "common" serializers in 0.3.0 (opentelemetry-python 1.12.0, where there is no opentelemetry-exporter-otlp-proto-common)
  • to using the standard implementation of "common" serializers in 0.4.0 (opentelemetry-python 1.23.0 which does have opentelemetry-exporter-otlp-proto-common)

Now that every release will use the standard implementation of "common" serializers, running these tests in this repo may be of limited value. We could just trust the upstream opentelemetry-exporter-otlp-proto-common package.

sfc-gh-tmonk
sfc-gh-tmonk approved these changes Aug 19, 2024
View reviewed changes
Copy link
Collaborator

@sfc-gh-tmonk sfc-gh-tmonk left a comment

Choose a reason for hiding this comment

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

Looks good to me. We can have a separate discussion about the value of maintaining the test_*_encoder.py tests in this repo.

@sfc-gh-bdrutu sfc-gh-bdrutu merged commit 2736fad into snowflakedb:main Aug 19, 2024
5 checks passed
@sfc-gh-bdrutu sfc-gh-bdrutu deleted the bump-otel-to-latest branch August 19, 2024 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sfc-gh-tmonk sfc-gh-tmonk sfc-gh-tmonk approved these changes

@sfc-gh-vguttha sfc-gh-vguttha sfc-gh-vguttha approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sfc-gh-bdrutu @sfc-gh-tmonk @sfc-gh-vguttha