From bed3ff7d9d5d9e2c55f3c298706a39ea7fc4500c Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 28 Mar 2024 13:10:47 -0700 Subject: [PATCH 01/11] Prototype HistogramAdvice API. --- ...em.Diagnostics.DiagnosticSourceActivity.cs | 8 +++++++- ...System.Diagnostics.DiagnosticSource.csproj | 1 + .../System/Diagnostics/Metrics/Histogram.cs | 8 ++++++-- .../Diagnostics/Metrics/HistogramAdvice.cs | 12 ++++++++++++ .../src/System/Diagnostics/Metrics/Meter.cs | 19 +++++++++++++++++-- 5 files changed, 43 insertions(+), 5 deletions(-) create mode 100644 src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/HistogramAdvice.cs diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs b/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs index 7cb90d245dcf93..906cfdb596bcdf 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs @@ -356,6 +356,7 @@ internal UpDownCounter(Meter meter, string name, string? unit, string? descripti } public sealed class Histogram : Instrument where T : struct { + public HistogramAdvice? Advice { get { throw null; } } internal Histogram(Meter meter, string name, string? unit, string? description) : base(meter, name, unit, description) { throw null; } public void Record(T value) { throw null; } public void Record(T value, System.Collections.Generic.KeyValuePair tag) { throw null; } @@ -410,7 +411,8 @@ public class Meter : IDisposable public UpDownCounter CreateUpDownCounter(string name, string? unit = null, string? description = null) where T : struct { throw null; } public UpDownCounter CreateUpDownCounter(string name, string? unit, string? description, System.Collections.Generic.IEnumerable> tags) where T : struct { throw null; } public Histogram CreateHistogram(string name, string? unit = null, string? description = null) where T : struct { throw null; } - public Histogram CreateHistogram(string name, string? unit, string? description, System.Collections.Generic.IEnumerable> tags) where T : struct { throw null; } + public Histogram CreateHistogram(string name, string? unit, string? description, System.Collections.Generic.IEnumerable>? tags) where T : struct { throw null; } + public Histogram CreateHistogram(string name, string? unit, string? description, System.Collections.Generic.IEnumerable>? tags, HistogramAdvice? advice) where T : struct { throw null; } public ObservableCounter CreateObservableCounter( string name, Func observeValue, @@ -567,4 +569,8 @@ public abstract class ObservableInstrument : Instrument where T : struct protected ObservableInstrument(Meter meter, string name, string? unit, string? description, System.Collections.Generic.IEnumerable> tags) : base(meter, name, unit, description) { throw null; } protected abstract System.Collections.Generic.IEnumerable> Observe(); } + public sealed class HistogramAdvice where T : struct + { + public System.Collections.Generic.IReadOnlyList? ExplicitBucketBoundaries { get { throw null; } init { throw null; } } + } } diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj b/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj index fb55fdbebd23b4..76a6685c3e5b09 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj @@ -54,6 +54,7 @@ System.Diagnostics.DiagnosticSource + diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Histogram.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Histogram.cs index ed3431edfce8be..656fd55b948456 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Histogram.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Histogram.cs @@ -15,12 +15,16 @@ namespace System.Diagnostics.Metrics /// public sealed class Histogram : Instrument where T : struct { - internal Histogram(Meter meter, string name, string? unit, string? description) : this(meter, name, unit, description, tags: null) + public HistogramAdvice? Advice { get; } + + internal Histogram(Meter meter, string name, string? unit, string? description) : this(meter, name, unit, description, tags: null, advice: null) { } - internal Histogram(Meter meter, string name, string? unit, string? description, IEnumerable>? tags) : base(meter, name, unit, description, tags) + internal Histogram(Meter meter, string name, string? unit, string? description, IEnumerable>? tags, HistogramAdvice? advice) : base(meter, name, unit, description, tags) { + Advice = advice; + Publish(); } diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/HistogramAdvice.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/HistogramAdvice.cs new file mode 100644 index 00000000000000..ab122ceb2b2c38 --- /dev/null +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/HistogramAdvice.cs @@ -0,0 +1,12 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; + +namespace System.Diagnostics.Metrics +{ + public sealed class HistogramAdvice where T : struct + { + public IReadOnlyList? ExplicitBucketBoundaries { get; init; } + } +} diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Meter.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Meter.cs index 856725680857c0..fbac05a3f0cd44 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Meter.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Meter.cs @@ -152,7 +152,8 @@ public Counter CreateCounter(string name, string? unit, string? descriptio /// /// Example uses for Histogram: the request duration and the size of the response payload. /// - public Histogram CreateHistogram(string name, string? unit = null, string? description = null) where T : struct => CreateHistogram(name, unit, description, tags: null); + public Histogram CreateHistogram(string name, string? unit = null, string? description = null) where T : struct + => CreateHistogram(name, unit, description, tags: null, advice: null); /// /// Histogram is an Instrument which can be used to report arbitrary values that are likely to be statistically meaningful. It is intended for statistics such as histograms, summaries, and percentile. @@ -165,7 +166,21 @@ public Counter CreateCounter(string name, string? unit, string? descriptio /// Example uses for Histogram: the request duration and the size of the response payload. /// public Histogram CreateHistogram(string name, string? unit, string? description, IEnumerable>? tags) where T : struct - => (Histogram)GetOrCreateInstrument(typeof(Histogram), name, unit, description, tags, () => new Histogram(this, name, unit, description, tags)); + => CreateHistogram(name, unit, description, tags, advice: null); + + /// + /// Histogram is an Instrument which can be used to report arbitrary values that are likely to be statistically meaningful. It is intended for statistics such as histograms, summaries, and percentile. + /// + /// The instrument name. cannot be null. + /// Optional instrument unit of measurements. + /// Optional instrument description. + /// tags to attach to the counter. + /// to attach to the counter. + /// + /// Example uses for Histogram: the request duration and the size of the response payload. + /// + public Histogram CreateHistogram(string name, string? unit, string? description, IEnumerable>? tags, HistogramAdvice? advice) where T : struct + => (Histogram)GetOrCreateInstrument(typeof(Histogram), name, unit, description, tags, () => new Histogram(this, name, unit, description, tags, advice)); /// /// Create a metrics UpDownCounter object. From 0aa4c7221d14d54da930d0e7a8301f2c4332b283 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 21 May 2024 13:39:48 -0700 Subject: [PATCH 02/11] Updates. --- ...em.Diagnostics.DiagnosticSourceActivity.cs | 10 ++- .../src/Resources/Strings.resx | 59 +++++++------- .../System/Diagnostics/Metrics/Histogram.cs | 3 + .../Diagnostics/Metrics/HistogramAdvice.cs | 53 ++++++++++++- .../src/System/Diagnostics/Metrics/Meter.cs | 79 +++++++++++-------- 5 files changed, 139 insertions(+), 65 deletions(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs b/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs index 635bceb910cd74..b801b5751b3202 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs @@ -411,9 +411,12 @@ public class Meter : IDisposable public Counter CreateCounter(string name, string? unit, string? description, System.Collections.Generic.IEnumerable> tags) where T : struct { throw null; } public UpDownCounter CreateUpDownCounter(string name, string? unit = null, string? description = null) where T : struct { throw null; } public UpDownCounter CreateUpDownCounter(string name, string? unit, string? description, System.Collections.Generic.IEnumerable> tags) where T : struct { throw null; } - public Histogram CreateHistogram(string name, string? unit = null, string? description = null) where T : struct { throw null; } + public Histogram CreateHistogram(string name) where T : struct { throw null; } + [System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)] + public Histogram CreateHistogram(string name, string? unit, string? description) where T : struct { throw null; } + [System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)] public Histogram CreateHistogram(string name, string? unit, string? description, System.Collections.Generic.IEnumerable>? tags) where T : struct { throw null; } - public Histogram CreateHistogram(string name, string? unit, string? description, System.Collections.Generic.IEnumerable>? tags, HistogramAdvice? advice) where T : struct { throw null; } + public Histogram CreateHistogram(string name, string? unit = default, string? description = default, System.Collections.Generic.IEnumerable>? tags = default, HistogramAdvice? advice = default) where T : struct { throw null; } public ObservableCounter CreateObservableCounter( string name, Func observeValue, @@ -572,6 +575,7 @@ public abstract class ObservableInstrument : Instrument where T : struct } public sealed class HistogramAdvice where T : struct { - public System.Collections.Generic.IReadOnlyList? ExplicitBucketBoundaries { get { throw null; } init { throw null; } } + public HistogramAdvice(System.Collections.Generic.IEnumerable explicitBucketBoundaries) { throw null; } + public System.Collections.Generic.IReadOnlyList? ExplicitBucketBoundaries { get { throw null; } } } } diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/Resources/Strings.resx b/src/libraries/System.Diagnostics.DiagnosticSource/src/Resources/Strings.resx index 6ad3bfd0b2a118..f1493d25fae37f 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/Resources/Strings.resx +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/Resources/Strings.resx @@ -1,17 +1,17 @@  - @@ -174,4 +174,7 @@ The instrument is of different generic type. - \ No newline at end of file + + Histogram explicit bucket boundaries MUST be specified in ascending order and CANNOT contain duplicate values. + + diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Histogram.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Histogram.cs index 87e3eef0414c64..7e3999695b0bad 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Histogram.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Histogram.cs @@ -15,6 +15,9 @@ namespace System.Diagnostics.Metrics /// public sealed class Histogram : Instrument where T : struct { + /// + /// Gets the associated with the instrument. + /// public HistogramAdvice? Advice { get; } internal Histogram(Meter meter, string name, string? unit, string? description) : this(meter, name, unit, description, tags: null, advice: null) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/HistogramAdvice.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/HistogramAdvice.cs index ab122ceb2b2c38..f40d2fb5cd0a0d 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/HistogramAdvice.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/HistogramAdvice.cs @@ -5,8 +5,59 @@ namespace System.Diagnostics.Metrics { + /// + /// Contains settings used to advise metrics consumers how to construct storage for instruments. + /// + /// Histogram value type. public sealed class HistogramAdvice where T : struct { - public IReadOnlyList? ExplicitBucketBoundaries { get; init; } + /// + /// Constructs a new instance of . + /// + /// + /// Explicit bucket boundaries advised to be used with the histogram. + /// Notes: + /// + /// Bucket boundaries MUST be specified in ascending order and CANNOT contain duplicate values. + /// An empty set of bucket boundaries hints that the histogram should NOT contain buckets and should only track count and sum values. + /// + /// + /// + public HistogramAdvice(IEnumerable explicitBucketBoundaries) + { + if (explicitBucketBoundaries is null) + { + throw new ArgumentNullException(nameof(explicitBucketBoundaries)); + } + + List explicitBucketBoundariesCopy = [.. explicitBucketBoundaries]; + + if (!IsSortedAndDistinct(explicitBucketBoundariesCopy)) + { + throw new ArgumentException(SR.InvalidHistogramExplicitBucketBoundaries, nameof(explicitBucketBoundaries)); + } + + ExplicitBucketBoundaries = explicitBucketBoundariesCopy; + } + + /// + /// Gets the explicit bucket boundaries advised to be used with the histogram. + /// + public IReadOnlyList? ExplicitBucketBoundaries { get; } + + private static bool IsSortedAndDistinct(List values) + { + Comparer comparer = Comparer.Default; + + for (int i = 1; i < values.Count; i++) + { + if (comparer.Compare(values[i - 1], values[i]) >= 0) + { + return false; + } + } + + return true; + } } } diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Meter.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Meter.cs index fbac05a3f0cd44..4ca5f0fe8daa54 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Meter.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Meter.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.ComponentModel; using System.Diagnostics; using System.Threading; @@ -120,7 +121,7 @@ private void Initialize(string name, string? version, IEnumerable /// Create a metrics Counter object. /// - /// The instrument name. cannot be null. + /// The instrument name. Cannot be null. /// Optional instrument unit of measurements. /// Optional instrument description. /// @@ -132,10 +133,10 @@ private void Initialize(string name, string? version, IEnumerable /// Create a metrics Counter object. /// - /// The instrument name. cannot be null. + /// The instrument name. Cannot be null. /// Optional instrument unit of measurements. /// Optional instrument description. - /// tags to attach to the counter. + /// Optional tags to attach to the counter. /// /// Counter is an Instrument which supports non-negative increments. /// Example uses for Counter: count the number of bytes received, count the number of requests completed, count the number of accounts created, count the number of checkpoints run, and count the number of HTTP 5xx errors. @@ -146,40 +147,52 @@ public Counter CreateCounter(string name, string? unit, string? descriptio /// /// Histogram is an Instrument which can be used to report arbitrary values that are likely to be statistically meaningful. It is intended for statistics such as histograms, summaries, and percentile. /// - /// The instrument name. cannot be null. + /// The instrument name. Cannot be null. + /// + /// Example uses for Histogram: the request duration and the size of the response payload. + /// + public Histogram CreateHistogram(string name) where T : struct + => CreateHistogram(name, unit: null, description: null, tags: null, advice: null); + + /// + /// Histogram is an Instrument which can be used to report arbitrary values that are likely to be statistically meaningful. It is intended for statistics such as histograms, summaries, and percentile. + /// + /// The instrument name. Cannot be null. /// Optional instrument unit of measurements. /// Optional instrument description. /// /// Example uses for Histogram: the request duration and the size of the response payload. /// - public Histogram CreateHistogram(string name, string? unit = null, string? description = null) where T : struct + [EditorBrowsable(EditorBrowsableState.Never)] + public Histogram CreateHistogram(string name, string? unit, string? description) where T : struct => CreateHistogram(name, unit, description, tags: null, advice: null); /// /// Histogram is an Instrument which can be used to report arbitrary values that are likely to be statistically meaningful. It is intended for statistics such as histograms, summaries, and percentile. /// - /// The instrument name. cannot be null. + /// The instrument name. Cannot be null. /// Optional instrument unit of measurements. /// Optional instrument description. - /// tags to attach to the counter. + /// Optional tags to attach to the histogram. /// /// Example uses for Histogram: the request duration and the size of the response payload. /// + [EditorBrowsable(EditorBrowsableState.Never)] public Histogram CreateHistogram(string name, string? unit, string? description, IEnumerable>? tags) where T : struct => CreateHistogram(name, unit, description, tags, advice: null); /// /// Histogram is an Instrument which can be used to report arbitrary values that are likely to be statistically meaningful. It is intended for statistics such as histograms, summaries, and percentile. /// - /// The instrument name. cannot be null. + /// The instrument name. Cannot be null. /// Optional instrument unit of measurements. /// Optional instrument description. - /// tags to attach to the counter. - /// to attach to the counter. + /// Optional tags to attach to the histogram. + /// Optional to attach to the histogram. /// /// Example uses for Histogram: the request duration and the size of the response payload. /// - public Histogram CreateHistogram(string name, string? unit, string? description, IEnumerable>? tags, HistogramAdvice? advice) where T : struct + public Histogram CreateHistogram(string name, string? unit = default, string? description = default, IEnumerable>? tags = default, HistogramAdvice? advice = default) where T : struct => (Histogram)GetOrCreateInstrument(typeof(Histogram), name, unit, description, tags, () => new Histogram(this, name, unit, description, tags, advice)); /// @@ -201,7 +214,7 @@ public UpDownCounter CreateUpDownCounter(string name, string? unit = null, /// The instrument name. Cannot be null. /// Optional instrument unit of measurements. /// Optional instrument description. - /// tags to attach to the counter. + /// Optional tags to attach to the counter. /// /// UpDownCounter is an Instrument which supports reporting positive or negative metric values. /// Example uses for UpDownCounter: reporting the change in active requests or queue size. @@ -229,7 +242,7 @@ public ObservableUpDownCounter CreateObservableUpDownCounter(string name, /// The callback to call to get the measurements when the is called by . /// Optional instrument unit of measurements. /// Optional instrument description. - /// tags to attach to the counter. + /// Optional tags to attach to the counter. /// /// Example uses for ObservableUpDownCounter: the process heap size or the approximate number of items in a lock-free circular buffer. /// @@ -257,7 +270,7 @@ public ObservableUpDownCounter CreateObservableUpDownCounter(string name, /// The callback to call to get the measurements when the is called by /// Optional instrument unit of measurements. /// Optional instrument description. - /// tags to attach to the counter. + /// Optional tags to attach to the counter. /// /// Example uses for ObservableUpDownCounter: the process heap size or the approximate number of items in a lock-free circular buffer. /// @@ -284,7 +297,7 @@ public ObservableUpDownCounter CreateObservableUpDownCounter(string name, /// The callback to call to get the measurements when the is called by . /// Optional instrument unit of measurements. /// Optional instrument description. - /// tags to attach to the counter. + /// Optional tags to attach to the counter. /// /// Example uses for ObservableUpDownCounter: the process heap size or the approximate number of items in a lock-free circular buffer. /// @@ -294,7 +307,7 @@ public ObservableUpDownCounter CreateObservableUpDownCounter(string name, /// /// ObservableCounter is an Instrument which reports monotonically increasing value(s) when the instrument is being observed. /// - /// The instrument name. cannot be null. + /// The instrument name. Cannot be null. /// The callback to call to get the measurements when the is called by . /// Optional instrument unit of measurements. /// Optional instrument description. @@ -307,11 +320,11 @@ public ObservableCounter CreateObservableCounter(string name, Func obse /// /// ObservableCounter is an Instrument which reports monotonically increasing value(s) when the instrument is being observed. /// - /// The instrument name. cannot be null. + /// The instrument name. Cannot be null. /// The callback to call to get the measurements when the is called by . /// Optional instrument unit of measurements. /// Optional instrument description. - /// tags to attach to the counter. + /// Optional tags to attach to the counter. /// /// Example uses for ObservableCounter: The number of page faults for each process. /// @@ -321,7 +334,7 @@ public ObservableCounter CreateObservableCounter(string name, Func obse /// /// ObservableCounter is an Instrument which reports monotonically increasing value(s) when the instrument is being observed. /// - /// The instrument name. cannot be null. + /// The instrument name. Cannot be null. /// The callback to call to get the measurements when the is called by /// Optional instrument unit of measurements. /// Optional instrument description. @@ -334,11 +347,11 @@ public ObservableCounter CreateObservableCounter(string name, Func /// ObservableCounter is an Instrument which reports monotonically increasing value(s) when the instrument is being observed. /// - /// The instrument name. cannot be null. + /// The instrument name. Cannot be null. /// The callback to call to get the measurements when the is called by /// Optional instrument unit of measurements. /// Optional instrument description. - /// tags to attach to the counter. + /// Optional tags to attach to the counter. /// /// Example uses for ObservableCounter: The number of page faults for each process. /// @@ -349,7 +362,7 @@ public ObservableCounter CreateObservableCounter(string name, Func /// ObservableCounter is an Instrument which reports monotonically increasing value(s) when the instrument is being observed. /// - /// The instrument name. cannot be null. + /// The instrument name. Cannot be null. /// The callback to call to get the measurements when the is called by . /// Optional instrument unit of measurements. /// Optional instrument description. @@ -362,11 +375,11 @@ public ObservableCounter CreateObservableCounter(string name, Func /// ObservableCounter is an Instrument which reports monotonically increasing value(s) when the instrument is being observed. /// - /// The instrument name. cannot be null. + /// The instrument name. Cannot be null. /// The callback to call to get the measurements when the is called by . /// Optional instrument unit of measurements. /// Optional instrument description. - /// tags to attach to the counter. + /// Optional tags to attach to the counter. /// /// Example uses for ObservableCounter: The number of page faults for each process. /// @@ -376,7 +389,7 @@ public ObservableCounter CreateObservableCounter(string name, Func /// ObservableGauge is an asynchronous Instrument which reports non-additive value(s) (e.g. the room temperature - it makes no sense to report the temperature value from multiple rooms and sum them up) when the instrument is being observed. /// - /// The instrument name. cannot be null. + /// The instrument name. Cannot be null. /// The callback to call to get the measurements when the is called by . /// Optional instrument unit of measurements. /// Optional instrument description. @@ -386,18 +399,18 @@ public ObservableGauge CreateObservableGauge(string name, Func observeV /// /// ObservableGauge is an asynchronous Instrument which reports non-additive value(s) (e.g. the room temperature - it makes no sense to report the temperature value from multiple rooms and sum them up) when the instrument is being observed. /// - /// The instrument name. cannot be null. + /// The instrument name. Cannot be null. /// The callback to call to get the measurements when the is called by . /// Optional instrument unit of measurements. /// Optional instrument description. - /// tags to attach to the counter. + /// Optional tags to attach to the gauge. public ObservableGauge CreateObservableGauge(string name, Func observeValue, string? unit, string? description, IEnumerable>? tags) where T : struct => new ObservableGauge(this, name, observeValue, unit, description, tags); /// /// ObservableGauge is an asynchronous Instrument which reports non-additive value(s) (e.g. the room temperature - it makes no sense to report the temperature value from multiple rooms and sum them up) when the instrument is being observed. /// - /// The instrument name. cannot be null. + /// The instrument name. Cannot be null. /// The callback to call to get the measurements when the is called by . /// Optional instrument unit of measurements. /// Optional instrument description. @@ -407,18 +420,18 @@ public ObservableGauge CreateObservableGauge(string name, Func /// ObservableGauge is an asynchronous Instrument which reports non-additive value(s) (e.g. the room temperature - it makes no sense to report the temperature value from multiple rooms and sum them up) when the instrument is being observed. /// - /// The instrument name. cannot be null. + /// The instrument name. Cannot be null. /// The callback to call to get the measurements when the is called by . /// Optional instrument unit of measurements. /// Optional instrument description. - /// tags to attach to the counter. + /// Optional tags to attach to the gauge. public ObservableGauge CreateObservableGauge(string name, Func> observeValue, string? unit, string? description, IEnumerable>? tags) where T : struct => new ObservableGauge(this, name, observeValue, unit, description, tags); /// /// ObservableGauge is an asynchronous Instrument which reports non-additive value(s) (e.g. the room temperature - it makes no sense to report the temperature value from multiple rooms and sum them up) when the instrument is being observed. /// - /// The instrument name. cannot be null. + /// The instrument name. Cannot be null. /// The callback to call to get the measurements when the is called by . /// Optional instrument unit of measurements. /// Optional instrument description. @@ -428,11 +441,11 @@ public ObservableGauge CreateObservableGauge(string name, Func /// ObservableGauge is an asynchronous Instrument which reports non-additive value(s) (e.g. the room temperature - it makes no sense to report the temperature value from multiple rooms and sum them up) when the instrument is being observed. /// - /// The instrument name. cannot be null. + /// The instrument name. Cannot be null. /// The callback to call to get the measurements when the is called by . /// Optional instrument unit of measurements. /// Optional instrument description. - /// tags to attach to the counter. + /// Optional tags to attach to the gauge. public ObservableGauge CreateObservableGauge(string name, Func>> observeValues, string? unit, string? description, IEnumerable>? tags) where T : struct => new ObservableGauge(this, name, observeValues, unit, description, tags); From 94958bc2f68b0da7125df07394a6c78a32fba717 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 21 May 2024 14:11:31 -0700 Subject: [PATCH 03/11] Add tests. --- .../tests/MetricsAdviceTests.cs | 38 +++++++++++++++++++ .../tests/MetricsTests.cs | 23 +++++++++-- ....Diagnostics.DiagnosticSource.Tests.csproj | 1 + 3 files changed, 59 insertions(+), 3 deletions(-) create mode 100644 src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsAdviceTests.cs diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsAdviceTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsAdviceTests.cs new file mode 100644 index 00000000000000..9c794812cee339 --- /dev/null +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsAdviceTests.cs @@ -0,0 +1,38 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Xunit; + +namespace System.Diagnostics.Metrics.Tests +{ + public class MetricsAdviceTests + { + [Fact] + public void HistogramAdviceConstructionTest() + { + Assert.Throws(() => new HistogramAdvice(explicitBucketBoundaries: null)); + + HistogramAdvice emptyExplicitBucketBoundariesAdvice = new HistogramAdvice(explicitBucketBoundaries: Array.Empty()); + + Assert.Equal(Array.Empty(), emptyExplicitBucketBoundariesAdvice.ExplicitBucketBoundaries); + + int[] singleExplicitBucketBoundary = new int[] { 0 }; + + HistogramAdvice singleExplicitBucketBoundaryAdvice = new HistogramAdvice(singleExplicitBucketBoundary); + + Assert.Equal(singleExplicitBucketBoundary, singleExplicitBucketBoundaryAdvice.ExplicitBucketBoundaries); + + // Verify data cannot be mutated after construction + singleExplicitBucketBoundary[0] = int.MaxValue; + Assert.NotEqual(singleExplicitBucketBoundary, singleExplicitBucketBoundaryAdvice.ExplicitBucketBoundaries); + + int[] invalidBucketBoundariesNondistinctValues = new int[] { 0, 1, 2, 3, 4, 4 }; + + Assert.Throws(() => new HistogramAdvice(invalidBucketBoundariesNondistinctValues)); + + int[] invalidBucketBoundariesOrdering = new int[] { 0, 1, 2, 3, 5, 4 }; + + Assert.Throws(() => new HistogramAdvice(invalidBucketBoundariesOrdering)); + } + } +} diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs index b292e55f1bcf20..2068ed056c6839 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs @@ -3,7 +3,6 @@ using Microsoft.DotNet.RemoteExecutor; using System.Collections.Generic; -using System.Diagnostics.Metrics; using System.Diagnostics.Tests; using System.Linq; using System.Threading; @@ -26,7 +25,7 @@ public void MeasurementConstructionTest() var measurement = new Measurement(i, tags); Assert.Equal(i, measurement.Value); TagListTests.ValidateTags(new TagList(measurement.Tags), tagsArray); - + measurement = new Measurement(i, tagsArray); Assert.Equal(i, measurement.Value); TagListTests.ValidateTags(new TagList(measurement.Tags), tagsArray); @@ -335,7 +334,7 @@ public void InstrumentMeasurementTest(bool useSpan) InstrumentMeasurementAggregationValidation(histogram6, (value, tags) => { Record(histogram6, value, tags, useSpan); } ); }, useSpan.ToString()).Dispose(); - + void AddToCounter(Counter counter, T delta, KeyValuePair[] tags, bool useSpan) where T : struct { if (useSpan) @@ -1461,6 +1460,24 @@ public void TestInstrumentCreationWithTags() }).Dispose(); } + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void TestHistogramCreationWithAdvice() + { + RemoteExecutor.Invoke(() => { + using Meter meter = new Meter(nameof(TestHistogramCreationWithAdvice)); + + int[] explicitBucketBoundaries = new int[] { 0, 100, 1000, 10000 }; + + Histogram histogramWithoutAdvice = meter.CreateHistogram(name: nameof(histogramWithoutAdvice)); + + Assert.Null(histogramWithoutAdvice.Advice); + + Histogram histogramWithAdvice = meter.CreateHistogram(name: nameof(histogramWithAdvice), advice: new HistogramAdvice(explicitBucketBoundaries)); + + Assert.NotNull(histogramWithAdvice.Advice?.ExplicitBucketBoundaries); + Assert.Equal(explicitBucketBoundaries, histogramWithAdvice.Advice.ExplicitBucketBoundaries); + }).Dispose(); + } private void PublishCounterMeasurement(Counter counter, T value, KeyValuePair[] tags) where T : struct { diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/System.Diagnostics.DiagnosticSource.Tests.csproj b/src/libraries/System.Diagnostics.DiagnosticSource/tests/System.Diagnostics.DiagnosticSource.Tests.csproj index 29a8e55dc2fc02..446c67800c4a07 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/System.Diagnostics.DiagnosticSource.Tests.csproj +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/System.Diagnostics.DiagnosticSource.Tests.csproj @@ -32,6 +32,7 @@ + From 8d65122e662af0cba3c86de980797a4fd9261777 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 21 May 2024 14:15:32 -0700 Subject: [PATCH 04/11] Code review. --- .../ref/System.Diagnostics.DiagnosticSourceActivity.cs | 10 +++++----- .../src/Resources/Strings.resx | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs b/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs index b801b5751b3202..8bfaeb309ea551 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs @@ -367,6 +367,11 @@ public sealed class Histogram : Instrument where T : struct public void Record(T value, params System.ReadOnlySpan> tags) { throw null; } public void Record(T value, params System.Collections.Generic.KeyValuePair[] tags) { throw null; } } + public sealed class HistogramAdvice where T : struct + { + public HistogramAdvice(System.Collections.Generic.IEnumerable explicitBucketBoundaries) { throw null; } + public System.Collections.Generic.IReadOnlyList? ExplicitBucketBoundaries { get { throw null; } } + } public interface IMeterFactory : System.IDisposable { System.Diagnostics.Metrics.Meter Create(System.Diagnostics.Metrics.MeterOptions options); @@ -573,9 +578,4 @@ public abstract class ObservableInstrument : Instrument where T : struct protected ObservableInstrument(Meter meter, string name, string? unit, string? description, System.Collections.Generic.IEnumerable> tags) : base(meter, name, unit, description) { throw null; } protected abstract System.Collections.Generic.IEnumerable> Observe(); } - public sealed class HistogramAdvice where T : struct - { - public HistogramAdvice(System.Collections.Generic.IEnumerable explicitBucketBoundaries) { throw null; } - public System.Collections.Generic.IReadOnlyList? ExplicitBucketBoundaries { get { throw null; } } - } } diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/Resources/Strings.resx b/src/libraries/System.Diagnostics.DiagnosticSource/src/Resources/Strings.resx index f1493d25fae37f..b228dca0765cc3 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/Resources/Strings.resx +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/Resources/Strings.resx @@ -175,6 +175,6 @@ The instrument is of different generic type. - Histogram explicit bucket boundaries MUST be specified in ascending order and CANNOT contain duplicate values. + Histogram explicit bucket boundaries must be specified in ascending order and cannot contain duplicate values. - + \ No newline at end of file From 922abfca3360ad44c82134e80bf649dd3dbeb3d9 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 21 May 2024 15:09:58 -0700 Subject: [PATCH 05/11] Code review. --- .../Diagnostics/Metrics/HistogramAdvice.cs | 13 +++++++++++-- .../tests/MetricsAdviceTests.cs | 3 ++- .../tests/MetricsTests.cs | 19 ++++++++++--------- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/HistogramAdvice.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/HistogramAdvice.cs index f40d2fb5cd0a0d..065dca591a4b2f 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/HistogramAdvice.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/HistogramAdvice.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Collections.ObjectModel; namespace System.Diagnostics.Metrics { @@ -30,7 +31,7 @@ public HistogramAdvice(IEnumerable explicitBucketBoundaries) throw new ArgumentNullException(nameof(explicitBucketBoundaries)); } - List explicitBucketBoundariesCopy = [.. explicitBucketBoundaries]; + IReadOnlyList explicitBucketBoundariesCopy = new ReadOnlyCollection(new List(explicitBucketBoundaries)); if (!IsSortedAndDistinct(explicitBucketBoundariesCopy)) { @@ -43,9 +44,17 @@ public HistogramAdvice(IEnumerable explicitBucketBoundaries) /// /// Gets the explicit bucket boundaries advised to be used with the histogram. /// + /// + /// Notes: + /// + /// A value means no bucket boundaries have been configured and default values should be used for bucket configuration. + /// An empty set of bucket boundaries hints that the histogram by default should NOT contain buckets and should only track count and sum values. + /// A set of non-distinct increasing values for bucket boundaries hints that the histogram should use those for its default bucket configuration. + /// + /// public IReadOnlyList? ExplicitBucketBoundaries { get; } - private static bool IsSortedAndDistinct(List values) + private static bool IsSortedAndDistinct(IReadOnlyList values) { Comparer comparer = Comparer.Default; diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsAdviceTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsAdviceTests.cs index 9c794812cee339..91549509c60d14 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsAdviceTests.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsAdviceTests.cs @@ -14,7 +14,8 @@ public void HistogramAdviceConstructionTest() HistogramAdvice emptyExplicitBucketBoundariesAdvice = new HistogramAdvice(explicitBucketBoundaries: Array.Empty()); - Assert.Equal(Array.Empty(), emptyExplicitBucketBoundariesAdvice.ExplicitBucketBoundaries); + Assert.NotNull(emptyExplicitBucketBoundariesAdvice.ExplicitBucketBoundaries); + Assert.Empty(emptyExplicitBucketBoundariesAdvice.ExplicitBucketBoundaries); int[] singleExplicitBucketBoundary = new int[] { 0 }; diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs index 2068ed056c6839..467531a5fd2045 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs @@ -1463,20 +1463,21 @@ public void TestInstrumentCreationWithTags() [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] public void TestHistogramCreationWithAdvice() { - RemoteExecutor.Invoke(() => { - using Meter meter = new Meter(nameof(TestHistogramCreationWithAdvice)); + RemoteExecutor.Invoke(() => + { + using Meter meter = new Meter(nameof(TestHistogramCreationWithAdvice)); - int[] explicitBucketBoundaries = new int[] { 0, 100, 1000, 10000 }; + Histogram histogramWithoutAdvice = meter.CreateHistogram(name: nameof(histogramWithoutAdvice)); - Histogram histogramWithoutAdvice = meter.CreateHistogram(name: nameof(histogramWithoutAdvice)); + Assert.Null(histogramWithoutAdvice.Advice); - Assert.Null(histogramWithoutAdvice.Advice); + int[] explicitBucketBoundaries = new int[] { 0, 100, 1000, 10000 }; - Histogram histogramWithAdvice = meter.CreateHistogram(name: nameof(histogramWithAdvice), advice: new HistogramAdvice(explicitBucketBoundaries)); + Histogram histogramWithAdvice = meter.CreateHistogram(name: nameof(histogramWithAdvice), advice: new HistogramAdvice(explicitBucketBoundaries)); - Assert.NotNull(histogramWithAdvice.Advice?.ExplicitBucketBoundaries); - Assert.Equal(explicitBucketBoundaries, histogramWithAdvice.Advice.ExplicitBucketBoundaries); - }).Dispose(); + Assert.NotNull(histogramWithAdvice.Advice?.ExplicitBucketBoundaries); + Assert.Equal(explicitBucketBoundaries, histogramWithAdvice.Advice.ExplicitBucketBoundaries); + }).Dispose(); } private void PublishCounterMeasurement(Counter counter, T value, KeyValuePair[] tags) where T : struct From 3f0fbccfeba2de466a2065d6db161e21784d66e7 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 21 May 2024 15:13:03 -0700 Subject: [PATCH 06/11] Tweaks. --- .../src/System/Diagnostics/Metrics/HistogramAdvice.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/HistogramAdvice.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/HistogramAdvice.cs index 065dca591a4b2f..5b000493cbb256 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/HistogramAdvice.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/HistogramAdvice.cs @@ -19,8 +19,8 @@ public sealed class HistogramAdvice where T : struct /// Explicit bucket boundaries advised to be used with the histogram. /// Notes: /// - /// Bucket boundaries MUST be specified in ascending order and CANNOT contain duplicate values. - /// An empty set of bucket boundaries hints that the histogram should NOT contain buckets and should only track count and sum values. + /// An empty set of bucket boundaries hints that the histogram by default should NOT contain buckets and should only track count and sum values. + /// A set of distinct increasing values for bucket boundaries hints that the histogram should use those for its default bucket configuration. /// /// /// @@ -49,7 +49,7 @@ public HistogramAdvice(IEnumerable explicitBucketBoundaries) /// /// A value means no bucket boundaries have been configured and default values should be used for bucket configuration. /// An empty set of bucket boundaries hints that the histogram by default should NOT contain buckets and should only track count and sum values. - /// A set of non-distinct increasing values for bucket boundaries hints that the histogram should use those for its default bucket configuration. + /// A set of distinct increasing values for bucket boundaries hints that the histogram should use those for its default bucket configuration. /// /// public IReadOnlyList? ExplicitBucketBoundaries { get; } From ea17f1734048230553a1593bb87451073f20e5c0 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 21 May 2024 22:52:16 -0700 Subject: [PATCH 07/11] Code review. --- .../System/Diagnostics/Metrics/HistogramAdvice.cs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/HistogramAdvice.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/HistogramAdvice.cs index 5b000493cbb256..5bd2c197efeb2c 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/HistogramAdvice.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/HistogramAdvice.cs @@ -15,15 +15,14 @@ public sealed class HistogramAdvice where T : struct /// /// Constructs a new instance of . /// - /// - /// Explicit bucket boundaries advised to be used with the histogram. - /// Notes: + /// + /// Notes: /// /// An empty set of bucket boundaries hints that the histogram by default should NOT contain buckets and should only track count and sum values. /// A set of distinct increasing values for bucket boundaries hints that the histogram should use those for its default bucket configuration. /// - /// - /// + /// + /// Explicit bucket boundaries advised to be used with the histogram. public HistogramAdvice(IEnumerable explicitBucketBoundaries) { if (explicitBucketBoundaries is null) @@ -31,14 +30,14 @@ public HistogramAdvice(IEnumerable explicitBucketBoundaries) throw new ArgumentNullException(nameof(explicitBucketBoundaries)); } - IReadOnlyList explicitBucketBoundariesCopy = new ReadOnlyCollection(new List(explicitBucketBoundaries)); + List explicitBucketBoundariesCopy = new List(explicitBucketBoundaries); if (!IsSortedAndDistinct(explicitBucketBoundariesCopy)) { throw new ArgumentException(SR.InvalidHistogramExplicitBucketBoundaries, nameof(explicitBucketBoundaries)); } - ExplicitBucketBoundaries = explicitBucketBoundariesCopy; + ExplicitBucketBoundaries = new ReadOnlyCollection(explicitBucketBoundariesCopy); } /// @@ -54,7 +53,7 @@ public HistogramAdvice(IEnumerable explicitBucketBoundaries) /// public IReadOnlyList? ExplicitBucketBoundaries { get; } - private static bool IsSortedAndDistinct(IReadOnlyList values) + private static bool IsSortedAndDistinct(List values) { Comparer comparer = Comparer.Default; From afc3facc8209a9dabb22916d3ee1755693931769 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 4 Jun 2024 11:07:10 -0700 Subject: [PATCH 08/11] Revisions. --- ...em.Diagnostics.DiagnosticSourceActivity.cs | 34 ++++++---- ...System.Diagnostics.DiagnosticSource.csproj | 11 ++-- .../System/Diagnostics/Metrics/Histogram.cs | 13 ++-- .../Diagnostics/Metrics/Instrument.common.cs | 62 +++++++++++++++---- .../System/Diagnostics/Metrics/Instrument.cs | 38 +++++++++--- ...HistogramAdvice.cs => InstrumentAdvice.cs} | 30 ++++----- .../src/System/Diagnostics/Metrics/Meter.cs | 4 +- .../tests/MetricsAdviceTests.cs | 18 +++--- .../tests/MetricsTests.cs | 6 +- 9 files changed, 139 insertions(+), 77 deletions(-) rename src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/{HistogramAdvice.cs => InstrumentAdvice.cs} (54%) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs b/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs index 8bfaeb309ea551..c5f1a73e575157 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs @@ -357,7 +357,6 @@ internal UpDownCounter(Meter meter, string name, string? unit, string? descripti } public sealed class Histogram : Instrument where T : struct { - public HistogramAdvice? Advice { get { throw null; } } internal Histogram(Meter meter, string name, string? unit, string? description) : base(meter, name, unit, description) { throw null; } public void Record(T value) { throw null; } public void Record(T value, System.Collections.Generic.KeyValuePair tag) { throw null; } @@ -367,11 +366,6 @@ public sealed class Histogram : Instrument where T : struct public void Record(T value, params System.ReadOnlySpan> tags) { throw null; } public void Record(T value, params System.Collections.Generic.KeyValuePair[] tags) { throw null; } } - public sealed class HistogramAdvice where T : struct - { - public HistogramAdvice(System.Collections.Generic.IEnumerable explicitBucketBoundaries) { throw null; } - public System.Collections.Generic.IReadOnlyList? ExplicitBucketBoundaries { get { throw null; } } - } public interface IMeterFactory : System.IDisposable { System.Diagnostics.Metrics.Meter Create(System.Diagnostics.Metrics.MeterOptions options); @@ -380,8 +374,10 @@ public abstract class Instrument { public string? Description { get {throw null;} } public bool Enabled { get {throw null; } } - protected Instrument(Meter meter, string name, string? unit, string? description) {throw null;} - protected Instrument(Meter meter, string name, string? unit, string? description, System.Collections.Generic.IEnumerable>? tags) {throw null;} + protected Instrument(Meter meter, string name) : this(meter, name, unit: null, description: null, tags: null) { throw null; } + [System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)] + protected Instrument(Meter meter, string name, string? unit, string? description) : this(meter, name, unit, description, tags: null) { throw null; } + protected Instrument(Meter meter, string name, string? unit = default, string? description = default, System.Collections.Generic.IEnumerable>? tags = default) { throw null; } public virtual bool IsObservable { get {throw null; } } public Meter Meter { get {throw null;} } public string Name { get {throw null;} } @@ -391,8 +387,19 @@ public abstract class Instrument } public abstract class Instrument : Instrument where T : struct { - protected Instrument(Meter meter, string name, string? unit, string? description) : base(meter, name, unit, description) { throw null; } - protected Instrument(Meter meter, string name, string? unit, string? description, System.Collections.Generic.IEnumerable>? tags) : base(meter, name, unit, description, tags) {throw null;} + public InstrumentAdvice? Advice { get { throw null; } } + protected Instrument(Meter meter, string name) : this(meter, name, unit: null, description: null, tags: null, advice: null) { throw null; } + [System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)] + protected Instrument(Meter meter, string name, string? unit, string? description) : this(meter, name, unit, description, tags: null, advice: null) { throw null; } + [System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)] + protected Instrument(Meter meter, string name, string? unit, string? description, System.Collections.Generic.IEnumerable>? tags) : this(meter, name, unit, description, tags, advice: null) { throw null; } + protected Instrument( + Meter meter, + string name, + string? unit = default, + string? description = default, + System.Collections.Generic.IEnumerable>? tags = default, + InstrumentAdvice? advice = default) : base(meter, name, unit, description, tags) { throw null; } protected void RecordMeasurement(T measurement) { throw null; } protected void RecordMeasurement(T measurement, System.Collections.Generic.KeyValuePair tag) { throw null; } protected void RecordMeasurement(T measurement, System.Collections.Generic.KeyValuePair tag1, System.Collections.Generic.KeyValuePair tag2) { throw null; } @@ -400,6 +407,11 @@ public abstract class Instrument : Instrument where T : struct protected void RecordMeasurement(T measurement, in TagList tagList) { throw null; } protected void RecordMeasurement(T measurement, ReadOnlySpan> tags) { throw null; } } + public sealed class InstrumentAdvice where T : struct + { + public InstrumentAdvice(System.Collections.Generic.IEnumerable histogramExplicitBucketBoundaries) { throw null; } + public System.Collections.Generic.IReadOnlyList? HistogramExplicitBucketBoundaries { get { throw null; } } + } public readonly struct Measurement where T : struct { public Measurement(T value) { throw null; } @@ -421,7 +433,7 @@ public class Meter : IDisposable public Histogram CreateHistogram(string name, string? unit, string? description) where T : struct { throw null; } [System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)] public Histogram CreateHistogram(string name, string? unit, string? description, System.Collections.Generic.IEnumerable>? tags) where T : struct { throw null; } - public Histogram CreateHistogram(string name, string? unit = default, string? description = default, System.Collections.Generic.IEnumerable>? tags = default, HistogramAdvice? advice = default) where T : struct { throw null; } + public Histogram CreateHistogram(string name, string? unit = default, string? description = default, System.Collections.Generic.IEnumerable>? tags = default, InstrumentAdvice? advice = default) where T : struct { throw null; } public ObservableCounter CreateObservableCounter( string name, Func observeValue, diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj b/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj index 76a6685c3e5b09..d8775312d8bcb5 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj @@ -54,7 +54,7 @@ System.Diagnostics.DiagnosticSource - + @@ -93,10 +93,8 @@ System.Diagnostics.DiagnosticSource - - + + @@ -116,8 +114,7 @@ System.Diagnostics.DiagnosticSource - + diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Histogram.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Histogram.cs index 7e3999695b0bad..88c99481670c06 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Histogram.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Histogram.cs @@ -15,19 +15,14 @@ namespace System.Diagnostics.Metrics /// public sealed class Histogram : Instrument where T : struct { - /// - /// Gets the associated with the instrument. - /// - public HistogramAdvice? Advice { get; } - - internal Histogram(Meter meter, string name, string? unit, string? description) : this(meter, name, unit, description, tags: null, advice: null) + internal Histogram(Meter meter, string name, string? unit, string? description) + : this(meter, name, unit, description, tags: null, advice: null) { } - internal Histogram(Meter meter, string name, string? unit, string? description, IEnumerable>? tags, HistogramAdvice? advice) : base(meter, name, unit, description, tags) + internal Histogram(Meter meter, string name, string? unit, string? description, IEnumerable>? tags, InstrumentAdvice? advice) + : base(meter, name, unit, description, tags, advice) { - Advice = advice; - Publish(); } diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Instrument.common.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Instrument.common.cs index bc813c0d4cfe47..ca1c7dd63d236a 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Instrument.common.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Instrument.common.cs @@ -2,11 +2,12 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.ComponentModel; namespace System.Diagnostics.Metrics { /// - /// Instrument{T} is the base class for all non-observable instruments. + /// is the base class for all non-observable instruments. /// /// /// This class supports only the following generic parameter types: , , , , , , and @@ -15,28 +16,67 @@ namespace System.Diagnostics.Metrics public abstract partial class Instrument : Instrument where T : struct { /// - /// Create the metrics instrument using the properties meter, name, description, and unit. - /// All classes extending Instrument{T} need to call this constructor when constructing object of the extended class. + /// Gets the associated with the instrument. /// - /// The meter that created the instrument. - /// The instrument name. cannot be null. + public InstrumentAdvice? Advice { get; } + + /// + /// Constructs a new instance of . + /// + /// The meter that created the instrument. Cannot be null. + /// The instrument name. Cannot be null. + protected Instrument(Meter meter, string name) + : this(meter, name, unit: null, description: null, tags: null, advice: null) + { + } + + /// + /// Constructs a new instance of . + /// + /// The meter that created the instrument. Cannot be null. + /// The instrument name. Cannot be null. /// Optional instrument unit of measurements. /// Optional instrument description. - protected Instrument(Meter meter, string name, string? unit, string? description) : this(meter, name, unit, description, tags: null) + [EditorBrowsable(EditorBrowsableState.Never)] + protected Instrument(Meter meter, string name, string? unit, string? description) + : this(meter, name, unit, description, tags: null, advice: null) { } /// - /// Create the metrics instrument using the properties meter, name, description, and unit. - /// All classes extending Instrument{T} need to call this constructor when constructing object of the extended class. + /// Constructs a new instance of . /// - /// The meter that created the instrument. - /// The instrument name. cannot be null. + /// The meter that created the instrument. Cannot be null. + /// The instrument name. Cannot be null. /// Optional instrument unit of measurements. /// Optional instrument description. /// Optional instrument tags. - protected Instrument(Meter meter, string name, string? unit, string? description, IEnumerable>? tags) : base(meter, name, unit, description, tags) + [EditorBrowsable(EditorBrowsableState.Never)] + protected Instrument(Meter meter, string name, string? unit, string? description, IEnumerable>? tags) + : this(meter, name, unit, description, tags, advice: null) { + } + + /// + /// Constructs a new instance of . + /// + /// The meter that created the instrument. Cannot be null. + /// The instrument name. Cannot be null. + /// Optional instrument unit of measurements. + /// Optional instrument description. + /// Optional instrument tags. + /// Optional . + protected Instrument( + Meter meter, + string name, + string? unit = default, + string? description = default, + IEnumerable>? tags = default, + InstrumentAdvice? advice = default) + : base(meter, name, unit, description, tags) + { + Advice = advice; + ValidateTypeParameter(); } diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Instrument.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Instrument.cs index 17941aa0d8b830..0a3362e015d1a7 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Instrument.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Instrument.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.ComponentModel; namespace System.Diagnostics.Metrics { @@ -27,25 +28,42 @@ public abstract class Instrument internal readonly DiagLinkedList _subscriptions = new DiagLinkedList(); /// - /// Protected constructor to initialize the common instrument properties like the meter, name, description, and unit. - /// All classes extending Instrument need to call this constructor when constructing object of the extended class. + /// Constructs a new instance of . /// - /// The meter that created the instrument. - /// The instrument name. cannot be null. + /// The meter that created the instrument. Cannot be null. + /// The instrument name. Cannot be null. + protected Instrument(Meter meter, string name) + : this(meter, name, unit: null, description: null, tags: null) + { + } + + /// + /// Constructs a new instance of . + /// + /// The meter that created the instrument. Cannot be null. + /// The instrument name. Cannot be null. /// Optional instrument unit of measurements. /// Optional instrument description. - protected Instrument(Meter meter, string name, string? unit, string? description) : this(meter, name, unit, description, null) { } + [EditorBrowsable(EditorBrowsableState.Never)] + protected Instrument(Meter meter, string name, string? unit, string? description) + : this(meter, name, unit, description, tags: null) + { + } /// - /// Protected constructor to initialize the common instrument properties like the meter, name, description, and unit. - /// All classes extending Instrument need to call this constructor when constructing object of the extended class. + /// Constructs a new instance of . /// - /// The meter that created the instrument. - /// The instrument name. cannot be null. + /// The meter that created the instrument. Cannot be null. + /// The instrument name. Cannot be null. /// Optional instrument unit of measurements. /// Optional instrument description. /// Optional instrument tags. - protected Instrument(Meter meter, string name, string? unit, string? description, IEnumerable>? tags) + protected Instrument( + Meter meter, + string name, + string? unit = default, + string? description = default, + IEnumerable>? tags = default) { Meter = meter ?? throw new ArgumentNullException(nameof(meter)); Name = name ?? throw new ArgumentNullException(nameof(name)); diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/HistogramAdvice.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/InstrumentAdvice.cs similarity index 54% rename from src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/HistogramAdvice.cs rename to src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/InstrumentAdvice.cs index 5bd2c197efeb2c..d651d83f350512 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/HistogramAdvice.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/InstrumentAdvice.cs @@ -7,41 +7,41 @@ namespace System.Diagnostics.Metrics { /// - /// Contains settings used to advise metrics consumers how to construct storage for instruments. + /// Contains configuration settings advised to be used by metrics consumers when recording measurements for a given . /// - /// Histogram value type. - public sealed class HistogramAdvice where T : struct + /// Instrument value type. + public sealed class InstrumentAdvice where T : struct { /// - /// Constructs a new instance of . + /// Constructs a new instance of . /// /// /// Notes: /// - /// An empty set of bucket boundaries hints that the histogram by default should NOT contain buckets and should only track count and sum values. - /// A set of distinct increasing values for bucket boundaries hints that the histogram should use those for its default bucket configuration. + /// An empty set of bucket boundaries hints that histogram instruments by default should NOT contain buckets and should only track count and sum values. + /// A set of distinct increasing values for histogram bucket boundaries hints that histogram instruments should use those for its default bucket configuration. /// /// - /// Explicit bucket boundaries advised to be used with the histogram. - public HistogramAdvice(IEnumerable explicitBucketBoundaries) + /// Explicit bucket boundaries advised to be used with histogram instruments. + public InstrumentAdvice(IEnumerable histogramExplicitBucketBoundaries) { - if (explicitBucketBoundaries is null) + if (histogramExplicitBucketBoundaries is null) { - throw new ArgumentNullException(nameof(explicitBucketBoundaries)); + throw new ArgumentNullException(nameof(histogramExplicitBucketBoundaries)); } - List explicitBucketBoundariesCopy = new List(explicitBucketBoundaries); + List explicitBucketBoundariesCopy = new List(histogramExplicitBucketBoundaries); if (!IsSortedAndDistinct(explicitBucketBoundariesCopy)) { - throw new ArgumentException(SR.InvalidHistogramExplicitBucketBoundaries, nameof(explicitBucketBoundaries)); + throw new ArgumentException(SR.InvalidHistogramExplicitBucketBoundaries, nameof(histogramExplicitBucketBoundaries)); } - ExplicitBucketBoundaries = new ReadOnlyCollection(explicitBucketBoundariesCopy); + HistogramExplicitBucketBoundaries = new ReadOnlyCollection(explicitBucketBoundariesCopy); } /// - /// Gets the explicit bucket boundaries advised to be used with the histogram. + /// Gets the explicit bucket boundaries advised to be used with histogram instruments. /// /// /// Notes: @@ -51,7 +51,7 @@ public HistogramAdvice(IEnumerable explicitBucketBoundaries) /// A set of distinct increasing values for bucket boundaries hints that the histogram should use those for its default bucket configuration. /// /// - public IReadOnlyList? ExplicitBucketBoundaries { get; } + public IReadOnlyList? HistogramExplicitBucketBoundaries { get; } private static bool IsSortedAndDistinct(List values) { diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Meter.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Meter.cs index 4ca5f0fe8daa54..8578f0a51fd640 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Meter.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Meter.cs @@ -188,11 +188,11 @@ public Histogram CreateHistogram(string name, string? unit, string? descri /// Optional instrument unit of measurements. /// Optional instrument description. /// Optional tags to attach to the histogram. - /// Optional to attach to the histogram. + /// Optional to attach to the histogram. /// /// Example uses for Histogram: the request duration and the size of the response payload. /// - public Histogram CreateHistogram(string name, string? unit = default, string? description = default, IEnumerable>? tags = default, HistogramAdvice? advice = default) where T : struct + public Histogram CreateHistogram(string name, string? unit = default, string? description = default, IEnumerable>? tags = default, InstrumentAdvice? advice = default) where T : struct => (Histogram)GetOrCreateInstrument(typeof(Histogram), name, unit, description, tags, () => new Histogram(this, name, unit, description, tags, advice)); /// diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsAdviceTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsAdviceTests.cs index 91549509c60d14..26d05183c50b00 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsAdviceTests.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsAdviceTests.cs @@ -10,30 +10,30 @@ public class MetricsAdviceTests [Fact] public void HistogramAdviceConstructionTest() { - Assert.Throws(() => new HistogramAdvice(explicitBucketBoundaries: null)); + Assert.Throws(() => new InstrumentAdvice(histogramExplicitBucketBoundaries: null)); - HistogramAdvice emptyExplicitBucketBoundariesAdvice = new HistogramAdvice(explicitBucketBoundaries: Array.Empty()); + InstrumentAdvice emptyExplicitBucketBoundariesAdvice = new InstrumentAdvice(histogramExplicitBucketBoundaries: Array.Empty()); - Assert.NotNull(emptyExplicitBucketBoundariesAdvice.ExplicitBucketBoundaries); - Assert.Empty(emptyExplicitBucketBoundariesAdvice.ExplicitBucketBoundaries); + Assert.NotNull(emptyExplicitBucketBoundariesAdvice.HistogramExplicitBucketBoundaries); + Assert.Empty(emptyExplicitBucketBoundariesAdvice.HistogramExplicitBucketBoundaries); int[] singleExplicitBucketBoundary = new int[] { 0 }; - HistogramAdvice singleExplicitBucketBoundaryAdvice = new HistogramAdvice(singleExplicitBucketBoundary); + InstrumentAdvice singleExplicitBucketBoundaryAdvice = new InstrumentAdvice(singleExplicitBucketBoundary); - Assert.Equal(singleExplicitBucketBoundary, singleExplicitBucketBoundaryAdvice.ExplicitBucketBoundaries); + Assert.Equal(singleExplicitBucketBoundary, singleExplicitBucketBoundaryAdvice.HistogramExplicitBucketBoundaries); // Verify data cannot be mutated after construction singleExplicitBucketBoundary[0] = int.MaxValue; - Assert.NotEqual(singleExplicitBucketBoundary, singleExplicitBucketBoundaryAdvice.ExplicitBucketBoundaries); + Assert.NotEqual(singleExplicitBucketBoundary, singleExplicitBucketBoundaryAdvice.HistogramExplicitBucketBoundaries); int[] invalidBucketBoundariesNondistinctValues = new int[] { 0, 1, 2, 3, 4, 4 }; - Assert.Throws(() => new HistogramAdvice(invalidBucketBoundariesNondistinctValues)); + Assert.Throws(() => new InstrumentAdvice(invalidBucketBoundariesNondistinctValues)); int[] invalidBucketBoundariesOrdering = new int[] { 0, 1, 2, 3, 5, 4 }; - Assert.Throws(() => new HistogramAdvice(invalidBucketBoundariesOrdering)); + Assert.Throws(() => new InstrumentAdvice(invalidBucketBoundariesOrdering)); } } } diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs index 467531a5fd2045..49a66f9e30def6 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs @@ -1473,10 +1473,10 @@ public void TestHistogramCreationWithAdvice() int[] explicitBucketBoundaries = new int[] { 0, 100, 1000, 10000 }; - Histogram histogramWithAdvice = meter.CreateHistogram(name: nameof(histogramWithAdvice), advice: new HistogramAdvice(explicitBucketBoundaries)); + Histogram histogramWithAdvice = meter.CreateHistogram(name: nameof(histogramWithAdvice), advice: new InstrumentAdvice(explicitBucketBoundaries)); - Assert.NotNull(histogramWithAdvice.Advice?.ExplicitBucketBoundaries); - Assert.Equal(explicitBucketBoundaries, histogramWithAdvice.Advice.ExplicitBucketBoundaries); + Assert.NotNull(histogramWithAdvice.Advice?.HistogramExplicitBucketBoundaries); + Assert.Equal(explicitBucketBoundaries, histogramWithAdvice.Advice.HistogramExplicitBucketBoundaries); }).Dispose(); } From 6f5607225aa2c78ad70213bfc575b2de769006ee Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 4 Jun 2024 15:49:55 -0700 Subject: [PATCH 09/11] Tweaks. --- ...em.Diagnostics.DiagnosticSourceActivity.cs | 3 +- .../Diagnostics/Metrics/InstrumentAdvice.cs | 47 ++++++++----------- .../tests/MetricsAdviceTests.cs | 18 +++---- .../tests/MetricsTests.cs | 6 +-- 4 files changed, 33 insertions(+), 41 deletions(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs b/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs index c5f1a73e575157..b6d6a352520e7f 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs @@ -409,8 +409,7 @@ protected Instrument( } public sealed class InstrumentAdvice where T : struct { - public InstrumentAdvice(System.Collections.Generic.IEnumerable histogramExplicitBucketBoundaries) { throw null; } - public System.Collections.Generic.IReadOnlyList? HistogramExplicitBucketBoundaries { get { throw null; } } + public System.Collections.Generic.IReadOnlyList? HistogramBucketBoundaries { get { throw null; } init { throw null; } } } public readonly struct Measurement where T : struct { diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/InstrumentAdvice.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/InstrumentAdvice.cs index d651d83f350512..1a78325ca53e9b 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/InstrumentAdvice.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/InstrumentAdvice.cs @@ -12,47 +12,40 @@ namespace System.Diagnostics.Metrics /// Instrument value type. public sealed class InstrumentAdvice where T : struct { + private readonly ReadOnlyCollection? _HistogramBucketBoundaries; + /// - /// Constructs a new instance of . + /// Gets the explicit bucket boundaries advised to be used with histogram instruments. /// /// /// Notes: /// - /// An empty set of bucket boundaries hints that histogram instruments by default should NOT contain buckets and should only track count and sum values. - /// A set of distinct increasing values for histogram bucket boundaries hints that histogram instruments should use those for its default bucket configuration. + /// A value means no bucket boundaries have been configured and default values should be used for bucket configuration. + /// An empty set of bucket boundaries hints that the histogram by default should NOT contain buckets and should only track count and sum values. + /// A set of distinct increasing values for bucket boundaries hints that the histogram should use those for its default bucket configuration. /// /// - /// Explicit bucket boundaries advised to be used with histogram instruments. - public InstrumentAdvice(IEnumerable histogramExplicitBucketBoundaries) + public IReadOnlyList? HistogramBucketBoundaries { - if (histogramExplicitBucketBoundaries is null) + get => _HistogramBucketBoundaries; + init { - throw new ArgumentNullException(nameof(histogramExplicitBucketBoundaries)); - } + if (value is null) + { + throw new ArgumentNullException(nameof(value)); + } - List explicitBucketBoundariesCopy = new List(histogramExplicitBucketBoundaries); + List bucketBoundariesCopy = new List(value); - if (!IsSortedAndDistinct(explicitBucketBoundariesCopy)) - { - throw new ArgumentException(SR.InvalidHistogramExplicitBucketBoundaries, nameof(histogramExplicitBucketBoundaries)); - } + if (!IsSortedAndDistinct(bucketBoundariesCopy)) + { + throw new ArgumentException(SR.InvalidHistogramExplicitBucketBoundaries, nameof(value)); + } - HistogramExplicitBucketBoundaries = new ReadOnlyCollection(explicitBucketBoundariesCopy); + _HistogramBucketBoundaries = new ReadOnlyCollection(bucketBoundariesCopy); + } } - /// - /// Gets the explicit bucket boundaries advised to be used with histogram instruments. - /// - /// - /// Notes: - /// - /// A value means no bucket boundaries have been configured and default values should be used for bucket configuration. - /// An empty set of bucket boundaries hints that the histogram by default should NOT contain buckets and should only track count and sum values. - /// A set of distinct increasing values for bucket boundaries hints that the histogram should use those for its default bucket configuration. - /// - /// - public IReadOnlyList? HistogramExplicitBucketBoundaries { get; } - private static bool IsSortedAndDistinct(List values) { Comparer comparer = Comparer.Default; diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsAdviceTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsAdviceTests.cs index 26d05183c50b00..e825fed29f8d40 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsAdviceTests.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsAdviceTests.cs @@ -10,30 +10,30 @@ public class MetricsAdviceTests [Fact] public void HistogramAdviceConstructionTest() { - Assert.Throws(() => new InstrumentAdvice(histogramExplicitBucketBoundaries: null)); + Assert.Throws(() => new InstrumentAdvice { HistogramBucketBoundaries = null }); - InstrumentAdvice emptyExplicitBucketBoundariesAdvice = new InstrumentAdvice(histogramExplicitBucketBoundaries: Array.Empty()); + InstrumentAdvice emptyExplicitBucketBoundariesAdvice = new InstrumentAdvice { HistogramBucketBoundaries = Array.Empty() }; - Assert.NotNull(emptyExplicitBucketBoundariesAdvice.HistogramExplicitBucketBoundaries); - Assert.Empty(emptyExplicitBucketBoundariesAdvice.HistogramExplicitBucketBoundaries); + Assert.NotNull(emptyExplicitBucketBoundariesAdvice.HistogramBucketBoundaries); + Assert.Empty(emptyExplicitBucketBoundariesAdvice.HistogramBucketBoundaries); int[] singleExplicitBucketBoundary = new int[] { 0 }; - InstrumentAdvice singleExplicitBucketBoundaryAdvice = new InstrumentAdvice(singleExplicitBucketBoundary); + InstrumentAdvice singleExplicitBucketBoundaryAdvice = new InstrumentAdvice { HistogramBucketBoundaries = singleExplicitBucketBoundary }; - Assert.Equal(singleExplicitBucketBoundary, singleExplicitBucketBoundaryAdvice.HistogramExplicitBucketBoundaries); + Assert.Equal(singleExplicitBucketBoundary, singleExplicitBucketBoundaryAdvice.HistogramBucketBoundaries); // Verify data cannot be mutated after construction singleExplicitBucketBoundary[0] = int.MaxValue; - Assert.NotEqual(singleExplicitBucketBoundary, singleExplicitBucketBoundaryAdvice.HistogramExplicitBucketBoundaries); + Assert.NotEqual(singleExplicitBucketBoundary, singleExplicitBucketBoundaryAdvice.HistogramBucketBoundaries); int[] invalidBucketBoundariesNondistinctValues = new int[] { 0, 1, 2, 3, 4, 4 }; - Assert.Throws(() => new InstrumentAdvice(invalidBucketBoundariesNondistinctValues)); + Assert.Throws(() => new InstrumentAdvice { HistogramBucketBoundaries = invalidBucketBoundariesNondistinctValues }); int[] invalidBucketBoundariesOrdering = new int[] { 0, 1, 2, 3, 5, 4 }; - Assert.Throws(() => new InstrumentAdvice(invalidBucketBoundariesOrdering)); + Assert.Throws(() => new InstrumentAdvice { HistogramBucketBoundaries = invalidBucketBoundariesOrdering }); } } } diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs index 49a66f9e30def6..035b45ea3d2fe0 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs @@ -1473,10 +1473,10 @@ public void TestHistogramCreationWithAdvice() int[] explicitBucketBoundaries = new int[] { 0, 100, 1000, 10000 }; - Histogram histogramWithAdvice = meter.CreateHistogram(name: nameof(histogramWithAdvice), advice: new InstrumentAdvice(explicitBucketBoundaries)); + Histogram histogramWithAdvice = meter.CreateHistogram(name: nameof(histogramWithAdvice), advice: new InstrumentAdvice { HistogramBucketBoundaries = explicitBucketBoundaries }); - Assert.NotNull(histogramWithAdvice.Advice?.HistogramExplicitBucketBoundaries); - Assert.Equal(explicitBucketBoundaries, histogramWithAdvice.Advice.HistogramExplicitBucketBoundaries); + Assert.NotNull(histogramWithAdvice.Advice?.HistogramBucketBoundaries); + Assert.Equal(explicitBucketBoundaries, histogramWithAdvice.Advice.HistogramBucketBoundaries); }).Dispose(); } From 8e76e819c2a4bbd0dad1fd57e252519af5fa45db Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 11 Jun 2024 13:29:27 -0700 Subject: [PATCH 10/11] Code review. --- .../src/System.Diagnostics.DiagnosticSource.csproj | 3 ++- .../src/System/Diagnostics/Metrics/InstrumentAdvice.cs | 8 ++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj b/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj index d8775312d8bcb5..b2654e92ee003b 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj @@ -114,7 +114,8 @@ System.Diagnostics.DiagnosticSource - + diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/InstrumentAdvice.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/InstrumentAdvice.cs index 1a78325ca53e9b..b06a5e4012a5bd 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/InstrumentAdvice.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/InstrumentAdvice.cs @@ -14,6 +14,14 @@ public sealed class InstrumentAdvice where T : struct { private readonly ReadOnlyCollection? _HistogramBucketBoundaries; + /// + /// Initializes a new instance of the class. + /// + public InstrumentAdvice() + { + Instrument.ValidateTypeParameter(); + } + /// /// Gets the explicit bucket boundaries advised to be used with histogram instruments. /// From 75868494f2b20261c8330518398af6ca7b59bb6e Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 11 Jun 2024 16:45:20 -0700 Subject: [PATCH 11/11] Add InstrumentAdvice ctor to ref. --- .../ref/System.Diagnostics.DiagnosticSourceActivity.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs b/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs index b6d6a352520e7f..e8ddcd2b140ae4 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs @@ -409,6 +409,7 @@ protected Instrument( } public sealed class InstrumentAdvice where T : struct { + public InstrumentAdvice() { throw null; } public System.Collections.Generic.IReadOnlyList? HistogramBucketBoundaries { get { throw null; } init { throw null; } } } public readonly struct Measurement where T : struct