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

Exporters #3550

Closed
cparkins opened this issue Nov 15, 2023 · 6 comments · Fixed by #3583
Closed

Exporters #3550

cparkins opened this issue Nov 15, 2023 · 6 comments · Fixed by #3583

Comments

@cparkins
Copy link

In the section OTLP Endpoint there is a flaw in the code provided in the third and fourth code example. If this example is used then the data will not make it to the HTTP Endpoints. This is because of a flaw in the .NET Library but there is a workaround. Here's the example code for reference:

var builder = WebApplication.CreateBuilder(args);

builder.Services.AddOpenTelemetry()
  .WithTracing(b =>
{
    b
    .AddOtlpExporter(opt =>
    {
        opt.Endpoint = new Uri("your-endpoint-here");
        opt.Protocol = OtlpExportProtocol.HttpProtobuf;
    })
    // The rest of your setup code goes here too
});

If this code is used with the default endpoint "http://localhost:4318" it will fail! To workaround this issue the Endpoint can be specified using an Environment Variable instead. To set the endpoint and get the default path set use the Environment Variable OTEL_EXPORTER_OTLP_ENDPOINT.

@cartermp
Copy link
Contributor

Thanks for the issue @cparkins -- I see you filed an issue here: open-telemetry/opentelemetry-dotnet#5054

Is the nature of this issue that you must specify /v1/traces to the end of the URI string to make this work to a local collector?

@cparkins
Copy link
Author

cparkins commented Nov 16, 2023

@cartermp - Sorry; I think I was conflating two issues I've run into. Which may have more to do with how we've implemented OTel. The Interim solution should work fine. I did find it confusing when I first started using the Library to have to specify the "default" endpoint path. From what I understand this is not necessary in other languages. Also the fact that it behaves differently is a bit odd.

@cartermp
Copy link
Contributor

I'll defer to @open-telemetry/dotnet-approvers on what the intended behavior should be, and we'll document that.

In the interim, we can certainly accept a change that uses localhost:4318/v1/traces as the example and maybe a code comment saying that you can plug your own endpoint in there.

I can also buy the argument that if the environment variable correctly appends the right signal type to the URL, then that should also happen if you do this programmatically (and forget it to do it yourself). I'd also prefer that behavior, but that's also the .NET SIG's call.

@cparkins
Copy link
Author

@cartermp - There's only one problem with the Interim solution. You can only append the /v1/traces if you are only sending Traces. If you are sending Logs, Metrics and Traces the only way to set the Endpoint with the Exporter Options (which is what is in the Documentation) is to use the Environment Variable. Obviously if you are specifying the endpoint for each Signal this problem goes away, and I assume that may be why it was written that way.

@cartermp
Copy link
Contributor

Today if you're doing things programmatically, you do need to specify an exporter for each signal. There's no current way to specify a single exporter with an HTTP endpoint for all signals - you need to add each signal support and within each configure an endpoint.

@cparkins
Copy link
Author

@cartermp - Sorry, you're right! I got confused with the .NET Library and the Library that our teams have been using which allows a single Exporter Options to be passed in for all three types! I think the interim solution is fine. Just noticed the disparity between doing it one way versus another.

cartermp added a commit that referenced this issue Nov 22, 2023
fixes #3550 as a stopgap

Eventually, we'll want to rewrite this page in accordance with #3559, but for now this addresses a known point of confusion.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants