`SetErrorStatusOnException` doesn't set `Activity.Status`

[MatisseHack]

Bug Report

List of all OpenTelemetry NuGet packages and version that you are using (e.g. OpenTelemetry 1.0.2):

• OpenTelemetry 1.4.0

Runtime version (e.g. net462, net48, netcoreapp3.1, net6.0 etc. You can find this information from the *.csproj file):

• net7.0

Symptom

SetErrorStatusOnException doesn't set Activity.Status. If I'm reading correctly, it looks like the exception processor calls into an extension method that sets tags on the activity instead:

opentelemetry-dotnet/src/OpenTelemetry.Api/Trace/ActivityExtensions.cs

Lines 42 to 48 in 094124e

	public static void SetStatus(this Activity activity, Status status)
	{
	Debug.Assert(activity != null, "Activity should not be null");
	activity.SetTag(SpanAttributeConstants.StatusCodeKey, StatusHelper.GetTagValueForStatusCode(status.StatusCode));
	activity.SetTag(SpanAttributeConstants.StatusDescriptionKey, status.Description);
	}

What is the expected behavior?

Spans that contained an exception should be marked as failed.

What is the actual behavior?

Some exporters (e.g. Jaeger via OTLP) recognize these tags and properly display the spans as failed. However, some exporters (e.g. Azure.Monitor.OpenTelemetry.Exporter) only look at Activity.Status and don't display the spans as failed.

Reproduce

The following program exhibits this issue where failed spans do not show up as such in Azure:

public class Program
{
    private static readonly ActivitySource MyActivitySource = new("MyCompany.MyProduct.MyLibrary");

    public static void Main()
    {
        using var tracerProvider = Sdk.CreateTracerProviderBuilder()
            .AddSource("MyCompany.MyProduct.MyLibrary")
            .SetErrorStatusOnException()
            .AddAzureMonitorTraceExporter()
            .Build();

        try
        {
            using (MyActivitySource.StartActivity("Foo"))
            {
                throw new Exception("Oops!");
            }
        }
        catch (Exception)
        {
            // swallow the exception
        }
    }
}

Additional Context

Is there any reason to avoid setting Activity.Status for uncaught exceptions or is this implementation just a historical artifact from earlier versions of .NET?

[cijothomas]

Thanks for reporting.
Storing status as tag was done as a workaround for the limitation that Activity did not support Status originally. We moved all components in this repo to switch to Status, but this was left out. This can be fixed easily though.

[cijothomas]

https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/docs/trace/extending-the-sdk#exporting-activity-status Removing bug tag, as this is not a bug.