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

AccessTokenHandler does not support sync methods #28

Open
codingbott opened this issue Nov 7, 2024 · 1 comment
Open

AccessTokenHandler does not support sync methods #28

codingbott opened this issue Nov 7, 2024 · 1 comment
Labels
area/access-token-managment Issues related to Access Token Management impact/non-breaking The fix or change will not be a breaking one priority/2 High

Comments

@codingbott
Copy link

Which version of Duende.AccessTokenManagement are you using?
3.0.0

Which version of .NET are you using?
6

Describe the bug

I'm try to use the OpenTelemetryClient from dotnet together with the AccessTokenManagement.

Setup like this:

builder.Services.AddClientCredentialsTokenManagement().AddClient("Token.Client.OtelAuditLogger", options =>
                {
                   // options setup here with crediential etc.
                });

            builder.Services
                .AddClientCredentialsHttpClient("Http.Client.OtelAuditLogger", "Token.Client.OtelAuditLogger");

Later on in my implementation I pass the IHttpClientFactory in my class:

    public AuditLogger(IHttpClientFactory httpClientFactory)
    {
        string plantShort = "tc5ot";
        string teamName = "BackboneInfra";
        string applicationName = "OtelDemoApp";

        var loggerFactory = LoggerFactory.Create(builder =>
        {
            builder.SetMinimumLevel(LogLevel.Trace);
            builder.AddOpenTelemetry(logging =>
            {
                logging.AddOtlpExporter(otlpOptions =>
                {
                    otlpOptions.Endpoint = new Uri(OtlpHttp);
                    otlpOptions.Protocol = OtlpExportProtocol.HttpProtobuf;

                    otlpOptions.HttpClientFactory = () =>
                    {
                        var httpClient = httpClientFactory.CreateClient("Http.Client.OtelAuditLogger");
                        httpClient.BaseAddress = new Uri(OtlpHttp);

                        // this lines test if the AccessTokenManagement is bound
                        // using SEND does NOT request a token and SEND is used by OTEL log exporter 
                        var failingGettingToken = httpClient.Send(new HttpRequestMessage());
                        // compared with SendAsync: This does request a token
                        var successGettingToken = httpClient.SendAsync(new HttpRequestMessage());

                        return httpClient;
                    };
                });
            });
        });

        var _logger = loggerFactory.CreateLogger<AuditLogger>();
        _logger.Log(LogLevel.Critical, "Hello World");
    }

To Reproduce

See the source above. Where the httpClient is created I'm doing a send which does not request tokens. I saw that in the logs.
When I call SendAsync, then the lib is requesting a token.

Expected behavior

Also Send method will request a token.

Additional context

src/Duende.AccessTokenManagement/AccessTokenHandler.cs

Does not override the Send method, but SendAync only.

@AndersAbel
Copy link
Member

It looks like Send was added in .NET 5, while SendAsync has been around since .NET Core 1.0 (and .NET Framework 4.5), so I guess the reason is that when the code was initially written, there was no Send method.

I'll transfer this issue to our FOSS repository (the home for Duende.AccessTokenManagement) for handling.

@AndersAbel AndersAbel transferred this issue from DuendeSoftware/Support Nov 11, 2024
@AndersAbel AndersAbel added area/access-token-managment Issues related to Access Token Management state/needs-triage Needs triaging by the maintainers labels Nov 11, 2024
@josephdecock josephdecock added priority/2 High impact/non-breaking The fix or change will not be a breaking one and removed state/needs-triage Needs triaging by the maintainers labels Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/access-token-managment Issues related to Access Token Management impact/non-breaking The fix or change will not be a breaking one priority/2 High
Projects
None yet
Development

No branches or pull requests

3 participants