From d55df48c6dac9c6f657aefcca6256719b2004258 Mon Sep 17 00:00:00 2001 From: kkeirstead <85592574+kkeirstead@users.noreply.github.com> Date: Wed, 30 Nov 2022 13:11:34 -0800 Subject: [PATCH 01/14] [Dotnet Monitor] Ignore - Adding System.Diagnostics.Metrics Support (#3529) * Got basic counter rate end-to-end working - currently in a broken state as I investigate other types of metrics * Leftovers from previous commit * Gauges working for systems diagnostics metrics * Added in histogram, adding in options for maxHistograms and maxTimeSeries * Added in error payloads for logging purposes * Temporarily changed visibility for testing - this may be reverted later * Now handling multiple sessions * Added filtering for counters, instead of allowing all counters for a provider to go through. * Handle observable... errors * Some cleanup, added error event check * Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20221129.1 (#3528) [main] Update dependencies from dotnet/source-build-reference-packages Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com> --- .../MetricSourceConfiguration.cs | 32 +++ .../Counters/CounterFilter.cs | 6 + .../Counters/CounterPayload.cs | 75 ++++- .../Counters/EventCounterPipeline.cs | 9 +- .../EventPipeCounterPipelineSettings.cs | 4 + .../Counters/ICounterPayload.cs | 6 +- .../Counters/ICountersLogger.cs | 2 +- .../Counters/TraceEventExtensions.cs | 268 +++++++++++++++++- .../EventCounter/EventCounterTrigger.cs | 4 +- 9 files changed, 393 insertions(+), 13 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/MetricSourceConfiguration.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/MetricSourceConfiguration.cs index 4047e11242..1c3891f123 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/MetricSourceConfiguration.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/MetricSourceConfiguration.cs @@ -16,6 +16,7 @@ namespace Microsoft.Diagnostics.Monitoring.EventPipe public sealed class MetricSourceConfiguration : MonitoringSourceConfiguration { private readonly IList _eventPipeProviders; + public string SessionId { get; private set; } public MetricSourceConfiguration(float metricIntervalSeconds, IEnumerable customProviderNames) { @@ -45,6 +46,37 @@ public MetricSourceConfiguration(float metricIntervalSeconds, IEnumerable customProviderNames, int maxHistograms, int maxTimeSeries) : this(metricIntervalSeconds, customProviderNames) + { + const long TimeSeriesValues = 0x2; + StringBuilder metrics = new StringBuilder(); + foreach (string provider in customProviderNames) + { + if (metrics.Length != 0) + { + metrics.Append(","); + } + + metrics.Append(provider); + } + + SessionId = Guid.NewGuid().ToString(); + + EventPipeProvider metricsEventSourceProvider = + new EventPipeProvider("System.Diagnostics.Metrics", EventLevel.Informational, TimeSeriesValues, + new Dictionary() + { + { "SessionId", SessionId }, + { "Metrics", metrics.ToString() }, + { "RefreshInterval", MetricIntervalSeconds.ToString() }, + { "MaxTimeSeries", maxTimeSeries.ToString() }, + { "MaxHistograms", maxHistograms.ToString() } + } + ); + + _eventPipeProviders = _eventPipeProviders.Append(metricsEventSourceProvider).ToArray(); + } + private string MetricIntervalSeconds { get; } public override IList GetProviders() => _eventPipeProviders; diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterFilter.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterFilter.cs index dd83243ef7..9ad08494d4 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterFilter.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterFilter.cs @@ -44,6 +44,12 @@ public bool IsIncluded(string providerName, string counterName, int intervalMill { return false; } + + return IsIncluded(providerName, counterName); + } + + public bool IsIncluded(string providerName, string counterName) + { if (_enabledCounters.Count == 0) { return true; diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs index 0108e722bc..fc9f8a9485 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs @@ -8,7 +8,7 @@ namespace Microsoft.Diagnostics.Monitoring.EventPipe { - internal class CounterPayload : ICounterPayload + public class CounterPayload : ICounterPayload { #if NETSTANDARD private static readonly IReadOnlyDictionary Empty = new ReadOnlyDictionary(new Dictionary(0)); @@ -35,13 +35,26 @@ public CounterPayload(DateTime timestamp, Provider = provider; Interval = interval; Metadata = metadata ?? Empty; + EventType = EventType.Gauge; + } + + // Copied from dotnet-counters + public CounterPayload(string providerName, string name, string displayName, string displayUnits, Dictionary metadata, double value, DateTime timestamp, string type, EventType eventType) + { + Provider = providerName; + Name = name; + Metadata = metadata ?? Empty; + Value = value; + Timestamp = timestamp; + CounterType = (CounterType)Enum.Parse(typeof(CounterType), type); + EventType = eventType; } public string Namespace { get; } public string Name { get; } - public string DisplayName { get; } + public string DisplayName { get; protected set; } public string Unit { get; } @@ -56,5 +69,63 @@ public CounterPayload(DateTime timestamp, public string Provider { get; } public IReadOnlyDictionary Metadata { get; } + + public EventType EventType { get; set; } + + } + + public class GaugePayload : CounterPayload + { + public GaugePayload(string providerName, string name, string displayName, string displayUnits, Dictionary metadata, double value, DateTime timestamp) : + base(providerName, name, displayName, displayUnits, metadata, value, timestamp, "Metric", EventType.Gauge) + { + // In case these properties are not provided, set them to appropriate values. + string counterName = string.IsNullOrEmpty(displayName) ? name : displayName; + DisplayName = !string.IsNullOrEmpty(displayUnits) ? $"{counterName} ({displayUnits})" : counterName; + } + } + + public class RatePayload : CounterPayload + { + public RatePayload(string providerName, string name, string displayName, string displayUnits, Dictionary metadata, double value, double intervalSecs, DateTime timestamp) : + base(providerName, name, displayName, displayUnits, metadata, value, timestamp, "Rate", EventType.Rate) + { + // In case these properties are not provided, set them to appropriate values. + string counterName = string.IsNullOrEmpty(displayName) ? name : displayName; + string unitsName = string.IsNullOrEmpty(displayUnits) ? "Count" : displayUnits; + string intervalName = intervalSecs.ToString() + " sec"; + DisplayName = $"{counterName} ({unitsName} / {intervalName})"; + } + } + + public class PercentilePayload : CounterPayload + { + public PercentilePayload(string providerName, string name, string displayName, string displayUnits, Dictionary metadata, double val, DateTime timestamp) : + base(providerName, name, displayName, displayUnits, metadata, val, timestamp, "Metric", EventType.Histogram) + { + // In case these properties are not provided, set them to appropriate values. + string counterName = string.IsNullOrEmpty(displayName) ? name : displayName; + DisplayName = !string.IsNullOrEmpty(displayUnits) ? $"{counterName} ({displayUnits})" : counterName; + } + } + + public class ErrorPayload : CounterPayload + { + public ErrorPayload(string providerName, string name, string displayName, string displayUnits, Dictionary metadata, double val, DateTime timestamp, string errorMessage) : + base(providerName, name, displayName, displayUnits, metadata, val, timestamp, "Metric", EventType.Error) + { + ErrorMessage = errorMessage; + } + + public string ErrorMessage { get; private set; } + } + + // If keep this, should probably put it somewhere else + public enum EventType : int + { + Rate, + Gauge, + Histogram, + Error } } \ No newline at end of file diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/EventCounterPipeline.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/EventCounterPipeline.cs index 14cdfe5b9f..280c552f58 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/EventCounterPipeline.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/EventCounterPipeline.cs @@ -15,6 +15,7 @@ internal class EventCounterPipeline : EventSourcePipeline _loggers; private readonly CounterFilter _filter; + private string _sessionId; public EventCounterPipeline(DiagnosticsClient client, EventPipeCounterPipelineSettings settings, @@ -38,7 +39,11 @@ public EventCounterPipeline(DiagnosticsClient client, protected override MonitoringSourceConfiguration CreateConfiguration() { - return new MetricSourceConfiguration(Settings.CounterIntervalSeconds, _filter.GetProviders()); + var config = new MetricSourceConfiguration(Settings.CounterIntervalSeconds, _filter.GetProviders(), Settings.MaxHistograms, Settings.MaxTimeSeries); + + _sessionId = config.SessionId; + + return config; } protected override async Task OnEventSourceAvailable(EventPipeEventSource eventSource, Func stopSessionAsync, CancellationToken token) @@ -49,7 +54,7 @@ protected override async Task OnEventSourceAvailable(EventPipeEventSource eventS { try { - if (traceEvent.TryGetCounterPayload(_filter, out ICounterPayload counterPayload)) + if (traceEvent.TryGetCounterPayload(_filter, _sessionId, out List counterPayload)) { ExecuteCounterLoggerAction((metricLogger) => metricLogger.Log(counterPayload)); } diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/EventPipeCounterPipelineSettings.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/EventPipeCounterPipelineSettings.cs index ffcb93924a..9aa3855634 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/EventPipeCounterPipelineSettings.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/EventPipeCounterPipelineSettings.cs @@ -15,6 +15,10 @@ internal class EventPipeCounterPipelineSettings : EventSourcePipelineSettings //Do not use TimeSpan here since we may need to synchronize this pipeline interval //with a different session and want to make sure the values are identical. public float CounterIntervalSeconds { get; set; } + + public int MaxHistograms { get; set; } + + public int MaxTimeSeries { get; set; } } internal class EventPipeCounterGroup diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICounterPayload.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICounterPayload.cs index b492a629bb..cc27e69d9b 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICounterPayload.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICounterPayload.cs @@ -8,7 +8,7 @@ namespace Microsoft.Diagnostics.Monitoring.EventPipe { - internal enum CounterType + public enum CounterType { //Same as average or mean Metric, @@ -17,7 +17,7 @@ internal enum CounterType Rate } - internal interface ICounterPayload + public interface ICounterPayload { string Name { get; } @@ -36,5 +36,7 @@ internal interface ICounterPayload float Interval { get; } IReadOnlyDictionary Metadata { get; } + + public EventType EventType { get; set; } } } diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICountersLogger.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICountersLogger.cs index 8a522c9fe9..b0ddf1f753 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICountersLogger.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICountersLogger.cs @@ -14,7 +14,7 @@ internal interface ICountersLogger { //TODO Consider making these async. - void Log(ICounterPayload counter); + void Log(List counter); void PipelineStarted(); void PipelineStopped(); } diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs index f0ec5c6310..96c531f3e7 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs @@ -5,14 +5,15 @@ using Microsoft.Diagnostics.Tracing; using System; using System.Collections.Generic; +using System.Linq; namespace Microsoft.Diagnostics.Monitoring.EventPipe { internal static class TraceEventExtensions { - public static bool TryGetCounterPayload(this TraceEvent traceEvent, CounterFilter filter, out ICounterPayload payload) + public static bool TryGetCounterPayload(this TraceEvent traceEvent, CounterFilter filter, string sessionId, out List payload) { - payload = null; + payload = new List(); if ("EventCounters".Equals(traceEvent.EventName)) { @@ -57,7 +58,8 @@ public static bool TryGetCounterPayload(this TraceEvent traceEvent, CounterFilte // Note that dimensional data such as pod and namespace are automatically added in prometheus and azure monitor scenarios. // We no longer added it here. - payload = new CounterPayload( + + payload.Add(new CounterPayload( traceEvent.TimeStamp, traceEvent.ProviderName, counterName, displayName, @@ -65,13 +67,247 @@ public static bool TryGetCounterPayload(this TraceEvent traceEvent, CounterFilte value, counterType, intervalSec, - metadataDict); + metadataDict)); + return true; } + if (sessionId != null && "System.Diagnostics.Metrics".Equals(traceEvent.ProviderName)) + { + + ICounterPayload individualPayload = null; + + if (traceEvent.EventName == "BeginInstrumentReporting") + { + // Do we want to log something for this? + //HandleBeginInstrumentReporting(traceEvent); + } + if (traceEvent.EventName == "HistogramValuePublished") + { + HandleHistogram(traceEvent, filter, sessionId, out payload); + } + else if (traceEvent.EventName == "GaugeValuePublished") + { + HandleGauge(traceEvent, filter, sessionId, out individualPayload); + } + else if (traceEvent.EventName == "CounterRateValuePublished") + { + HandleCounterRate(traceEvent, filter, sessionId, out individualPayload); + } + else if (traceEvent.EventName == "TimeSeriesLimitReached") + { + HandleTimeSeriesLimitReached(traceEvent, sessionId, out individualPayload); + } + else if (traceEvent.EventName == "HistogramLimitReached") + { + HandleHistogramLimitReached(traceEvent, sessionId, out individualPayload); + } + else if (traceEvent.EventName == "Error") + { + HandleError(traceEvent, sessionId, out individualPayload); + } + else if (traceEvent.EventName == "ObservableInstrumentCallbackError") + { + HandleObservableInstrumentCallbackError(traceEvent, sessionId, out individualPayload); + } + else if (traceEvent.EventName == "MultipleSessionsNotSupportedError") + { + HandleMultipleSessionsNotSupportedError(traceEvent, sessionId, out individualPayload); + } + + if (null != individualPayload) + { + payload.Add(individualPayload); + } + + return null != payload && payload.Any(); + } + return false; } + public static bool TryGetCounterPayload(this TraceEvent traceEvent, CounterFilter filter, out List payload) + { + return TryGetCounterPayload(traceEvent, filter, null, out payload); + } + + private static void HandleGauge(TraceEvent obj, CounterFilter filter, string sessionId, out ICounterPayload payload) + { + payload = null; + + string payloadSessionId = (string)obj.PayloadValue(0); + + if (payloadSessionId != sessionId) + { + return; + } + + string meterName = (string)obj.PayloadValue(1); + //string meterVersion = (string)obj.PayloadValue(2); + string instrumentName = (string)obj.PayloadValue(3); + string unit = (string)obj.PayloadValue(4); + string tags = (string)obj.PayloadValue(5); + string lastValueText = (string)obj.PayloadValue(6); + + Dictionary metadataDict = GetMetadata(tags); + + if (!filter.IsIncluded(meterName, instrumentName)) + { + return; + } + + // the value might be an empty string indicating no measurement was provided this collection interval + if (double.TryParse(lastValueText, out double lastValue)) + { + payload = new GaugePayload(meterName, instrumentName, null, unit, metadataDict, lastValue, obj.TimeStamp); + } + } + + private static void HandleCounterRate(TraceEvent traceEvent, CounterFilter filter, string sessionId, out ICounterPayload payload) + { + payload = null; + + string payloadSessionId = (string)traceEvent.PayloadValue(0); + + if (payloadSessionId != sessionId) + { + return; + } + + string meterName = (string)traceEvent.PayloadValue(1); + //string meterVersion = (string)obj.PayloadValue(2); + string instrumentName = (string)traceEvent.PayloadValue(3); + string unit = (string)traceEvent.PayloadValue(4); + string tags = (string)traceEvent.PayloadValue(5); + string rateText = (string)traceEvent.PayloadValue(6); + + Dictionary metadataDict = GetMetadata(tags); + + if (double.TryParse(rateText, out double rate)) + { + payload = new RatePayload(meterName, instrumentName, null, unit, metadataDict, rate, 10, traceEvent.TimeStamp); + } + } + + private static void HandleHistogram(TraceEvent obj, CounterFilter filter, string sessionId, out List payload) + { + payload = new List(); + + string payloadSessionId = (string)obj.PayloadValue(0); + + if (payloadSessionId != sessionId) + { + return; + } + + string meterName = (string)obj.PayloadValue(1); + //string meterVersion = (string)obj.PayloadValue(2); + string instrumentName = (string)obj.PayloadValue(3); + string unit = (string)obj.PayloadValue(4); + string tags = (string)obj.PayloadValue(5); + string quantilesText = (string)obj.PayloadValue(6); + + if (!filter.IsIncluded(meterName, instrumentName)) + { + return; + } + + KeyValuePair[] quantiles = ParseQuantiles(quantilesText); + foreach ((double key, double val) in quantiles) + { + Dictionary metadataDict = GetMetadata(tags); + metadataDict.Add("quantile", key.ToString()); + payload.Add(new PercentilePayload(meterName, instrumentName, null, unit, metadataDict, val, obj.TimeStamp)); + } + } + + private static void HandleHistogramLimitReached(TraceEvent obj, string sessionId, out ICounterPayload payload) + { + payload = null; + + string payloadSessionId = (string)obj.PayloadValue(0); + + if (payloadSessionId != sessionId) + { + return; + } + + string errorMessage = $"Warning: Histogram tracking limit reached. Not all data is being shown. The limit can be changed with maxHistograms but will use more memory in the target process."; + + payload = new ErrorPayload(string.Empty, string.Empty, string.Empty, string.Empty, new(), 0, obj.TimeStamp, errorMessage); + } + + private static void HandleTimeSeriesLimitReached(TraceEvent obj, string sessionId, out ICounterPayload payload) + { + payload = null; + + string payloadSessionId = (string)obj.PayloadValue(0); + + if (payloadSessionId != sessionId) + { + return; + } + + string errorMessage = "Warning: Time series tracking limit reached. Not all data is being shown. The limit can be changed with maxTimeSeries but will use more memory in the target process."; + + payload = new ErrorPayload(string.Empty, string.Empty, string.Empty, string.Empty, new(), 0, obj.TimeStamp, errorMessage); + } + + private static void HandleError(TraceEvent obj, string sessionId, out ICounterPayload payload) + { + payload = null; + + string payloadSessionId = (string)obj.PayloadValue(0); + string error = (string)obj.PayloadValue(1); + if (sessionId != payloadSessionId) + { + return; + } + + string errorMessage = "Error reported from target process:" + Environment.NewLine + error; + + payload = new ErrorPayload(string.Empty, string.Empty, string.Empty, string.Empty, new(), 0, obj.TimeStamp, errorMessage); + } + + private static void HandleMultipleSessionsNotSupportedError(TraceEvent obj, string sessionId, out ICounterPayload payload) + { + payload = null; + + string payloadSessionId = (string)obj.PayloadValue(0); + if (payloadSessionId == sessionId) + { + // If our session is the one that is running then the error is not for us, + // it is for some other session that came later + return; + } + else + { + string errorMessage = "Error: Another metrics collection session is already in progress for the target process, perhaps from another tool? " + Environment.NewLine + + "Concurrent sessions are not supported."; + + payload = new ErrorPayload(string.Empty, string.Empty, string.Empty, string.Empty, new(), 0, obj.TimeStamp, errorMessage); + } + } + + private static void HandleObservableInstrumentCallbackError(TraceEvent obj, string sessionId, out ICounterPayload payload) + { + payload = null; + + string payloadSessionId = (string)obj.PayloadValue(0); + string error = (string)obj.PayloadValue(1); + + if (payloadSessionId != sessionId) + { + return; + } + + string errorMessage = "Exception thrown from an observable instrument callback in the target process:" + Environment.NewLine + + error; + + payload = new ErrorPayload(string.Empty, string.Empty, string.Empty, string.Empty, new(), 0, obj.TimeStamp, errorMessage); + } + + //The metadata payload is formatted as a string of comma separated key:value pairs. //This limitation means that metadata values cannot include commas; otherwise, the //metadata will be parsed incorrectly. If a value contains a comma, then all metadata @@ -114,6 +350,30 @@ internal static Dictionary GetMetadata(string metadataPayload) return metadataDict; } + private static KeyValuePair[] ParseQuantiles(string quantileList) + { + string[] quantileParts = quantileList.Split(';', StringSplitOptions.RemoveEmptyEntries); + List> quantiles = new List>(); + foreach (string quantile in quantileParts) + { + string[] keyValParts = quantile.Split('=', StringSplitOptions.RemoveEmptyEntries); + if (keyValParts.Length != 2) + { + continue; + } + if (!double.TryParse(keyValParts[0], out double key)) + { + continue; + } + if (!double.TryParse(keyValParts[1], out double val)) + { + continue; + } + quantiles.Add(new KeyValuePair(key, val)); + } + return quantiles.ToArray(); + } + private static int GetInterval(string series) { const string comparison = "Interval="; diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Triggers/EventCounter/EventCounterTrigger.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Triggers/EventCounter/EventCounterTrigger.cs index 622171d07c..51fd9f3de8 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Triggers/EventCounter/EventCounterTrigger.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Triggers/EventCounter/EventCounterTrigger.cs @@ -59,9 +59,9 @@ public IReadOnlyDictionary> GetProviderEvent public bool HasSatisfiedCondition(TraceEvent traceEvent) { // Filter to the counter of interest before forwarding to the implementation - if (traceEvent.TryGetCounterPayload(_filter, out ICounterPayload payload)) + if (traceEvent.TryGetCounterPayload(_filter, out List payload)) { - return _impl.HasSatisfiedCondition(payload); + return _impl.HasSatisfiedCondition(payload[0]); // Need to check if this is safe - in theory just want the first (and only) result } return false; } From e6b5f21989e37ed0843b532bf9c0aa7b7ba3f182 Mon Sep 17 00:00:00 2001 From: kkeirstead <85592574+kkeirstead@users.noreply.github.com> Date: Wed, 7 Dec 2022 07:11:36 -0800 Subject: [PATCH 02/14] [Dotnet Monitor] Revert To Logging a single payload (not a list) (#3538) --- .../Counters/CounterPayload.cs | 1 - .../Counters/EventCounterPipeline.cs | 7 ++++++- .../Counters/ICountersLogger.cs | 8 +------- .../Counters/TraceEventExtensions.cs | 10 ++++++---- .../Triggers/EventCounter/EventCounterTrigger.cs | 4 ++-- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs index fc9f8a9485..28e811b0b6 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs @@ -71,7 +71,6 @@ public CounterPayload(string providerName, string name, string displayName, stri public IReadOnlyDictionary Metadata { get; } public EventType EventType { get; set; } - } public class GaugePayload : CounterPayload diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/EventCounterPipeline.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/EventCounterPipeline.cs index 280c552f58..baa5228562 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/EventCounterPipeline.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/EventCounterPipeline.cs @@ -56,7 +56,12 @@ protected override async Task OnEventSourceAvailable(EventPipeEventSource eventS { if (traceEvent.TryGetCounterPayload(_filter, _sessionId, out List counterPayload)) { - ExecuteCounterLoggerAction((metricLogger) => metricLogger.Log(counterPayload)); + ExecuteCounterLoggerAction((metricLogger) => { + foreach (var payload in counterPayload) + { + metricLogger.Log(payload); + } + }); } } catch (Exception) diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICountersLogger.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICountersLogger.cs index b0ddf1f753..7348735888 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICountersLogger.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICountersLogger.cs @@ -2,19 +2,13 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System; -using System.Collections.Generic; -using System.Text; -using System.Threading; -using System.Threading.Tasks; - namespace Microsoft.Diagnostics.Monitoring.EventPipe { internal interface ICountersLogger { //TODO Consider making these async. - void Log(List counter); + void Log(ICounterPayload counter); void PipelineStarted(); void PipelineStopped(); } diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs index 96c531f3e7..75133b5f5c 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs @@ -74,7 +74,6 @@ public static bool TryGetCounterPayload(this TraceEvent traceEvent, CounterFilte if (sessionId != null && "System.Diagnostics.Metrics".Equals(traceEvent.ProviderName)) { - ICounterPayload individualPayload = null; if (traceEvent.EventName == "BeginInstrumentReporting") @@ -126,9 +125,13 @@ public static bool TryGetCounterPayload(this TraceEvent traceEvent, CounterFilte return false; } - public static bool TryGetCounterPayload(this TraceEvent traceEvent, CounterFilter filter, out List payload) + public static bool TryGetIndividualCounterPayload(this TraceEvent traceEvent, CounterFilter filter, out ICounterPayload payload) { - return TryGetCounterPayload(traceEvent, filter, null, out payload); + bool gotCounterPayload = TryGetCounterPayload(traceEvent, filter, null, out List payloadsList); + + payload = payloadsList.FirstOrDefault(); + + return gotCounterPayload; } private static void HandleGauge(TraceEvent obj, CounterFilter filter, string sessionId, out ICounterPayload payload) @@ -307,7 +310,6 @@ private static void HandleObservableInstrumentCallbackError(TraceEvent obj, stri payload = new ErrorPayload(string.Empty, string.Empty, string.Empty, string.Empty, new(), 0, obj.TimeStamp, errorMessage); } - //The metadata payload is formatted as a string of comma separated key:value pairs. //This limitation means that metadata values cannot include commas; otherwise, the //metadata will be parsed incorrectly. If a value contains a comma, then all metadata diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Triggers/EventCounter/EventCounterTrigger.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Triggers/EventCounter/EventCounterTrigger.cs index 51fd9f3de8..bdbe92591c 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Triggers/EventCounter/EventCounterTrigger.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Triggers/EventCounter/EventCounterTrigger.cs @@ -59,9 +59,9 @@ public IReadOnlyDictionary> GetProviderEvent public bool HasSatisfiedCondition(TraceEvent traceEvent) { // Filter to the counter of interest before forwarding to the implementation - if (traceEvent.TryGetCounterPayload(_filter, out List payload)) + if (traceEvent.TryGetIndividualCounterPayload(_filter, out ICounterPayload payload)) { - return _impl.HasSatisfiedCondition(payload[0]); // Need to check if this is safe - in theory just want the first (and only) result + return _impl.HasSatisfiedCondition(payload); } return false; } From ddf091ed6eaa26892ff801c748e8b5bee082ed13 Mon Sep 17 00:00:00 2001 From: Wiktor Kopec Date: Thu, 15 Dec 2022 16:00:55 -0800 Subject: [PATCH 03/14] [Feature branch changes, no review] Fixup counter apis (#3559) * Fix ICountersLogger contract * MetadataUpdates * Fixup api protection levels * Fixup tests and add CounterEnded payload * Fixup metadata parsing --- .../Counters/CounterPayload.cs | 54 +++++++++++-------- .../Counters/EventCounterPipeline.cs | 18 ++++++- .../Counters/ICounterPayload.cs | 6 +-- .../Counters/ICountersLogger.cs | 10 ++-- .../Counters/TraceEventExtensions.cs | 42 ++++++++------- ...ft.Diagnostics.Monitoring.EventPipe.csproj | 2 + .../Microsoft.Diagnostics.Monitoring.csproj | 1 + .../EventCounterPipelineUnitTests.cs | 8 +-- .../EventCounterTriggerTests.cs | 4 +- 9 files changed, 87 insertions(+), 58 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs index 28e811b0b6..522c6be188 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs @@ -8,14 +8,8 @@ namespace Microsoft.Diagnostics.Monitoring.EventPipe { - public class CounterPayload : ICounterPayload + internal class CounterPayload : ICounterPayload { -#if NETSTANDARD - private static readonly IReadOnlyDictionary Empty = new ReadOnlyDictionary(new Dictionary(0)); -#else - private static readonly IReadOnlyDictionary Empty = System.Collections.Immutable.ImmutableDictionary.Empty; -#endif - public CounterPayload(DateTime timestamp, string provider, string name, @@ -24,7 +18,7 @@ public CounterPayload(DateTime timestamp, double value, CounterType counterType, float interval, - Dictionary metadata) + string metadata) { Timestamp = timestamp; Name = name; @@ -34,16 +28,16 @@ public CounterPayload(DateTime timestamp, CounterType = counterType; Provider = provider; Interval = interval; - Metadata = metadata ?? Empty; + Metadata = metadata; EventType = EventType.Gauge; } // Copied from dotnet-counters - public CounterPayload(string providerName, string name, string displayName, string displayUnits, Dictionary metadata, double value, DateTime timestamp, string type, EventType eventType) + public CounterPayload(string providerName, string name, string displayName, string displayUnits, string metadata, double value, DateTime timestamp, string type, EventType eventType) { Provider = providerName; Name = name; - Metadata = metadata ?? Empty; + Metadata = metadata; Value = value; Timestamp = timestamp; CounterType = (CounterType)Enum.Parse(typeof(CounterType), type); @@ -68,14 +62,14 @@ public CounterPayload(string providerName, string name, string displayName, stri public string Provider { get; } - public IReadOnlyDictionary Metadata { get; } + public string Metadata { get; } public EventType EventType { get; set; } } - public class GaugePayload : CounterPayload + internal class GaugePayload : CounterPayload { - public GaugePayload(string providerName, string name, string displayName, string displayUnits, Dictionary metadata, double value, DateTime timestamp) : + public GaugePayload(string providerName, string name, string displayName, string displayUnits, string metadata, double value, DateTime timestamp) : base(providerName, name, displayName, displayUnits, metadata, value, timestamp, "Metric", EventType.Gauge) { // In case these properties are not provided, set them to appropriate values. @@ -84,9 +78,18 @@ public GaugePayload(string providerName, string name, string displayName, string } } - public class RatePayload : CounterPayload + internal class CounterEndedPayload : CounterPayload + { + public CounterEndedPayload(string providerName, string name, string displayName, DateTime timestamp) + : base(providerName, name, displayName, string.Empty, null, 0.0, timestamp, "Metric", EventType.CounterEnded) + { + + } + } + + internal class RatePayload : CounterPayload { - public RatePayload(string providerName, string name, string displayName, string displayUnits, Dictionary metadata, double value, double intervalSecs, DateTime timestamp) : + public RatePayload(string providerName, string name, string displayName, string displayUnits, string metadata, double value, double intervalSecs, DateTime timestamp) : base(providerName, name, displayName, displayUnits, metadata, value, timestamp, "Rate", EventType.Rate) { // In case these properties are not provided, set them to appropriate values. @@ -97,9 +100,9 @@ public RatePayload(string providerName, string name, string displayName, string } } - public class PercentilePayload : CounterPayload + internal class PercentilePayload : CounterPayload { - public PercentilePayload(string providerName, string name, string displayName, string displayUnits, Dictionary metadata, double val, DateTime timestamp) : + public PercentilePayload(string providerName, string name, string displayName, string displayUnits, string metadata, double val, DateTime timestamp) : base(providerName, name, displayName, displayUnits, metadata, val, timestamp, "Metric", EventType.Histogram) { // In case these properties are not provided, set them to appropriate values. @@ -108,10 +111,14 @@ public PercentilePayload(string providerName, string name, string displayName, s } } - public class ErrorPayload : CounterPayload + internal class ErrorPayload : CounterPayload { - public ErrorPayload(string providerName, string name, string displayName, string displayUnits, Dictionary metadata, double val, DateTime timestamp, string errorMessage) : - base(providerName, name, displayName, displayUnits, metadata, val, timestamp, "Metric", EventType.Error) + public ErrorPayload(string errorMessage) : this(errorMessage, DateTime.UtcNow) + { + } + + public ErrorPayload(string errorMessage, DateTime timestamp) : + base(string.Empty, string.Empty, string.Empty, string.Empty, null, 0.0, timestamp, "Metric", EventType.Error) { ErrorMessage = errorMessage; } @@ -120,11 +127,12 @@ public ErrorPayload(string providerName, string name, string displayName, string } // If keep this, should probably put it somewhere else - public enum EventType : int + internal enum EventType : int { Rate, Gauge, Histogram, - Error + Error, + CounterEnded } } \ No newline at end of file diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/EventCounterPipeline.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/EventCounterPipeline.cs index baa5228562..b6a175e0a0 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/EventCounterPipeline.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/EventCounterPipeline.cs @@ -48,7 +48,7 @@ protected override MonitoringSourceConfiguration CreateConfiguration() protected override async Task OnEventSourceAvailable(EventPipeEventSource eventSource, Func stopSessionAsync, CancellationToken token) { - ExecuteCounterLoggerAction((metricLogger) => metricLogger.PipelineStarted()); + await ExecuteCounterLoggerActionAsync((metricLogger) => metricLogger.PipelineStarted(token)); eventSource.Dynamic.All += traceEvent => { @@ -77,7 +77,21 @@ protected override async Task OnEventSourceAvailable(EventPipeEventSource eventS await sourceCompletedTaskSource.Task; - ExecuteCounterLoggerAction((metricLogger) => metricLogger.PipelineStopped()); + await ExecuteCounterLoggerActionAsync((metricLogger) => metricLogger.PipelineStopped(token)); + } + + private async Task ExecuteCounterLoggerActionAsync(Func action) + { + foreach (ICountersLogger logger in _loggers) + { + try + { + await action(logger); + } + catch (ObjectDisposedException) + { + } + } } private void ExecuteCounterLoggerAction(Action action) diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICounterPayload.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICounterPayload.cs index cc27e69d9b..cb6e8e9248 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICounterPayload.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICounterPayload.cs @@ -17,7 +17,7 @@ public enum CounterType Rate } - public interface ICounterPayload + internal interface ICounterPayload { string Name { get; } @@ -35,8 +35,8 @@ public interface ICounterPayload float Interval { get; } - IReadOnlyDictionary Metadata { get; } + string Metadata { get; } - public EventType EventType { get; set; } + EventType EventType { get; set; } } } diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICountersLogger.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICountersLogger.cs index 7348735888..7a84c607e5 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICountersLogger.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICountersLogger.cs @@ -2,14 +2,16 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Threading; +using System.Threading.Tasks; + namespace Microsoft.Diagnostics.Monitoring.EventPipe { internal interface ICountersLogger { - //TODO Consider making these async. - void Log(ICounterPayload counter); - void PipelineStarted(); - void PipelineStopped(); + + Task PipelineStarted(CancellationToken token); + Task PipelineStopped(CancellationToken token); } } diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs index 75133b5f5c..23061f93ed 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs @@ -24,7 +24,7 @@ public static bool TryGetCounterPayload(this TraceEvent traceEvent, CounterFilte string series = payloadFields["Series"].ToString(); string counterName = payloadFields["Name"].ToString(); - Dictionary metadataDict = GetMetadata(payloadFields["Metadata"].ToString()); + string metadata = payloadFields["Metadata"].ToString(); //CONSIDER //Concurrent counter sessions do not each get a separate interval. Instead the payload @@ -67,7 +67,7 @@ public static bool TryGetCounterPayload(this TraceEvent traceEvent, CounterFilte value, counterType, intervalSec, - metadataDict)); + metadata)); return true; } @@ -152,8 +152,6 @@ private static void HandleGauge(TraceEvent obj, CounterFilter filter, string ses string tags = (string)obj.PayloadValue(5); string lastValueText = (string)obj.PayloadValue(6); - Dictionary metadataDict = GetMetadata(tags); - if (!filter.IsIncluded(meterName, instrumentName)) { return; @@ -162,7 +160,16 @@ private static void HandleGauge(TraceEvent obj, CounterFilter filter, string ses // the value might be an empty string indicating no measurement was provided this collection interval if (double.TryParse(lastValueText, out double lastValue)) { - payload = new GaugePayload(meterName, instrumentName, null, unit, metadataDict, lastValue, obj.TimeStamp); + payload = new GaugePayload(meterName, instrumentName, null, unit, tags, lastValue, obj.TimeStamp); + } + else + { + // for observable instruments we assume the lack of data is meaningful and remove it from the UI + // this happens when the Gauge callback function throws an exception. + + //TODO Can this occur for other meter types? + payload = new CounterEndedPayload(meterName, instrumentName, null, obj.TimeStamp); + } } @@ -184,11 +191,9 @@ private static void HandleCounterRate(TraceEvent traceEvent, CounterFilter filte string tags = (string)traceEvent.PayloadValue(5); string rateText = (string)traceEvent.PayloadValue(6); - Dictionary metadataDict = GetMetadata(tags); - if (double.TryParse(rateText, out double rate)) { - payload = new RatePayload(meterName, instrumentName, null, unit, metadataDict, rate, 10, traceEvent.TimeStamp); + payload = new RatePayload(meterName, instrumentName, null, unit, tags, rate, 10, traceEvent.TimeStamp); } } @@ -218,12 +223,13 @@ private static void HandleHistogram(TraceEvent obj, CounterFilter filter, string KeyValuePair[] quantiles = ParseQuantiles(quantilesText); foreach ((double key, double val) in quantiles) { - Dictionary metadataDict = GetMetadata(tags); - metadataDict.Add("quantile", key.ToString()); - payload.Add(new PercentilePayload(meterName, instrumentName, null, unit, metadataDict, val, obj.TimeStamp)); + string tagsWithQuantile = AppendQuantile(tags, FormattableString.Invariant($"quantile={key}")); + payload.Add(new PercentilePayload(meterName, instrumentName, null, unit, tagsWithQuantile, val, obj.TimeStamp)); } } + private static string AppendQuantile(string tags, string quantile) => string.IsNullOrEmpty(tags) ? quantile : FormattableString.Invariant($"{tags},{quantile}"); + private static void HandleHistogramLimitReached(TraceEvent obj, string sessionId, out ICounterPayload payload) { payload = null; @@ -237,7 +243,7 @@ private static void HandleHistogramLimitReached(TraceEvent obj, string sessionId string errorMessage = $"Warning: Histogram tracking limit reached. Not all data is being shown. The limit can be changed with maxHistograms but will use more memory in the target process."; - payload = new ErrorPayload(string.Empty, string.Empty, string.Empty, string.Empty, new(), 0, obj.TimeStamp, errorMessage); + payload = new ErrorPayload(errorMessage); } private static void HandleTimeSeriesLimitReached(TraceEvent obj, string sessionId, out ICounterPayload payload) @@ -253,7 +259,7 @@ private static void HandleTimeSeriesLimitReached(TraceEvent obj, string sessionI string errorMessage = "Warning: Time series tracking limit reached. Not all data is being shown. The limit can be changed with maxTimeSeries but will use more memory in the target process."; - payload = new ErrorPayload(string.Empty, string.Empty, string.Empty, string.Empty, new(), 0, obj.TimeStamp, errorMessage); + payload = new ErrorPayload(errorMessage, obj.TimeStamp); } private static void HandleError(TraceEvent obj, string sessionId, out ICounterPayload payload) @@ -269,7 +275,7 @@ private static void HandleError(TraceEvent obj, string sessionId, out ICounterPa string errorMessage = "Error reported from target process:" + Environment.NewLine + error; - payload = new ErrorPayload(string.Empty, string.Empty, string.Empty, string.Empty, new(), 0, obj.TimeStamp, errorMessage); + payload = new ErrorPayload(errorMessage, obj.TimeStamp); } private static void HandleMultipleSessionsNotSupportedError(TraceEvent obj, string sessionId, out ICounterPayload payload) @@ -288,7 +294,7 @@ private static void HandleMultipleSessionsNotSupportedError(TraceEvent obj, stri string errorMessage = "Error: Another metrics collection session is already in progress for the target process, perhaps from another tool? " + Environment.NewLine + "Concurrent sessions are not supported."; - payload = new ErrorPayload(string.Empty, string.Empty, string.Empty, string.Empty, new(), 0, obj.TimeStamp, errorMessage); + payload = new ErrorPayload(errorMessage, obj.TimeStamp); } } @@ -307,14 +313,14 @@ private static void HandleObservableInstrumentCallbackError(TraceEvent obj, stri string errorMessage = "Exception thrown from an observable instrument callback in the target process:" + Environment.NewLine + error; - payload = new ErrorPayload(string.Empty, string.Empty, string.Empty, string.Empty, new(), 0, obj.TimeStamp, errorMessage); + payload = new ErrorPayload(errorMessage, obj.TimeStamp); } //The metadata payload is formatted as a string of comma separated key:value pairs. //This limitation means that metadata values cannot include commas; otherwise, the //metadata will be parsed incorrectly. If a value contains a comma, then all metadata //is treated as invalid and excluded from the payload. - internal static Dictionary GetMetadata(string metadataPayload) + public static IDictionary GetMetadata(string metadataPayload, char kvSeparator = ':') { var metadataDict = new Dictionary(); @@ -337,7 +343,7 @@ internal static Dictionary GetMetadata(string metadataPayload) metadata = metadata.Slice(commaIndex + 1); } - int colonIndex = kvPair.IndexOf(':'); + int colonIndex = kvPair.IndexOf(kvSeparator); if (colonIndex < 0) { metadataDict.Clear(); diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Microsoft.Diagnostics.Monitoring.EventPipe.csproj b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Microsoft.Diagnostics.Monitoring.EventPipe.csproj index 19fce8c296..21ebf73be1 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Microsoft.Diagnostics.Monitoring.EventPipe.csproj +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Microsoft.Diagnostics.Monitoring.EventPipe.csproj @@ -42,5 +42,7 @@ + + diff --git a/src/Microsoft.Diagnostics.Monitoring/Microsoft.Diagnostics.Monitoring.csproj b/src/Microsoft.Diagnostics.Monitoring/Microsoft.Diagnostics.Monitoring.csproj index d1157e98da..d0352a014e 100644 --- a/src/Microsoft.Diagnostics.Monitoring/Microsoft.Diagnostics.Monitoring.csproj +++ b/src/Microsoft.Diagnostics.Monitoring/Microsoft.Diagnostics.Monitoring.csproj @@ -28,6 +28,7 @@ + diff --git a/src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterPipelineUnitTests.cs b/src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterPipelineUnitTests.cs index bfb15c6b6b..95abce2c06 100644 --- a/src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterPipelineUnitTests.cs +++ b/src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterPipelineUnitTests.cs @@ -67,13 +67,9 @@ public void Log(ICounterPayload metric) } } - public void PipelineStarted() - { - } + public Task PipelineStarted(CancellationToken token) => Task.CompletedTask; - public void PipelineStopped() - { - } + public Task PipelineStopped(CancellationToken token) => Task.CompletedTask; private static string CreateKey(ICounterPayload payload) { diff --git a/src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterTriggerTests.cs b/src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterTriggerTests.cs index 551e53e4b2..1e4609d859 100644 --- a/src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterTriggerTests.cs +++ b/src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterTriggerTests.cs @@ -499,7 +499,7 @@ public void ValidateMetadataParsing_Success() const string value1 = "V1"; const string key2 = "K2"; const string value2 = "V:2"; - Dictionary metadataDict = TraceEventExtensions.GetMetadata($"{key1}:{value1},{key2}:{value2}"); + IDictionary metadataDict = TraceEventExtensions.GetMetadata($"{key1}:{value1},{key2}:{value2}"); Assert.Equal(2, metadataDict.Count); Assert.Equal(value1, metadataDict[key1]); @@ -515,7 +515,7 @@ public void ValidateMetadataParsing_Success() [InlineData("K1")] public void ValidateMetadataParsing_Failure(string invalidMetadata) { - Dictionary metadataDict = TraceEventExtensions.GetMetadata(invalidMetadata); + IDictionary metadataDict = TraceEventExtensions.GetMetadata(invalidMetadata); Assert.Empty(metadataDict); } From 959ebd648b8123e7121cfd9e33756d8be7a79ffc Mon Sep 17 00:00:00 2001 From: kkeirstead <85592574+kkeirstead@users.noreply.github.com> Date: Wed, 21 Dec 2022 11:59:00 -0800 Subject: [PATCH 04/14] [Dotnet-Monitor] [Feature Branch] Switched quantile to Percentile (#3567) --- .../Counters/TraceEventExtensions.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs index 23061f93ed..86c5b2fd80 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs @@ -223,12 +223,12 @@ private static void HandleHistogram(TraceEvent obj, CounterFilter filter, string KeyValuePair[] quantiles = ParseQuantiles(quantilesText); foreach ((double key, double val) in quantiles) { - string tagsWithQuantile = AppendQuantile(tags, FormattableString.Invariant($"quantile={key}")); - payload.Add(new PercentilePayload(meterName, instrumentName, null, unit, tagsWithQuantile, val, obj.TimeStamp)); + string tagsWithPercentile = AppendPercentile(tags, FormattableString.Invariant($"Percentile={(int)(100*key)}")); + payload.Add(new PercentilePayload(meterName, instrumentName, null, unit, tagsWithPercentile, val, obj.TimeStamp)); } } - private static string AppendQuantile(string tags, string quantile) => string.IsNullOrEmpty(tags) ? quantile : FormattableString.Invariant($"{tags},{quantile}"); + private static string AppendPercentile(string tags, string percentile) => string.IsNullOrEmpty(tags) ? percentile : FormattableString.Invariant($"{tags},{percentile}"); private static void HandleHistogramLimitReached(TraceEvent obj, string sessionId, out ICounterPayload payload) { From 5052752fcfd7c836855364938005bc22bb81a7b3 Mon Sep 17 00:00:00 2001 From: wiktork Date: Tue, 3 Jan 2023 16:55:23 -0800 Subject: [PATCH 05/14] Minor branch cleanup --- .../Counters/CounterPayload.cs | 3 +++ .../Counters/ICounterPayload.cs | 9 ++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs index 522c6be188..459440ee21 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs @@ -8,6 +8,9 @@ namespace Microsoft.Diagnostics.Monitoring.EventPipe { + /// + /// TODO This is currently a duplication of the src\Tools\dotnet-counters\CounterPayload.cs stack. The two will be unified in a separate change. + /// internal class CounterPayload : ICounterPayload { public CounterPayload(DateTime timestamp, diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICounterPayload.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICounterPayload.cs index cb6e8e9248..bec5523d51 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICounterPayload.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICounterPayload.cs @@ -8,7 +8,7 @@ namespace Microsoft.Diagnostics.Monitoring.EventPipe { - public enum CounterType + internal enum CounterType { //Same as average or mean Metric, @@ -33,8 +33,15 @@ internal interface ICounterPayload DateTime Timestamp { get; } + /// + /// The interval between counters. Note this is the actual measure of time elapsed, not the requested interval. + /// float Interval { get; } + /// + /// Optional metadata for counters. Note that normal counters use ':' as a separator character, while System.Diagnostics.Metrics use ';'. + /// We do not immediately convert string to Dictionary, since dotnet-counters does not need this conversion. + /// string Metadata { get; } EventType EventType { get; set; } From 308cffc39cbf6434dd1aeb0a8719563e0450dba6 Mon Sep 17 00:00:00 2001 From: wiktork Date: Thu, 12 Jan 2023 21:05:18 -0800 Subject: [PATCH 06/14] PR for feature branch --- .../Counters/CounterFilter.cs | 2 ++ .../{EventCounterPipeline.cs => CounterPipeline.cs} | 6 +++--- ...unterPipelineSettings.cs => CounterPipelineSettings.cs} | 2 +- .../Counters/TraceEventExtensions.cs | 7 ++++++- .../EventCounterPipelineUnitTests.cs | 2 +- 5 files changed, 13 insertions(+), 6 deletions(-) rename src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/{EventCounterPipeline.cs => CounterPipeline.cs} (94%) rename src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/{EventPipeCounterPipelineSettings.cs => CounterPipelineSettings.cs} (91%) diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterFilter.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterFilter.cs index 9ad08494d4..c065514796 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterFilter.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterFilter.cs @@ -38,6 +38,8 @@ public void AddFilter(string providerName, string[] counters) public IEnumerable GetProviders() => _enabledCounters.Keys; + public int IntervalSeconds => _intervalMilliseconds / 1000; + public bool IsIncluded(string providerName, string counterName, int intervalMilliseconds) { if (_intervalMilliseconds != intervalMilliseconds) diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/EventCounterPipeline.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipeline.cs similarity index 94% rename from src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/EventCounterPipeline.cs rename to src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipeline.cs index b6a175e0a0..76c81143d6 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/EventCounterPipeline.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipeline.cs @@ -11,14 +11,14 @@ namespace Microsoft.Diagnostics.Monitoring.EventPipe { - internal class EventCounterPipeline : EventSourcePipeline + internal class CounterPipeline : EventSourcePipeline { private readonly IEnumerable _loggers; private readonly CounterFilter _filter; private string _sessionId; - public EventCounterPipeline(DiagnosticsClient client, - EventPipeCounterPipelineSettings settings, + public CounterPipeline(DiagnosticsClient client, + CounterPipelineSettings settings, IEnumerable loggers) : base(client, settings) { _loggers = loggers ?? throw new ArgumentNullException(nameof(loggers)); diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/EventPipeCounterPipelineSettings.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipelineSettings.cs similarity index 91% rename from src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/EventPipeCounterPipelineSettings.cs rename to src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipelineSettings.cs index 9aa3855634..51ce90dbfd 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/EventPipeCounterPipelineSettings.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipelineSettings.cs @@ -8,7 +8,7 @@ namespace Microsoft.Diagnostics.Monitoring.EventPipe { - internal class EventPipeCounterPipelineSettings : EventSourcePipelineSettings + internal class CounterPipelineSettings : EventSourcePipelineSettings { public EventPipeCounterGroup[] CounterGroups { get; set; } diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs index 86c5b2fd80..d4fec2e295 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs @@ -191,9 +191,14 @@ private static void HandleCounterRate(TraceEvent traceEvent, CounterFilter filte string tags = (string)traceEvent.PayloadValue(5); string rateText = (string)traceEvent.PayloadValue(6); + if (!filter.IsIncluded(meterName, instrumentName)) + { + return; + } + if (double.TryParse(rateText, out double rate)) { - payload = new RatePayload(meterName, instrumentName, null, unit, tags, rate, 10, traceEvent.TimeStamp); + payload = new RatePayload(meterName, instrumentName, null, unit, tags, rate, filter.IntervalSeconds, traceEvent.TimeStamp); } } diff --git a/src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterPipelineUnitTests.cs b/src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterPipelineUnitTests.cs index 95abce2c06..5b20262333 100644 --- a/src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterPipelineUnitTests.cs +++ b/src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterPipelineUnitTests.cs @@ -99,7 +99,7 @@ public async Task TestCounterEventPipeline(TestConfiguration config) { var client = new DiagnosticsClient(testRunner.Pid); - await using EventCounterPipeline pipeline = new EventCounterPipeline(client, new EventPipeCounterPipelineSettings + await using CounterPipeline pipeline = new CounterPipeline(client, new CounterPipelineSettings { Duration = Timeout.InfiniteTimeSpan, CounterGroups = new[] From 149f3adb4b0d4b36e835a91907b3566fe8664d4a Mon Sep 17 00:00:00 2001 From: wiktork Date: Thu, 19 Jan 2023 13:58:32 -0800 Subject: [PATCH 07/14] PR feedback --- .../MetricSourceConfiguration.cs | 97 +++++++++++-------- .../Counters/CounterPayload.cs | 1 - .../Counters/CounterPipeline.cs | 9 +- .../Counters/CounterPipelineSettings.cs | 12 +++ .../Counters/TraceEventExtensions.cs | 9 +- src/Tools/dotnet-counters/CounterMonitor.cs | 1 - 6 files changed, 82 insertions(+), 47 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/MetricSourceConfiguration.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/MetricSourceConfiguration.cs index 1c3891f123..13567ee754 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/MetricSourceConfiguration.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/MetricSourceConfiguration.cs @@ -13,70 +13,85 @@ namespace Microsoft.Diagnostics.Monitoring.EventPipe { + [Flags] + public enum MetricType + { + EventCounter = 0x1, + Meter = 0x2 + } + + public sealed class MetricEventPipeProvider + { + public string Provider { get; set; } + + public float IntervalSeconds { get; set; } + + public MetricType Type { get; set; } = MetricType.EventCounter | MetricType.Meter; + } + public sealed class MetricSourceConfiguration : MonitoringSourceConfiguration { private readonly IList _eventPipeProviders; public string SessionId { get; private set; } + public static readonly string[] DefaultProviders = new[] { SystemRuntimeEventSourceName, MicrosoftAspNetCoreHostingEventSourceName, GrpcAspNetCoreServer }; + public MetricSourceConfiguration(float metricIntervalSeconds, IEnumerable customProviderNames) + : this(metricIntervalSeconds, customProviderNames?.Any() == true ? CreateProviders(metricIntervalSeconds, customProviderNames) : + CreateProviders(metricIntervalSeconds, DefaultProviders)) { - RequestRundown = false; - if (customProviderNames == null) - { + } + + public MetricSourceConfiguration(float metricIntervalSeconds, IEnumerable customProviderNames, int maxHistograms = 20, int maxTimeSeries = 1000) { + if (customProviderNames == null) { throw new ArgumentNullException(nameof(customProviderNames)); } + + RequestRundown = false; MetricIntervalSeconds = metricIntervalSeconds.ToString(CultureInfo.InvariantCulture); - IEnumerable providers = null; - if (customProviderNames.Any()) - { - providers = customProviderNames; - } - else - { - providers = new[] { SystemRuntimeEventSourceName, MicrosoftAspNetCoreHostingEventSourceName, GrpcAspNetCoreServer }; - } + _eventPipeProviders = customProviderNames.Where(provider => provider.Type.HasFlag(MetricType.EventCounter)) + .Select((MetricEventPipeProvider provider) => new EventPipeProvider(provider.Provider, + EventLevel.Informational, + (long)ClrTraceEventParser.Keywords.None, + new Dictionary() + { + { "EventCounterIntervalSec", provider.IntervalSeconds.ToString(CultureInfo.InvariantCulture)} + })).ToList(); - _eventPipeProviders = providers.Select((string provider) => new EventPipeProvider(provider, - EventLevel.Informational, - (long)ClrTraceEventParser.Keywords.None, - new Dictionary() - { - { "EventCounterIntervalSec", MetricIntervalSeconds } - })).ToList(); - } + IEnumerable meterProviders = customProviderNames.Where(provider => provider.Type.HasFlag(MetricType.Meter)); - public MetricSourceConfiguration(float metricIntervalSeconds, IEnumerable customProviderNames, int maxHistograms, int maxTimeSeries) : this(metricIntervalSeconds, customProviderNames) - { - const long TimeSeriesValues = 0x2; - StringBuilder metrics = new StringBuilder(); - foreach (string provider in customProviderNames) + if (meterProviders.Any()) { - if (metrics.Length != 0) - { - metrics.Append(","); - } - - metrics.Append(provider); - } + const long TimeSeriesValues = 0x2; + string metrics = string.Join(',', meterProviders.Select(p => p.Provider)); - SessionId = Guid.NewGuid().ToString(); + SessionId = Guid.NewGuid().ToString(); - EventPipeProvider metricsEventSourceProvider = - new EventPipeProvider("System.Diagnostics.Metrics", EventLevel.Informational, TimeSeriesValues, - new Dictionary() - { + EventPipeProvider metricsEventSourceProvider = + new EventPipeProvider("System.Diagnostics.Metrics", EventLevel.Informational, TimeSeriesValues, + new Dictionary() + { { "SessionId", SessionId }, - { "Metrics", metrics.ToString() }, + { "Metrics", metrics }, { "RefreshInterval", MetricIntervalSeconds.ToString() }, { "MaxTimeSeries", maxTimeSeries.ToString() }, { "MaxHistograms", maxHistograms.ToString() } - } - ); + } + ); - _eventPipeProviders = _eventPipeProviders.Append(metricsEventSourceProvider).ToArray(); + _eventPipeProviders = _eventPipeProviders.Append(metricsEventSourceProvider).ToArray(); + } } + private static IEnumerable CreateProviders(float metricIntervalSeconds, IEnumerable customProviderNames) => + customProviderNames.Select(provider => new MetricEventPipeProvider { + Provider = provider, + IntervalSeconds = metricIntervalSeconds, + Type = MetricType.EventCounter + }); + + private string MetricIntervalSeconds { get; } public override IList GetProviders() => _eventPipeProviders; diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs index 459440ee21..cbdee69e79 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs @@ -129,7 +129,6 @@ public ErrorPayload(string errorMessage, DateTime timestamp) : public string ErrorMessage { get; private set; } } - // If keep this, should probably put it somewhere else internal enum EventType : int { Rate, diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipeline.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipeline.cs index 76c81143d6..f764f99834 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipeline.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipeline.cs @@ -6,6 +6,7 @@ using Microsoft.Diagnostics.Tracing; using System; using System.Collections.Generic; +using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -39,7 +40,13 @@ public CounterPipeline(DiagnosticsClient client, protected override MonitoringSourceConfiguration CreateConfiguration() { - var config = new MetricSourceConfiguration(Settings.CounterIntervalSeconds, _filter.GetProviders(), Settings.MaxHistograms, Settings.MaxTimeSeries); + var config = new MetricSourceConfiguration(Settings.CounterIntervalSeconds, Settings.CounterGroups.Select((EventPipeCounterGroup counterGroup) => new MetricEventPipeProvider + { + Provider = counterGroup.ProviderName, + IntervalSeconds = counterGroup.IntervalSeconds, + Type = (MetricType)counterGroup.Type + }), + Settings.MaxHistograms, Settings.MaxTimeSeries); _sessionId = config.SessionId; diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipelineSettings.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipelineSettings.cs index 51ce90dbfd..3926db3f8f 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipelineSettings.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipelineSettings.cs @@ -21,9 +21,21 @@ internal class CounterPipelineSettings : EventSourcePipelineSettings public int MaxTimeSeries { get; set; } } + [Flags] + internal enum CounterGroupType + { + EventCounter = 0x1, + Meter = 0x2, + } + internal class EventPipeCounterGroup { public string ProviderName { get; set; } + public string[] CounterNames { get; set; } + + public CounterGroupType Type { get; set; } = CounterGroupType.EventCounter | CounterGroupType.Meter; + + public float IntervalSeconds { get; set; } } } diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs index d4fec2e295..01bf1aa8a3 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs @@ -166,10 +166,7 @@ private static void HandleGauge(TraceEvent obj, CounterFilter filter, string ses { // for observable instruments we assume the lack of data is meaningful and remove it from the UI // this happens when the Gauge callback function throws an exception. - - //TODO Can this occur for other meter types? payload = new CounterEndedPayload(meterName, instrumentName, null, obj.TimeStamp); - } } @@ -200,6 +197,12 @@ private static void HandleCounterRate(TraceEvent traceEvent, CounterFilter filte { payload = new RatePayload(meterName, instrumentName, null, unit, tags, rate, filter.IntervalSeconds, traceEvent.TimeStamp); } + else + { + // for observable instruments we assume the lack of data is meaningful and remove it from the UI + // this happens when the ObservableCounter callback function throws an exception. + payload = new CounterEndedPayload(meterName, instrumentName, null, traceEvent.TimeStamp); + } } private static void HandleHistogram(TraceEvent obj, CounterFilter filter, string sessionId, out List payload) diff --git a/src/Tools/dotnet-counters/CounterMonitor.cs b/src/Tools/dotnet-counters/CounterMonitor.cs index beb1e009c4..5aaa1ef4eb 100644 --- a/src/Tools/dotnet-counters/CounterMonitor.cs +++ b/src/Tools/dotnet-counters/CounterMonitor.cs @@ -171,7 +171,6 @@ private void HandleCounterRate(TraceEvent obj) CounterPayload payload = new RatePayload(meterName, instrumentName, null, unit, tags, rate, _interval, obj.TimeStamp); _renderer.CounterPayloadReceived(payload, _pauseCmdSet); } - } private void HandleGauge(TraceEvent obj) From 22ff6446cdd75ba19fc1a9d14da2b32f94edfbb5 Mon Sep 17 00:00:00 2001 From: wiktork Date: Mon, 23 Jan 2023 21:36:26 -0800 Subject: [PATCH 08/14] Pr feedback feedback --- .../MetricSourceConfiguration.cs | 56 +++++++++---------- .../Counters/CounterPipelineSettings.cs | 2 +- 2 files changed, 27 insertions(+), 31 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/MetricSourceConfiguration.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/MetricSourceConfiguration.cs index 13567ee754..701d5e8c34 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/MetricSourceConfiguration.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/MetricSourceConfiguration.cs @@ -19,12 +19,12 @@ public enum MetricType EventCounter = 0x1, Meter = 0x2 } - + public sealed class MetricEventPipeProvider { public string Provider { get; set; } - public float IntervalSeconds { get; set; } + public float? IntervalSeconds { get; set; } public MetricType Type { get; set; } = MetricType.EventCounter | MetricType.Meter; } @@ -34,32 +34,32 @@ public sealed class MetricSourceConfiguration : MonitoringSourceConfiguration private readonly IList _eventPipeProviders; public string SessionId { get; private set; } - public static readonly string[] DefaultProviders = new[] { SystemRuntimeEventSourceName, MicrosoftAspNetCoreHostingEventSourceName, GrpcAspNetCoreServer }; - - public MetricSourceConfiguration(float metricIntervalSeconds, IEnumerable customProviderNames) - : this(metricIntervalSeconds, customProviderNames?.Any() == true ? CreateProviders(metricIntervalSeconds, customProviderNames) : - CreateProviders(metricIntervalSeconds, DefaultProviders)) + public MetricSourceConfiguration(float metricIntervalSeconds, IEnumerable eventCounterProviderNames) + : this(metricIntervalSeconds, CreateProviders(eventCounterProviderNames?.Any() == true ? eventCounterProviderNames : DefaultMetricProviders)) { } - public MetricSourceConfiguration(float metricIntervalSeconds, IEnumerable customProviderNames, int maxHistograms = 20, int maxTimeSeries = 1000) { - if (customProviderNames == null) { - throw new ArgumentNullException(nameof(customProviderNames)); + public MetricSourceConfiguration(float metricIntervalSeconds, IEnumerable providers, int maxHistograms = 20, int maxTimeSeries = 1000) + { + if (providers == null) + { + throw new ArgumentNullException(nameof(providers)); } RequestRundown = false; - MetricIntervalSeconds = metricIntervalSeconds.ToString(CultureInfo.InvariantCulture); - _eventPipeProviders = customProviderNames.Where(provider => provider.Type.HasFlag(MetricType.EventCounter)) + _eventPipeProviders = providers.Where(provider => provider.Type.HasFlag(MetricType.EventCounter)) .Select((MetricEventPipeProvider provider) => new EventPipeProvider(provider.Provider, - EventLevel.Informational, - (long)ClrTraceEventParser.Keywords.None, - new Dictionary() - { - { "EventCounterIntervalSec", provider.IntervalSeconds.ToString(CultureInfo.InvariantCulture)} - })).ToList(); + EventLevel.Informational, + (long)ClrTraceEventParser.Keywords.None, + new Dictionary() + { + { + "EventCounterIntervalSec", (provider.IntervalSeconds ?? metricIntervalSeconds).ToString(CultureInfo.InvariantCulture) + } + })).ToList(); - IEnumerable meterProviders = customProviderNames.Where(provider => provider.Type.HasFlag(MetricType.Meter)); + IEnumerable meterProviders = providers.Where(provider => provider.Type.HasFlag(MetricType.Meter)); if (meterProviders.Any()) { @@ -72,11 +72,11 @@ public MetricSourceConfiguration(float metricIntervalSeconds, IEnumerable() { - { "SessionId", SessionId }, - { "Metrics", metrics }, - { "RefreshInterval", MetricIntervalSeconds.ToString() }, - { "MaxTimeSeries", maxTimeSeries.ToString() }, - { "MaxHistograms", maxHistograms.ToString() } + { "SessionId", SessionId }, + { "Metrics", metrics }, + { "RefreshInterval", metricIntervalSeconds.ToString(CultureInfo.InvariantCulture) }, + { "MaxTimeSeries", maxTimeSeries.ToString() }, + { "MaxHistograms", maxHistograms.ToString() } } ); @@ -84,16 +84,12 @@ public MetricSourceConfiguration(float metricIntervalSeconds, IEnumerable CreateProviders(float metricIntervalSeconds, IEnumerable customProviderNames) => - customProviderNames.Select(provider => new MetricEventPipeProvider { + private static IEnumerable CreateProviders(IEnumerable providers) => + providers.Select(provider => new MetricEventPipeProvider { Provider = provider, - IntervalSeconds = metricIntervalSeconds, Type = MetricType.EventCounter }); - - private string MetricIntervalSeconds { get; } - public override IList GetProviders() => _eventPipeProviders; } } diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipelineSettings.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipelineSettings.cs index 3926db3f8f..9833f71038 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipelineSettings.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipelineSettings.cs @@ -36,6 +36,6 @@ internal class EventPipeCounterGroup public CounterGroupType Type { get; set; } = CounterGroupType.EventCounter | CounterGroupType.Meter; - public float IntervalSeconds { get; set; } + public float? IntervalSeconds { get; set; } } } From fe36113ef52c968f5bac3454d08c9a5e83e92bbe Mon Sep 17 00:00:00 2001 From: wiktork Date: Wed, 25 Jan 2023 09:02:30 -0800 Subject: [PATCH 09/14] Convert Histogram to single payload --- .../Counters/CounterPayload.cs | 10 +- .../Counters/CounterPipeline.cs | 9 +- .../Counters/CounterUtilities.cs | 54 +++++++++ .../Counters/TraceEventExtensions.cs | 105 ++++-------------- .../EventCounter/EventCounterTrigger.cs | 2 +- 5 files changed, 88 insertions(+), 92 deletions(-) create mode 100644 src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterUtilities.cs diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs index cbdee69e79..810f0cff00 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Collections.ObjectModel; +using System.Linq; namespace Microsoft.Diagnostics.Monitoring.EventPipe { @@ -105,15 +106,20 @@ public RatePayload(string providerName, string name, string displayName, string internal class PercentilePayload : CounterPayload { - public PercentilePayload(string providerName, string name, string displayName, string displayUnits, string metadata, double val, DateTime timestamp) : - base(providerName, name, displayName, displayUnits, metadata, val, timestamp, "Metric", EventType.Histogram) + public PercentilePayload(string providerName, string name, string displayName, string displayUnits, string metadata, IEnumerable<(double quantile, double value)> quantiles, DateTime timestamp) : + base(providerName, name, displayName, displayUnits, metadata, 0.0, timestamp, "Metric", EventType.Histogram) { // In case these properties are not provided, set them to appropriate values. string counterName = string.IsNullOrEmpty(displayName) ? name : displayName; DisplayName = !string.IsNullOrEmpty(displayUnits) ? $"{counterName} ({displayUnits})" : counterName; + Quantiles = quantiles.Select(v => new Quantile(v.quantile, v.value)).ToArray(); } + + public Quantile[] Quantiles { get; } } + internal record struct Quantile(double Percentage, double Value); + internal class ErrorPayload : CounterPayload { public ErrorPayload(string errorMessage) : this(errorMessage, DateTime.UtcNow) diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipeline.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipeline.cs index f764f99834..ebb0c79301 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipeline.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipeline.cs @@ -61,14 +61,9 @@ protected override async Task OnEventSourceAvailable(EventPipeEventSource eventS { try { - if (traceEvent.TryGetCounterPayload(_filter, _sessionId, out List counterPayload)) + if (traceEvent.TryGetCounterPayload(_filter, _sessionId, out ICounterPayload counterPayload)) { - ExecuteCounterLoggerAction((metricLogger) => { - foreach (var payload in counterPayload) - { - metricLogger.Log(payload); - } - }); + ExecuteCounterLoggerAction((metricLogger) => metricLogger.Log(counterPayload)); } } catch (Exception) diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterUtilities.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterUtilities.cs new file mode 100644 index 0000000000..b09c4bc19e --- /dev/null +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterUtilities.cs @@ -0,0 +1,54 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Collections.Generic; +using System; + +namespace Microsoft.Diagnostics.Monitoring.EventPipe +{ + internal static class CounterUtilities + { + //The metadata payload is formatted as a string of comma separated key:value pairs. + //This limitation means that metadata values cannot include commas; otherwise, the + //metadata will be parsed incorrectly. If a value contains a comma, then all metadata + //is treated as invalid and excluded from the payload. + public static IDictionary GetMetadata(string metadataPayload, char kvSeparator = ':') + { + var metadataDict = new Dictionary(); + + ReadOnlySpan metadata = metadataPayload; + + while (!metadata.IsEmpty) { + int commaIndex = metadata.IndexOf(','); + + ReadOnlySpan kvPair; + + if (commaIndex < 0) { + kvPair = metadata; + metadata = default; + } + else { + kvPair = metadata[..commaIndex]; + metadata = metadata.Slice(commaIndex + 1); + } + + int colonIndex = kvPair.IndexOf(kvSeparator); + if (colonIndex < 0) { + metadataDict.Clear(); + break; + } + + string metadataKey = kvPair[..colonIndex].ToString(); + string metadataValue = kvPair.Slice(colonIndex + 1).ToString(); + metadataDict[metadataKey] = metadataValue; + } + + return metadataDict; + } + + public static string AppendPercentile(string tags, double quantile) => AppendPercentile(tags, FormattableString.Invariant($"Percentile={(int)(100 * quantile)}")); + + private static string AppendPercentile(string tags, string percentile) => string.IsNullOrEmpty(tags) ? percentile : string.Concat(tags, ",", percentile); + } +} \ No newline at end of file diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs index 01bf1aa8a3..1212afe20b 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs @@ -11,9 +11,9 @@ namespace Microsoft.Diagnostics.Monitoring.EventPipe { internal static class TraceEventExtensions { - public static bool TryGetCounterPayload(this TraceEvent traceEvent, CounterFilter filter, string sessionId, out List payload) + public static bool TryGetCounterPayload(this TraceEvent traceEvent, CounterFilter filter, string sessionId, out ICounterPayload payload) { - payload = new List(); + payload = null; if ("EventCounters".Equals(traceEvent.EventName)) { @@ -59,7 +59,7 @@ public static bool TryGetCounterPayload(this TraceEvent traceEvent, CounterFilte // Note that dimensional data such as pod and namespace are automatically added in prometheus and azure monitor scenarios. // We no longer added it here. - payload.Add(new CounterPayload( + payload = new CounterPayload( traceEvent.TimeStamp, traceEvent.ProviderName, counterName, displayName, @@ -67,15 +67,13 @@ public static bool TryGetCounterPayload(this TraceEvent traceEvent, CounterFilte value, counterType, intervalSec, - metadata)); + metadata); return true; } if (sessionId != null && "System.Diagnostics.Metrics".Equals(traceEvent.ProviderName)) { - ICounterPayload individualPayload = null; - if (traceEvent.EventName == "BeginInstrumentReporting") { // Do we want to log something for this? @@ -87,53 +85,39 @@ public static bool TryGetCounterPayload(this TraceEvent traceEvent, CounterFilte } else if (traceEvent.EventName == "GaugeValuePublished") { - HandleGauge(traceEvent, filter, sessionId, out individualPayload); + HandleGauge(traceEvent, filter, sessionId, out payload); } else if (traceEvent.EventName == "CounterRateValuePublished") { - HandleCounterRate(traceEvent, filter, sessionId, out individualPayload); + HandleCounterRate(traceEvent, filter, sessionId, out payload); } else if (traceEvent.EventName == "TimeSeriesLimitReached") { - HandleTimeSeriesLimitReached(traceEvent, sessionId, out individualPayload); + HandleTimeSeriesLimitReached(traceEvent, sessionId, out payload); } else if (traceEvent.EventName == "HistogramLimitReached") { - HandleHistogramLimitReached(traceEvent, sessionId, out individualPayload); + HandleHistogramLimitReached(traceEvent, sessionId, out payload); } else if (traceEvent.EventName == "Error") { - HandleError(traceEvent, sessionId, out individualPayload); + HandleError(traceEvent, sessionId, out payload); } else if (traceEvent.EventName == "ObservableInstrumentCallbackError") { - HandleObservableInstrumentCallbackError(traceEvent, sessionId, out individualPayload); + HandleObservableInstrumentCallbackError(traceEvent, sessionId, out payload); } else if (traceEvent.EventName == "MultipleSessionsNotSupportedError") { - HandleMultipleSessionsNotSupportedError(traceEvent, sessionId, out individualPayload); - } - - if (null != individualPayload) - { - payload.Add(individualPayload); + HandleMultipleSessionsNotSupportedError(traceEvent, sessionId, out payload); } - return null != payload && payload.Any(); + return payload != null; } return false; } - public static bool TryGetIndividualCounterPayload(this TraceEvent traceEvent, CounterFilter filter, out ICounterPayload payload) - { - bool gotCounterPayload = TryGetCounterPayload(traceEvent, filter, null, out List payloadsList); - - payload = payloadsList.FirstOrDefault(); - - return gotCounterPayload; - } - private static void HandleGauge(TraceEvent obj, CounterFilter filter, string sessionId, out ICounterPayload payload) { payload = null; @@ -205,12 +189,11 @@ private static void HandleCounterRate(TraceEvent traceEvent, CounterFilter filte } } - private static void HandleHistogram(TraceEvent obj, CounterFilter filter, string sessionId, out List payload) + private static void HandleHistogram(TraceEvent obj, CounterFilter filter, string sessionId, out ICounterPayload payload) { - payload = new List(); + payload = null; string payloadSessionId = (string)obj.PayloadValue(0); - if (payloadSessionId != sessionId) { return; @@ -228,15 +211,15 @@ private static void HandleHistogram(TraceEvent obj, CounterFilter filter, string return; } - KeyValuePair[] quantiles = ParseQuantiles(quantilesText); - foreach ((double key, double val) in quantiles) + if (string.IsNullOrEmpty(quantilesText)) { - string tagsWithPercentile = AppendPercentile(tags, FormattableString.Invariant($"Percentile={(int)(100*key)}")); - payload.Add(new PercentilePayload(meterName, instrumentName, null, unit, tagsWithPercentile, val, obj.TimeStamp)); } + + IList<(double, double)> quantiles = ParseQuantiles(quantilesText); + payload = new PercentilePayload(meterName, instrumentName, null, unit, tags, quantiles, obj.TimeStamp); } - private static string AppendPercentile(string tags, string percentile) => string.IsNullOrEmpty(tags) ? percentile : FormattableString.Invariant($"{tags},{percentile}"); + private static void HandleHistogramLimitReached(TraceEvent obj, string sessionId, out ICounterPayload payload) { @@ -324,52 +307,10 @@ private static void HandleObservableInstrumentCallbackError(TraceEvent obj, stri payload = new ErrorPayload(errorMessage, obj.TimeStamp); } - //The metadata payload is formatted as a string of comma separated key:value pairs. - //This limitation means that metadata values cannot include commas; otherwise, the - //metadata will be parsed incorrectly. If a value contains a comma, then all metadata - //is treated as invalid and excluded from the payload. - public static IDictionary GetMetadata(string metadataPayload, char kvSeparator = ':') - { - var metadataDict = new Dictionary(); - - ReadOnlySpan metadata = metadataPayload; - - while (!metadata.IsEmpty) - { - int commaIndex = metadata.IndexOf(','); - - ReadOnlySpan kvPair; - - if (commaIndex < 0) - { - kvPair = metadata; - metadata = default; - } - else - { - kvPair = metadata[..commaIndex]; - metadata = metadata.Slice(commaIndex + 1); - } - - int colonIndex = kvPair.IndexOf(kvSeparator); - if (colonIndex < 0) - { - metadataDict.Clear(); - break; - } - - string metadataKey = kvPair[..colonIndex].ToString(); - string metadataValue = kvPair.Slice(colonIndex + 1).ToString(); - metadataDict[metadataKey] = metadataValue; - } - - return metadataDict; - } - - private static KeyValuePair[] ParseQuantiles(string quantileList) + private static IList<(double, double)> ParseQuantiles(string quantileList) { string[] quantileParts = quantileList.Split(';', StringSplitOptions.RemoveEmptyEntries); - List> quantiles = new List>(); + var quantiles = new List<(double, double)>(); foreach (string quantile in quantileParts) { string[] keyValParts = quantile.Split('=', StringSplitOptions.RemoveEmptyEntries); @@ -385,9 +326,9 @@ private static KeyValuePair[] ParseQuantiles(string quantileList { continue; } - quantiles.Add(new KeyValuePair(key, val)); + quantiles.Add((key, val)); } - return quantiles.ToArray(); + return quantiles; } private static int GetInterval(string series) diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Triggers/EventCounter/EventCounterTrigger.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Triggers/EventCounter/EventCounterTrigger.cs index bdbe92591c..1bc46fc7b0 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Triggers/EventCounter/EventCounterTrigger.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Triggers/EventCounter/EventCounterTrigger.cs @@ -59,7 +59,7 @@ public IReadOnlyDictionary> GetProviderEvent public bool HasSatisfiedCondition(TraceEvent traceEvent) { // Filter to the counter of interest before forwarding to the implementation - if (traceEvent.TryGetIndividualCounterPayload(_filter, out ICounterPayload payload)) + if (traceEvent.TryGetCounterPayload(_filter, null, out ICounterPayload payload)) { return _impl.HasSatisfiedCondition(payload); } From b47a9579c5d03afbc8f9586da9c5a3b14bc4b0b6 Mon Sep 17 00:00:00 2001 From: kkeirstead Date: Wed, 25 Jan 2023 12:31:32 -0800 Subject: [PATCH 10/14] Tweaks to account for new All flag --- .../Configuration/MetricSourceConfiguration.cs | 5 +++-- .../Counters/CounterPipelineSettings.cs | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/MetricSourceConfiguration.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/MetricSourceConfiguration.cs index 701d5e8c34..f5bb9eae30 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/MetricSourceConfiguration.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/MetricSourceConfiguration.cs @@ -17,7 +17,8 @@ namespace Microsoft.Diagnostics.Monitoring.EventPipe public enum MetricType { EventCounter = 0x1, - Meter = 0x2 + Meter = 0x2, + All = 0xFF } public sealed class MetricEventPipeProvider @@ -26,7 +27,7 @@ public sealed class MetricEventPipeProvider public float? IntervalSeconds { get; set; } - public MetricType Type { get; set; } = MetricType.EventCounter | MetricType.Meter; + public MetricType Type { get; set; } = MetricType.All; } public sealed class MetricSourceConfiguration : MonitoringSourceConfiguration diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipelineSettings.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipelineSettings.cs index 9833f71038..7fdc85e24f 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipelineSettings.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipelineSettings.cs @@ -26,6 +26,7 @@ internal enum CounterGroupType { EventCounter = 0x1, Meter = 0x2, + All = 0xFF } internal class EventPipeCounterGroup @@ -34,7 +35,7 @@ internal class EventPipeCounterGroup public string[] CounterNames { get; set; } - public CounterGroupType Type { get; set; } = CounterGroupType.EventCounter | CounterGroupType.Meter; + public CounterGroupType Type { get; set; } = CounterGroupType.All; public float? IntervalSeconds { get; set; } } From 887c6d4411c709eeafdfdaac55a6af00c10a72f2 Mon Sep 17 00:00:00 2001 From: kkeirstead Date: Wed, 25 Jan 2023 12:36:51 -0800 Subject: [PATCH 11/14] Fixed build/test failures from Wiktor's changes --- .../EventCounterTriggerTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterTriggerTests.cs b/src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterTriggerTests.cs index 1e4609d859..8770c98269 100644 --- a/src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterTriggerTests.cs +++ b/src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterTriggerTests.cs @@ -499,7 +499,7 @@ public void ValidateMetadataParsing_Success() const string value1 = "V1"; const string key2 = "K2"; const string value2 = "V:2"; - IDictionary metadataDict = TraceEventExtensions.GetMetadata($"{key1}:{value1},{key2}:{value2}"); + IDictionary metadataDict = CounterUtilities.GetMetadata($"{key1}:{value1},{key2}:{value2}"); Assert.Equal(2, metadataDict.Count); Assert.Equal(value1, metadataDict[key1]); @@ -515,7 +515,7 @@ public void ValidateMetadataParsing_Success() [InlineData("K1")] public void ValidateMetadataParsing_Failure(string invalidMetadata) { - IDictionary metadataDict = TraceEventExtensions.GetMetadata(invalidMetadata); + IDictionary metadataDict = CounterUtilities.GetMetadata(invalidMetadata); Assert.Empty(metadataDict); } From ba1fac53de0cacce5713f5bfb5ddd4213bd5e265 Mon Sep 17 00:00:00 2001 From: wiktork Date: Thu, 26 Jan 2023 17:35:39 -0800 Subject: [PATCH 12/14] Fix issue with empty quantiles --- .../Counters/TraceEventExtensions.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs index 1212afe20b..645bb456f4 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs @@ -213,6 +213,7 @@ private static void HandleHistogram(TraceEvent obj, CounterFilter filter, string if (string.IsNullOrEmpty(quantilesText)) { + return; } IList<(double, double)> quantiles = ParseQuantiles(quantilesText); From 8f8bce29e51fbe4dc72720843abf6741dd388d04 Mon Sep 17 00:00:00 2001 From: Wiktor Kopec Date: Fri, 27 Jan 2023 15:22:07 -0800 Subject: [PATCH 13/14] Initial PR feedback (#3620) --- .../MetricSourceConfiguration.cs | 8 +++---- .../MonitoringSourceConfiguration.cs | 1 + .../Counters/CounterPayload.cs | 4 ++-- .../Counters/CounterUtilities.cs | 3 ++- ...{CounterPipeline.cs => MetricsPipeline.cs} | 6 +++--- ...Settings.cs => MetricsPipelineSettings.cs} | 2 +- .../Counters/TraceEventExtensions.cs | 21 ++++++++----------- src/Tools/dotnet-counters/CounterMonitor.cs | 1 + 8 files changed, 23 insertions(+), 23 deletions(-) rename src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/{CounterPipeline.cs => MetricsPipeline.cs} (95%) rename src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/{CounterPipelineSettings.cs => MetricsPipelineSettings.cs} (94%) diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/MetricSourceConfiguration.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/MetricSourceConfiguration.cs index f5bb9eae30..a045a343ec 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/MetricSourceConfiguration.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/MetricSourceConfiguration.cs @@ -64,20 +64,20 @@ public MetricSourceConfiguration(float metricIntervalSeconds, IEnumerable p.Provider)); SessionId = Guid.NewGuid().ToString(); EventPipeProvider metricsEventSourceProvider = - new EventPipeProvider("System.Diagnostics.Metrics", EventLevel.Informational, TimeSeriesValues, + new EventPipeProvider(MonitoringSourceConfiguration.SystemDiagnosticsMetricsProviderName, EventLevel.Informational, TimeSeriesValuesEventKeyword, new Dictionary() { { "SessionId", SessionId }, { "Metrics", metrics }, { "RefreshInterval", metricIntervalSeconds.ToString(CultureInfo.InvariantCulture) }, - { "MaxTimeSeries", maxTimeSeries.ToString() }, - { "MaxHistograms", maxHistograms.ToString() } + { "MaxTimeSeries", maxTimeSeries.ToString(CultureInfo.InvariantCulture) }, + { "MaxHistograms", maxHistograms.ToString(CultureInfo.InvariantCulture) } } ); diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/MonitoringSourceConfiguration.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/MonitoringSourceConfiguration.cs index 00486370f5..ec1aec13d0 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/MonitoringSourceConfiguration.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Configuration/MonitoringSourceConfiguration.cs @@ -27,6 +27,7 @@ public abstract class MonitoringSourceConfiguration public const string TplEventSource = "System.Threading.Tasks.TplEventSource"; public const string SampleProfilerProviderName = "Microsoft-DotNETCore-SampleProfiler"; public const string EventPipeProviderName = "Microsoft-DotNETCore-EventPipe"; + public const string SystemDiagnosticsMetricsProviderName = "System.Diagnostics.Metrics"; public static IEnumerable DefaultMetricProviders => new[] { SystemRuntimeEventSourceName, MicrosoftAspNetCoreHostingEventSourceName, GrpcAspNetCoreServer }; diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs index 810f0cff00..187cbfc387 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs @@ -106,13 +106,13 @@ public RatePayload(string providerName, string name, string displayName, string internal class PercentilePayload : CounterPayload { - public PercentilePayload(string providerName, string name, string displayName, string displayUnits, string metadata, IEnumerable<(double quantile, double value)> quantiles, DateTime timestamp) : + public PercentilePayload(string providerName, string name, string displayName, string displayUnits, string metadata, IEnumerable quantiles, DateTime timestamp) : base(providerName, name, displayName, displayUnits, metadata, 0.0, timestamp, "Metric", EventType.Histogram) { // In case these properties are not provided, set them to appropriate values. string counterName = string.IsNullOrEmpty(displayName) ? name : displayName; DisplayName = !string.IsNullOrEmpty(displayUnits) ? $"{counterName} ({displayUnits})" : counterName; - Quantiles = quantiles.Select(v => new Quantile(v.quantile, v.value)).ToArray(); + Quantiles = quantiles.ToArray(); } public Quantile[] Quantiles { get; } diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterUtilities.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterUtilities.cs index b09c4bc19e..26ead38882 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterUtilities.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterUtilities.cs @@ -51,4 +51,5 @@ public static IDictionary GetMetadata(string metadataPayload, ch private static string AppendPercentile(string tags, string percentile) => string.IsNullOrEmpty(tags) ? percentile : string.Concat(tags, ",", percentile); } -} \ No newline at end of file +} + \ No newline at end of file diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipeline.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/MetricsPipeline.cs similarity index 95% rename from src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipeline.cs rename to src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/MetricsPipeline.cs index ebb0c79301..1dfad989c7 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipeline.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/MetricsPipeline.cs @@ -12,14 +12,14 @@ namespace Microsoft.Diagnostics.Monitoring.EventPipe { - internal class CounterPipeline : EventSourcePipeline + internal class MetricsPipeline : EventSourcePipeline { private readonly IEnumerable _loggers; private readonly CounterFilter _filter; private string _sessionId; - public CounterPipeline(DiagnosticsClient client, - CounterPipelineSettings settings, + public MetricsPipeline(DiagnosticsClient client, + MetricsPipelineSettings settings, IEnumerable loggers) : base(client, settings) { _loggers = loggers ?? throw new ArgumentNullException(nameof(loggers)); diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipelineSettings.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/MetricsPipelineSettings.cs similarity index 94% rename from src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipelineSettings.cs rename to src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/MetricsPipelineSettings.cs index 7fdc85e24f..f0c9d6e64e 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPipelineSettings.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/MetricsPipelineSettings.cs @@ -8,7 +8,7 @@ namespace Microsoft.Diagnostics.Monitoring.EventPipe { - internal class CounterPipelineSettings : EventSourcePipelineSettings + internal class MetricsPipelineSettings : EventSourcePipelineSettings { public EventPipeCounterGroup[] CounterGroups { get; set; } diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs index 645bb456f4..34255c6d34 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs @@ -5,6 +5,7 @@ using Microsoft.Diagnostics.Tracing; using System; using System.Collections.Generic; +using System.Globalization; using System.Linq; namespace Microsoft.Diagnostics.Monitoring.EventPipe @@ -72,7 +73,7 @@ public static bool TryGetCounterPayload(this TraceEvent traceEvent, CounterFilte return true; } - if (sessionId != null && "System.Diagnostics.Metrics".Equals(traceEvent.ProviderName)) + if (sessionId != null && MonitoringSourceConfiguration.SystemDiagnosticsMetricsProviderName.Equals(traceEvent.ProviderName)) { if (traceEvent.EventName == "BeginInstrumentReporting") { @@ -142,7 +143,7 @@ private static void HandleGauge(TraceEvent obj, CounterFilter filter, string ses } // the value might be an empty string indicating no measurement was provided this collection interval - if (double.TryParse(lastValueText, out double lastValue)) + if (double.TryParse(lastValueText, NumberStyles.Number | NumberStyles.Float, CultureInfo.InvariantCulture, out double lastValue)) { payload = new GaugePayload(meterName, instrumentName, null, unit, tags, lastValue, obj.TimeStamp); } @@ -177,7 +178,7 @@ private static void HandleCounterRate(TraceEvent traceEvent, CounterFilter filte return; } - if (double.TryParse(rateText, out double rate)) + if (double.TryParse(rateText, NumberStyles.Number | NumberStyles.Float, CultureInfo.InvariantCulture, out double rate)) { payload = new RatePayload(meterName, instrumentName, null, unit, tags, rate, filter.IntervalSeconds, traceEvent.TimeStamp); } @@ -211,12 +212,8 @@ private static void HandleHistogram(TraceEvent obj, CounterFilter filter, string return; } - if (string.IsNullOrEmpty(quantilesText)) - { - return; - } - - IList<(double, double)> quantiles = ParseQuantiles(quantilesText); + //Note quantiles can be empty. + IList quantiles = ParseQuantiles(quantilesText); payload = new PercentilePayload(meterName, instrumentName, null, unit, tags, quantiles, obj.TimeStamp); } @@ -308,10 +305,10 @@ private static void HandleObservableInstrumentCallbackError(TraceEvent obj, stri payload = new ErrorPayload(errorMessage, obj.TimeStamp); } - private static IList<(double, double)> ParseQuantiles(string quantileList) + private static IList ParseQuantiles(string quantileList) { string[] quantileParts = quantileList.Split(';', StringSplitOptions.RemoveEmptyEntries); - var quantiles = new List<(double, double)>(); + var quantiles = new List(); foreach (string quantile in quantileParts) { string[] keyValParts = quantile.Split('=', StringSplitOptions.RemoveEmptyEntries); @@ -327,7 +324,7 @@ private static void HandleObservableInstrumentCallbackError(TraceEvent obj, stri { continue; } - quantiles.Add((key, val)); + quantiles.Add(new Quantile(key, val)); } return quantiles; } diff --git a/src/Tools/dotnet-counters/CounterMonitor.cs b/src/Tools/dotnet-counters/CounterMonitor.cs index 5aaa1ef4eb..beb1e009c4 100644 --- a/src/Tools/dotnet-counters/CounterMonitor.cs +++ b/src/Tools/dotnet-counters/CounterMonitor.cs @@ -171,6 +171,7 @@ private void HandleCounterRate(TraceEvent obj) CounterPayload payload = new RatePayload(meterName, instrumentName, null, unit, tags, rate, _interval, obj.TimeStamp); _renderer.CounterPayloadReceived(payload, _pauseCmdSet); } + } private void HandleGauge(TraceEvent obj) From b2971ce320e436ba16422ac7ffd8364280fa641f Mon Sep 17 00:00:00 2001 From: kkeirstead Date: Mon, 30 Jan 2023 07:09:23 -0800 Subject: [PATCH 14/14] Fixes outdated naming in test --- .../EventCounterPipelineUnitTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterPipelineUnitTests.cs b/src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterPipelineUnitTests.cs index 5c9f36eafa..5a05bd0f8d 100644 --- a/src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterPipelineUnitTests.cs +++ b/src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterPipelineUnitTests.cs @@ -103,7 +103,7 @@ public async Task TestCounterEventPipeline(TestConfiguration config) { var client = new DiagnosticsClient(testRunner.Pid); - await using CounterPipeline pipeline = new CounterPipeline(client, new CounterPipelineSettings + await using MetricsPipeline pipeline = new MetricsPipeline(client, new MetricsPipelineSettings { Duration = Timeout.InfiniteTimeSpan, CounterGroups = new[]