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.42k stars 4.8k forks source link

[BUG] Issues with setting (authenticated) user id using OpenTelemetry exporter #46753

Open hansmbakker opened 4 days ago

hansmbakker commented 4 days ago

Library name and version

Azure.Monitor.OpenTelemetry.Exporter 1.2.0

Describe the bug

This is a follow-up on https://github.com/Azure/azure-sdk-for-net/issues/45089.

It looks like setting the opentelemetry tag enduser.id indeed sets the top-level user_AuthenticatedId, but it does not fully work as expected:

Apart from the functionality, enduser.id is deprecated and is recommended to be replaced by user.id.

Expected behavior

Actual behavior

Reproduction Steps

  1. Create a project using the OpenTelemetry Azure Monitor Exporter, e.g. create a new Aspire.NET project.
  2. Add a (fictional) userId value by setting activity.SetTag("enduser.id", userId). The value for now does not matter, this issue is about how the value is processed by the OpenTelemetry Exporter and how it is finally picked up by Application Insights.
  3. See that the Users page in Application Insights only works when Authenticated Users is selected from the dropdown.

activity.SetTag("user.id", userId); results in the user.id tag being added as custom dimension which is not picked up by the Out of the Box Application Insights pages and workbooks.

Environment

No response

github-actions[bot] commented 4 days ago

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

johncrim commented 3 days ago

Note that https://github.com/Azure/azure-sdk-for-net/issues/45089 has some additional details on this. It was incorrectly closed (it has not been fixed, and will not be fixed by the consolidating issue) IMO.

TimothyMothra commented 3 days ago

I think the UX is broken because we're only setting the User Auth Id, but not the Anon Id. This is on our backlog, but we're blocked waiting for the community to define stable attributes.

Apart from the functionality, enduser.id is deprecated and is recommended to be replaced by user.id.

Yeah we're aware that enduser.id was depricated. We took a dependency on this while it was still experimental and we got burned. user.id is also experimental so we've decided to wait until the community has stable attributes.

We have someone from our team helping to driving this spec in the community.