[BUG] OpenTelemetryExporter prevents setting the span name for Request telemetry

[johncrim]

Library name and version

Azure.Monitor.OpenTelemetry.Exporter 1.2.0

Describe the bug

OpenTelemetryExporter overrides Activity.DisplayName with hard-coded logic for Request telemetry, which effectively makes it impossible to set the span name for requests. So, if a developer is able to figure out open-telemetry/opentelemetry-dotnet-contrib#1948 and determine that they have to set the Activity.DisplayName sometime after the request Activity has stopped, that value is ignored when the AppInsights telemetry is generated by the exporter - the exporter re-creates the default value that was replaced and ignores the overridden value.

Expected behavior

The value set in Activity.DisplayName should be used for request telemetry in the Request.Name and OperationName values. DisplayName should provide a good default (which it does), but the developer should be able to override it to make their telemetry more useful.

This is both the correct behavior, and I believe is consistent with App Insights classic.

Actual behavior

The value set in Activity.DisplayName is ignored; instead the logic used by the OpenTelemetry AspNetCore instrumentation is roughly repeated and the re-computed value is used for the Request.Name and OperationName.

Reproduction Steps

Here's a test (that runs in the Azure.Monitor.OpenTelemetry.Exporter.Integration.Tests project b/c there's no other reasonable way to validate the telemetry values):

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Net.Http;
using System.Threading.Tasks;
using Azure.Core.TestFramework;
using Azure.Monitor.OpenTelemetry.Exporter.Integration.Tests.TestFramework;
using Azure.Monitor.OpenTelemetry.Exporter.Models;
using Azure.Monitor.OpenTelemetry.Exporter.Tests.CommonTestFramework;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Http.Features;
using Microsoft.Extensions.DependencyInjection;
using OpenTelemetry;
using OpenTelemetry.Trace;
using Xunit;
using Xunit.Abstractions;

namespace Azure.Monitor.OpenTelemetry.Exporter.Integration.Tests.TelemetryCustomization;

public class WebServerTelemetryCustomizationTests : WebApplicationTestsBase
{
    private const string TestServerPort = "9996";
    private const string TestServerTarget = $"localhost:{TestServerPort}";
    private const string TestServerUrl = $"http://{TestServerTarget}/";

    public WebServerTelemetryCustomizationTests(ITestOutputHelper output)
    : base(output)
    {}

#if !NET462
    [Fact]
    public async Task ActivityDisplayName_CanBeSetForRequests()
    {
        List<TelemetryItem>? telemetryItems = null;

        var builder = WebApplication.CreateBuilder();
        builder.Services.AddOpenTelemetry()
            .WithTracing(builder =>
            {
                builder.AddAspNetCoreInstrumentation()
                    .AddProcessor<FixupDisplayNameProcessor>()
                    .AddAzureMonitorTraceExporterForTest(out telemetryItems);
            });
        builder.Services.AddRouting();

        await using var app = builder.Build();
        app.UseRouting();
        app.Map("{**path}",
                context =>
                {
                    // Here I'm setting the Activity.DisplayName to something more useful for this specific route
                    // However, I have to store a value for use by FixupDisplayNameProcessor due to https://github.com/open-telemetry/opentelemetry-dotnet-contrib/issues/1948
                    var activity = context.Features.Get<IHttpActivityFeature>()?.Activity;
                    if (activity?.IsAllDataRequested == true)
                    {
                        var request = context.Request;
                        var spanName = $"{request.Method.ToUpperInvariant()} {request.Path}";
                        activity.AddTag(FixupDisplayNameProcessor.OverrideSpanNameTag, spanName);
                    }

                    context.Response.StatusCode = 200;
                    return Task.CompletedTask;
                });

        _ = app.RunAsync(TestServerUrl);

        // ACT
        using var httpClient = new HttpClient { BaseAddress = new Uri(TestServerUrl) };
        var res = await httpClient.GetAsync("/foo/bar");

        // SHUTDOWN
        var tracerProvider = app.Services.GetRequiredService<TracerProvider>();
        tracerProvider.ForceFlush();
        tracerProvider.Shutdown();

        // ASSERT
        Assert.NotNull(telemetryItems);
        WaitForActivityExport(telemetryItems);

        // Assert
        Assert.True(telemetryItems.Any(), "test project did not capture telemetry");
        var telemetryItem = telemetryItems.Last()!;
        telemetryOutput.Write(telemetryItem);

        AssertRequestTelemetry(
            telemetryItem: telemetryItem,
            expectedResponseCode: "200",
            expectedUrl: TestServerUrl + "foo/bar");

        // BUG: Both of these Asserts fail, b/c the value for both is "GET {**path}",
        // even though Activity.DisplayName was set to "GET /foo/bar" before the telemetryItem was created
        Assert.Equal("GET /foo/bar", (telemetryItem.Data.BaseData as RequestData)!.Name);
        Assert.Equal("GET /foo/bar", telemetryItem.Tags["ai.operation.name"]);
    }
#endif

    private class FixupDisplayNameProcessor : BaseProcessor<Activity>
    {
        public const string OverrideSpanNameTag = "override.span.name";

        public override void OnEnd(Activity activity)
        {
            // Have to set Activity.DisplayName here instead of where I want to due to https://github.com/open-telemetry/opentelemetry-dotnet-contrib/issues/1948
            if (activity.IsAllDataRequested == true)
            {
                var overrideSpanName = activity.GetTagItem(OverrideSpanNameTag)?.ToString();
                if (overrideSpanName != null)
                {
                    activity.DisplayName = overrideSpanName;
                    activity.SetTag(OverrideSpanNameTag, null); // Delete the tag
                }
            }
        }
    }
}

Environment

PS > dotnet --info
.NET SDK:
 Version:           8.0.303
 Commit:            29ab8e3268
 Workload version:  8.0.300-manifests.34944930
 MSBuild version:   17.10.4+10fbfbf2e

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.22631
 OS Platform: Windows
 RID:         win-x64
 Base Path:   C:\Program Files\dotnet\sdk\8.0.303\

Host:
  Version:      8.0.7
  Architecture: x64
  Commit:       2aade6beb0

.NET SDKs installed:
  6.0.424 [C:\Program Files\dotnet\sdk]
  8.0.303 [C:\Program Files\dotnet\sdk]

[github-actions]

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @cijothomas @rajkumar-rangaraj @reyang @TimothyMothra @vishweshbankwar.

[johncrim]

To provide a visual, here's what we get using Azure.Monitor.OpenTelemetry.Exporter :

and here's the equivalent view of requests in AppInsights classic:

and there's no reasonable way to fix it (there's at least 1 unreasonable way).

[TimothyMothra]

I'm consolidating all issues regarding overriding specific fields in the Application Insights schema into another issue to help with tracking. #46021