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: Add error logging for failed OpenTelemetry HTTP export #1498

Merged
merged 14 commits into from
Feb 1, 2024

Conversation

mustakimali
Copy link
Contributor

@mustakimali mustakimali commented Jan 27, 2024

I spent quite some time trying to get the exporter to send traces to Honeycomb.

The only error I could see wasn't entirely useful.

OpenTelemetry trace error occurred. error processing span "SendError(..)"

I had to

  • clone this & some other repo,
  • use path dependency
  • add some logs

To see the actual reason my traces were not being published. So it should be useful for others too.

Changes

The HTTP exporter prints the endpoint URL and the raw response from the telemetry provider if the request fails.

Before

OpenTelemetry trace error occurred. error processing span "SendError(..)"

After

OpenTelemetry trace error occurred. OpenTelemetry export failed. Url: https://api.honeycomb.io/v1/traces, Response: b"{\"error\":\"unknown API key - check your credentials, region, and API URL\"}\n"

image

ℹ️ The Url requires allocation, so the log won't be emitted in release mode. The log is mostly useful when setting up tracing locally.

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)

Copy link

linux-foundation-easycla bot commented Jan 27, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@mustakimali mustakimali changed the title Add error logging for failed OpenTelemetry export Add error logging for failed OpenTelemetry HTTP export Jan 27, 2024
Copy link

codecov bot commented Jan 27, 2024

Codecov Report

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

Comparison is base (57c3aa3) 63.7% compared to head (8212218) 63.6%.

Files Patch % Lines
opentelemetry-otlp/src/exporter/http/logs.rs 0.0% 11 Missing ⚠️
opentelemetry-otlp/src/exporter/http/trace.rs 0.0% 11 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main   #1498     +/-   ##
=======================================
- Coverage   63.7%   63.6%   -0.1%     
=======================================
  Files        144     144             
  Lines      19983   20005     +22     
=======================================
  Hits       12741   12741             
- Misses      7242    7264     +22     

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

@mustakimali mustakimali marked this pull request as ready for review January 27, 2024 20:38
@mustakimali mustakimali requested a review from a team January 27, 2024 20:38
@mustakimali mustakimali changed the title Add error logging for failed OpenTelemetry HTTP export chore: Add error logging for failed OpenTelemetry HTTP export Jan 27, 2024
@lalitb
Copy link
Member

lalitb commented Jan 29, 2024

@mustakimali It seems the OTLP HTTP export for logs also suffer from this issue, would you be able to replicate these changes for logs too -

@mustakimali
Copy link
Contributor Author

@mustakimali It seems the OTLP HTTP export for logs also suffer from this issue, would you be able to replicate these changes for logs too -

@lalitb I've pushed a1b4b3b 👍🏽

Copy link
Member

@cijothomas cijothomas 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 adding these!

@cijothomas cijothomas merged commit d19187d into open-telemetry:main Feb 1, 2024
14 of 15 checks passed
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.

4 participants