From bf7e92147a16f5b05c5e814c2ee8bd16ec5ae42f Mon Sep 17 00:00:00 2001 From: Russ Cam Date: Thu, 28 Oct 2021 11:45:54 +1000 Subject: [PATCH 1/3] Improve Performance counter handling This commit improves the handling of performance counters used to get the total CPU % time on Windows. When the "Processor" performance counter category does not exist, try using the "Processor Information" category name. Include information in the log message about checking permissions only when a SecurityException is thrown. Closes #1505 --- .../MetricsProvider/SystemTotalCpuProvider.cs | 46 ++++++++++++++----- 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/src/Elastic.Apm/Metrics/MetricsProvider/SystemTotalCpuProvider.cs b/src/Elastic.Apm/Metrics/MetricsProvider/SystemTotalCpuProvider.cs index 2c6ff8bbf..2d1591cc0 100644 --- a/src/Elastic.Apm/Metrics/MetricsProvider/SystemTotalCpuProvider.cs +++ b/src/Elastic.Apm/Metrics/MetricsProvider/SystemTotalCpuProvider.cs @@ -10,6 +10,7 @@ using System.IO; using System.Linq; using System.Runtime.InteropServices; +using System.Security; using Elastic.Apm.Api; using Elastic.Apm.Helpers; using Elastic.Apm.Logging; @@ -30,31 +31,52 @@ public SystemTotalCpuProvider(IApmLogger logger) _logger = logger.Scoped(nameof(SystemTotalCpuProvider)); if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { + var categoryName = "Processor"; try { - _processorTimePerfCounter = new PerformanceCounter("Processor", "% Processor Time", "_Total"); + try + { + _processorTimePerfCounter = new PerformanceCounter(categoryName, "% Processor Time", "_Total"); + } + catch (InvalidOperationException e) + { + _logger.Debug()?.LogException(e, "Error instantiating '{CategoryName}' performance counter.", categoryName); + _processorTimePerfCounter?.Dispose(); + // If the Processor performance counter category does not exist, try Processor Information. + categoryName = "Processor Information"; + _processorTimePerfCounter = new PerformanceCounter(categoryName, "% Processor Time", "_Total"); + } + //The perf. counter API returns 0 the for the 1. call (probably because there is no delta in the 1. call) - so we just call it here first _processorTimePerfCounter.NextValue(); } catch (Exception e) { - _logger.Error() - ?.LogException(e, "Failed instantiating PerformanceCounter " - + "- please make sure the current user has permissions to read performance counters. E.g. make sure the current user is member of " - + "the 'Performance Monitor Users' group"); + if (e is SecurityException) + { + _logger.Error() + ?.LogException(e, "Error instantiating '{CategoryName}' performance counter." + + "- please make sure the current user has permissions to read performance counters. E.g. make sure the current user is member of " + + "the 'Performance Monitor Users' group", categoryName); + } + else + { + _logger.Error() + ?.LogException(e, "Error instantiating '{CategoryName}' performance counter", categoryName); + } _processorTimePerfCounter?.Dispose(); _processorTimePerfCounter = null; } } + else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) + { + var (success, idle, total) = ReadProcStat(); + if (!success) return; - if (!RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) return; - - var (success, idle, total) = ReadProcStat(); - if (!success) return; - - _prevIdleTime = idle; - _prevTotalTime = total; + _prevIdleTime = idle; + _prevTotalTime = total; + } } internal SystemTotalCpuProvider(IApmLogger logger, StreamReader procStatStreamReader) From 16202942ab70ebc02ef0c48db0b25e2be180aa88 Mon Sep 17 00:00:00 2001 From: Russ Cam Date: Mon, 1 Nov 2021 15:19:24 +1000 Subject: [PATCH 2/3] Add docs for performance counter permissions Catch correct security exception --- docs/metrics.asciidoc | 20 +++++++++++++++++++ .../MetricsProvider/SystemTotalCpuProvider.cs | 8 ++++---- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/docs/metrics.asciidoc b/docs/metrics.asciidoc index 2b1a1589e..c2dac6cfd 100644 --- a/docs/metrics.asciidoc +++ b/docs/metrics.asciidoc @@ -22,6 +22,26 @@ The metrics will be stored in the `apm-*` index and have the `processor.event` p As of APM version 6.6, these metrics will be visualized in the APM app. +[IMPORTANT] +-- +System metrics are collected using Performance Counters on Windows. The account under which a traced +application runs must be part of the **Performance Monitor Users** group to be able to access +performance counter values. + +An account can be added to the **Performance Monitor Users** group from the command line + +[source,sh] +---- +net localgroup "Performance Monitor Users" "" /add <1> +---- +<1> `` is the account under which the traced application runs + +For applications running in IIS, +https://docs.microsoft.com/en-us/iis/manage/configuring-security/application-pool-identities[IIS application pool identities use _virtual_ accounts] +with a name following the convention `IIS APPPOOL\`. An individual application pool identity +can be added to the **Performance Monitor Users** group. +-- + For more system metrics, consider installing {metricbeat-ref}/index.html[metricbeat] on your hosts. *`system.cpu.total.norm.pct`*:: diff --git a/src/Elastic.Apm/Metrics/MetricsProvider/SystemTotalCpuProvider.cs b/src/Elastic.Apm/Metrics/MetricsProvider/SystemTotalCpuProvider.cs index 2d1591cc0..51675e8ab 100644 --- a/src/Elastic.Apm/Metrics/MetricsProvider/SystemTotalCpuProvider.cs +++ b/src/Elastic.Apm/Metrics/MetricsProvider/SystemTotalCpuProvider.cs @@ -10,7 +10,6 @@ using System.IO; using System.Linq; using System.Runtime.InteropServices; -using System.Security; using Elastic.Apm.Api; using Elastic.Apm.Helpers; using Elastic.Apm.Logging; @@ -52,12 +51,12 @@ public SystemTotalCpuProvider(IApmLogger logger) } catch (Exception e) { - if (e is SecurityException) + if (e is UnauthorizedAccessException) { _logger.Error() ?.LogException(e, "Error instantiating '{CategoryName}' performance counter." - + "- please make sure the current user has permissions to read performance counters. E.g. make sure the current user is member of " - + "the 'Performance Monitor Users' group", categoryName); + + " Process does not have permissions to read performance counters." + + " See https://www.elastic.co/guide/en/apm/agent/dotnet/current/metrics.html#metrics-system to see how to configure.", categoryName); } else { @@ -65,6 +64,7 @@ public SystemTotalCpuProvider(IApmLogger logger) ?.LogException(e, "Error instantiating '{CategoryName}' performance counter", categoryName); } + _logger.Warning()?.Log("System metrics won't be collected"); _processorTimePerfCounter?.Dispose(); _processorTimePerfCounter = null; } From 60ff3af4ac5494d40aa8dd8d74991cefec252fc7 Mon Sep 17 00:00:00 2001 From: Russ Cam Date: Wed, 3 Nov 2021 07:13:40 +1000 Subject: [PATCH 3/3] update wording from PR review --- docs/metrics.asciidoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/metrics.asciidoc b/docs/metrics.asciidoc index c2dac6cfd..e36429920 100644 --- a/docs/metrics.asciidoc +++ b/docs/metrics.asciidoc @@ -24,7 +24,7 @@ As of APM version 6.6, these metrics will be visualized in the APM app. [IMPORTANT] -- -System metrics are collected using Performance Counters on Windows. The account under which a traced +System CPU usage metric is collected using Performance Counters on Windows. The account under which a traced application runs must be part of the **Performance Monitor Users** group to be able to access performance counter values. @@ -39,7 +39,7 @@ net localgroup "Performance Monitor Users" "" /add <1> For applications running in IIS, https://docs.microsoft.com/en-us/iis/manage/configuring-security/application-pool-identities[IIS application pool identities use _virtual_ accounts] with a name following the convention `IIS APPPOOL\`. An individual application pool identity -can be added to the **Performance Monitor Users** group. +can be added to the **Performance Monitor Users** group using the `net localgroup` command above. -- For more system metrics, consider installing {metricbeat-ref}/index.html[metricbeat] on your hosts.