Address Metrics APIs issues (#86740)

diff --git a/src/libraries/Common/src/System/Diagnostics/DiagnosticsHelper.cs b/src/libraries/Common/src/System/Diagnostics/DiagnosticsHelper.cs
index 6d018ef..18f10c0 100644 (file)
--- a/src/libraries/Common/src/System/Diagnostics/DiagnosticsHelper.cs
+++ b/src/libraries/Common/src/System/Diagnostics/DiagnosticsHelper.cs
@@ -7,35 +7,67 @@ namespace System.Diagnostics
 {
     internal static class DiagnosticsHelper
     {
-        internal static bool CompareTags(IEnumerable<KeyValuePair<string, object?>>? tags1, IEnumerable<KeyValuePair<string, object?>>? tags2)
+        /// <summary>
+        /// Compares two tag collections for equality.
+        /// </summary>
+        /// <param name="sortedTags">The first collection of tags. it has to be a sorted List</param>
+        /// <param name="tags2">The second collection of tags. This one doesn't have to be sorted nor be specific collection type</param>
+        /// <returns>True if the two collections are equal, false otherwise</returns>
+        /// <remarks>
+        /// This method is used to compare two collections of tags for equality. The first collection is expected to be a sorted array
+        /// of tags. The second collection can be any collection of tags.
+        /// we avoid the allocation of a new array by using the second collection as is and not converting it to an array. the reason
+        /// is we call this every time we try to create a meter or instrument and we don't want to allocate a new array every time.
+        /// </remarks>
+        internal static bool CompareTags(List<KeyValuePair<string, object?>>? sortedTags, IEnumerable<KeyValuePair<string, object?>>? tags2)
         {
-            if (tags1 == tags2)
+            if (sortedTags == tags2)
             {
                 return true;
             }
 
-            if (tags1 is null || tags2 is null)
+            if (sortedTags is null || tags2 is null)
             {
                 return false;
             }
 
-            if (tags1 is ICollection<KeyValuePair<string, object?>> firstCol && tags2 is ICollection<KeyValuePair<string, object?>> secondCol)
+            int count = sortedTags.Count;
+            int size = count / (sizeof(long) * 8) + 1;
+            BitMapper bitMapper = new BitMapper(size <= 100 ? stackalloc ulong[size] : new ulong[size]);
+
+            if (tags2 is ICollection<KeyValuePair<string, object?>> tagsCol)
             {
-                int count = firstCol.Count;
-                if (count != secondCol.Count)
+                if (tagsCol.Count != count)
                 {
                     return false;
                 }
 
-                if (firstCol is IList<KeyValuePair<string, object?>> firstList && secondCol is IList<KeyValuePair<string, object?>> secondList)
+                if (tagsCol is IList<KeyValuePair<string, object?>> secondList)
                 {
                     for (int i = 0; i < count; i++)
                     {
-                        KeyValuePair<string, object?> pair1 = firstList[i];
-                        KeyValuePair<string, object?> pair2 = secondList[i];
-                        if (pair1.Key != pair2.Key || !object.Equals(pair1.Value, pair2.Value))
+                        KeyValuePair<string, object?> pair = secondList[i];
+
+                        for (int j = 0; j < count; j++)
                         {
-                            return false;
+                            if (bitMapper.IsSet(j))
+                            {
+                                continue;
+                            }
+
+                            KeyValuePair<string, object?> pair1 = sortedTags[j];
+
+                            int compareResult = string.CompareOrdinal(pair.Key, pair1.Key);
+                            if (compareResult == 0 && object.Equals(pair.Value, pair1.Value))
+                            {
+                                bitMapper.SetBit(j);
+                                break;
+                            }
+
+                            if (compareResult < 0 || j == count - 1)
+                            {
+                                return false;
+                            }
                         }
                     }
 
@@ -43,26 +75,86 @@ namespace System.Diagnostics
                 }
             }
 
-            using (IEnumerator<KeyValuePair<string, object?>> e1 = tags1.GetEnumerator())
-            using (IEnumerator<KeyValuePair<string, object?>> e2 = tags2.GetEnumerator())
+            int listCount = 0;
+            using (IEnumerator<KeyValuePair<string, object?>> enumerator = tags2.GetEnumerator())
             {
-                while (e1.MoveNext())
+                while (enumerator.MoveNext())
                 {
-                    KeyValuePair<string, object?> pair1 = e1.Current;
-                    if (!e2.MoveNext())
+                    listCount++;
+                    if (listCount > sortedTags.Count)
                     {
                         return false;
                     }
 
-                    KeyValuePair<string, object?> pair2 = e2.Current;
-                    if (pair1.Key != pair2.Key || !object.Equals(pair1.Value, pair2.Value))
+                    KeyValuePair<string, object?> pair = enumerator.Current;
+                    for (int j = 0; j < count; j++)
                     {
-                        return false;
+                        if (bitMapper.IsSet(j))
+                        {
+                            continue;
+                        }
+
+                        KeyValuePair<string, object?> pair1 = sortedTags[j];
+
+                        int compareResult = string.CompareOrdinal(pair.Key, pair1.Key);
+                        if (compareResult == 0 && object.Equals(pair.Value, pair1.Value))
+                        {
+                            bitMapper.SetBit(j);
+                            break;
+                        }
+
+                        if (compareResult < 0 || j == count - 1)
+                        {
+                            return false;
+                        }
                     }
                 }
 
-                return !e2.MoveNext();
+                return listCount == sortedTags.Count;
             }
         }
     }
+
+    internal ref struct BitMapper
+    {
+        private int _maxIndex;
+        private Span<ulong> _bitMap;
+
+        public BitMapper(Span<ulong> bitMap)
+        {
+            _bitMap = bitMap;
+            _bitMap.Clear();
+            _maxIndex = bitMap.Length * sizeof(long) * 8;
+        }
+
+        public int MaxIndex => _maxIndex;
+
+        private static void GetIndexAndMask(int index, out int bitIndex, out ulong mask)
+        {
+            bitIndex = index >> 6;
+            int bit = index & (sizeof(long) * 8 - 1);
+            mask = 1UL << bit;
+        }
+
+        public bool SetBit(int index)
+        {
+            Debug.Assert(index >= 0);
+            Debug.Assert(index < _maxIndex);
+
+            GetIndexAndMask(index, out int bitIndex, out ulong mask);
+            ulong value = _bitMap[bitIndex];
+            _bitMap[bitIndex] = value | mask;
+            return true;
+        }
+
+        public bool IsSet(int index)
+        {
+            Debug.Assert(index >= 0);
+            Debug.Assert(index < _maxIndex);
+
+            GetIndexAndMask(index, out int bitIndex, out ulong mask);
+            ulong value = _bitMap[bitIndex];
+            return ((value & mask) != 0);
+        }
+    }
 }

diff --git a/src/libraries/Microsoft.Bcl.TimeProvider/src/System/Threading/Tasks/TimeProviderTaskExtensions.cs b/src/libraries/Microsoft.Bcl.TimeProvider/src/System/Threading/Tasks/TimeProviderTaskExtensions.cs
index 9f8ba32..e410f4a 100644 (file)
--- a/src/libraries/Microsoft.Bcl.TimeProvider/src/System/Threading/Tasks/TimeProviderTaskExtensions.cs
+++ b/src/libraries/Microsoft.Bcl.TimeProvider/src/System/Threading/Tasks/TimeProviderTaskExtensions.cs
@@ -96,7 +96,7 @@ namespace System.Threading.Tasks
 
             // There are race conditions where the timer fires after we have attached the cancellation callback but before the
             // registration is stored in state.Registration, or where cancellation is requested prior to the registration being
-            // stored into state.Registration, or where the timer could fire after it's been createdbut before it's been stored
+            // stored into state.Registration, or where the timer could fire after it's been created but before it's been stored
             // in state.Timer. In such cases, the cancellation registration and/or the Timer might be stored into state after the
             // callbacks and thus left undisposed.  So, we do a subsequent check here. If the task isn't completed by this point,
             // then the callbacks won't have called TrySetResult (the callbacks invoke TrySetResult before disposing of the fields),

diff --git a/src/libraries/Microsoft.Extensions.Diagnostics.Abstractions/ref/Microsoft.Extensions.Diagnostics.Abstractions.cs b/src/libraries/Microsoft.Extensions.Diagnostics.Abstractions/ref/Microsoft.Extensions.Diagnostics.Abstractions.cs
index 121f454..3853005 100644 (file)
--- a/src/libraries/Microsoft.Extensions.Diagnostics.Abstractions/ref/Microsoft.Extensions.Diagnostics.Abstractions.cs
+++ b/src/libraries/Microsoft.Extensions.Diagnostics.Abstractions/ref/Microsoft.Extensions.Diagnostics.Abstractions.cs
@@ -7,4 +7,8 @@ namespace Microsoft.Extensions.Diagnostics.Metrics
     {
         System.Diagnostics.Metrics.Meter Create(System.Diagnostics.Metrics.MeterOptions options);
     }
+    public static class MeterFactoryExtensions
+    {
+        public static System.Diagnostics.Metrics.Meter Create(this Microsoft.Extensions.Diagnostics.Metrics.IMeterFactory meterFactory, string name, string? version = null, System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string, object?>>? tags = null) { return null!; }
+    }
 }
