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); }