Address Metrics APIs issues

src/libraries/Common/src/System/Diagnostics/DiagnosticsHelper.cs

@@ -7,62 +7,154 @@ 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;
+                            }
                         }
                     }
 
                     return true;
                 }
             }
 
-            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);
+        }
+    }
 }

src/libraries/Microsoft.Bcl.TimeProvider/src/System/Threading/Tasks/TimeProviderTaskExtensions.cs

@@ -96,7 +96,7 @@ public static Task Delay(this TimeProvider timeProvider, TimeSpan delay, Cancell
 
             // 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),

src/libraries/Microsoft.Extensions.Diagnostics.Abstractions/ref/Microsoft.Extensions.Diagnostics.Abstractions.cs

@@ -7,4 +7,8 @@ public interface IMeterFactory : System.IDisposable
     {
         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!; }
+    }
 }

src/libraries/Microsoft.Extensions.Diagnostics/src/Metrics/MeterFactoryExtensions.cs → 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 @@ public static Meter Create(this IMeterFactory meterFactory, string name, string?
             {
                 Version = version,
                 Tags = tags,
-                Scope = scope
+                Scope = meterFactory
             });
         }
     }

src/libraries/Microsoft.Extensions.Diagnostics/ref/Microsoft.Extensions.Diagnostics.cs

@@ -7,8 +7,4 @@ public static class MetricsServiceExtensions
     {
         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!; }
-    }
 }

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 @@ public Meter Create(MeterOptions options)
                 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,23 +36,27 @@ public Meter Create(MeterOptions options)
                     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;
                         }
                     }
                 }
                 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,16 +73,31 @@ public void Dispose()
 
                 _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();
                     }
                 }
 
                 _cachedMeters.Clear();
             }
         }
     }
+
+    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.
+        }
+    }
 }

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>