'Counter<T>' and 'Instrument<T>' APIs are too redundant

[julealgon]

I started working with Counter<T> recently and noticed that its API surface is very redundant, with lots of overloads that do "basically the same thing" but with additional data attached to the measurement:

runtime/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Counter.cs

Lines 26 to 75 in 65008a9

	/// <summary>
	/// Record the increment value of the measurement.
	/// </summary>
	/// <param name="delta">The increment measurement.</param>
	public void Add(T delta) => RecordMeasurement(delta);
	/// <summary>
	/// Record the increment value of the measurement.
	/// </summary>
	/// <param name="delta">The increment measurement.</param>
	/// <param name="tag">A key-value pair tag associated with the measurement.</param>
	public void Add(T delta, KeyValuePair<string, object?> tag) => RecordMeasurement(delta, tag);
	/// <summary>
	/// Record the increment value of the measurement.
	/// </summary>
	/// <param name="delta">The increment measurement.</param>
	/// <param name="tag1">A first key-value pair tag associated with the measurement.</param>
	/// <param name="tag2">A second key-value pair tag associated with the measurement.</param>
	public void Add(T delta, KeyValuePair<string, object?> tag1, KeyValuePair<string, object?> tag2) => RecordMeasurement(delta, tag1, tag2);
	/// <summary>
	/// Record the increment value of the measurement.
	/// </summary>
	/// <param name="delta">The increment measurement.</param>
	/// <param name="tag1">A first key-value pair tag associated with the measurement.</param>
	/// <param name="tag2">A second key-value pair tag associated with the measurement.</param>
	/// <param name="tag3">A third key-value pair tag associated with the measurement.</param>
	public void Add(T delta, KeyValuePair<string, object?> tag1, KeyValuePair<string, object?> tag2, KeyValuePair<string, object?> tag3) => RecordMeasurement(delta, tag1, tag2, tag3);
	/// <summary>
	/// Record the increment value of the measurement.
	/// </summary>
	/// <param name="delta">The increment measurement.</param>
	/// <param name="tags">A span of key-value pair tags associated with the measurement.</param>
	public void Add(T delta, ReadOnlySpan<KeyValuePair<string, object?>> tags) => RecordMeasurement(delta, tags);
	/// <summary>
	/// Record the increment value of the measurement.
	/// </summary>
	/// <param name="delta">The increment measurement.</param>
	/// <param name="tags">A list of key-value pair tags associated with the measurement.</param>
	public void Add(T delta, params KeyValuePair<string, object?>[] tags) => RecordMeasurement(delta, tags.AsSpan());
	/// <summary>
	/// Record the increment value of the measurement.
	/// </summary>
	/// <param name="delta">The measurement value.</param>
	/// <param name="tagList">A <see cref="T:System.Diagnostics.TagList" /> of tags associated with the measurement.</param>
	public void Add(T delta, in TagList tagList) => RecordMeasurement(delta, in tagList);

As we can see, the implementation basically redirects every call to a similar abstraction in the Instrument<T> abstract class:

    public abstract class Instrument<T> : Instrument where T : struct
    {
        protected void RecordMeasurement(T measurement)

        protected void RecordMeasurement(T measurement, KeyValuePair<string, object?> tag)

        protected void RecordMeasurement(T measurement, KeyValuePair<string, object?> tag1, KeyValuePair<string, object?> tag2)

        protected void RecordMeasurement(T measurement, KeyValuePair<string, object?> tag1, KeyValuePair<string, object?> tag2, KeyValuePair<string, object?> tag3)

        protected void RecordMeasurement(T measurement, in TagList tagList)

        protected void RecordMeasurement(T measurement, ReadOnlySpan<KeyValuePair<string, object?>> tags)
    }

I'd like to propose that this design be reconsidered to something simpler, where the main abstraction contains a "core" method that performs the work, while "auxiliary" overloads are hosted in a separate place.

The framework currently has a few places where this approach is taken today. For example:

1. HttpClient contains the "friendly access methods" and wraps a HttpClientHandler which actually contains a single method and is the "core" interface. The behavior is controlled in a centralized manner and can be easily replaced due to it being a single entry point.
2. ILogger (and consequently ILogger<T>) have most of the "friendly" overloads defined as extension methods in Microsoft.Extensions.Logging.LoggerExtensions, while the core functionality of logging is defined in a single method in the actual interface, the void Log<TState>(LogLevel, EventId, TState, Exception?, Func<TState, Exception?, string>) method. This again means the core behavior is unified in a single entry point that is easily implemented/mocked, while the "friendly" overloads are isolated and can be reused
3. In the same vein as ILogger, we also have IServiceCollection, which defines most "helper" methods in an extensions class in Microsoft.Extensions.DependencyInjection.ServiceCollectionServiceExtensions, while the main functionality is abstracted in an extremely simple fashion by IServiceCollection being "a collection of ServiceDescriptor" that can be appended to.

If we look at the Metrics/Instrument API and compare them with these examples, its clear to see how redundant/verbose their APIs are.

I think it would be cleaner if there was a single, main API for instruments that is then extended with "helper" overloads using the extension methods (ILogger/IServiceCollection) or wrapper (HttpClient) approaches. This would make it much easier to implement new instruments and to create a more testable abstraction that relies on a single main entry point to do its work.

Something like:

    public abstract class Instrument<T> : Instrument where T : struct
    {
        protected void RecordMeasurement(T measurement, in TagList tagList)
    }

    public static class InstrumentExtensions
    {
        public static void RecordMeasurement<T>(this Instrument<T> instrument, T measurement) 
            => // redirect to core 'RecordMeasurement(T, in TagList)

        // ... other overloads here
    }

Also, I must say that having "the same interface" with "different method names" (RecordMeasurement vs Add) seems like a hacky workaround to a missing language feature here which would allow "renaming" the abstraction on inheritance... not sure if this has ever been discussed but it just feels ugly to me the way it was done here (making RecordMeasurement invisible, and expose it using a different name in the inheritor).

For context, this comes from the perspective of testing code that relies on Counter<int> to perform conditional increments based on business logic. More details in:

• How to properly test code relying on Instruments/Counters? #68269