From: Sung Yoon Whang Date: Wed, 10 Apr 2019 08:19:30 +0000 (-0700) Subject: Expose new EventCounter APIs (#23808) X-Git-Tag: accepted/tizen/unified/20190813.215958~46^2~139 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=95d37e097086187692c770471d79810482971b34;p=platform%2Fupstream%2Fcoreclr.git Expose new EventCounter APIs (#23808) * rename BaseCounter to DiagnosticCounter * Change MetaData->Metadata * Make EventSource and Name a property for counter classes * Make the counter APIs public * fix build errors * Change float to double * Few cleanups, fix test * fix GetMetadataString * PR feedback * More PR feedback --- diff --git a/src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems b/src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems index 0ba0bbb..f85cb14 100644 --- a/src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems +++ b/src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems @@ -908,7 +908,7 @@ - + diff --git a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/CounterGroup.cs b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/CounterGroup.cs index 010cbd5..0382556 100644 --- a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/CounterGroup.cs +++ b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/CounterGroup.cs @@ -20,22 +20,22 @@ namespace System.Diagnostics.Tracing internal class CounterGroup { private readonly EventSource _eventSource; - private readonly List _counters; + private readonly List _counters; internal CounterGroup(EventSource eventSource) { _eventSource = eventSource; - _counters = new List(); + _counters = new List(); RegisterCommandCallback(); } - internal void Add(BaseCounter eventCounter) + internal void Add(DiagnosticCounter eventCounter) { lock (this) // Lock the CounterGroup _counters.Add(eventCounter); } - internal void Remove(BaseCounter eventCounter) + internal void Remove(DiagnosticCounter eventCounter) { lock (this) // Lock the CounterGroup _counters.Remove(eventCounter); diff --git a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/CounterPayload.cs b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/CounterPayload.cs index ae4a1a7..1be7d54 100644 --- a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/CounterPayload.cs +++ b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/CounterPayload.cs @@ -17,52 +17,6 @@ namespace Microsoft.Diagnostics.Tracing namespace System.Diagnostics.Tracing #endif { - // TODO: This should be removed as we make the new payloads public - [EventData] - internal class EventCounterPayload : IEnumerable> - { - public string Name { get; set; } - - public float Mean { get; set; } - - public float StandardDeviation { get; set; } - - public int Count { get; set; } - - public float Min { get; set; } - - public float Max { get; set; } - - public float IntervalSec { get; internal set; } - - #region Implementation of the IEnumerable interface - - public IEnumerator> GetEnumerator() - { - return ForEnumeration.GetEnumerator(); - } - - IEnumerator IEnumerable.GetEnumerator() - { - return ForEnumeration.GetEnumerator(); - } - - private IEnumerable> ForEnumeration - { - get - { - yield return new KeyValuePair("Name", Name); - yield return new KeyValuePair("Mean", Mean); - yield return new KeyValuePair("StandardDeviation", StandardDeviation); - yield return new KeyValuePair("Count", Count); - yield return new KeyValuePair("Min", Min); - yield return new KeyValuePair("Max", Max); - } - } - - #endregion // Implementation of the IEnumerable interface - } - [EventData] internal class CounterPayload : IEnumerable> { @@ -70,19 +24,19 @@ namespace System.Diagnostics.Tracing public string DisplayName { get; set; } - public float Mean { get; set; } + public double Mean { get; set; } - public float StandardDeviation { get; set; } + public double StandardDeviation { get; set; } public int Count { get; set; } - public float Min { get; set; } + public double Min { get; set; } - public float Max { get; set; } + public double Max { get; set; } public float IntervalSec { get; internal set; } - public string MetaData { get; set; } + public string Metadata { get; set; } #region Implementation of the IEnumerable interface @@ -110,7 +64,7 @@ namespace System.Diagnostics.Tracing yield return new KeyValuePair("IntervalSec", IntervalSec); yield return new KeyValuePair("Series", $"Interval={IntervalSec}"); yield return new KeyValuePair("CounterType", "Mean"); - yield return new KeyValuePair("MetaData", MetaData); + yield return new KeyValuePair("Metadata", Metadata); } } @@ -126,11 +80,11 @@ namespace System.Diagnostics.Tracing public string DisplayRateTimeScale { get; set; } - public float Increment { get; set; } + public double Increment { get; set; } public float IntervalSec { get; internal set; } - public string MetaData { get; set; } + public string Metadata { get; set; } #region Implementation of the IEnumerable interface @@ -155,7 +109,7 @@ namespace System.Diagnostics.Tracing yield return new KeyValuePair("IntervalSec", IntervalSec); yield return new KeyValuePair("Series", $"Interval={IntervalSec}"); yield return new KeyValuePair("CounterType", "Sum"); - yield return new KeyValuePair("MetaData", MetaData); + yield return new KeyValuePair("Metadata", Metadata); } } diff --git a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/BaseCounter.cs b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/DiagnosticCounter.cs similarity index 71% rename from src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/BaseCounter.cs rename to src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/DiagnosticCounter.cs index 447852e..ea4cb92 100644 --- a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/BaseCounter.cs +++ b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/DiagnosticCounter.cs @@ -19,10 +19,10 @@ namespace System.Diagnostics.Tracing #endif { /// - /// BaseCounter is an abstract class that serves as the parent class for various Counter* classes, + /// DiagnosticCounter is an abstract class that serves as the parent class for various Counter* classes, /// namely EventCounter, PollingCounter, IncrementingEventCounter, and IncrementingPollingCounter. /// - public abstract class BaseCounter : IDisposable + public abstract class DiagnosticCounter : IDisposable { /// /// All Counters live as long as the EventSource that they are attached to unless they are @@ -30,23 +30,22 @@ namespace System.Diagnostics.Tracing /// /// The name. /// The event source. - public BaseCounter(string name, EventSource eventSource) + public DiagnosticCounter(string name, EventSource eventSource) { if (name == null) { - throw new ArgumentNullException(nameof(_name)); + throw new ArgumentNullException(nameof(Name)); } if (eventSource == null) { - throw new ArgumentNullException(nameof(eventSource)); + throw new ArgumentNullException(nameof(EventSource)); } _group = CounterGroup.GetCounterGroup(eventSource); _group.Add(this); - _eventSource = eventSource; - _name = name; - _metaData = new Dictionary(); + Name = name; + EventSource = eventSource; } /// @@ -67,38 +66,45 @@ namespace System.Diagnostics.Tracing /// /// Adds a key-value metadata to the EventCounter that will be included as a part of the payload /// - internal void AddMetaData(string key, string value) + public void AddMetadata(string key, string value) { lock (MyLock) { - _metaData.Add(key, value); + _metadata = _metadata ?? new Dictionary(); + _metadata.Add(key, value); } } - internal string DisplayName { get; set; } + public string DisplayName { get; set; } - #region private implementation + public string Name { get; } + + public EventSource EventSource { get; } - internal readonly string _name; + #region private implementation private CounterGroup _group; - private Dictionary _metaData; - internal EventSource _eventSource; + private Dictionary _metadata; internal abstract void WritePayload(float intervalSec); // arbitrarily we use name as the lock object. - internal object MyLock { get { return _name; } } + internal object MyLock { get { return Name; } } internal void ReportOutOfBandMessage(string message) { - _eventSource.ReportOutOfBandMessage(message, true); + EventSource.ReportOutOfBandMessage(message, true); } - internal string GetMetaDataString() + internal string GetMetadataString() { + if (_metadata == null) + { + return ""; + } + StringBuilder sb = new StringBuilder(""); - foreach(KeyValuePair kvPair in _metaData) + foreach(KeyValuePair kvPair in _metadata) { sb.Append($"{kvPair.Key}:{kvPair.Value},"); } diff --git a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/EventCounter.cs b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/EventCounter.cs index 85fc40a..a9fc089 100644 --- a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/EventCounter.cs +++ b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/EventCounter.cs @@ -26,7 +26,7 @@ namespace System.Diagnostics.Tracing /// See https://github.com/dotnet/corefx/blob/master/src/System.Diagnostics.Tracing/tests/BasicEventSourceTest/TestEventCounter.cs /// which shows tests, which are also useful in seeing actual use. /// - public partial class EventCounter : BaseCounter + public partial class EventCounter : DiagnosticCounter { /// /// Initializes a new instance of the class. @@ -37,8 +37,8 @@ namespace System.Diagnostics.Tracing /// The event source. public EventCounter(string name, EventSource eventSource) : base(name, eventSource) { - _min = float.PositiveInfinity; - _max = float.NegativeInfinity; + _min = double.PositiveInfinity; + _max = double.NegativeInfinity; InitializeBuffer(); } @@ -50,21 +50,26 @@ namespace System.Diagnostics.Tracing /// The value. public void WriteMetric(float value) { + Enqueue((double)value); + } + + public void WriteMetric(double value) + { Enqueue(value); } - public override string ToString() => $"EventCounter '{_name}' Count {_count} Mean {(((double)_sum) / _count).ToString("n3")}"; + public override string ToString() => $"EventCounter '{Name}' Count {_count} Mean {(_sum / _count).ToString("n3")}"; #region Statistics Calculation // Statistics private int _count; - private float _sum; - private float _sumSquared; - private float _min; - private float _max; + private double _sum; + private double _sumSquared; + private double _min; + private double _max; - internal void OnMetricWritten(float value) + internal void OnMetricWritten(double value) { Debug.Assert(Monitor.IsEntered(MyLock)); _sum += value; @@ -83,14 +88,13 @@ namespace System.Diagnostics.Tracing lock (MyLock) { Flush(); - EventCounterPayload payload = new EventCounterPayload(); - payload.Name = _name; + CounterPayload payload = new CounterPayload(); payload.Count = _count; payload.IntervalSec = intervalSec; if (0 < _count) { payload.Mean = _sum / _count; - payload.StandardDeviation = (float)Math.Sqrt(_sumSquared / _count - _sum * _sum / _count / _count); + payload.StandardDeviation = Math.Sqrt(_sumSquared / _count - _sum * _sum / _count / _count); } else { @@ -99,8 +103,12 @@ namespace System.Diagnostics.Tracing } payload.Min = _min; payload.Max = _max; + + payload.Metadata = GetMetadataString(); + payload.DisplayName = DisplayName; + payload.Name = Name; ResetStatistics(); - _eventSource.Write("EventCounters", new EventSourceOptions() { Level = EventLevel.LogAlways }, new EventCounterPayloadType(payload)); + EventSource.Write("EventCounters", new EventSourceOptions() { Level = EventLevel.LogAlways }, new CounterPayloadType(payload)); } } private void ResetStatistics() @@ -109,35 +117,35 @@ namespace System.Diagnostics.Tracing _count = 0; _sum = 0; _sumSquared = 0; - _min = float.PositiveInfinity; - _max = float.NegativeInfinity; + _min = double.PositiveInfinity; + _max = double.NegativeInfinity; } #endregion // Statistics Calculation // Values buffering private const int BufferedSize = 10; - private const float UnusedBufferSlotValue = float.NegativeInfinity; + private const double UnusedBufferSlotValue = double.NegativeInfinity; private const int UnsetIndex = -1; - private volatile float[] _bufferedValues; + private volatile double[] _bufferedValues; private volatile int _bufferedValuesIndex; private void InitializeBuffer() { - _bufferedValues = new float[BufferedSize]; + _bufferedValues = new double[BufferedSize]; for (int i = 0; i < _bufferedValues.Length; i++) { _bufferedValues[i] = UnusedBufferSlotValue; } } - protected void Enqueue(float value) + private void Enqueue(double value) { // It is possible that two threads read the same bufferedValuesIndex, but only one will be able to write the slot, so that is okay. int i = _bufferedValuesIndex; while (true) { - float result = Interlocked.CompareExchange(ref _bufferedValues[i], value, UnusedBufferSlotValue); + double result = Interlocked.CompareExchange(ref _bufferedValues[i], value, UnusedBufferSlotValue); i++; if (_bufferedValues.Length <= i) { @@ -178,10 +186,10 @@ namespace System.Diagnostics.Tracing /// This is the payload that is sent in the with EventSource.Write /// [EventData] - class EventCounterPayloadType + class CounterPayloadType { - public EventCounterPayloadType(EventCounterPayload payload) { Payload = payload; } - public EventCounterPayload Payload { get; set; } + public CounterPayloadType(CounterPayload payload) { Payload = payload; } + public CounterPayload Payload { get; set; } } } diff --git a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/IncrementingEventCounter.cs b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/IncrementingEventCounter.cs index 569c9a0..4058105 100644 --- a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/IncrementingEventCounter.cs +++ b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/IncrementingEventCounter.cs @@ -23,7 +23,7 @@ namespace System.Diagnostics.Tracing /// It does not calculate statistics like mean, standard deviation, etc. because it only accumulates /// the counter value. /// - internal partial class IncrementingEventCounter : BaseCounter + public partial class IncrementingEventCounter : DiagnosticCounter { /// /// Initializes a new instance of the class. @@ -41,7 +41,7 @@ namespace System.Diagnostics.Tracing /// be logged on the next timer interval. /// /// The value to increment by. - public void Increment(float increment = 1) + public void Increment(double increment = 1) { lock(MyLock) { @@ -49,25 +49,25 @@ namespace System.Diagnostics.Tracing } } - internal TimeSpan DisplayRateTimeScale { get; set; } - private float _increment; - private float _prevIncrement; + public TimeSpan DisplayRateTimeScale { get; set; } + private double _increment; + private double _prevIncrement; - public override string ToString() => $"IncrementingEventCounter '{_name}' Increment {_increment}"; + public override string ToString() => $"IncrementingEventCounter '{Name}' Increment {_increment}"; internal override void WritePayload(float intervalSec) { lock (MyLock) // Lock the counter { IncrementingCounterPayload payload = new IncrementingCounterPayload(); - payload.Name = _name; + payload.Name = Name; payload.IntervalSec = intervalSec; payload.DisplayName = DisplayName ?? ""; payload.DisplayRateTimeScale = (DisplayRateTimeScale == TimeSpan.Zero) ? "" : DisplayRateTimeScale.ToString("c"); - payload.MetaData = GetMetaDataString(); + payload.Metadata = GetMetadataString(); payload.Increment = _increment - _prevIncrement; _prevIncrement = _increment; - _eventSource.Write("EventCounters", new EventSourceOptions() { Level = EventLevel.LogAlways }, new IncrementingEventCounterPayloadType(payload)); + EventSource.Write("EventCounters", new EventSourceOptions() { Level = EventLevel.LogAlways }, new IncrementingEventCounterPayloadType(payload)); } } } diff --git a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/IncrementingPollingCounter.cs b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/IncrementingPollingCounter.cs index 8bad728..1b8ee75 100644 --- a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/IncrementingPollingCounter.cs +++ b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/IncrementingPollingCounter.cs @@ -25,7 +25,7 @@ namespace System.Diagnostics.Tracing /// Unlike IncrementingEventCounter, this takes in a polling callback that it can call to update /// its own metric periodically. /// - internal partial class IncrementingPollingCounter : BaseCounter + public partial class IncrementingPollingCounter : DiagnosticCounter { /// /// Initializes a new instance of the class. @@ -34,20 +34,20 @@ namespace System.Diagnostics.Tracing /// /// The name. /// The event source. - public IncrementingPollingCounter(string name, EventSource eventSource, Func getCountFunction) : base(name, eventSource) + public IncrementingPollingCounter(string name, EventSource eventSource, Func totalValueProvider) : base(name, eventSource) { - _getCountFunction = getCountFunction; + _totalValueProvider = totalValueProvider; } - public override string ToString() => $"IncrementingPollingCounter '{_name}' Increment {_increment}"; + public override string ToString() => $"IncrementingPollingCounter '{Name}' Increment {_increment}"; - internal TimeSpan DisplayRateTimeScale { get; set; } - private float _increment; - private float _prevIncrement; - private Func _getCountFunction; + public TimeSpan DisplayRateTimeScale { get; set; } + private double _increment; + private double _prevIncrement; + private Func _totalValueProvider; /// - /// Calls "_getCountFunction" to enqueue the counter value to the queue. + /// Calls "_totalValueProvider" to enqueue the counter value to the queue. /// private void UpdateMetric() { @@ -55,12 +55,12 @@ namespace System.Diagnostics.Tracing { lock(MyLock) { - _increment = _getCountFunction(); + _increment = _totalValueProvider(); } } catch (Exception ex) { - ReportOutOfBandMessage($"ERROR: Exception during EventCounter {_name} getMetricFunction callback: " + ex.Message); + ReportOutOfBandMessage($"ERROR: Exception during EventCounter {Name} getMetricFunction callback: " + ex.Message); } } @@ -70,14 +70,14 @@ namespace System.Diagnostics.Tracing lock (MyLock) // Lock the counter { IncrementingCounterPayload payload = new IncrementingCounterPayload(); - payload.Name = _name; + payload.Name = Name; payload.DisplayName = DisplayName ?? ""; payload.DisplayRateTimeScale = (DisplayRateTimeScale == TimeSpan.Zero) ? "" : DisplayRateTimeScale.ToString("c"); payload.IntervalSec = intervalSec; - payload.MetaData = GetMetaDataString(); + payload.Metadata = GetMetadataString(); payload.Increment = _increment - _prevIncrement; _prevIncrement = _increment; - _eventSource.Write("EventCounters", new EventSourceOptions() { Level = EventLevel.LogAlways }, new IncrementingPollingCounterPayloadType(payload)); + EventSource.Write("EventCounters", new EventSourceOptions() { Level = EventLevel.LogAlways }, new IncrementingPollingCounterPayloadType(payload)); } } } diff --git a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/PollingCounter.cs b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/PollingCounter.cs index 695b357..e057718 100644 --- a/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/PollingCounter.cs +++ b/src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/PollingCounter.cs @@ -23,7 +23,7 @@ namespace System.Diagnostics.Tracing /// function to collect metrics on its own rather than the user having to call WriteMetric() /// every time. /// - internal partial class PollingCounter : BaseCounter + public partial class PollingCounter : DiagnosticCounter { /// /// Initializes a new instance of the class. @@ -32,41 +32,42 @@ namespace System.Diagnostics.Tracing /// /// The name. /// The event source. - public PollingCounter(string name, EventSource eventSource, Func getMetricFunction) : base(name, eventSource) + public PollingCounter(string name, EventSource eventSource, Func metricProvider) : base(name, eventSource) { - _getMetricFunction = getMetricFunction; + _metricProvider = metricProvider; } - public override string ToString() => $"PollingCounter '{_name}' Count {1} Mean {_lastVal.ToString("n3")}"; + public override string ToString() => $"PollingCounter '{Name}' Count {1} Mean {_lastVal.ToString("n3")}"; - private Func _getMetricFunction; - private float _lastVal; + private Func _metricProvider; + private double _lastVal; internal override void WritePayload(float intervalSec) { lock (MyLock) { - float value = 0; + double value = 0; try { - value = _getMetricFunction(); + value = _metricProvider(); } catch (Exception ex) { - ReportOutOfBandMessage($"ERROR: Exception during EventCounter {_name} getMetricFunction callback: " + ex.Message); + ReportOutOfBandMessage($"ERROR: Exception during EventCounter {Name} metricProvider callback: " + ex.Message); } CounterPayload payload = new CounterPayload(); - payload.Name = _name; + payload.Name = Name; payload.DisplayName = DisplayName ?? ""; payload.Count = 1; // NOTE: These dumb-looking statistics is intentional payload.IntervalSec = intervalSec; payload.Mean = value; payload.Max = value; payload.Min = value; + payload.Metadata = GetMetadataString(); payload.StandardDeviation = 0; _lastVal = value; - _eventSource.Write("EventCounters", new EventSourceOptions() { Level = EventLevel.LogAlways }, new PollingPayloadType(payload)); + EventSource.Write("EventCounters", new EventSourceOptions() { Level = EventLevel.LogAlways }, new PollingPayloadType(payload)); } } } diff --git a/tests/src/tracing/eventcounter/incrementingpollingcounter.cs b/tests/src/tracing/eventcounter/incrementingpollingcounter.cs index f215460..2b21ebb 100644 --- a/tests/src/tracing/eventcounter/incrementingpollingcounter.cs +++ b/tests/src/tracing/eventcounter/incrementingpollingcounter.cs @@ -21,7 +21,7 @@ namespace BasicEventSourceTests { private object _failureCounter; - public SimpleEventSource(Func getFailureCount, Type IncrementingPollingCounterType) + public SimpleEventSource(Func getFailureCount, Type IncrementingPollingCounterType) { _failureCounter = Activator.CreateInstance(IncrementingPollingCounterType, "failureCount", this, getFailureCount); } @@ -93,7 +93,7 @@ namespace BasicEventSourceTests public static int failureCountCalled = 0; - public static float getFailureCount() + public static double getFailureCount() { failureCountCalled++; return failureCountCalled; diff --git a/tests/src/tracing/eventcounter/pollingcounter.cs b/tests/src/tracing/eventcounter/pollingcounter.cs index ba2369d..9e8dff2 100644 --- a/tests/src/tracing/eventcounter/pollingcounter.cs +++ b/tests/src/tracing/eventcounter/pollingcounter.cs @@ -22,7 +22,7 @@ namespace BasicEventSourceTests private object _failureCounter; private object _successCounter; - public SimpleEventSource(Func getFailureCount, Func getSuccessCount, Type PollingCounterType) + public SimpleEventSource(Func getFailureCount, Func getSuccessCount, Type PollingCounterType) { _failureCounter = Activator.CreateInstance(PollingCounterType, "failureCount", this, getSuccessCount); _successCounter = Activator.CreateInstance(PollingCounterType, "successCount", this, getFailureCount); @@ -140,13 +140,13 @@ namespace BasicEventSourceTests public static int failureCountCalled = 0; public static int successCountCalled = 0; - public static float getFailureCount() + public static double getFailureCount() { failureCountCalled++; return failureCountCalled; } - public static float getSuccessCount() + public static double getSuccessCount() { successCountCalled++; return successCountCalled;