From 4ff6150df6f488852677793438e207eee6de8c2b Mon Sep 17 00:00:00 2001 From: Yun-Ting Lin Date: Mon, 6 Feb 2023 13:10:54 -0800 Subject: [PATCH 1/6] removed cpu utilization and corresponding tests --- .../ProcessMetrics.cs | 54 +++---------------- .../ProcessMetricsTests.cs | 44 +-------------- 2 files changed, 9 insertions(+), 89 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.Process/ProcessMetrics.cs b/src/OpenTelemetry.Instrumentation.Process/ProcessMetrics.cs index 410c2f5ce4..5835ce228c 100644 --- a/src/OpenTelemetry.Instrumentation.Process/ProcessMetrics.cs +++ b/src/OpenTelemetry.Instrumentation.Process/ProcessMetrics.cs @@ -16,7 +16,6 @@ #nullable enable -using System; using System.Collections.Generic; using System.Diagnostics.Metrics; using System.Reflection; @@ -24,25 +23,16 @@ 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> 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(Math.Min(userProcessorUtilization, 1D), new KeyValuePair("state", "user")), - new Measurement(Math.Min(privilegedProcessorUtilization, 1D), new KeyValuePair("state", "system")), - }; } } diff --git a/test/OpenTelemetry.Instrumentation.Process.Tests/ProcessMetricsTests.cs b/test/OpenTelemetry.Instrumentation.Process.Tests/ProcessMetricsTests.cs index fd0e4e94b3..789bc52d55 100644 --- a/test/OpenTelemetry.Instrumentation.Process.Tests/ProcessMetricsTests.cs +++ b/test/OpenTelemetry.Instrumentation.Process.Tests/ProcessMetricsTests.cs @@ -36,15 +36,13 @@ public void ProcessMetricsAreCaptured() meterProvider.ForceFlush(MaxTimeToAllowForFlush); - Assert.True(exportedItems.Count == 5); + Assert.True(exportedItems.Count == 4); var physicalMemoryMetric = exportedItems.FirstOrDefault(i => i.Name == "process.memory.usage"); Assert.NotNull(physicalMemoryMetric); var virtualMemoryMetric = exportedItems.FirstOrDefault(i => i.Name == "process.memory.virtual"); Assert.NotNull(virtualMemoryMetric); var cpuTimeMetric = exportedItems.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"); Assert.NotNull(threadMetric); } @@ -87,44 +85,6 @@ public void CpuTimeMetricsAreCaptured() } [Fact] - public void CpuUtilizationMetricsAreCaptured() - { - var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() - .AddProcessInstrumentation() - .AddInMemoryExporter(exportedItems) - .Build(); - - meterProvider.ForceFlush(MaxTimeToAllowForFlush); - - var cpuUtilizationMetric = exportedItems.FirstOrDefault(i => i.Name == "process.cpu.utilization"); - Assert.NotNull(cpuUtilizationMetric); - - var userCpuUtilizationCaptured = false; - var systemCpuUtilizationCaptured = false; - - var iter = cpuUtilizationMetric.GetMetricPoints().GetEnumerator(); - while (iter.MoveNext() && (!userCpuUtilizationCaptured || !systemCpuUtilizationCaptured)) - { - foreach (var tag in iter.Current.Tags) - { - 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); - } - - // See: https://github.com/open-telemetry/opentelemetry-dotnet-contrib/issues/831 - [Fact(Skip = "There are known issues with this test.")] public void CheckValidGaugeValueWhen2MeterProviderInstancesHaveTheSameMeterName() { var exportedItemsA = new List(); @@ -173,8 +133,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); } From 21dc392937ccef79b3d3f19eee975536974b173d Mon Sep 17 00:00:00 2001 From: Yun-Ting Lin Date: Mon, 6 Feb 2023 13:34:04 -0800 Subject: [PATCH 2/6] add tests --- .../ProcessMetricsTests.cs | 97 +++++++++++++++++-- 1 file changed, 88 insertions(+), 9 deletions(-) diff --git a/test/OpenTelemetry.Instrumentation.Process.Tests/ProcessMetricsTests.cs b/test/OpenTelemetry.Instrumentation.Process.Tests/ProcessMetricsTests.cs index 789bc52d55..5687d03d9a 100644 --- a/test/OpenTelemetry.Instrumentation.Process.Tests/ProcessMetricsTests.cs +++ b/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,23 +30,44 @@ public class ProcessMetricsTests [Fact] public void ProcessMetricsAreCaptured() { - var exportedItems = new List(); - using var meterProvider = Sdk.CreateMeterProviderBuilder() + var exportedItemsA = new List(); + var meterProviderA = Sdk.CreateMeterProviderBuilder() .AddProcessInstrumentation() - .AddInMemoryExporter(exportedItems) + .AddInMemoryExporter(exportedItemsA) .Build(); - meterProvider.ForceFlush(MaxTimeToAllowForFlush); + meterProviderA.ForceFlush(MaxTimeToAllowForFlush); - Assert.True(exportedItems.Count == 4); - 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 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(); + + 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] @@ -84,6 +107,62 @@ public void CpuTimeMetricsAreCaptured() Assert.True(systemTimeCaptured); } + [Fact] + public void ProcessMetricsAreCapturedWhenTasksOverlap() + { + var exportedItemsA =new List(); + var exportedItemsB = new List(); + + var tasks = new List() + { + Task.Run(() => + { + var meterProviderA = Sdk.CreateMeterProviderBuilder() + .AddProcessInstrumentation() + .AddInMemoryExporter(exportedItemsA) + .Build(); + + Thread.Sleep(3000); // increase the odds of 2 tasks overlaps + + meterProviderA.ForceFlush(MaxTimeToAllowForFlush); + }), + + Task.Run(() => + { + 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); + } + [Fact] public void CheckValidGaugeValueWhen2MeterProviderInstancesHaveTheSameMeterName() { From 0699f9c97018eafc5842e856f9a64b67a1ffa1ed Mon Sep 17 00:00:00 2001 From: Yun-Ting Lin Date: Mon, 6 Feb 2023 14:06:43 -0800 Subject: [PATCH 3/6] changelog --- src/OpenTelemetry.Instrumentation.Process/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/OpenTelemetry.Instrumentation.Process/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.Process/CHANGELOG.md index 6e9571be36..5e8db79a86 100644 --- a/src/OpenTelemetry.Instrumentation.Process/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.Process/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +* [Instrumentation.Process] Removed CPU utilization metric. + ([#972](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/972)) + ## 1.0.0-alpha.5 Released 2023-Feb-02 From ddd4c93020184fd96c6f6b5e52993a2ee241b812 Mon Sep 17 00:00:00 2001 From: Yun-Ting Lin Date: Mon, 6 Feb 2023 14:08:23 -0800 Subject: [PATCH 4/6] SA --- .../ProcessMetricsTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/OpenTelemetry.Instrumentation.Process.Tests/ProcessMetricsTests.cs b/test/OpenTelemetry.Instrumentation.Process.Tests/ProcessMetricsTests.cs index 5687d03d9a..e63b89f1fe 100644 --- a/test/OpenTelemetry.Instrumentation.Process.Tests/ProcessMetricsTests.cs +++ b/test/OpenTelemetry.Instrumentation.Process.Tests/ProcessMetricsTests.cs @@ -110,7 +110,7 @@ public void CpuTimeMetricsAreCaptured() [Fact] public void ProcessMetricsAreCapturedWhenTasksOverlap() { - var exportedItemsA =new List(); + var exportedItemsA = new List(); var exportedItemsB = new List(); var tasks = new List() From 1e2cc977da5d612d98b8cb31df4e4754939fd214 Mon Sep 17 00:00:00 2001 From: Yun-Ting Lin Date: Tue, 7 Feb 2023 13:21:25 -0800 Subject: [PATCH 5/6] comment --- src/OpenTelemetry.Instrumentation.Process/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Instrumentation.Process/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.Process/CHANGELOG.md index 5e8db79a86..ccc3129958 100644 --- a/src/OpenTelemetry.Instrumentation.Process/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.Process/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased -* [Instrumentation.Process] Removed CPU utilization metric. +* Removed CPU utilization metric `process.cpu.utilization`. ([#972](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/972)) ## 1.0.0-alpha.5 From edb9dafdafeaa96d15a65fe03d19b27d44feaee3 Mon Sep 17 00:00:00 2001 From: Yun-Ting Lin Date: Tue, 7 Feb 2023 13:43:48 -0800 Subject: [PATCH 6/6] readmd --- .../README.md | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.Process/README.md b/src/OpenTelemetry.Instrumentation.Process/README.md index 6d3562b42a..9481e7fb58 100644 --- a/src/OpenTelemetry.Instrumentation.Process/README.md +++ b/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.