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

Upgrade hyper and otel dependencies #97

Merged
merged 5 commits into from
Aug 2, 2024

Conversation

kcking-openai
Copy link
Contributor

Changes

Upgrade otel and hyper dependencies, change HttpClient example to hyper over unix socket.

I removed writing the resource for every span, as the field was removed from SpanData.

Merge requirement checklist

  • CONTRIBUTING guidelines followed
  • Unit tests added/updated (if applicable)
  • Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
  • Changes in public API reviewed (if applicable)

@kcking-openai kcking-openai requested a review from a team July 30, 2024 03:34
Copy link

linux-foundation-easycla bot commented Jul 30, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

Copy link

codecov bot commented Jul 30, 2024

Codecov Report

Attention: Patch coverage is 88.88889% with 4 lines in your changes missing coverage. Please review.

Project coverage is 56.4%. Comparing base (56a6f45) to head (0720841).
Report is 23 commits behind head on main.

Files Patch % Lines
opentelemetry-etw-logs/src/logs/exporter.rs 0.0% 2 Missing ⚠️
opentelemetry-datadog/src/exporter/mod.rs 91.6% 1 Missing ⚠️
...pentelemetry-user-events-logs/src/logs/exporter.rs 0.0% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main     #97     +/-   ##
=======================================
+ Coverage   52.3%   56.4%   +4.1%     
=======================================
  Files         38      39      +1     
  Lines       4967    5441    +474     
=======================================
+ Hits        2598    3074    +476     
+ Misses      2369    2367      -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lalitb
Copy link
Member

lalitb commented Jul 30, 2024

I removed writing the resource for every span, as the field was removed from SpanData.

@kcking-openai thanks for the PR. Regarding resource, while it was removed from SpanData, SpanExporter::set_resource() method was introduced for one time setting of resource in Exporter. Default implementation was added for it in the SpanExporter trait so that custom exporters don't break. You may want to implement the DatadogExporter ::set_resource() method instead of removing resource from the code.

@lalitb
Copy link
Member

lalitb commented Jul 30, 2024

Also, you have to sign the easyCLA before the PR can be approved.

@kcking-openai kcking-openai force-pushed the main branch 2 times, most recently from a9c06b5 to 8727213 Compare July 30, 2024 17:27
Deleted resource publishing as `resource` is no longer a field on SpanData.
@kcking-openai
Copy link
Contributor Author

Thank you @lalitb, I have completed the CLA and pushed a fix for the lint. If you would like an alternative solution to the too_many_arguments lint I could wrap them in a struct, but I wasn't sure it was worth the effort at this point.

@kcking-openai
Copy link
Contributor Author

And I just pushed a fix for some lints that had broken in a couple unrelated directories.

Copy link
Member

@lalitb lalitb left a comment

Choose a reason for hiding this comment

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

Thanks @kcking-openai. This looks good. Will publish a release shortly.

@lalitb lalitb merged commit 6655787 into open-telemetry:main Aug 2, 2024
9 checks passed
@lalitb
Copy link
Member

lalitb commented Aug 2, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants