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.2k stars 4.55k forks source link

[FEATURE REQ] Resource Tags not saved to sink with OpenTelemetryMetrics #40630

Open joebeernink opened 7 months ago

joebeernink commented 7 months ago

Library name and version

Azure.Monitor.OpenTelemetry.Exporter 1.0.0

Describe the bug

When using Azure.Monitor.OpenTelemetry.Exporter to export metrics with custom tags set via the SetResourceBuilder(), the custom tags are not added to the metrics saved to Azure Monitor.

i.e. the following call should add a collection of ResourceAttributes to all metrics, including the HttpCientInstrumentation metrics. According to the OpenTelemetry.net folks, this is happening, but the AzureMonitor Exporter (and also Prometheus) are not saving these custome dimensions to Azure. This works for Logs and Traces, but not for metrics. This is a major problem. Without those custom dimensions on the metrics, we have to fall back to generating metrics from our logs, which greatly increases time for our dashboards and alerts to run, and increases costs. 

.WithMetrics(builder => builder
    .SetResourceBuilder(ResourceBuilder.CreateDefault()
                         .AddAttributes(resourceAttributes))
    .AddHttpClientInstrumentation()
    .AddMeter(willowContext?.MeterOptions.Name ?? "Unknown", willowContext?.MeterOptions.Version ?? "Unknown")
    .AddAzureMonitorMetricExporter(o =>
    {
        o.ConnectionString = appInsightsConnection;
    }));

Expected behavior

A new custom dimension for all metrics should appear in AppInsights or Prometheus for any ResourceAttributes added to the metrics at the global level, in addition to any custom dimensions added to the meter through the new MeterOptions capability, or to the counter generate from the meter.

Actual behavior

Dimensions added through the .SetResourceBuilder(ResourceBuilder.CreateDefault().AddAttributes(resourceAttributes)) call are missing from the custom properties list when the metrics are stored in Azure

Reproduction Steps

        public static IServiceCollection ConfigureOpenTelemetry(this IServiceCollection services, IConfiguration configuration)
        {
            var appInsightsConnection = configuration.GetSection("ApplicationInsights")["ConnectionString"];
            var cloudRoleName = Assembly.GetEntryAssembly()?.GetName()?.Name ?? "Unknown";

            var resourceAttributes = new Dictionary<string, object>()
            {
                { "service.name", cloudRoleName },
                { "environment", "Dev" },
                { "region", "eus" }
            };
            services.AddOpenTelemetry()
                .WithTracing(builder =>
                {
                    builder.AddSource(cloudRoleName)
                        .SetResourceBuilder(ResourceBuilder.CreateDefault().AddAttributes(resourceAttributes))
                        .AddAspNetCoreInstrumentation()
                        .AddHttpClientInstrumentation()
                        .AddAzureMonitorTraceExporter(o =>
                        {
                            o.ConnectionString = appInsightsConnection;
                            o.Diagnostics.IsDistributedTracingEnabled = true;
                        });
                })
                .WithMetrics(builder =>
                {
                    builder
                        .SetResourceBuilder(ResourceBuilder.CreateDefault()
                                            .AddAttributes(resourceAttributes))
                        .AddMeter("test")
                        .AddAspNetCoreInstrumentation()
                        .AddHttpClientInstrumentation()
                        .AddAzureMonitorMetricExporter(o =>
                        {
                            o.ConnectionString = appInsightsConnection;
                        });
                });
            return services;
        }

Environment

PS C:\repos\TwinPlatform> dotnet --info .NET SDK: Version: 8.0.100 Commit: 57efcf1350 Workload version: 8.0.100-manifests.6a1e483a

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

.NET workloads installed: Workload version: 8.0.100-manifests.6a1e483a [maui-windows] Installation Source: VS 17.8.34309.116 Manifest Version: 8.0.3/8.0.100 Manifest Path: C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.maui\8.0.3\WorkloadManifest.json Install Type: Msi

[maccatalyst] Installation Source: VS 17.8.34309.116 Manifest Version: 17.0.8478/8.0.100 Manifest Path: C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.maccatalyst\17.0.8478\WorkloadManifest.json Install Type: Msi

[ios] Installation Source: VS 17.8.34309.116 Manifest Version: 17.0.8478/8.0.100 Manifest Path: C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.ios\17.0.8478\WorkloadManifest.json Install Type: Msi

[android] Installation Source: VS 17.8.34309.116 Manifest Version: 34.0.43/8.0.100 Manifest Path: C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.android\34.0.43\WorkloadManifest.json Install Type: Msi

Host: Version: 8.0.0 Architecture: x64 Commit: 5535e31a71

.NET SDKs installed: 6.0.417 [C:\Program Files\dotnet\sdk] 8.0.100 [C:\Program Files\dotnet\sdk]

.NET runtimes installed: Microsoft.AspNetCore.App 6.0.25 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 7.0.14 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 8.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.NETCore.App 6.0.25 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 7.0.14 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 8.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.WindowsDesktop.App 6.0.25 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 7.0.14 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 8.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found: x86 [C:\Program Files (x86)\dotnet] registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation]

Environment variables: Not set

global.json file: Not found

Learn more: https://aka.ms/dotnet/info

Download .NET: https://aka.ms/dotnet/download

github-actions[bot] commented 7 months ago

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

joebeernink commented 7 months ago

Following up on this one. I understand this is probably a significant effort (looking at the amount of code that is in the Trace Exporter to support this), but it really is critical for us to get these dimensions added consistently to all metrics. We're currently forced to use Azure Log Analytics tables to monitor and alert on for our production customers with Grafana, and that just isn't an efficient way to operate at scale. Any estimate on when this issue might be addressed?

mattmccleary commented 7 months ago

Hi @joebeernink - thanks for reaching out.

Just a heads up, you have to explicitly opt-in to dimensions on custom metrics.

https://learn.microsoft.com/azure/azure-monitor/app/pre-aggregated-metrics-log-metrics#custom-metrics-dimensions-and-pre-aggregation

Before we dig further, can you confirm they're enabled?

joebeernink commented 7 months ago

Yes, we have turned this on.

Sent via the Samsung Galaxy S21 5G, an AT&T 5G smartphone Get Outlook for Androidhttps://aka.ms/AAb9ysg

