search

Azure / azure-sdk-for-net

This repository is for active development of the Azure SDK for .NET. For consumers of the SDK we recommend visiting our public developer docs at https://learn.microsoft.com/dotnet/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-net.
MIT License
5.25k stars 4.59k forks source link

[BUG] AzMon Exporter removes semantic attributes from telemetry #45288

Open johncrim opened 1 month ago

johncrim commented 1 month ago

Library name and version

Azure.Monitor.OpenTelemetry.Exporter 1.3.0

Describe the bug

When an Activity is processed by the Azure.Monitor.OpenTelemetry.Exporter, any tags with keys in this list:

https://github.com/Azure/azure-sdk-for-net/blob/ede7138b0150024daa440819cc0008cd51f46bc1/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Internals/ActivityTagsProcessor.cs#L12C1-L69C1

are removed from the resulting telemetry. Some of the corresponding values are mapped to other AI telemetry fields, but many of them are just removed from the telemetry. For example, these fieldsare added by the GRPC request instrumentation :

But then all of these fields except rpc.method are removed from the resulting telemetry in Application Insights, yielding the following output:

Expected behavior

Activity tags (particularly semantic tags) that are not mapped to other TelemetryItem fields show up in the TelemetryItem.Properties.

Actual behavior

All Activity tags in the s_semantics list are effectively stripped from the TelemetryItem. A few of them are mapped to TelemetryItem predefined values (good) but the others are dropped.

The only workaround is to purposefully instrument code with non-semantic tags, and/or copy the semantic values to non-semantic tags.

Reproduction Steps

This behavior is present for all types of Activity telemetry (request and dependency TelemetryItems in the exporter), but it's particularly noticeable with GRPC. Start with a GRPC sample app (eg Examples.GrpcCore.AspNetCore) and remove GrpcCore dependency, and add Azure.Monitor.OpenTelemetry.AspNetCore and enable the exporter, then view the output.

Environment

dotnet --info .NET SDK: Version: 8.0.302 Commit: ef14e02af8 Workload version: 8.0.300-manifests.5273bb1c 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.302\

Host: Version: 8.0.6 Architecture: x64 Commit: 3b8b000a0e

.NET SDKs installed: 6.0.423 [C:\Program Files\dotnet\sdk] 8.0.302 [C:\Program Files\dotnet\sdk]

github-actions[bot] commented 1 month ago

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

cijothomas commented 1 month ago

Sounds like a bug, if tags are removed (without mapping to existing top level field in ApplicationInsights).

cijothomas commented 1 month ago

Thanks @johncrim for your efforts in reporting issues! Really appreciate it. Thanks @TimothyMothra for the quick fixes.

johncrim commented 1 month ago

Thank you @TimothyMothra !

johncrim commented 1 month ago

Hi @cijothomas and @TimothyMothra - I think this should be re-opened. The fix only addresses RPC/GRPC semantic attributes. There are a number of other attributes that are also stripped; and also can only be included in the output by using non-semantic tag names.

For example:

In short, I think none of the semantic attributes should be removed unless there is an explicit equivalent in the AI telemetry output.

This came up just now b/c I'm not able to report network.protocol.version even though it's provided by the HTTP instrumentation library.

TimothyMothra commented 1 month ago

Hi @johncrim, Thank you for all your work reporting issues.

none of the semantic attributes should be removed unless there is an explicit equivalent in the AI telemetry output.

Fully agree! We initially expected to map all of these attributes. We've since struggled with some of OpenTelemetry's attributes stuck in Experimental. I'm seeing that quite a few of the attributes we expected to map are not currently used. I'm going to do a full audit of this list and get this corrected. I'm sorry for the trouble, but we really appreciate you.

johncrim commented 1 month ago

That sounds good @TimothyMothra - thank you for the explanation. I had guessed that was the case.