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

Add OTLP HTTP Log Exporter example using tracing appender. #1294

Merged
merged 9 commits into from
Oct 17, 2023

Conversation

lalitb
Copy link
Member

@lalitb lalitb commented Oct 12, 2023

Changes

  • Update the existing OTLP HTTP exporter to export logs instrumented through tokio library, using tracing appender. This change could have suffered from infinite live-loop issue described in Need a way to suppress telemetry from SDKs own operation #1171, but the Hyper HTTP library emits tracing/debug logs only if the tracing feature (which is disabled by default) is enabled for this library, and Cargo.toml for this example doesn't enable it. Also, there are no errors reported from this library (which would also get routed to OpenTelemetry appender) if the steps in examples are executed properly. So only the logs, traces and metrics geerated from the example code are exported to the collector.

  • Also for simplicity removed all-in-one jaeger exporter from the collector configuration, and instead used Debug Exporter which dumps all events to console.

  • The infinite live-issue discussed in Need a way to suppress telemetry from SDKs own operation #1171 need to be investigated separately, one option could be to filter all such non-required events using event_enabled function by checking the target/module.

Please provide a brief description of the changes here.

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)

@lalitb lalitb requested a review from a team October 12, 2023 00:02
@codecov
Copy link

codecov bot commented Oct 12, 2023

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Files Coverage Δ
opentelemetry-sdk/src/metrics/view.rs 78.4% <89.2%> (+10.2%) ⬆️

... and 1 file with indirect coverage changes

📢 Thoughts on this report? Let us know!.

Copy link
Contributor

@shaun-cox shaun-cox left a comment

Choose a reason for hiding this comment

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

Thanks for doing this. Just one nit which would be nice to address before merging...

@lalitb
Copy link
Member Author

lalitb commented Oct 12, 2023

The CI is failing with:

error: package `regex-syntax v0.8.1` cannot be built because it requires rustc 1.65 or newer, while the currently active rustc version is 1.64.0 Error: Process completed with exit code 101.

Does it mean we should bump up msrv to 1.65.0 ?

lalitb and others added 2 commits October 12, 2023 13:58
@lalitb
Copy link
Member Author

lalitb commented Oct 13, 2023

The CI is failing with:

error: package `regex-syntax v0.8.1` cannot be built because it requires rustc 1.65 or newer, while the currently active rustc version is 1.64.0 Error: Process completed with exit code 101.

Does it mean we should bump up msrv to 1.65.0 ?

@jtescher @TommyCpp Seems the PRs are blocked on this error. Should we raise a separate PR to bump the msrv version ?

@jtescher
Copy link
Member

@lalitb We could do that, an alternative would be to swap the view regex impl to use a glob impl instead as suggested in #1262, which would drop the need for the dependency entirely

@TommyCpp TommyCpp merged commit 03b94b3 into open-telemetry:main Oct 17, 2023
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.

5 participants