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

OTLP HttpExporter: Bearer Authentication #2504

Open
zakimaksyutov opened this issue Oct 21, 2021 · 17 comments
Open

OTLP HttpExporter: Bearer Authentication #2504

zakimaksyutov opened this issue Oct 21, 2021 · 17 comments
Assignees
Labels
pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package question Further information is requested

Comments

@zakimaksyutov
Copy link

Question

What are you trying to achieve?

.NET Core app uses native OTLP HttpExporter to upload telemetry to an endpoint requiring Bearer Authentication.

How can I configure OTLP in a way that it adds $"Authorization=Bearer {token.Token}" header for every outgoing request with a fresh token?

Additional Context

There is a way to configure headers through .AddOtlpExporter:

.AddOtlpExporter(opt =>
{
    opt.Endpoint = new Uri("https://bla");
    opt.ExportProcessorType = ExportProcessorType.Batch;
    opt.Protocol = OtlpExportProtocol.HttpProtobuf;
    opt.Headers = $"Authorization=Bearer {token.Token}";
})

The issue with this approach is that token finally expires and OTLP fails to upload a telemetry.

@zakimaksyutov zakimaksyutov added the question Further information is requested label Oct 21, 2021
@pellared
Copy link
Member

pellared commented Oct 21, 2021

Interesting use case. I do not think it is currently possible. At least without some excessive hacking of the internals e.g. by trying to change the HttpMessageHandler using reflection).

I think to do that we could add an option in OtlpExporterOptions to set the HttpClient and then the user could use the HttpClient(HttpMessageHandler) constructor to handle the token refresh logic. Maybe you could propose a PR to add such an option?

@ktmitton
Copy link
Contributor

Was thinking about this, it's similar to a problem I had to solve at work, but there I had access to DI, which made this simpler. Not sure yet I'm sold on this, but wanted to keep the conversation going, so I through together a simple change to the export options that adds a delegating handler:
ktmitton@eba1256

Not sure how I feel about putting in a setting that only gets applied when a different setting has a specific value, though this isn't the only such setting there...

A couple pros I see to this approach:

  • It would allow us to remove the HttpClient from the BaseOtlpHttpExportClient constructor. At a glance, looks like that is only exposed to allow for internal testing, but maybe this could serve that purpose?
  • Since it would expose access to both the request and response, it would allow for processing such as "if 401, trigger a credentials refresh", which would allow a way to handle both expired and revoked credentials.

Let me know what you guys think, and whether this is worth pursuing further, or if we should go a different direction!

@zakimaksyutov
Copy link
Author

Thank you @pellared and @ktmitton. Though it will work (relying on 401s) it might be suboptimal and less reliable. Besides doing extra calls when a token expires, auth layers usually provide logic which refresh a token asynchronously (if it is still in use). Async refresh also helps to survive minor outages when token refresh fails.