\ No newline at end of file

diff --git a/src/libraries/Microsoft.Extensions.Diagnostics/src/Metrics/MeterFactoryExtensions.cs b/src/libraries/Microsoft.Extensions.Diagnostics.Abstractions/src/Metrics/MeterFactoryExtensions.cs
similarity index 77%
rename from src/libraries/Microsoft.Extensions.Diagnostics/src/Metrics/MeterFactoryExtensions.cs
rename to src/libraries/Microsoft.Extensions.Diagnostics.Abstractions/src/Metrics/MeterFactoryExtensions.cs
index ad1e82e..5d744f9 100644 (file)
--- a/src/libraries/Microsoft.Extensions.Diagnostics/src/Metrics/MeterFactoryExtensions.cs
+++ b/src/libraries/Microsoft.Extensions.Diagnostics.Abstractions/src/Metrics/MeterFactoryExtensions.cs
@@ -13,15 +13,14 @@ namespace Microsoft.Extensions.Diagnostics.Metrics
     public static class MeterFactoryExtensions
     {
         /// <summary>
-        /// Creates a <see cref="Meter" /> with the specified <paramref name="name" />, <paramref name="version" />, <paramref name="tags" />, and <paramref name="scope" />.
+        /// Creates a <see cref="Meter" /> with the specified <paramref name="name" />, <paramref name="version" />, and <paramref name="tags" />.
         /// </summary>
         /// <param name="meterFactory">The <see cref="IMeterFactory" /> to use to create the <see cref="Meter" />.</param>
         /// <param name="name">The name of the <see cref="Meter" />.</param>
         /// <param name="version">The version of the <see cref="Meter" />.</param>
         /// <param name="tags">The tags to associate with the <see cref="Meter" />.</param>
-        /// <param name="scope">The scope to associate with the <see cref="Meter" />.</param>
-        /// <returns>A <see cref="Meter" /> with the specified <paramref name="name" />, <paramref name="version" />, <paramref name="tags" />, and <paramref name="scope" />.</returns>
-        public static Meter Create(this IMeterFactory meterFactory, string name, string? version = null, IEnumerable<KeyValuePair<string, object?>>? tags = null, object? scope = null)
+        /// <returns>A <see cref="Meter" /> with the specified <paramref name="name" />, <paramref name="version" />, and <paramref name="tags" />.</returns>
+        public static Meter Create(this IMeterFactory meterFactory, string name, string? version = null, IEnumerable<KeyValuePair<string, object?>>? tags = null)
         {
             if (meterFactory is null)
             {
@@ -32,7 +31,7 @@ namespace Microsoft.Extensions.Diagnostics.Metrics
             {
                 Version = version,
                 Tags = tags,
-                Scope = scope
+                Scope = meterFactory
             });
         }
     }

