From 5d2ca481c61307faaa9dc264512e245df53e9e58 Mon Sep 17 00:00:00 2001 From: wiktork Date: Fri, 9 Dec 2022 15:02:25 -0800 Subject: [PATCH 1/5] Fix ICountersLogger contract --- .../Counters/EventCounterPipeline.cs | 18 ++++++++++++++++-- .../Counters/ICountersLogger.cs | 10 ++++++---- .../EventCounterPipelineUnitTests.cs | 8 ++------ 3 files changed, 24 insertions(+), 12 deletions(-) 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/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/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterPipelineUnitTests.cs b/src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterPipelineUnitTests.cs index bfb15c6b6b..4909f0ccfc 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() => Task.CompletedTask; - public void PipelineStopped() - { - } + public Task PipelineStopped() => Task.CompletedTask; private static string CreateKey(ICounterPayload payload) { From b5cf8207d435b00ec65bbd5e9052ce09fb078fc3 Mon Sep 17 00:00:00 2001 From: wiktork Date: Fri, 9 Dec 2022 15:11:17 -0800 Subject: [PATCH 2/5] MetadataUpdates --- .../Counters/CounterPayload.cs | 24 ++++++--------- .../Counters/ICounterPayload.cs | 2 +- .../Counters/TraceEventExtensions.cs | 29 ++++++++----------- .../EventCounterPipelineUnitTests.cs | 4 +-- .../EventCounterTriggerTests.cs | 4 +-- 5 files changed, 26 insertions(+), 37 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs index 28e811b0b6..5da604ea3c 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs @@ -10,12 +10,6 @@ namespace Microsoft.Diagnostics.Monitoring.EventPipe { public 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 { - 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. @@ -86,7 +80,7 @@ public GaugePayload(string providerName, string name, string displayName, string public 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. @@ -99,7 +93,7 @@ public RatePayload(string providerName, string name, string displayName, string public 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. @@ -110,7 +104,7 @@ public PercentilePayload(string providerName, string name, string displayName, s public class ErrorPayload : CounterPayload { - public ErrorPayload(string providerName, string name, string displayName, string displayUnits, Dictionary metadata, double val, DateTime timestamp, string errorMessage) : + public ErrorPayload(string providerName, string name, string displayName, string displayUnits, string metadata, double val, DateTime timestamp, string errorMessage) : base(providerName, name, displayName, displayUnits, metadata, val, timestamp, "Metric", EventType.Error) { ErrorMessage = errorMessage; diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICounterPayload.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICounterPayload.cs index cc27e69d9b..f565ab6067 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICounterPayload.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICounterPayload.cs @@ -35,7 +35,7 @@ public interface ICounterPayload float Interval { get; } - IReadOnlyDictionary Metadata { get; } + string Metadata { get; } public EventType EventType { get; set; } } diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs index 75133b5f5c..782d044412 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,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)) { - payload = new GaugePayload(meterName, instrumentName, null, unit, metadataDict, lastValue, obj.TimeStamp); + payload = new GaugePayload(meterName, instrumentName, null, unit, tags, lastValue, obj.TimeStamp); } } @@ -184,11 +182,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,9 +214,8 @@ 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)); + tags = FormattableString.Invariant($"{tags},quantile={key}"); + payload.Add(new PercentilePayload(meterName, instrumentName, null, unit, tags, val, obj.TimeStamp)); } } @@ -237,7 +232,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(string.Empty, string.Empty, string.Empty, string.Empty, null, 0, obj.TimeStamp, errorMessage); } private static void HandleTimeSeriesLimitReached(TraceEvent obj, string sessionId, out ICounterPayload payload) @@ -253,7 +248,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(string.Empty, string.Empty, string.Empty, string.Empty, null, 0, obj.TimeStamp, errorMessage); } private static void HandleError(TraceEvent obj, string sessionId, out ICounterPayload payload) @@ -269,7 +264,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(string.Empty, string.Empty, string.Empty, string.Empty, null, 0, obj.TimeStamp, errorMessage); } private static void HandleMultipleSessionsNotSupportedError(TraceEvent obj, string sessionId, out ICounterPayload payload) @@ -288,7 +283,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(string.Empty, string.Empty, string.Empty, string.Empty, null, 0, obj.TimeStamp, errorMessage); } } @@ -307,14 +302,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(string.Empty, string.Empty, string.Empty, string.Empty, null, 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 //is treated as invalid and excluded from the payload. - internal static Dictionary GetMetadata(string metadataPayload) + public static IDictionary GetMetadata(string metadataPayload) { var metadataDict = new Dictionary(); diff --git a/src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterPipelineUnitTests.cs b/src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterPipelineUnitTests.cs index 4909f0ccfc..95abce2c06 100644 --- a/src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterPipelineUnitTests.cs +++ b/src/tests/Microsoft.Diagnostics.Monitoring.EventPipe/EventCounterPipelineUnitTests.cs @@ -67,9 +67,9 @@ public void Log(ICounterPayload metric) } } - public Task PipelineStarted() => Task.CompletedTask; + public Task PipelineStarted(CancellationToken token) => Task.CompletedTask; - public Task PipelineStopped() => Task.CompletedTask; + 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 d697b3f0f2c06a73edfae0db34cb65008086bd4c Mon Sep 17 00:00:00 2001 From: wiktork Date: Wed, 14 Dec 2022 16:03:30 -0800 Subject: [PATCH 3/5] Fixup api protection levels --- .../Counters/CounterPayload.cs | 20 +++++++++++-------- .../Counters/ICounterPayload.cs | 4 ++-- .../Counters/TraceEventExtensions.cs | 10 +++++----- ...ft.Diagnostics.Monitoring.EventPipe.csproj | 2 ++ .../Microsoft.Diagnostics.Monitoring.csproj | 1 + 5 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs index 5da604ea3c..2968c72a62 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 { - public class CounterPayload : ICounterPayload + internal class CounterPayload : ICounterPayload { public CounterPayload(DateTime timestamp, string provider, @@ -67,7 +67,7 @@ public CounterPayload(string providerName, string name, string displayName, stri public EventType EventType { get; set; } } - public class GaugePayload : CounterPayload + internal class GaugePayload : CounterPayload { 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) @@ -78,7 +78,7 @@ public GaugePayload(string providerName, string name, string displayName, string } } - public class RatePayload : CounterPayload + internal class RatePayload : CounterPayload { 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) @@ -91,7 +91,7 @@ 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, string metadata, double val, DateTime timestamp) : base(providerName, name, displayName, displayUnits, metadata, val, timestamp, "Metric", EventType.Histogram) @@ -102,10 +102,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, string 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; } @@ -114,7 +118,7 @@ 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, diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICounterPayload.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/ICounterPayload.cs index f565ab6067..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; } @@ -37,6 +37,6 @@ public interface ICounterPayload string Metadata { get; } - public EventType EventType { get; set; } + EventType EventType { get; set; } } } diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs index 782d044412..de16a1ecff 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs @@ -232,7 +232,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, null, 0, obj.TimeStamp, errorMessage); + payload = new ErrorPayload(errorMessage); } private static void HandleTimeSeriesLimitReached(TraceEvent obj, string sessionId, out ICounterPayload payload) @@ -248,7 +248,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, null, 0, obj.TimeStamp, errorMessage); + payload = new ErrorPayload(errorMessage, obj.TimeStamp); } private static void HandleError(TraceEvent obj, string sessionId, out ICounterPayload payload) @@ -264,7 +264,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, null, 0, obj.TimeStamp, errorMessage); + payload = new ErrorPayload(errorMessage, obj.TimeStamp); } private static void HandleMultipleSessionsNotSupportedError(TraceEvent obj, string sessionId, out ICounterPayload payload) @@ -283,7 +283,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, null, 0, obj.TimeStamp, errorMessage); + payload = new ErrorPayload(errorMessage, obj.TimeStamp); } } @@ -302,7 +302,7 @@ 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, null, 0, obj.TimeStamp, errorMessage); + payload = new ErrorPayload(errorMessage, obj.TimeStamp); } //The metadata payload is formatted as a string of comma separated key:value pairs. 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 @@ + From 8cc29c95acc12701415e12b4c3740147a952bc04 Mon Sep 17 00:00:00 2001 From: wiktork Date: Wed, 14 Dec 2022 17:09:32 -0800 Subject: [PATCH 4/5] Fixup tests and add CounterEnded payload --- .../Counters/CounterPayload.cs | 12 +++++++++++- .../Counters/TraceEventExtensions.cs | 9 +++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs index 2968c72a62..522c6be188 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/CounterPayload.cs @@ -78,6 +78,15 @@ public GaugePayload(string providerName, string name, string displayName, string } } + 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, string metadata, double value, double intervalSecs, DateTime timestamp) : @@ -123,6 +132,7 @@ 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/TraceEventExtensions.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs index de16a1ecff..8cf3a37b8d 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs @@ -162,6 +162,15 @@ private static void HandleGauge(TraceEvent obj, CounterFilter filter, string ses { 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); + + } } private static void HandleCounterRate(TraceEvent traceEvent, CounterFilter filter, string sessionId, out ICounterPayload payload) From 811135d843399891eed4eeb883ae276b9d90dfbd Mon Sep 17 00:00:00 2001 From: wiktork Date: Thu, 15 Dec 2022 15:47:30 -0800 Subject: [PATCH 5/5] Fixup metadata parsing --- .../Counters/TraceEventExtensions.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs index 8cf3a37b8d..23061f93ed 100644 --- a/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs +++ b/src/Microsoft.Diagnostics.Monitoring.EventPipe/Counters/TraceEventExtensions.cs @@ -223,11 +223,13 @@ private static void HandleHistogram(TraceEvent obj, CounterFilter filter, string KeyValuePair[] quantiles = ParseQuantiles(quantilesText); foreach ((double key, double val) in quantiles) { - tags = FormattableString.Invariant($"{tags},quantile={key}"); - payload.Add(new PercentilePayload(meterName, instrumentName, null, unit, tags, 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; @@ -318,7 +320,7 @@ private static void HandleObservableInstrumentCallbackError(TraceEvent obj, stri //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) + public static IDictionary GetMetadata(string metadataPayload, char kvSeparator = ':') { var metadataDict = new Dictionary(); @@ -341,7 +343,7 @@ public static IDictionary GetMetadata(string metadataPayload) metadata = metadata.Slice(commaIndex + 1); } - int colonIndex = kvPair.IndexOf(':'); + int colonIndex = kvPair.IndexOf(kvSeparator); if (colonIndex < 0) { metadataDict.Clear();