From: Matt McCleary @.> Sent: Monday, December 18, 2023 8:15:25 PM To: Azure/azure-sdk-for-net @.> Cc: Joe Beernink @.>; Mention @.> Subject: Re: [Azure/azure-sdk-for-net] [BUG] Resource Tags not saved to sink with OpenTelemetryMetrics (Issue #40630)

Hi @joebeerninkhttps://github.com/joebeernink - thanks for reaching out.

Just a heads up, you have to explicitly opt-in to dimensions on custom metrics.

https://learn.microsoft.com/azure/azure-monitor/app/pre-aggregated-metrics-log-metrics#custom-metrics-dimensions-and-pre-aggregation

Before we dig further, can you confirm they're enabled?

— Reply to this email directly, view it on GitHubhttps://github.com/Azure/azure-sdk-for-net/issues/40630#issuecomment-1861942920, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AA5UXDMVKUDABKDBNLAXUWLYKDTC3AVCNFSM6AAAAABALROYSCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRRHE2DEOJSGA. You are receiving this because you were mentioned.Message ID: @.***>

mattmccleary commented 7 months ago

Thanks Joe. Am I correct that you are running on an Azure VM? Looks like you also have some ios and android deployment?

joebeernink commented 7 months ago

We're running on Azure Container Environment. Docker images. Other custom metrics dimensions do come through. Only the ones added in the ResourceBuilder are not. Works for digging and tracing. Just not for metrics.

Sent via the Samsung Galaxy S21 5G, an AT&T 5G smartphone Get Outlook for Androidhttps://aka.ms/AAb9ysg

From: Matt McCleary @.> Sent: Tuesday, December 19, 2023 1:59:15 PM To: Azure/azure-sdk-for-net @.> Cc: Joe Beernink @.>; Mention @.> Subject: Re: [Azure/azure-sdk-for-net] [BUG] Resource Tags not saved to sink with OpenTelemetryMetrics (Issue #40630)

Thanks Joe. Am I correct that you are running on an Azure VM? Looks like you also have some ios and android deployment?

— Reply to this email directly, view it on GitHubhttps://github.com/Azure/azure-sdk-for-net/issues/40630#issuecomment-1863319279, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AA5UXDOVTHVSLKDFBNO4JEDYKHPYHAVCNFSM6AAAAABALROYSCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRTGMYTSMRXHE. You are receiving this because you were mentioned.Message ID: @.***>

mattmccleary commented 7 months ago

Thx Joe. I'm told we translate the Resource Builder dims to "Cloud Role Instance" and "Cloud Role Name" for custom metrics. Are you seeing these come through? Just curious - what specific resource dims do you want and why?

joebeernink commented 7 months ago

We add a number of values to each metric like:

CustomerId (we are running single tenant instances of our compute that all report to a common app insights instance for the region) Region, Scale unit, Environment (prod, dev, ci, ppe) CustomerInstanceId (a customer may have multiple instances for scaling purposes)

These are all set when the app is deployed to a customer instance. When we log any Metric, they should be dimensions on each row.

Thanks

Sent via the Samsung Galaxy S21 5G, an AT&T 5G smartphone Get Outlook for Androidhttps://aka.ms/AAb9ysg

From: Matt McCleary @.> Sent: Thursday, December 21, 2023 12:39:48 PM To: Azure/azure-sdk-for-net @.> Cc: Joe Beernink @.>; Mention @.> Subject: Re: [Azure/azure-sdk-for-net] [BUG] Resource Tags not saved to sink with OpenTelemetryMetrics (Issue #40630)

Thx Joe. I'm told we translate the Resource Builder dims to "Cloud Role Instance" and "Cloud Role Name". Are you seeing these come through on custom metrics? Just curious - what specific resource dims do you want and why?

— Reply to this email directly, view it on GitHubhttps://github.com/Azure/azure-sdk-for-net/issues/40630#issuecomment-1866697858, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AA5UXDLUFMYASF3GXWMNN3TYKRX6JAVCNFSM6AAAAABALROYSCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRWGY4TOOBVHA. You are receiving this because you were mentioned.Message ID: @.***>

joebeernink commented 6 months ago

@mattmccleary Do you have an ETA on when we might have a fix for this? I believe this is the class that needs to be updated https://github.com/Azure/azure-sdk-for-net/blob/Azure.Monitor.OpenTelemetry.Exporter_1.1.0/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Internals/MetricHelper.cs

to do some of the same things that this class does: https://github.com/Azure/azure-sdk-for-net/blob/Azure.Monitor.OpenTelemetry.Exporter_1.1.0/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Internals/LogsHelper.cs

mattmccleary commented 6 months ago

Hi Joe - I agree that custom metric dimensions is important. I'm working to get an update. One thing I'm still trying to understand - wouldn't CustomerId and CustomerInstanceId be high cardinality dimensions? As customers grow, seems like the number of time-series would blow up and hit cardinality caps in OTel SDK and possibly Azure backend. Feel free to shoot me a quick email at mmcc@microsoft.com if you want to set up a few minutes to chat.

cijothomas commented 6 months ago

As customers grow, seems like the number of time-series would blow up and hit cardinality caps in OTel SDK and possibly Azure backend

The caps in OTel SDKs are to protect accidental/buggy code from causing Out-Of-Memory! The limit (default 2000) is configurable to match user needs.

CustomerId, despite high-cardinality, is a valid dimension used in some large organizations built on Azure, so unlike to hit any Azure backend limits. (eg: Services needing to track per custom SLO.)