Skip to content

[Bug]: opentelemetry-otlp inconsistency between http and grpc export endpoint for traces #1618

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
Closed
bnjjj opened this issue Mar 13, 2024 · 1 comment · Fixed by #1706
Closed

[Bug]: opentelemetry-otlp inconsistency between http and grpc export endpoint for traces #1618

bnjjj opened this issue Mar 13, 2024 · 1 comment · Fixed by #1706
Assignees
@TommyCpp
Labels
bug Something isn't working triage:todo Needs to be traiged.

Comments

@bnjjj
Copy link
Contributor

bnjjj commented Mar 13, 2024

What happened?

When using the http and grpc exporter for traces you have to fill the endpoint value but when using the http one you have to specify the /v1/traces path and not with the grpc one. You can see in your examples here and here it's both different. But under the hood it's configuring the struct ExportConfig and in the documentation for endpoint it's explicit that it should be the base address. So I think there is a bug in the configuration here.

To keep backward compatibility maybe it could be interesting to directly check if the address already contains /v1/traces we don't modify the URL.

API Version

0.13.0

SDK Version

0.13.0

What Exporters are you seeing the problem on?

OTLP

Relevant log output

No response
@bnjjj bnjjj added bug Something isn't working triage:todo Needs to be traiged. labels Mar 13, 2024
This was referenced Mar 13, 2024
Closed
Merged
@TommyCpp TommyCpp self-assigned this Mar 18, 2024
@TommyCpp
Copy link
Contributor

TommyCpp commented Mar 18, 2024 •
edited
Loading

Seems like an issue that we didn't handle the endpoint provided in builder correclty. Will put a fix next week.

Just to double check: if there is no environment variable configuration but user provided a URL in builder, we should not modify the URL

TommyCpp added a commit to TommyCpp/opentelemetry-rust that referenced this issue Mar 18, 2024
@TommyCpp
Fix open-telemetry#1618
7cfc188
bnjjj added a commit to apollographql/router that referenced this issue Mar 27, 2024
@bnjjj
fix(telemetry): keep consistency between tracing otlp endpoint (#4801)
670b29d 
In our
[documentation](https://www.apollographql.com/docs/router/configuration/telemetry/exporters/tracing/otlp/#endpoint)
when configuration http endpoint for tracing otlp exporter we say that
users should only include the base address of the otlp endpoint. It was
only working for grpc protocol and not http due to [this
bug](open-telemetry/opentelemetry-rust#1618).
This inconsistency is now fixed with a workaround in the router waiting
for the fix in openetelemetry crate.

So for example you'll have to specify the right path for http:

```yaml
telemetry:
  exporters:
    tracing:
      otlp:
        enabled: true
        endpoint: "http://localhost:4318"
        protocol: http
```


Fixes #4798

---------

Signed-off-by: Benjamin Coenen <[email protected]>
@BrynCooke BrynCooke mentioned this issue Mar 29, 2024
Merged
TommyCpp added a commit to TommyCpp/opentelemetry-rust that referenced this issue May 6, 2024
@TommyCpp
Fix open-telemetry#1618
5ca76f0
@TommyCpp TommyCpp mentioned this issue May 6, 2024
Merged
4 tasks
TommyCpp added a commit to TommyCpp/opentelemetry-rust that referenced this issue May 6, 2024
@TommyCpp
Fix open-telemetry#1618
73432d8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@TommyCpp TommyCpp

Labels
bug Something isn't working triage:todo Needs to be traiged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: URLs in OTLP exporters TommyCpp/opentelemetry-rust
2 participants
@bnjjj @TommyCpp