Publish Aggregated Values For Counter and UpDownCounter For System.Diagnostics.Metrics (#84846)

kkeirstead · web-flow · commit e210d9034285 · 2023-04-18T10:15:24.000-07:00
diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj b/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj
@@ -62,7 +62,7 @@ System.Diagnostics.DiagnosticSource</PackageDescription>
     <Compile Include="System\Diagnostics\Metrics\ObservableGauge.cs" />
     <Compile Include="System\Diagnostics\Metrics\ObservableInstrument.cs" />
     <Compile Include="System\Diagnostics\Metrics\ObservableUpDownCounter.cs" />
-    <Compile Include="System\Diagnostics\Metrics\RateAggregator.cs" />
+    <Compile Include="System\Diagnostics\Metrics\CounterAggregator.cs" />
     <Compile Include="System\Diagnostics\Metrics\StringSequence.cs" />
     <Compile Include="System\Diagnostics\Metrics\TagList.cs" />
     <Compile Include="System\Diagnostics\Metrics\UpDownCounter.cs" />
diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/AggregationManager.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/AggregationManager.cs
@@ -275,7 +275,7 @@ private void RemoveInstrumentState(Instrument instrument)
                 {
                     lock (this)
                     {
-                        return CheckTimeSeriesAllowed() ? new RateSumAggregator(isMonotonic: true) : null;
+                        return CheckTimeSeriesAllowed() ? new CounterAggregator(isMonotonic: true) : null;
                     }
                 };
             }
@@ -285,7 +285,7 @@ private void RemoveInstrumentState(Instrument instrument)
                 {
                     lock (this)
                     {
-                        return CheckTimeSeriesAllowed() ? new RateAggregator(isMonotonic: true) : null;
+                        return CheckTimeSeriesAllowed() ? new ObservableCounterAggregator(isMonotonic: true) : null;
                     }
                 };
             }
@@ -318,7 +318,7 @@ private void RemoveInstrumentState(Instrument instrument)
                 {
                     lock (this)
                     {
-                        return CheckTimeSeriesAllowed() ? new RateSumAggregator(isMonotonic: false) : null;
+                        return CheckTimeSeriesAllowed() ? new CounterAggregator(isMonotonic: false) : null;
                     }
                 };
             }
@@ -328,7 +328,7 @@ private void RemoveInstrumentState(Instrument instrument)
                 {
                     lock (this)
                     {
-                        return CheckTimeSeriesAllowed() ? new RateAggregator(isMonotonic: false) : null;
+                        return CheckTimeSeriesAllowed() ? new ObservableCounterAggregator(isMonotonic: false) : null;
                     }
                 };
             }
diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/CounterAggregator.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/CounterAggregator.cs
@@ -3,12 +3,13 @@
 
 namespace System.Diagnostics.Metrics
 {
-    internal sealed class RateSumAggregator : Aggregator
+    internal sealed class CounterAggregator : Aggregator
     {
         private readonly bool _isMonotonic;
-        private double _sum;
+        private double _delta;
+        private double _aggregatedValue;
 
-        public RateSumAggregator(bool isMonotonic)
+        public CounterAggregator(bool isMonotonic)
         {
             _isMonotonic = isMonotonic;
         }
@@ -17,28 +18,29 @@ public override void Update(double value)
         {
             lock (this)
             {
-                _sum += value;
+                _delta += value;
             }
         }
 
         public override IAggregationStatistics Collect()
         {
             lock (this)
             {
-                RateStatistics? stats = new RateStatistics(_sum, _isMonotonic);
-                _sum = 0;
+                _aggregatedValue += _delta;
+                CounterStatistics? stats = new CounterStatistics(_delta, _isMonotonic, _aggregatedValue);
+                _delta = 0;
                 return stats;
             }
         }
     }
 
-    internal sealed class RateAggregator : Aggregator
+    internal sealed class ObservableCounterAggregator : Aggregator
     {
         private readonly bool _isMonotonic;
         private double? _prevValue;
-        private double _value;
+        private double _currValue;
 
-        public RateAggregator(bool isMonotonic)
+        public ObservableCounterAggregator(bool isMonotonic)
         {
             _isMonotonic = isMonotonic;
         }
@@ -47,7 +49,7 @@ public override void Update(double value)
         {
             lock (this)
             {
-                _value = value;
+                _currValue = value;
             }
         }
 
@@ -58,25 +60,29 @@ public override IAggregationStatistics Collect()
                 double? delta = null;
                 if (_prevValue.HasValue)
                 {
-                    delta = _value - _prevValue.Value;
+                    delta = _currValue - _prevValue.Value;
                 }
-                RateStatistics stats = new RateStatistics(delta, _isMonotonic);
-                _prevValue = _value;
+
+                CounterStatistics stats = new CounterStatistics(delta, _isMonotonic, _currValue);
+                _prevValue = _currValue;
                 return stats;
             }
         }
     }
 
-    internal sealed class RateStatistics : IAggregationStatistics
+    internal sealed class CounterStatistics : IAggregationStatistics
     {
-        public RateStatistics(double? delta, bool isMonotonic)
+        public CounterStatistics(double? delta, bool isMonotonic, double value)
         {
             Delta = delta;
             IsMonotonic = isMonotonic;
+            Value = value;
         }
 
         public double? Delta { get; }
 
         public bool IsMonotonic { get; }
+
+        public double Value { get; }
     }
 }
diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/MetricsEventSource.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/MetricsEventSource.cs
@@ -102,12 +102,12 @@ public void CollectionStop(string sessionId, DateTime intervalStartTime, DateTim
             WriteEvent(3, sessionId, intervalStartTime, intervalEndTime);
         }
 
-        [Event(4, Keywords = Keywords.TimeSeriesValues)]
+        [Event(4, Keywords = Keywords.TimeSeriesValues, Version=1)]
         [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
                             Justification = "This calls WriteEvent with all primitive arguments which is safe. Primitives are always serialized properly.")]
-        public void CounterRateValuePublished(string sessionId, string meterName, string? meterVersion, string instrumentName, string? unit, string tags, string rate)
+        public void CounterRateValuePublished(string sessionId, string meterName, string? meterVersion, string instrumentName, string? unit, string tags, string rate, string value)
         {
-            WriteEvent(4, sessionId, meterName, meterVersion ?? "", instrumentName, unit ?? "", tags, rate);
+            WriteEvent(4, sessionId, meterName, meterVersion ?? "", instrumentName, unit ?? "", tags, rate, value);
         }
 
         [Event(5, Keywords = Keywords.TimeSeriesValues)]
@@ -191,12 +191,12 @@ public void MultipleSessionsNotSupportedError(string runningSessionId)
             WriteEvent(15, runningSessionId);
         }
 
-        [Event(16, Keywords = Keywords.TimeSeriesValues)]
+        [Event(16, Keywords = Keywords.TimeSeriesValues, Version=1)]
         [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
                             Justification = "This calls WriteEvent with all primitive arguments which is safe. Primitives are always serialized properly.")]
-        public void UpDownCounterRateValuePublished(string sessionId, string meterName, string? meterVersion, string instrumentName, string? unit, string tags, string rate)
+        public void UpDownCounterRateValuePublished(string sessionId, string meterName, string? meterVersion, string instrumentName, string? unit, string tags, string rate, string value)
         {
-            WriteEvent(16, sessionId, meterName, meterVersion ?? "", instrumentName, unit ?? "", tags, rate);
+            WriteEvent(16, sessionId, meterName, meterVersion ?? "", instrumentName, unit ?? "", tags, rate, value);
         }
 
         /// <summary>
@@ -413,17 +413,19 @@ private void ParseSpecs(string? metricsSpecs)
 
             private static void TransmitMetricValue(Instrument instrument, LabeledAggregationStatistics stats, string sessionId)
             {
-                if (stats.AggregationStatistics is RateStatistics rateStats)
+                if (stats.AggregationStatistics is CounterStatistics rateStats)
                 {
                     if (rateStats.IsMonotonic)
                     {
                         Log.CounterRateValuePublished(sessionId, instrument.Meter.Name, instrument.Meter.Version, instrument.Name, instrument.Unit, FormatTags(stats.Labels),
-                            rateStats.Delta.HasValue ? rateStats.Delta.Value.ToString(CultureInfo.InvariantCulture) : "");
+                            rateStats.Delta.HasValue ? rateStats.Delta.Value.ToString(CultureInfo.InvariantCulture) : "",
+                            rateStats.Value.ToString(CultureInfo.InvariantCulture));
                     }
                     else
                     {
                         Log.UpDownCounterRateValuePublished(sessionId, instrument.Meter.Name, instrument.Meter.Version, instrument.Name, instrument.Unit, FormatTags(stats.Labels),
-                            rateStats.Delta.HasValue ? rateStats.Delta.Value.ToString(CultureInfo.InvariantCulture) : "");
+                            rateStats.Delta.HasValue ? rateStats.Delta.Value.ToString(CultureInfo.InvariantCulture) : "",
+                            rateStats.Value.ToString(CultureInfo.InvariantCulture));
                     }
                 }
                 else if (stats.AggregationStatistics is LastValueStatistics lastValueStats)
diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricEventSourceTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricEventSourceTests.cs

Original file line number	Diff line number	Diff line change
`@@ -275,7 +275,7 @@ private void RemoveInstrumentState(Instrument instrument)`
`275`	`275`	`{`
`276`	`276`	`lock (this)`
`277`	`277`	`{`
`278`		`- return CheckTimeSeriesAllowed() ? new RateSumAggregator(isMonotonic: true) : null;`
	`278`	`+ return CheckTimeSeriesAllowed() ? new CounterAggregator(isMonotonic: true) : null;`
`279`	`279`	`}`
`280`	`280`	`};`
`281`	`281`	`}`
`@@ -285,7 +285,7 @@ private void RemoveInstrumentState(Instrument instrument)`
`285`	`285`	`{`
`286`	`286`	`lock (this)`
`287`	`287`	`{`
`288`		`- return CheckTimeSeriesAllowed() ? new RateAggregator(isMonotonic: true) : null;`
	`288`	`+ return CheckTimeSeriesAllowed() ? new ObservableCounterAggregator(isMonotonic: true) : null;`
`289`	`289`	`}`
`290`	`290`	`};`
`291`	`291`	`}`
`@@ -318,7 +318,7 @@ private void RemoveInstrumentState(Instrument instrument)`
`318`	`318`	`{`
`319`	`319`	`lock (this)`
`320`	`320`	`{`
`321`		`- return CheckTimeSeriesAllowed() ? new RateSumAggregator(isMonotonic: false) : null;`
	`321`	`+ return CheckTimeSeriesAllowed() ? new CounterAggregator(isMonotonic: false) : null;`
`322`	`322`	`}`
`323`	`323`	`};`
`324`	`324`	`}`
`@@ -328,7 +328,7 @@ private void RemoveInstrumentState(Instrument instrument)`
`328`	`328`	`{`
`329`	`329`	`lock (this)`
`330`	`330`	`{`
`331`		`- return CheckTimeSeriesAllowed() ? new RateAggregator(isMonotonic: false) : null;`
	`331`	`+ return CheckTimeSeriesAllowed() ? new ObservableCounterAggregator(isMonotonic: false) : null;`
`332`	`332`	`}`
`333`	`333`	`};`
`334`	`334`	`}`
Original file line number	Diff line number	Diff line change
`@@ -3,12 +3,13 @@`
`3`	`3`
`4`	`4`	`namespace System.Diagnostics.Metrics`
`5`	`5`	`{`
`6`		`- internal sealed class RateSumAggregator : Aggregator`
	`6`	`+ internal sealed class CounterAggregator : Aggregator`
`7`	`7`	`{`
`8`	`8`	`private readonly bool _isMonotonic;`
`9`		`- private double _sum;`
	`9`	`+ private double _delta;`
	`10`	`+ private double _aggregatedValue;`
`10`	`11`
`11`		`- public RateSumAggregator(bool isMonotonic)`
	`12`	`+ public CounterAggregator(bool isMonotonic)`
`12`	`13`	`{`
`13`	`14`	`_isMonotonic = isMonotonic;`
`14`	`15`	`}`
`@@ -17,28 +18,29 @@ public override void Update(double value)`
`17`	`18`	`{`
`18`	`19`	`lock (this)`
`19`	`20`	`{`
`20`		`- _sum += value;`
	`21`	`+ _delta += value;`
`21`	`22`	`}`
`22`	`23`	`}`
`23`	`24`
`24`	`25`	`public override IAggregationStatistics Collect()`
`25`	`26`	`{`
`26`	`27`	`lock (this)`
`27`	`28`	`{`
`28`		`- RateStatistics? stats = new RateStatistics(_sum, _isMonotonic);`
`29`		`- _sum = 0;`
	`29`	`+ _aggregatedValue += _delta;`
	`30`	`+ CounterStatistics? stats = new CounterStatistics(_delta, _isMonotonic, _aggregatedValue);`
	`31`	`+ _delta = 0;`
`30`	`32`	`return stats;`
`31`	`33`	`}`
`32`	`34`	`}`
`33`	`35`	`}`
`34`	`36`
`35`		`- internal sealed class RateAggregator : Aggregator`
	`37`	`+ internal sealed class ObservableCounterAggregator : Aggregator`
`36`	`38`	`{`
`37`	`39`	`private readonly bool _isMonotonic;`
`38`	`40`	`private double? _prevValue;`
`39`		`- private double _value;`
	`41`	`+ private double _currValue;`
`40`	`42`
`41`		`- public RateAggregator(bool isMonotonic)`
	`43`	`+ public ObservableCounterAggregator(bool isMonotonic)`
`42`	`44`	`{`
`43`	`45`	`_isMonotonic = isMonotonic;`
`44`	`46`	`}`
`@@ -47,7 +49,7 @@ public override void Update(double value)`
`47`	`49`	`{`
`48`	`50`	`lock (this)`
`49`	`51`	`{`
`50`		`- _value = value;`
	`52`	`+ _currValue = value;`
`51`	`53`	`}`
`52`	`54`	`}`
`53`	`55`
`@@ -58,25 +60,29 @@ public override IAggregationStatistics Collect()`
`58`	`60`	`double? delta = null;`
`59`	`61`	`if (_prevValue.HasValue)`
`60`	`62`	`{`
`61`		`- delta = _value - _prevValue.Value;`
	`63`	`+ delta = _currValue - _prevValue.Value;`
`62`	`64`	`}`
`63`		`- RateStatistics stats = new RateStatistics(delta, _isMonotonic);`
`64`		`- _prevValue = _value;`
	`65`	`+`
	`66`	`+ CounterStatistics stats = new CounterStatistics(delta, _isMonotonic, _currValue);`
	`67`	`+ _prevValue = _currValue;`
`65`	`68`	`return stats;`
`66`	`69`	`}`
`67`	`70`	`}`
`68`	`71`	`}`
`69`	`72`
`70`		`- internal sealed class RateStatistics : IAggregationStatistics`
	`73`	`+ internal sealed class CounterStatistics : IAggregationStatistics`
`71`	`74`	`{`
`72`		`- public RateStatistics(double? delta, bool isMonotonic)`
	`75`	`+ public CounterStatistics(double? delta, bool isMonotonic, double value)`
`73`	`76`	`{`
`74`	`77`	`Delta = delta;`
`75`	`78`	`IsMonotonic = isMonotonic;`
	`79`	`+ Value = value;`
`76`	`80`	`}`
`77`	`81`
`78`	`82`	`public double? Delta { get; }`
`79`	`83`
`80`	`84`	`public bool IsMonotonic { get; }`
	`85`	`+`
	`86`	`+ public double Value { get; }`
`81`	`87`	`}`
`82`	`88`	`}`
Original file line number	Diff line number	Diff line change
`@@ -102,12 +102,12 @@ public void CollectionStop(string sessionId, DateTime intervalStartTime, DateTim`
`102`	`102`	`WriteEvent(3, sessionId, intervalStartTime, intervalEndTime);`
`103`	`103`	`}`
`104`	`104`
`105`		`- [Event(4, Keywords = Keywords.TimeSeriesValues)]`
	`105`	`+ [Event(4, Keywords = Keywords.TimeSeriesValues, Version=1)]`