diff --git a/src/libraries/Microsoft.Extensions.Diagnostics/ref/Microsoft.Extensions.Diagnostics.cs b/src/libraries/Microsoft.Extensions.Diagnostics/ref/Microsoft.Extensions.Diagnostics.cs
index 6e034a5..52fa2fc 100644 (file)
--- a/src/libraries/Microsoft.Extensions.Diagnostics/ref/Microsoft.Extensions.Diagnostics.cs
+++ b/src/libraries/Microsoft.Extensions.Diagnostics/ref/Microsoft.Extensions.Diagnostics.cs
@@ -7,8 +7,4 @@ namespace Microsoft.Extensions.Diagnostics.Metrics
     {
         public static Microsoft.Extensions.DependencyInjection.IServiceCollection AddMetrics(this Microsoft.Extensions.DependencyInjection.IServiceCollection services) { return null!; }
     }
-    public static class MeterFactoryExtensions
-    {
-        public static System.Diagnostics.Metrics.Meter Create(this Microsoft.Extensions.Diagnostics.Metrics.IMeterFactory meterFactory, string name, string? version = null, System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string, object?>>? tags = null,  object? scope = null) { return null!; }
-    }
 }
\ No newline at end of file

diff --git a/src/libraries/Microsoft.Extensions.Diagnostics/src/Metrics/DefaultMeterFactory.cs b/src/libraries/Microsoft.Extensions.Diagnostics/src/Metrics/DefaultMeterFactory.cs
index c6ceaee..dd1be2d 100644 (file)
--- a/src/libraries/Microsoft.Extensions.Diagnostics/src/Metrics/DefaultMeterFactory.cs
+++ b/src/libraries/Microsoft.Extensions.Diagnostics/src/Metrics/DefaultMeterFactory.cs
@@ -10,7 +10,7 @@ namespace Microsoft.Extensions.Diagnostics.Metrics
 {
     internal sealed class DefaultMeterFactory : IMeterFactory
     {
-        private readonly Dictionary<string, List<Meter>> _cachedMeters = new();
+        private readonly Dictionary<string, List<FactoryMeter>> _cachedMeters = new();
         private bool _disposed;
 
         public DefaultMeterFactory() { }
@@ -22,6 +22,11 @@ namespace Microsoft.Extensions.Diagnostics.Metrics
                 throw new ArgumentNullException(nameof(options));
             }
 
+            if (options.Scope is not null && !object.ReferenceEquals(options.Scope, this))
+            {
+                throw new InvalidOperationException(SR.InvalidScope);
+            }
+
             Debug.Assert(options.Name is not null);
 
             lock (_cachedMeters)
@@ -31,11 +36,11 @@ namespace Microsoft.Extensions.Diagnostics.Metrics
                     throw new ObjectDisposedException(nameof(DefaultMeterFactory));
                 }
 
-                if (_cachedMeters.TryGetValue(options.Name, out List<Meter>? meterList))
+                if (_cachedMeters.TryGetValue(options.Name, out List<FactoryMeter>? meterList))
                 {
                     foreach (Meter meter in meterList)
                     {
-                        if (meter.Version == options.Version && DiagnosticsHelper.CompareTags(meter.Tags, options.Tags))
+                        if (meter.Version == options.Version && DiagnosticsHelper.CompareTags(meter.Tags as List<KeyValuePair<string, object?>>, options.Tags))
                         {
                             return meter;
                         }
@@ -43,11 +48,15 @@ namespace Microsoft.Extensions.Diagnostics.Metrics
                 }
                 else
                 {
-                    meterList = new List<Meter>();
+                    meterList = new List<FactoryMeter>();
                     _cachedMeters.Add(options.Name, meterList);
                 }
 
-                Meter m = new Meter(options.Name, options.Version, options.Tags, scope: this);
+                object? scope = options.Scope;
+                options.Scope = this;
+                FactoryMeter m = new FactoryMeter(options.Name, options.Version, options.Tags, scope: this);
+                options.Scope = scope;
+
                 meterList.Add(m);
                 return m;
             }
@@ -64,11 +73,11 @@ namespace Microsoft.Extensions.Diagnostics.Metrics
 
                 _disposed = true;
 
-                foreach (List<Meter> meterList in _cachedMeters.Values)
+                foreach (List<FactoryMeter> meterList in _cachedMeters.Values)
                 {
-                    foreach (Meter meter in meterList)
+                    foreach (FactoryMeter meter in meterList)
                     {
-                        meter.Dispose();
+                        meter.Release();
                     }
                 }
 
@@ -76,4 +85,19 @@ namespace Microsoft.Extensions.Diagnostics.Metrics
             }
         }
     }
+
+    internal sealed class FactoryMeter : Meter
+    {
+        public FactoryMeter(string name, string? version, IEnumerable<KeyValuePair<string, object?>>? tags, object? scope)
+            : base(name, version, tags, scope)
+        {
+        }
+
+        public void Release() => base.Dispose(true); // call the protected Dispose(bool)
+
+        protected override void Dispose(bool disposing)
+        {
+            // no-op, disallow users from disposing of the meters created from the factory.
+        }
+    }
 }

diff --git a/src/libraries/Microsoft.Extensions.Diagnostics/src/Resources/Strings.resx b/src/libraries/Microsoft.Extensions.Diagnostics/src/Resources/Strings.resx
new file mode 100644 (file)
index 0000000..ab0d5ca
--- /dev/null
+++ b/src/libraries/Microsoft.Extensions.Diagnostics/src/Resources/Strings.resx
@@ -0,0 +1,63 @@
+<root>
+  <xsd:schema id="root" xmlns="" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:msdata="urn:schemas-microsoft-com:xml-msdata">
+    <xsd:import namespace="http://www.w3.org/XML/1998/namespace" />
+    <xsd:element name="root" msdata:IsDataSet="true">
+      <xsd:complexType>
+        <xsd:choice maxOccurs="unbounded">
+          <xsd:element name="metadata">
+            <xsd:complexType>
+              <xsd:sequence>
+                <xsd:element name="value" type="xsd:string" minOccurs="0" />
+              </xsd:sequence>
+              <xsd:attribute name="name" use="required" type="xsd:string" />
+              <xsd:attribute name="type" type="xsd:string" />
+              <xsd:attribute name="mimetype" type="xsd:string" />
+              <xsd:attribute ref="xml:space" />
+            </xsd:complexType>
+          </xsd:element>
+          <xsd:element name="assembly">
+            <xsd:complexType>
+              <xsd:attribute name="alias" type="xsd:string" />
+              <xsd:attribute name="name" type="xsd:string" />
+            </xsd:complexType>
+          </xsd:element>
+          <xsd:element name="data">
+            <xsd:complexType>
+              <xsd:sequence>
+                <xsd:element name="value" type="xsd:string" minOccurs="0" msdata:Ordinal="1" />
+                <xsd:element name="comment" type="xsd:string" minOccurs="0" msdata:Ordinal="2" />
+              </xsd:sequence>
+              <xsd:attribute name="name" type="xsd:string" use="required" msdata:Ordinal="1" />
+              <xsd:attribute name="type" type="xsd:string" msdata:Ordinal="3" />
+              <xsd:attribute name="mimetype" type="xsd:string" msdata:Ordinal="4" />
+              <xsd:attribute ref="xml:space" />
+            </xsd:complexType>
+          </xsd:element>
+          <xsd:element name="resheader">
+            <xsd:complexType>
+              <xsd:sequence>
+                <xsd:element name="value" type="xsd:string" minOccurs="0" msdata:Ordinal="1" />
+              </xsd:sequence>
+              <xsd:attribute name="name" type="xsd:string" use="required" />
+            </xsd:complexType>
+          </xsd:element>
+        </xsd:choice>
+      </xsd:complexType>
+    </xsd:element>
+  </xsd:schema>
+  <resheader name="resmimetype">
+    <value>text/microsoft-resx</value>
+  </resheader>
+  <resheader name="version">
+    <value>2.0</value>
+  </resheader>
+  <resheader name="reader">
+    <value>System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
+  </resheader>
+  <resheader name="writer">
+    <value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
+  </resheader>
+  <data name="InvalidScope" xml:space="preserve">
+    <value>The meter factory does not allow a custom scope value when creating a meter.</value>
+  </data>
+</root>

diff --git a/src/libraries/Microsoft.Extensions.Diagnostics/tests/MetricsTests.cs b/src/libraries/Microsoft.Extensions.Diagnostics/tests/MetricsTests.cs
index bb9e91d..cfda92c 100644 (file)
--- a/src/libraries/Microsoft.Extensions.Diagnostics/tests/MetricsTests.cs
+++ b/src/libraries/Microsoft.Extensions.Diagnostics/tests/MetricsTests.cs
@@ -49,6 +49,47 @@ namespace Microsoft.Extensions.Diagnostics.Metrics.Tests
             Assert.Same(meterFactory, meter3.Scope);
 
             Assert.NotSame(meter1, meter3);
+
+            //
+            // Test meter creation with unordered tags
+            //
+
+            object o = new object();
+            TagList l1 = new TagList() { { "z", "a" }, { "y", "b" }, { "x", "c" }, { "w", o }, { "N", null }, { "c", "d" }, { "q", "d" }, { "c", null } };
+            List<KeyValuePair<string, object?>> l2 = new List<KeyValuePair<string, object?>>()
+            {
+                new KeyValuePair<string, object?>("y", "b"), new KeyValuePair<string, object?>("c", null), new KeyValuePair<string, object?>("N", null),
+                new KeyValuePair<string, object?>("x", "c"), new KeyValuePair<string, object?>("w", o), new KeyValuePair<string, object?>("z", "a"),
+                new KeyValuePair<string, object?>("c", "d"), new KeyValuePair<string, object?>("q", "d")
+            };
+            HashSet<KeyValuePair<string, object?>> l3 = new HashSet<KeyValuePair<string, object?>>()
+            {
+                new KeyValuePair<string, object?>("q", "d"), new KeyValuePair<string, object?>("c", null), new KeyValuePair<string, object?>("N", null),
+                new KeyValuePair<string, object?>("w", o), new KeyValuePair<string, object?>("c", "d"), new KeyValuePair<string, object?>("x", "c"),
+                new KeyValuePair<string, object?>("z", "a"), new KeyValuePair<string, object?>("y", "b")
+            };
+
+            Meter meter4 = meterFactory.Create("name4", "4", l1);
+            Meter meter5 = meterFactory.Create("name4", "4", l2);
+            Meter meter6 = meterFactory.Create("name4", "4", l3);
+
+            Assert.Same(meter4, meter5);
+            Assert.Same(meter4, meter6);
+
+            KeyValuePair<string, object?>[] t1 = meter4.Tags.ToArray();
+            Assert.Equal(l1.Count, t1.Length);
+            t1[0] = new KeyValuePair<string, object?>(t1[0].Key, "newValue"); // change value of one item;
+            Meter meter7 = meterFactory.Create("name4", "4", t1);
+            Assert.NotSame(meter4, meter7);
+
+            //
+            // Ensure the tags in the meter are sorted
+            //
+            t1 = meter4.Tags.ToArray();
+            for (int i = 0; i < t1.Length - 1; i++)
+            {
+                Assert.True(string.Compare(t1[i].Key, t1[i + 1].Key, StringComparison.Ordinal) <= 0);
+            }
         }
 
         [Fact]
@@ -60,6 +101,41 @@ namespace Microsoft.Extensions.Diagnostics.Metrics.Tests
             using IMeterFactory meterFactory = sp.GetRequiredService<IMeterFactory>();
 
             Assert.Throws<ArgumentNullException>(() => meterFactory.Create(name: null));
+            Assert.Throws<InvalidOperationException>(() => meterFactory.Create(new MeterOptions("name") { Name = "SomeName", Scope = new object() }));
+
+            Meter meter = meterFactory.Create(new MeterOptions("name") { Name = "SomeName", Scope = meterFactory });
+            Assert.Equal(meterFactory, meter.Scope);
+        }
+
+        [Fact]
+        public void MeterDisposeTest()
+        {
+            ServiceCollection services = new ServiceCollection();
+            services.AddMetrics();
+            var sp = services.BuildServiceProvider();
+            IMeterFactory meterFactory = sp.GetRequiredService<IMeterFactory>();
+
+            Meter meter = meterFactory.Create("DisposableMeter");
+
+            Counter<int> counter = meter.CreateCounter<int>("MyCounter");
+            InstrumentRecorder<int> recorder = new InstrumentRecorder<int>(counter);
+            counter.Add(10);
+            Assert.Equal(1, recorder.GetMeasurements().Count());
+            Assert.Equal(10, recorder.GetMeasurements().ElementAt(0).Value);
+            meter.Dispose(); // should be no-op
+            counter.Add(20);
+            Assert.Equal(2, recorder.GetMeasurements().Count());
+            Assert.Equal(20, recorder.GetMeasurements().ElementAt(1).Value);
+
+            meter.Dispose(); // dispose again, should be no-op too
+            counter.Add(30);
+            Assert.Equal(3, recorder.GetMeasurements().Count());
+            Assert.Equal(30, recorder.GetMeasurements().ElementAt(2).Value);
+
+            // Now dispose the factory, the meter should be disposed too
+            meterFactory.Dispose();
+            counter.Add(40); // recorder shouldn't observe this value as the meter created this instrument is disposed
+            Assert.Equal(3, recorder.GetMeasurements().Count());
         }
 
         [Fact]

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 17d2489..235c490 100644 (file)
--- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj
+++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj
@@ -126,7 +126,6 @@ System.Diagnostics.DiagnosticSource</PackageDescription>
     <Reference Include="System.Collections" />
     <Reference Include="System.Collections.Concurrent" />
     <Reference Include="System.Diagnostics.Tracing" />
