[bug] OpenTelemetry.Instrumentation.AspNetCore does not respect ForwardedHeaders

[jeremy-morren]

Package

OpenTelemetry.Extensions.Hosting

Package Version

Package Name	Version
OpenTelemetry.Instrumentation.AspNetCore	1.12.0
OpenTelemetry.Extensions.Hosting	1.12.0
OpenTelemetry.Exporter.Console	1.12.0
OpenTelemetry	1.12.0

Runtime Version

net8.0

Description

Using app.UseForwardedHeaders(new ForwardedHeadersOptions() { ForwardedHeaders = ForwardedHeaders.XForwardedProto }); works with ASP.NET Core, but the exported telemetry still has url.scheme as http.

Steps to Reproduce

Minimal api:

var builder = WebApplication.CreateBuilder(args);

builder.Services.AddOpenTelemetry()
    .WithTracing(b => b
        .AddAspNetCoreInstrumentation()
        .AddConsoleExporter());

var app = builder.Build();

app.UseForwardedHeaders(new ForwardedHeadersOptions() { ForwardedHeaders = ForwardedHeaders.XForwardedProto });
app.MapGet("/", context => Results.Ok(context.Request.GetDisplayUrl()).ExecuteAsync(context));
app.Run();

curl -H X-Forwarded-Proto:https 'http://localhost:5000/' returns https://localhost:5000/.

Open telemetry export:

Activity.Tags:
    server.address: localhost
    server.port: 5074
    http.request.method: GET
    url.scheme: http
    url.path: /
    network.protocol.version: 1.1
    user_agent.original: curl/8.14.1
    http.route: /
    http.response.status_code: 200
Instrumentation scope (ActivitySource):
    Name: Microsoft.AspNetCore
Resource associated with Activity:
    telemetry.sdk.name: opentelemetry
    telemetry.sdk.language: dotnet
    telemetry.sdk.version: 1.12.0

Expected Result

Activity.Tags:
    url.scheme: https

Actual Result

Activity.Tags:
    url.scheme: http

[Kielek]

It seems that for some part of span attributes it might be valid request: https://github.com/open-telemetry/semantic-conventions/blob/v1.37.0/docs/http/http-spans.md#http-server-definitions

@jeremy-morren, would you like to contribute this feature?

BTW I have transferred package from SDK to contrib repository, as it looks like impacting AspNetCore package.

[jeremy-morren]

Based on a preliminary investigation, It seems the issue is that the request activity is started (i.e. when the url.scheme is determined) before any middleware (i.e. ForwardedHeadersMiddleware) runs, therefore Request.Scheme is still http.

A fix could be to update the url.scheme when the response is complete, i.e. by which time Request.Scheme will have been updated.

[joegoldman2]

This issue is a duplicate of #2231. All forwarded headers are impacted. It's because the attributes server.address, server.port and url.scheme are set when the activity Microsoft.AspNetCore.Hosting.HttpRequestIn starts and not when it stops. When this activity starts, the ASP.NET Core's middleware pipeline is not yet executed and forwarded headers have not yet been processed. A simple solution might be to set these attributes when the activity stops. We may also need to keep these attributes when the activity starts because they allow to make the decision for sampling (see: dotnet/aspnetcore#62090).

[Kielek]

Closing as duplicate, lets move discussion about potential fixes for the #2231. It will be great if you want to contribute fix.