One standard pattern used in other libraries is to take IAuthSomething object and call it before every request (and that object will take care of token caching, it's refresh, etc.). Not sure whether it fits here though.

@ktmitton
Copy link
Contributor

@zakimaksyutov if I'm understanding what you're saying, I think the sendasync function in the delegating handler would allow you to do everything you want. Since you'd be creating your own implementation of the handler, you could pass your IAuthSomething into the constructor, so you'd get that, too.

Quick side note, if I'm readying all the code in the project correctly, concerns about async may be moot. It looks like the exporter uses synchronous sending, or forces SendAsync into sync via GetAwaiter().GetResult(), so in the end you're still locking up the thread. I think this is where your IAuthSomething shines, though, if you are using some other process to maintain it. Sounds similar to how I've managed custom configuration providers. Since the configuration pipeline is synchronous, I used a background process to do all the polling, but have a "middleman" cache that service writes to and the configuration pipeline reads. Again, if delegating handlers are the way people want to go, we can support that workflow since you can make the handler do whatever you want.

That being said, what if we look at this a different way? I'm wondering if this is a problem that is better addressed with collectors. If you spun app a separate local collector app, your main app would direct all the otlp traffic there, and since it's local, you can lock it down with whatever firewall rules or something you see fit. Then, your collector wouldn't need an authentication that is prone to updates, it could use a static api key.

Once you've got your data in the collector, you can then have your collector export the data to the final destination, and the collector can be set up to manage the more complex credentials. In this instance where you are running multiple apps you want to observe, this has the added benefit of consolidating the important credentials to a single spot (the collector), instead of having to share them with all your apps, and incurring all the overhead that comes with maintaining those securely.

@ktmitton
Copy link
Contributor

Oh...I'm thinking the collector path is definitely the intended path, they happen to have this in their documentation...
https://opentelemetry.io/docs/collector/custom-auth/

@ktmitton
Copy link
Contributor

Plus, in general I think it'd probably be a good idea to use a collector instead of having your application reach out to the destination directly (assuming you are using something outside your ecosystem like new relic or stackify). If you have your applications hitting your destination directly, your application is taking on the overhead that comes with general latency in the connection, and dealing with those services having issues. If you have your apps go through a local collector, though, the exports would generally be expected to be quicker and less prone to issues. The collector can then handle things like slow collections or services being down, and the effects wouldn't be felt by your application(s).

@ktmitton
Copy link
Contributor

Disclaimer: I've been hitting new relic directly myself, but that's been for testing purposes. I think before we go to production, we'll want to through a collector into the mix 😃

@mdrakiburrahman
Copy link

mdrakiburrahman commented Jun 11, 2023

It's possible to override the otlpOptions.HttpClientFactory and add in a Handler that caches OAuth token and injects a new one when it's about to expire.

Here's a fully working implementation tested with Azure AD:

https://github.com/mdrakiburrahman/authenticated-otel-logger/blob/3f4283b09acce392b71ac7a5b8e52fc07ba1e2cc/src/AuthenticatedOtelLogger/Program.cs#L65

@jr-ri
Copy link

jr-ri commented Oct 5, 2023

It's possible to override the otlpOptions.HttpClientFactory and add in a Handler that caches OAuth token and injects a new one when it's about to expire.

Here's a fully working implementation tested with Azure AD:

https://github.com/mdrakiburrahman/authenticated-otel-logger/blob/main/src/Program.cs#L64

Hi @mdrakiburrahman, your link is returning a 404 - is this the correct replacement link?

@mdrakiburrahman
Copy link

@jr-ri - yeap, thanks for the catch, updated to permalink!

@Yogesh19921
Copy link

It's possible to override the otlpOptions.HttpClientFactory and add in a Handler that caches OAuth token and injects a new one when it's about to expire.

Here's a fully working implementation tested with Azure AD:

https://github.com/mdrakiburrahman/authenticated-otel-logger/blob/3f4283b09acce392b71ac7a5b8e52fc07ba1e2cc/src/AuthenticatedOtelLogger/Program.cs#L65

I realized that the example here is using a blocking call to send method through HTTP Message handler. This means that opentelemetry sdk itself calls the blocking send method of the http client supplied. This in my opinion can bring down application performance (unless otlp runs on a separate thread).
I already did try to define an async method and otlp doesn't call async sendAsync method.

Are there plans to move it to using async sendAsync method ? Or can someone guide me to the piece of code where http call to send method is being made.

@cijothomas
Copy link
Member

unless otlp runs on a separate thread

Yes, by default, exporter is called on a dedicated thread.

@codingbott
Copy link

codingbott commented Sep 10, 2024

There is really a lack on authentication in the dotnet otel packages.

I also run in the same problem, that I can't authenticate when tokens/keys etc. might expire.
Having access to the http factory is nice, but also introduces the problem, that I can't use the DI when the settings option is accessible. I really like to inject something which can handle the token refresh in the background,

            builder.Services.AddOpenTelemetry()
            .WithLogging(bldr =>
            {
                bldr.AddOtlpExporter(otlpOptions =>
                {
                    otlpOptions.Endpoint = new Uri(OtelServerHttp);
                    otlpOptions.Protocol = OtlpExportProtocol.HttpProtobuf;
                });
                bldr.AddConsoleExporter();
            }, options =>
            {
                options.IncludeScopes = true;
            });

See also "Use DI services to configure options", please.
https://learn.microsoft.com/en-us/aspnet/core/fundamentals/configuration/options?view=aspnetcore-8.0#use-di-services-to-configure-options

Yes, it's possible, but I need to register an own OtlpExporterOptions before upper code.

            builder.Services.AddOptions<OtlpExporterOptions>().Configure<TokenRefresher>((options, tokenRefresher) =>
            {
                // options.HttpClientFactory = (() => new HttpClient(new HandlerUsingTokenRefresher() ));
            });

Also GRPC auth is not really present.

Are there already other suggestions to solve the auth problem?
How can I help?

@TimothyMothra
Copy link
Contributor

@bonfi7
Copy link

bonfi7 commented Jan 28, 2025

Hi @codingbott , @Yogesh19921 , @mdrakiburrahman
is the injection really working for logs? In the code seems that IHttpClientFactory for OtlpLogExporter is disabled and also documentation states that .

Is it an issue in documentation?

@codingbott
Copy link

@TimothyMothra Yes, that static injection of the headers works until you are using tokes which might expire. Then you don't have any chance to update them. In a oidc scenario not useful.

I tried to get the token manager from duende running.
https://docs.duendesoftware.com/identityserver/v7/bff/tokens/

The main problem here is that they have only implemented they token refresh for async calls.
Deep in the dotnet otel code is every only in sync according to the HTTPCLIENT.

As workaround I patched that duende code for my own tests. Finally I got it working with my patched version.
I have created a ticket at duende, that they add support for sync calls in their token manager. Then the part for http is a no brainer.

I'm still not satisfied with the current implementation of otel, because this was just the talk about HTTP and NOT grpc.
We need working authentication for all protocols!

@codingbott
Copy link

@bonfi7 Hi and thank you for the additional information. I think the documentation lacks on samples to show the options to use authentication in HTTP and GRPC. Special case when your are not using a simple authentication with user and password.

OIDC in state of the art and must be supported! The duende lib is perfect, but this includes that the otel part should get async calls for HTTPCLIENT.

@TimothyMothra TimothyMothra added the pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package label Jan 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package question Further information is requested
Projects
None yet
Development

No branches or pull requests