[Instrumentation.Process] Removed CPU utilization metric process.cpu.utilization.

src/OpenTelemetry.Instrumentation.Process/CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## Unreleased
 
+* Removed CPU utilization metric `process.cpu.utilization`.
+  ([#972](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/972))
+
 ## 1.0.0-alpha.5
 
 Released 2023-Feb-02

src/OpenTelemetry.Instrumentation.Process/ProcessMetrics.cs

@@ -16,33 +16,23 @@
 
 #nullable enable
 
-using System;
 using System.Collections.Generic;
 using System.Diagnostics.Metrics;
 using System.Reflection;
 using Diagnostics = System.Diagnostics;
 
 namespace OpenTelemetry.Instrumentation.Process;
 
-internal sealed class ProcessMetrics : IDisposable
+internal sealed class ProcessMetrics
 {
     internal static readonly AssemblyName AssemblyName = typeof(ProcessMetrics).Assembly.GetName();
     internal static readonly string MeterName = AssemblyName.Name;
 
-    private readonly Meter meterInstance = new(MeterName, AssemblyName.Version.ToString());
+    private static readonly Meter MeterInstance = new(MeterName, AssemblyName.Version.ToString());
 
-    // vars for calculating CPU utilization
-    private DateTime lastCollectionTimeUtc;
-    private double lastCollectedUserProcessorTime;
-    private double lastCollectedPrivilegedProcessorTime;
-
-    public ProcessMetrics(ProcessInstrumentationOptions options)
+    static ProcessMetrics()
     {
-        this.lastCollectionTimeUtc = DateTime.UtcNow;
-        this.lastCollectedUserProcessorTime = Diagnostics.Process.GetCurrentProcess().UserProcessorTime.TotalSeconds;
-        this.lastCollectedPrivilegedProcessorTime = Diagnostics.Process.GetCurrentProcess().PrivilegedProcessorTime.TotalSeconds;
-
-        this.meterInstance.CreateObservableUpDownCounter(
+        MeterInstance.CreateObservableUpDownCounter(
             "process.memory.usage",
             () =>
             {
@@ -51,7 +41,7 @@ public ProcessMetrics(ProcessInstrumentationOptions options)
             unit: "By",
             description: "The amount of physical memory allocated for this process.");
 
-        this.meterInstance.CreateObservableUpDownCounter(
+        MeterInstance.CreateObservableUpDownCounter(
             "process.memory.virtual",
             () =>
             {
@@ -60,7 +50,7 @@ public ProcessMetrics(ProcessInstrumentationOptions options)
             unit: "By",
             description: "The amount of committed virtual memory for this process.");
 
-        this.meterInstance.CreateObservableCounter(
+        MeterInstance.CreateObservableCounter(
             "process.cpu.time",
             () =>
             {
@@ -74,16 +64,7 @@ public ProcessMetrics(ProcessInstrumentationOptions options)
             unit: "s",
             description: "Total CPU seconds broken down by different states.");
 
-        this.meterInstance.CreateObservableGauge(
-            "process.cpu.utilization",
-            () =>
-            {
-                return this.GetCpuUtilization();
-            },
-            unit: "1",
-            description: "Difference in process.cpu.time since the last measurement, divided by the elapsed time and number of CPUs available to the process.");
-
-        this.meterInstance.CreateObservableUpDownCounter(
+        MeterInstance.CreateObservableUpDownCounter(
             "process.threads",
             () =>
             {
@@ -93,26 +74,7 @@ public ProcessMetrics(ProcessInstrumentationOptions options)
             description: "Process threads count.");
     }
 
-    public void Dispose()
-    {
-        this.meterInstance.Dispose();
-    }
-
-    private IEnumerable<Measurement<double>> GetCpuUtilization()
+    public ProcessMetrics(ProcessInstrumentationOptions options)
     {
-        var process = Diagnostics.Process.GetCurrentProcess();
-        var elapsedTimeForAllCpus = (DateTime.UtcNow - this.lastCollectionTimeUtc).TotalSeconds * Environment.ProcessorCount;
-        var userProcessorUtilization = (process.UserProcessorTime.TotalSeconds - this.lastCollectedUserProcessorTime) / elapsedTimeForAllCpus;
-        var privilegedProcessorUtilization = (process.PrivilegedProcessorTime.TotalSeconds - this.lastCollectedPrivilegedProcessorTime) / elapsedTimeForAllCpus;
-
-        this.lastCollectionTimeUtc = DateTime.UtcNow;
-        this.lastCollectedUserProcessorTime = process.UserProcessorTime.TotalSeconds;
-        this.lastCollectedPrivilegedProcessorTime = process.PrivilegedProcessorTime.TotalSeconds;
-
-        return new[]
-        {
-            new Measurement<double>(Math.Min(userProcessorUtilization, 1D), new KeyValuePair<string, object?>("state", "user")),
-            new Measurement<double>(Math.Min(privilegedProcessorUtilization, 1D), new KeyValuePair<string, object?>("state", "system")),
-        };
     }
 }

src/OpenTelemetry.Instrumentation.Process/README.md

@@ -92,23 +92,6 @@ Gets the user processor time for this process.
 * [Process.PrivilegedProcessorTime](https://learn.microsoft.com/dotnet/api/system.diagnostics.process.privilegedprocessortime):
 Gets the privileged processor time for this process.
 
-### process.cpu.utilization
-
-Difference in process.cpu.time since the last measurement,
-divided by the elapsed time and number of CPUs available to the process.
-
-| Units | Instrument Type   | Value Type | Attribute Key(s) | Attribute Values |
-|-------|-------------------|------------|------------------|------------------|
-|  `1`  | ObservableCounter | `Double`   | state            | user, system     |
-
-The APIs used to retrieve the values are:
-
-* [Process.UserProcessorTime](https://learn.microsoft.com/dotnet/api/system.diagnostics.process.userprocessortime):
-Gets the user processor time for this process.
-
-* [Process.PrivilegedProcessorTime](https://learn.microsoft.com/dotnet/api/system.diagnostics.process.privilegedprocessortime):
-Gets the privileged processor time for this process.
-
 ### process.threads
 
 Process threads count.

test/OpenTelemetry.Instrumentation.Process.Tests/ProcessMetricsTests.cs

@@ -16,6 +16,8 @@
 
 using System.Collections.Generic;
 using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
 using OpenTelemetry.Metrics;
 using Xunit;
 
@@ -28,25 +30,44 @@ public class ProcessMetricsTests
     [Fact]
     public void ProcessMetricsAreCaptured()
     {
-        var exportedItems = new List<Metric>();
-        using var meterProvider = Sdk.CreateMeterProviderBuilder()
+        var exportedItemsA = new List<Metric>();
+        var meterProviderA = Sdk.CreateMeterProviderBuilder()
             .AddProcessInstrumentation()
-            .AddInMemoryExporter(exportedItems)
+            .AddInMemoryExporter(exportedItemsA)
             .Build();
 
-        meterProvider.ForceFlush(MaxTimeToAllowForFlush);
+        meterProviderA.ForceFlush(MaxTimeToAllowForFlush);
 
-        Assert.True(exportedItems.Count == 5);
-        var physicalMemoryMetric = exportedItems.FirstOrDefault(i => i.Name == "process.memory.usage");
+        Assert.True(exportedItemsA.Count == 4);
+        var physicalMemoryMetric = exportedItemsA.FirstOrDefault(i => i.Name == "process.memory.usage");
         Assert.NotNull(physicalMemoryMetric);
-        var virtualMemoryMetric = exportedItems.FirstOrDefault(i => i.Name == "process.memory.virtual");
+        var virtualMemoryMetric = exportedItemsA.FirstOrDefault(i => i.Name == "process.memory.virtual");
         Assert.NotNull(virtualMemoryMetric);
-        var cpuTimeMetric = exportedItems.FirstOrDefault(i => i.Name == "process.cpu.time");
+        var cpuTimeMetric = exportedItemsA.FirstOrDefault(i => i.Name == "process.cpu.time");
         Assert.NotNull(cpuTimeMetric);
-        var cpuUtilizationMetric = exportedItems.FirstOrDefault(i => i.Name == "process.cpu.utilization");
-        Assert.NotNull(cpuUtilizationMetric);
-        var threadMetric = exportedItems.FirstOrDefault(i => i.Name == "process.threads");
+        var threadMetric = exportedItemsA.FirstOrDefault(i => i.Name == "process.threads");
         Assert.NotNull(threadMetric);
+
+        exportedItemsA.Clear();
+
+        var exportedItemsB = new List<Metric>();
+
+        using var meterProviderB = Sdk.CreateMeterProviderBuilder()
+            .AddProcessInstrumentation()
+            .AddInMemoryExporter(exportedItemsA, metricReaderOptions =>
+            {
+                metricReaderOptions.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds = 1500;
+            })
+            .AddInMemoryExporter(exportedItemsB, metricReaderOptions =>
+            {
+                metricReaderOptions.PeriodicExportingMetricReaderOptions.ExportIntervalMilliseconds = 5000;
+            })
+            .Build();
+
+        meterProviderB.ForceFlush(MaxTimeToAllowForFlush);
+
+        Assert.True(exportedItemsA.Count == 4);
+        Assert.True(exportedItemsB.Count == 4);
     }
 
     [Fact]
@@ -87,44 +108,62 @@ public void CpuTimeMetricsAreCaptured()
     }
 
     [Fact]
-    public void CpuUtilizationMetricsAreCaptured()
+    public void ProcessMetricsAreCapturedWhenTasksOverlap()
     {
-        var exportedItems = new List<Metric>();
-        using var meterProvider = Sdk.CreateMeterProviderBuilder()
-            .AddProcessInstrumentation()
-            .AddInMemoryExporter(exportedItems)
-            .Build();
+        var exportedItemsA = new List<Metric>();
+        var exportedItemsB = new List<Metric>();
 
-        meterProvider.ForceFlush(MaxTimeToAllowForFlush);
+        var tasks = new List<Task>()
+        {
+            Task.Run(() =>
+            {
+                var meterProviderA = Sdk.CreateMeterProviderBuilder()
+                    .AddProcessInstrumentation()
+                    .AddInMemoryExporter(exportedItemsA)
+                    .Build();
 
-        var cpuUtilizationMetric = exportedItems.FirstOrDefault(i => i.Name == "process.cpu.utilization");
-        Assert.NotNull(cpuUtilizationMetric);
+                Thread.Sleep(3000); // increase the odds of 2 tasks overlaps
 
-        var userCpuUtilizationCaptured = false;
-        var systemCpuUtilizationCaptured = false;
+                meterProviderA.ForceFlush(MaxTimeToAllowForFlush);
+            }),
 
-        var iter = cpuUtilizationMetric.GetMetricPoints().GetEnumerator();
-        while (iter.MoveNext() && (!userCpuUtilizationCaptured || !systemCpuUtilizationCaptured))
-        {
-            foreach (var tag in iter.Current.Tags)
+            Task.Run(() =>
             {
-                if (tag.Key == "state" && tag.Value.ToString() == "user")
-                {
-                    userCpuUtilizationCaptured = true;
-                }
-                else if (tag.Key == "state" && tag.Value.ToString() == "system")
-                {
-                    systemCpuUtilizationCaptured = true;
-                }
-            }
-        }
-
-        Assert.True(userCpuUtilizationCaptured);
-        Assert.True(systemCpuUtilizationCaptured);
+                var meterProviderB = Sdk.CreateMeterProviderBuilder()
+                    .AddProcessInstrumentation()
+                    .AddInMemoryExporter(exportedItemsB)
+                    .Build();
+
+                Thread.Sleep(3000); // increase the odds of 2 tasks overlaps
+
+                meterProviderB.ForceFlush(MaxTimeToAllowForFlush);
+            }),
+        };
+
+        Task.WaitAll(tasks.ToArray());
+
+        Assert.True(exportedItemsA.Count == 4);
+        var physicalMemoryMetricA = exportedItemsA.FirstOrDefault(i => i.Name == "process.memory.usage");
+        Assert.NotNull(physicalMemoryMetricA);
+        var virtualMemoryMetricA = exportedItemsA.FirstOrDefault(i => i.Name == "process.memory.virtual");
+        Assert.NotNull(virtualMemoryMetricA);
+        var cpuTimeMetricA = exportedItemsA.FirstOrDefault(i => i.Name == "process.cpu.time");
+        Assert.NotNull(cpuTimeMetricA);
+        var threadMetricA = exportedItemsA.FirstOrDefault(i => i.Name == "process.threads");
+        Assert.NotNull(threadMetricA);
+
+        Assert.True(exportedItemsB.Count == 4);
+        var physicalMemoryMetricB = exportedItemsB.FirstOrDefault(i => i.Name == "process.memory.usage");
+        Assert.NotNull(physicalMemoryMetricB);
+        var virtualMemoryMetricB = exportedItemsB.FirstOrDefault(i => i.Name == "process.memory.virtual");
+        Assert.NotNull(virtualMemoryMetricB);
+        var cpuTimeMetricB = exportedItemsB.FirstOrDefault(i => i.Name == "process.cpu.time");
+        Assert.NotNull(cpuTimeMetricB);
+        var threadMetricB = exportedItemsB.FirstOrDefault(i => i.Name == "process.threads");
+        Assert.NotNull(threadMetricB);
     }
 
-    // See: https://github.com/open-telemetry/opentelemetry-dotnet-contrib/issues/831
-    [Fact(Skip = "There are known issues with this test.")]
+    [Fact]
     public void CheckValidGaugeValueWhen2MeterProviderInstancesHaveTheSameMeterName()
     {
         var exportedItemsA = new List<Metric>();
@@ -173,8 +212,6 @@ public void CheckValidGaugeValueWhen2MeterProviderInstancesHaveTheSameMeterName(
 
         meterProviderA.ForceFlush(MaxTimeToAllowForFlush);
 
-        // Note: This fails due to:
-        // https://github.com/open-telemetry/opentelemetry-dotnet/blob/main/src/OpenTelemetry/Metrics/MetricReaderExt.cs#L244-L249
         Assert.NotEmpty(exportedItemsA);
         Assert.Empty(exportedItemsB);
     }