-    <Reference Include="System.Linq" />
     <Reference Include="System.Memory" />
     <Reference Include="System.Runtime" />
     <Reference Include="System.Runtime.InteropServices" />

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 be33619..0cb50f5 100644 (file)
--- 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,7 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Collections.Generic;
-using System.Linq;
 
 namespace System.Diagnostics.Metrics
 {
@@ -54,9 +53,9 @@ namespace System.Diagnostics.Metrics
             Unit = unit;
             if (tags is not null)
             {
-                var tagsArray = tags.ToArray();
-                // Array.Sort(tagsArray, (left, right) => string.Compare(left.Key, right.Key, StringComparison.Ordinal));
-                Tags = tagsArray;
+                var tagList = new List<KeyValuePair<string, object?>>(tags);
+                tagList.Sort((left, right) => string.Compare(left.Key, right.Key, StringComparison.Ordinal));
+                Tags = tagList;
             }
         }
 

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 6fe7ac1..a5722aa 100644 (file)
--- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Meter.cs
+++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Meter.cs
@@ -3,7 +3,6 @@
 
 using System.Collections.Generic;
 using System.Diagnostics;
-using System.Linq;
 using System.Threading;
 
 namespace System.Diagnostics.Metrics
@@ -72,9 +71,9 @@ namespace System.Diagnostics.Metrics
             Version = version;
             if (tags is not null)
             {
-                var tagsArray = tags.ToArray();
-                // Array.Sort(tagsArray, (left, right) => string.Compare(left.Key, right.Key, StringComparison.Ordinal));
-                Tags = tagsArray;
+                var tagList = new List<KeyValuePair<string, object?>>(tags);
+                tagList.Sort((left, right) => string.Compare(left.Key, right.Key, StringComparison.Ordinal));
+                Tags = tagList;
             }
             Scope = scope;
 
@@ -462,7 +461,7 @@ namespace System.Diagnostics.Metrics
             foreach (Instrument instrument in instrumentList)
             {
                 if (instrument.GetType() == instrumentType && instrument.Unit == unit &&
-                    instrument.Description == description && DiagnosticsHelper.CompareTags(instrument.Tags, tags))
+                    instrument.Description == description && DiagnosticsHelper.CompareTags(instrument.Tags as List<KeyValuePair<string, object?>>, tags))
                 {
                     return instrument;
                 }

diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs
index aafbbed..f6d86ef 100644 (file)
--- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs
+++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs
@@ -1221,6 +1221,16 @@ namespace System.Diagnostics.Metrics.Tests
                 Assert.Equal("8.0", meter8.Version);
                 Assert.Equal(new[] { new KeyValuePair<string, object?>("Key8", "Value8") }, meter8.Tags);
                 Assert.Equal("Scope8", meter8.Scope);
+
+                // Test tags sorting order
+                TagList l = new TagList() { { "f", "a" }, { "d", "b" }, { "w", "b" }, { "h", new object() }, { "N", null }, { "a", "b" }, { "a", null } };
+                using Meter meter9 = new Meter(new MeterOptions("TestMeterCreationWithOptions9") { Version = "8.0", Tags = l, Scope = "Scope8" });
+                var insArray = meter9.Tags.ToArray();
+                Assert.Equal(l.Count, insArray.Length);
+                for (int i = 0; i < insArray.Length - 1; i++)
+                {
+                    Assert.True(string.Compare(insArray[i].Key, insArray[i + 1].Key, StringComparison.Ordinal) <= 0);
+                }
             }).Dispose();
         }
 
@@ -1297,6 +1307,38 @@ namespace System.Diagnostics.Metrics.Tests
                 UpDownCounter<int> upDownCounter3 = meter.CreateUpDownCounter<int>("name", null, null, list3);
 
                 Assert.False(object.ReferenceEquals(upDownCounter3, upDownCounter1));
