Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address Metrics APIs issues #86740

Merged
merged 6 commits into from
May 26, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
219 changes: 199 additions & 20 deletions src/libraries/Common/src/System/Diagnostics/DiagnosticsHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,62 +7,241 @@ namespace System.Diagnostics
{
internal static class DiagnosticsHelper
{
internal static bool CompareTags(IEnumerable<KeyValuePair<string, object?>>? tags1, IEnumerable<KeyValuePair<string, object?>>? tags2)
// This is similar to System.Linq ToArray. We are not using System.Linq here to avoid the dependency.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the dependency problematic?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The auto-instrumentation can force loading higher version of System.Diagnostics.DiagnosticSource than the version of the one that app built with. Increasing the dependency of System.Diagnostics.DiagnosticSource can increase the risk of breaking the auto-instrumentation. Here is some related issue talking about similar thing #42244

internal static KeyValuePair<string, object?>[]? ToArray(IEnumerable<KeyValuePair<string, object?>>? tags)
tarekgh marked this conversation as resolved.
Show resolved Hide resolved
{
if (tags1 == tags2)
if (tags is null)
{
return null;
}

KeyValuePair<string, object?>[]? array = null;
if (tags is ICollection<KeyValuePair<string, object?>> tagsCol)
{
array = new KeyValuePair<string, object?>[tagsCol.Count];
if (tagsCol is IList<KeyValuePair<string, object?>> secondList)
{
for (int i = 0; i < tagsCol.Count; i++)
{
array[i] = secondList[i];
}

return array;
}
tarekgh marked this conversation as resolved.
Show resolved Hide resolved
}

if (array is null)
{
int count = 0;
using (IEnumerator<KeyValuePair<string, object?>> enumerator = tags.GetEnumerator())
{
while (enumerator.MoveNext())
{
count++;
}
}

array = new KeyValuePair<string, object?>[count];
}
tarekgh marked this conversation as resolved.
Show resolved Hide resolved

Debug.Assert(array is not null);

int index = 0;
using (IEnumerator<KeyValuePair<string, object?>> enumerator = tags.GetEnumerator())
{
while (enumerator.MoveNext())
{
array[index++] = enumerator.Current;
}
}

return array;
}

/// <summary>
/// Compares two tag collections for equality.
/// </summary>
/// <param name="sortedTags">The first collection of tags. it has to be a sorted array</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(KeyValuePair<string, object?>[]? sortedTags, IEnumerable<KeyValuePair<string, object?>>? 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)
// creating with 2 longs which can initially handle 2 * 64 = 128 tags
BitMapper bitMapper = new BitMapper(stackalloc ulong[2], true);

int count = sortedTags.Length;
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.Length)
{
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.Length;
}
}
}

internal ref struct BitMapper
{
private int _maxIndex;
private Span<ulong> _bitMap;

public BitMapper(Span<ulong> bitMap, bool zeroInitialize = false)
tarekgh marked this conversation as resolved.
Show resolved Hide resolved
{
_bitMap = bitMap;
_maxIndex = bitMap.Length * sizeof(long) * 8;

if (zeroInitialize)
{
for (int i = 0; i < _bitMap.Length; i++)
{
_bitMap[i] = 0;
}
}
}

public int MaxIndex => _maxIndex;

private void Expand(int index)
tarekgh marked this conversation as resolved.
Show resolved Hide resolved
{
if (_maxIndex > index)
{
return;
}

int newMax = (index / sizeof(long)) + 10;

Span<ulong> newBitMap = new ulong[newMax];
_bitMap.CopyTo(newBitMap);
_bitMap = newBitMap;
_maxIndex = newMax * sizeof(long);
}

private static void GetIndexAndMask(int index, out int bitIndex, out ulong mask)
{
bitIndex = index >> sizeof(long);
tarekgh marked this conversation as resolved.
Show resolved Hide resolved
int bit = index & (sizeof(long) - 1);
mask = 1UL << bit;
}

public bool SetBit(int index)
{
if (index < 0)
tarekgh marked this conversation as resolved.
Show resolved Hide resolved
{
throw new OutOfMemoryException(nameof(index));
}

if (index >= _maxIndex)
{
Expand(index);
}

GetIndexAndMask(index, out int bitIndex, out ulong mask);
ulong value = _bitMap[bitIndex];
_bitMap[bitIndex] = value | mask;
return true;
}

public bool IsSet(int index)
{
if (index < 0)
{
throw new OutOfMemoryException(nameof(index));
}

if (index >= _maxIndex)
{
return false;
}

GetIndexAndMask(index, out int bitIndex, out ulong mask);
ulong value = _bitMap[bitIndex];
return ((value & mask) != 0);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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!; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -32,7 +31,7 @@ public static Meter Create(this IMeterFactory meterFactory, string name, string?
{
Version = version,
Tags = tags,
Scope = scope
Scope = meterFactory
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
});
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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!; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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() { }
Expand All @@ -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)
Expand All @@ -31,23 +36,23 @@ 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 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);
FactoryMeter m = new FactoryMeter(options.Name, options.Version, options.Tags, scope: this);
tarekgh marked this conversation as resolved.
Show resolved Hide resolved
meterList.Add(m);
return m;
}
Expand All @@ -64,16 +69,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.
}
}
}
Loading