`106`	`106`	`[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",`
`107`	`107`	`Justification = "This calls WriteEvent with all primitive arguments which is safe. Primitives are always serialized properly.")]`
`108`		`- public void CounterRateValuePublished(string sessionId, string meterName, string? meterVersion, string instrumentName, string? unit, string tags, string rate)`
	`108`	`+ public void CounterRateValuePublished(string sessionId, string meterName, string? meterVersion, string instrumentName, string? unit, string tags, string rate, string value)`
`109`	`109`	`{`
`110`		`- WriteEvent(4, sessionId, meterName, meterVersion ?? "", instrumentName, unit ?? "", tags, rate);`
	`110`	`+ WriteEvent(4, sessionId, meterName, meterVersion ?? "", instrumentName, unit ?? "", tags, rate, value);`
`111`	`111`	`}`
`112`	`112`
`113`	`113`	`[Event(5, Keywords = Keywords.TimeSeriesValues)]`
`@@ -191,12 +191,12 @@ public void MultipleSessionsNotSupportedError(string runningSessionId)`
`191`	`191`	`WriteEvent(15, runningSessionId);`
`192`	`192`	`}`
`193`	`193`
`194`		`- [Event(16, Keywords = Keywords.TimeSeriesValues)]`
	`194`	`+ [Event(16, Keywords = Keywords.TimeSeriesValues, Version=1)]`
`195`	`195`	`[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",`
`196`	`196`	`Justification = "This calls WriteEvent with all primitive arguments which is safe. Primitives are always serialized properly.")]`
`197`		`- public void UpDownCounterRateValuePublished(string sessionId, string meterName, string? meterVersion, string instrumentName, string? unit, string tags, string rate)`
	`197`	`+ public void UpDownCounterRateValuePublished(string sessionId, string meterName, string? meterVersion, string instrumentName, string? unit, string tags, string rate, string value)`
`198`	`198`	`{`
`199`		`- WriteEvent(16, sessionId, meterName, meterVersion ?? "", instrumentName, unit ?? "", tags, rate);`
	`199`	`+ WriteEvent(16, sessionId, meterName, meterVersion ?? "", instrumentName, unit ?? "", tags, rate, value);`
`200`	`200`	`}`
`201`	`201`
`202`	`202`	`/// <summary>`
`@@ -413,17 +413,19 @@ private void ParseSpecs(string? metricsSpecs)`
`413`	`413`
`414`	`414`	`private static void TransmitMetricValue(Instrument instrument, LabeledAggregationStatistics stats, string sessionId)`
`415`	`415`	`{`
`416`		`- if (stats.AggregationStatistics is RateStatistics rateStats)`
	`416`	`+ if (stats.AggregationStatistics is CounterStatistics rateStats)`
`417`	`417`	`{`
`418`	`418`	`if (rateStats.IsMonotonic)`
`419`	`419`	`{`
`420`	`420`	`Log.CounterRateValuePublished(sessionId, instrument.Meter.Name, instrument.Meter.Version, instrument.Name, instrument.Unit, FormatTags(stats.Labels),`
`421`		`- rateStats.Delta.HasValue ? rateStats.Delta.Value.ToString(CultureInfo.InvariantCulture) : "");`
	`421`	`+ rateStats.Delta.HasValue ? rateStats.Delta.Value.ToString(CultureInfo.InvariantCulture) : "",`
	`422`	`+ rateStats.Value.ToString(CultureInfo.InvariantCulture));`
`422`	`423`	`}`
`423`	`424`	`else`
`424`	`425`	`{`
`425`	`426`	`Log.UpDownCounterRateValuePublished(sessionId, instrument.Meter.Name, instrument.Meter.Version, instrument.Name, instrument.Unit, FormatTags(stats.Labels),`
`426`		`- rateStats.Delta.HasValue ? rateStats.Delta.Value.ToString(CultureInfo.InvariantCulture) : "");`
	`427`	`+ rateStats.Delta.HasValue ? rateStats.Delta.Value.ToString(CultureInfo.InvariantCulture) : "",`
	`428`	`+ rateStats.Value.ToString(CultureInfo.InvariantCulture));`
`427`	`429`	`}`
`428`	`430`	`}`
`429`	`431`	`else if (stats.AggregationStatistics is LastValueStatistics lastValueStats)`