+
+                //
+                // Test instrument creation with unordered tags
+                //
+
+                object o = new object();
+                TagList l1 = new TagList() { { "f", "a" }, { "d", "b" }, { "w", "b" }, { "h", o}, { "N", null }, { "a", "b" }, { "a", null } };
+                List<KeyValuePair<string, object?>> l2 = new List<KeyValuePair<string, object?>>()
+                {
+                    new KeyValuePair<string, object?>("w", "b"), new KeyValuePair<string, object?>("h", o), new KeyValuePair<string, object?>("a", null),
+                    new KeyValuePair<string, object?>("d", "b"), new KeyValuePair<string, object?>("f", "a"), new KeyValuePair<string, object?>("N", null),
+                    new KeyValuePair<string, object?>("a", "b")
+                };
+                HashSet<KeyValuePair<string, object?>> l3 = new HashSet<KeyValuePair<string, object?>>()
+                {
+                    new KeyValuePair<string, object?>("d", "b"), new KeyValuePair<string, object?>("f", "a"), new KeyValuePair<string, object?>("a", null),
+                    new KeyValuePair<string, object?>("w", "b"), new KeyValuePair<string, object?>("h", o), new KeyValuePair<string, object?>("a", "b"),
+                    new KeyValuePair<string, object?>("N", null)
+                };
+
+                Counter<int> counter9 = meter.CreateCounter<int>("name9", null, null, l1);
+                Counter<int> counter10 = meter.CreateCounter<int>("name9", null, null, l2);
+                Counter<int> counter11 = meter.CreateCounter<int>("name9", null, null, l3);
+                Assert.Same(counter9, counter10);
+                Assert.Same(counter9, counter11);
+
+                KeyValuePair<string, object?>[] t1 = counter9.Tags.ToArray();
+                Assert.Equal(l1.Count, t1.Length);
+                t1[0] = new KeyValuePair<string, object?>(t1[0].Key, "newValue"); // change value of one item;
+                Counter<int> counter12 = meter.CreateCounter<int>("name9", null, null, t1);
+                Assert.NotSame(counter9, counter12);
+
             }).Dispose();
         }
 
@@ -1342,6 +1384,17 @@ namespace System.Diagnostics.Metrics.Tests
                 Instrument ins12 = meter.CreateObservableGauge<float>("ObservableUpDownCounter3", () => new Measurement<float>[] { new Measurement<float>(3) }, null, null, new TagList() { { "oudc3", "oudc-v3" } });
                 Assert.Equal(new[] { new KeyValuePair<string, object?>("oudc3", "oudc-v3") }, ins12.Tags);
 
+                // Test tags sorting order
+
+                TagList l = new TagList() { { "z", "a" }, { "y", "b" }, { "x", "b" }, { "m", new object() }, { "N", null }, { "a", "b" }, { "a", null } };
+                Instrument ins13 = meter.CreateCounter<int>("counter", null, null, l);
+                var insArray = ins13.Tags.ToArray();
+                Assert.Equal(l.Count, insArray.Length);
+                for (int i = 0; i < insArray.Length - 1; i++)
+                {
+                    Assert.True(string.Compare(insArray[i].Key, insArray[i + 1].Key, StringComparison.Ordinal) <= 0);
+                }
+
             }).Dispose();
         }
 
@@ -1454,10 +1507,6 @@ namespace System.Diagnostics.Metrics.Tests
                 Assert.True(recorder7.GetMeasurements().Same(new Measurement<long>[] { measurementWith10Value, measurementWith10Value }));
                 Assert.True(recorder7.GetMeasurements(true).Same(new Measurement<long>[] { measurementWith10Value, measurementWith10Value, measurementWith10Value }));
                 Assert.True(recorder7.GetMeasurements().Same(new Measurement<long>[] { measurementWith10Value }));
-                // Assert.Equal(new Measurement<long>[] { measurementWith10Value, measurementWith10Value }, recorder7.GetMeasurements());
-                // Assert.Equal(new Measurement<long>[] { measurementWith10Value, measurementWith10Value, measurementWith10Value }, recorder7.GetMeasurements(clear: true));
-                // Assert.Equal(new Measurement<long>[] { measurementWith10Value }, recorder7.GetMeasurements());
-
             }).Dispose();
         }