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

[API Proposal]: Introduce DI friendly IMeter<T> for modern services #77514

Closed
dpk83 opened this issue Oct 26, 2022 · 53 comments · Fixed by #86567
Closed

[API Proposal]: Introduce DI friendly IMeter<T> for modern services #77514

dpk83 opened this issue Oct 26, 2022 · 53 comments · Fixed by #86567
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Diagnostics.Metric enhancement Product code improvement that does NOT require public API changes/additions partner-impact This issue impacts a partner who needs to be kept updated
Milestone

Comments

@dpk83
Copy link

dpk83 commented Oct 26, 2022

This is edited by @tarekgh

Runtime Metrics APIs Proposal

At present, the metrics APIs that are exposed do not integrate smoothly with DI. This is because the current support relies on creating Meter objects as static, which cannot be utilized within DI containers. As a solution, this proposal recommends developing new APIs that will enable metrics to work seamlessly with DI.

Meter Factory

The IMeterFactory interface is being introduced, which can be registered with the DI container and utilized to create or obtain Meter objects.

IMeterFactory

namespace System.Diagnostics.Metrics
{
    public interface IMeterFactory : IDisposable
    {
        Meter Create(
                string name,
                string? version = null,
                IEnumerable<KeyValuePair<string, object?>>? tags = null);
    }
}

A default implementation will be offered, which should be adequate for most use cases. Nonetheless, users may choose to provide their own implementation for alternative purposes, such as testing.

Meter factories will be accountable for the following responsibilities:

  • Creating a new meter.
  • Attaching the factory instance to the Meter constructor for all created Meter objects. Additional details regarding this will be addressed later in the metrics APIs' additions.
  • Storing created meters in a cache and returning a cached instance if a meter with the same parameters (name, version, and tags) is requested.
  • Disposing of all cached Meter objects upon factory disposal.

DI IMeterFactory Registration

The default IMeterFactory registration can be done by the API:

namespace Microsoft.Extensions.Metrics
{
    public static class MetricsServiceExtensions
    {
        public static IServiceCollection AddMetrics(this IServiceCollection services);        
    }
}

Core Metrics APIs Addition

The proposed modifications aim to facilitate the following:

  • Inclusion of default tags to the Meter
  • Addition of default tags to the instruments
  • Incorporation of scope into the Meter involves assigning an IMeterFactory object to the Meter, which serves as a marker for all Meters associated with a particular factory.
  • The Meter will now cache non-Observable instruments internally to prevent re-creation when attempting to recreate the same instrument with identical parameters such as name, unit, tags, and generic type.
namespace System.Diagnostics.Metrics
{
    public abstract class Instrument
    {
        protected Instrument(
                    Meter meter,
                    string name,
                    string? unit,
                    string? description,
                    IEnumerable<KeyValuePair<string, object?>>? tags);

         public IEnumerable<KeyValuePair<string, object?>>? Tags { get; }
    }

    public abstract class Instrument<T> : Instrument where T : struct
    {
        protected Instrument(
                    Meter meter,
                    string name,
                    string? unit,
                    string? description, IEnumerable<KeyValuePair<string, object?>>? tags);
    }

    public abstract class ObservableInstrument<T> : Instrument where T : struct
    {
        protected ObservableInstrument(
                    Meter meter,
                    string name,
                    string? unit,
                    string? description,
                    IEnumerable<KeyValuePair<string, object?>> tags);
    }

    public class Meter : IDisposable
    {
        public Meter(
                string name,
                string? version,
                IEnumerable<KeyValuePair<string, object?>>? tags,
                IMeterFactory? factory = null);

        public IEnumerable<KeyValuePair<string, object?>>? Tags { get ; }}

        public IMeterFactory? Factory { get; }

        public Counter<T> CreateCounter<T>(
                            string name,
                            string? unit,
                            string? description,
                            IEnumerable<KeyValuePair<string, object?>> tags) where T : struct;

        public UpDownCounter<T> CreateUpDownCounter<T>(
                            string name,
                            string? unit,
                            string? description,
                            IEnumerable<KeyValuePair<string, object?>> tags) where T : struct;

        public Histogram<T> CreateHistogram<T>(
                            string name,
                            string? unit,
                            string? description,
                            IEnumerable<KeyValuePair<string, object?>> tags) where T : struct;

        public ObservableCounter<T> CreateObservableCounter<T>(
                            string name,
                            Func<T> observeValue,
                            string? unit,
                            string? description,
                            IEnumerable<KeyValuePair<string, object?>> tags) where T : struct;

        public ObservableCounter<T> CreateObservableCounter<T>(
                            string name,
                            Func<Measurement<T>> observeValue,
                            string? unit,
                            string? description,
                            IEnumerable<KeyValuePair<string, object?>> tags) where T : struct;

        public ObservableCounter<T> CreateObservableCounter<T>(
                            string name,
                            Func<IEnumerable<Measurement<T>>> observeValues,
                            string? unit,
                            string? description,
                            IEnumerable<KeyValuePair<string, object?>> tags) where T : struct;

        public ObservableUpDownCounter<T> CreateObservableUpDownCounter<T>(
                            string name,
                            Func<T> observeValue,
                            string? unit,
                            string? description,
                            IEnumerable<KeyValuePair<string, object?>> tags) where T : struct;

        public ObservableUpDownCounter<T> CreateObservableUpDownCounter<T>(
                            string name,
                            Func<Measurement<T>> observeValue,
                            string? unit,
                            string? description,
                            IEnumerable<KeyValuePair<string, object?>> tags) where T : struct;

        public ObservableUpDownCounter<T> CreateObservableUpDownCounter<T>(
                            string name,
                            Func<IEnumerable<Measurement<T>>> observeValues,
                            string? unit,
                            string? description,
                            IEnumerable<KeyValuePair<string, object?>> tags) where T : struct;

        public ObservableGauge<T> CreateObservableGauge<T>(
                            string name,
                            Func<T> observeValue,
                            string? unit,
                            string? description,
                            IEnumerable<KeyValuePair<string, object?>> tags) where T : struct;

        public ObservableGauge<T> CreateObservableGauge<T>(
                            string name,
                            Func<Measurement<T>> observeValue,
                            string? unit,
                            string? description,
                            IEnumerable<KeyValuePair<string, object?>> tags) where T : struct;

        public ObservableGauge<T> CreateObservableGauge<T>(
                            string name,
                            Func<IEnumerable<Measurement<T>>> observeValues,
                            string? unit,
                            string? description,
                            IEnumerable<KeyValuePair<string, object?>> tags) where T : struct;
}

Testing Helper

The InstrumentRecorder class has been introduced to facilitate metric testing with DI. With this class, it is straightforward to monitor a particular instrument and its meter readings, and then generate reports on the values published by the instrument.

namespace Microsoft.Extensions.Metrics
{
    public sealed class InstrumentRecorder<T> : IDisposable where T : struct
    {
        public InstrumentRecorder(Instrument instrument);
        public InstrumentRecorder(
                        IMeterFactory? meterFactory,
                        string meterName,
                        string instrumentName);
        public InstrumentRecorder(
                        Meter meter,
                        string instrumentName);
    
        public Instrument Instrument { get; }

        public IEnumerable<Measurement<T>> GetMeasurements();
        public void Dispose();
    }
}

Code Example

    //
    // Register Metrics
    //

    services.AddMetrics();

    //
    // Access the MeterFactory
    //

    IMeterFactory meterFactory = serviceProvider.GetRequiredService<IMeterFactory>();

    //
    // Create Meter
    //

    Meter meter = meterFactory.Create(
                            "MeterName",
                            "version",
                            new TagList()
                            {
                                { "Key1", "Value1" },
                                { "Key2", "Value2" }
                            });

    //
    // Create Instrument
    //

    Counter<int> counter = meter.CreateCounter<int>(
                            "CounterName",
                            "cm",
                            "Test counter 1",
                            new TagList() { { "K1", "V1" } });

    //
    // Test instrument published values
    //

    var recorder = new InstrumentRecorder<int>(meterFactory, "MeterName", "CounterName");
    // can do the following too:
    // var recorder = new InstrumentRecorder<int>(counter);
    counter. Add(1);
    Assert.Equal(1, recorder.GetMeasurements().ElementAt(0));

end of @tarekgh edit and start of old description

Background and motivation

.NET exposes Meter API to instrument code for collecting metrics. Currently the Meter is a class which doesn't work well with DI (Dependency Injection) based model. In order to create a meter object a developer currently need to create an object of the Meter class and pass the meter name, something like this

var meter = new Meter("myMeter");

While this works find on a smaller scale, for large scale complex services this starts becoming difficult to manage and having a DI based API surface could make things much simpler. Apart from the fact that DI would improve the testability of the code, it will also allow for better management of creation of Meter object in a consistent fashion. With DI based approach a meter can be consistently created with the type name of the class where the meter is injected, which opens up a lot of possibilities for further extensions that could simplify the programming model.

We have a metrics solution that our teams uses. When we implemented the solution .NET metrics API was not in existence so we had created our own API surface for metrics. We introduced DI friendly IMeter and IMeter which works similar to ILogger and ILogger. Over the time we found the DI friendly surface extremely useful and extensible, it turned out to be super easy for services and has allowed us to have a consistent approach of creating meter object everywhere. Service owners and library owners don't need to worry about cooking up names for the meter object and the meter object automatically gets created with consistent naming. This allows us to add various capabilities by providing filtering based on the meter names via config. Few examples of the capabilities

  • Services can configure the state of individual meters via config (i.e. enable/disable a meter by configuring the metering state in their appsettings.json using the type name of namespace, similar to how one can configure logging levels with .NET ILogger)
  • Services can configure different destinations for metrics generated by different meter objects. This works even when using third party libraries seamlessly as long as the libraries are using DI to inject meter. etc.

Modern .NET services use DI heavily due to it's benefits and the trend will only increase going forward. Based on our experience with IMeter API and the feedback of simplicity from our customers, we think it would be highly useful for the rest of the community too.

API Proposal

namespace System.Diagnostics.Metrics;

public interface IMeter
{
}

public interface IMeter<out TCategoryName> : IMeter
{
}

OR

namespace System.Diagnostics.Metrics;

public class Meter<out TCategoryName>
{
}

API Usage

public class MyClass
{
    // Inject Meter<T> object using DI
    public MyClass(IMeter<MyClass> meter)
    {
        var counter = meter.CreateCounter<long>("counter1");
    }
}

public class AnotherClass
{
    // Inject a meter with the full name of the provided T gets injected using DI.
    public AnotherClass(IMeter<AnotherClass> meter)
    {
        var counter = meter.CreateCounter<long>("counter2");
    }
}

Alternative Designs

The alternative is to write a custom DI wrapper for Meter and introduce Meter in a wrapper which then defeats the purpose of exposing the Meter API exposed from .NET directly.

Risks

It's a new addition to existing APIs so shouldn't cause breaking changes.

@dpk83 dpk83 added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Oct 26, 2022
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Oct 26, 2022
@ghost
Copy link

ghost commented Oct 26, 2022

Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

.NET exposes Meter API to instrument code for collecting metrics. Currently the Meter is a class which doesn't work well with DI (Dependency Injection) based model. In order to create a meter object a developer currently need to create an object of the Meter class and pass the meter name, something like this

var meter = new Meter("myMeter");

While this works find on a smaller scale, for large scale complex services this starts becoming difficult to manage and having a DI based API surface could make things much simpler. Apart from the fact that DI would improve the testability of the code, it will also allow for better management of creation of Meter object in a consistent fashion. With DI based approach a meter can be consistently created with the type name of the class where the meter is injected, which opens up a lot of possibilities for further extensions that could simplify the programming model.

In R9, we have a metrics solution. When we implemented the solution .NET metrics API was not in existence so we had created our own API surface for metrics. We introduced DI friendly IMeter and IMeter which works similar to ILogger and ILogger. Over the time we found the DI friendly surface extremely useful and extensible, it turned out to be super easy for services and has allowed us to have a consistent approach of creating meter object everywhere. Service owners and library owners don't need to worry about cooking up names for the meter object and the meter object automatically gets created with consistent naming. This allows us to add various capabilities by providing filtering based on the meter names via config. Few examples of the capabilities

  • Services can configure the state of individual meters via config (i.e. enable/disable a meter by configuring the metering state in their appsettings.json using the type name of namespace, similar to how one can configure logging levels with .NET ILogger)
  • Services can configure different destinations for metrics generated by different meter objects. This works even when using third party libraries seamlessly as long as the libraries are using DI to inject meter. etc.

Modern .NET services use DI heavily due to it's benefits and the trend will only increase going forward. Based on our experience with IMeter API and the feedback of simplicity from our customers, we think it would be highly useful for the rest of the community too.

Here are examples to implementations and usage in R9

API Proposal

namespace System.Diagnostics.Metrics;

public interface IMeter
{
}

public interface IMeter<out TCategoryName> : IMeter
{
}

OR

namespace System.Diagnostics.Metrics;

public class Meter<out TCategoryName>
{
}

API Usage

public class MyClass
{
    // Inject Meter<T> object using DI
    public MyClass(IMeter<MyClass> meter)
    {
        var counter = meter.CreateCounter<long>("counter1");
    }
}

public class AnotherClass
{
    // Inject a meter with the full name of the provided T gets injected using DI.
    public AnotherClass(IMeter<AnotherClass> meter)
    {
        var counter = meter.CreateCounter<long>("counter2");
    }
}

Alternative Designs

The alternative is to write a custom DI wrapper for Meter and introduce Meter in a wrapper which then defeats the purpose of exposing the Meter API exposed from .NET directly.

Risks

It's a new addition to existing APIs so shouldn't cause breaking changes.

Author: dpk83
Assignees: -
Labels:

api-suggestion, area-Extensions-DependencyInjection

Milestone: -

@tarekgh tarekgh added area-System.Diagnostics.Metric and removed area-Extensions-DependencyInjection untriaged New issue has not been triaged by the area owner labels Oct 26, 2022
@tarekgh tarekgh added this to the 8.0.0 milestone Oct 26, 2022
@tommcdon tommcdon added the enhancement Product code improvement that does NOT require public API changes/additions label Oct 31, 2022
@jeffhandley jeffhandley added the partner-impact This issue impacts a partner who needs to be kept updated label Nov 21, 2022
@julealgon
Copy link

With DI based approach a meter can be consistently created with the type name of the class where the meter is injected, which opens up a lot of possibilities for further extensions that could simplify the programming model.

This part turned me off.

The same pattern is used with ILogger and I see it more as a workaround to lack of injection target inspection in the container itself. If you look at the ILogger<T> type, it "does nothing", it's basically a marker interface to differentiate registrations in the container: again, a workaround.

Instead of expanding on that (IMHO terrible) practice, I'd rather see something similar to how HttpClient is handled today. It suffers from similar issues, but they were solved without "hurting" the type itself: HttpClient remains non-generic, but there are injection helpers that facilitate associating a given HttpClient with the service that is going to consume it.

Now... don't get me wrong: I still think even the AddHttpClient methods are also hacky workarounds to the underlying root problem here which is an inability of inspecting the target injection type during dependency injection. It would be nice to have that as it would allow you to have all these patterns without dedicated container extensions, but that's probably a separate discussion altogether.

Lastly, about this:

public class MyClass
{
    // Inject Meter<T> object using DI
    public MyClass(IMeter<MyClass> meter)
    {
        var counter = meter.CreateCounter<long>("counter1");
    }
}

I'm not very fond of having instrument creation happening inside my classes either. In all scenarios where I've used custom metrics, I've always made sure to configure and set them all up in the container, and inject the counters. Consumers shouldn't have to know how to create the counters (and gauges, and other instruments); they should only use them.

On that front, what I've also done is extract a ICounter<T> interface for my own purposes, and test around that.

If this proposal is seriously taken into consideration, I'd also recommend potentially extracting common interfaces for the instruments themselves as they are, IMHO, the core of the untestability in regards to metrics classes.

@tarekgh tarekgh self-assigned this Dec 17, 2022
@tarekgh
Copy link
Member

tarekgh commented Dec 17, 2022

Thanks @julealgon for the feedback, it is helpful. We'll look at this at some point next year. I am wondering if you have any specific proposal that you think addresses the request in the proper way.

@julealgon
Copy link

We'll look at this at some point next year.

That's good to hear. This API is indeed extremely unfriendly to modern patterns using Extensions.Hosting.

I am wondering if you have any specific proposal that you think addresses the request in the proper way.

Aside from providing an interface to each instrument type, I think this entire namespace needs a companion "Extensions.Hosting" extension with methods to add metrics and instruments to the container. In a sense, this approach wouldn't be too far off from what Azure and OTEL provide with their Microsoft.Extensions.Azure and OpenTelemetry.Extensions.Hosting respectively.

As for the API, I could see very similar needs to what the various AddHttpClient extensions provide:

  • Possibility of "tying" a specific instrument to a service
  • Possibility of tying a named instrument to a service
  • Possibility of resolving a named instrument from inside a service (some sort of I{Instrument}Factory analogous to IHttpClientFactory)

As well as some stuff that would have to be thought from scratch I think:

  • Possibility of tying multiple instruments to the same service (I actually have this exact requirement in one of our APIs)
  • Fluent API to create and add Meter instances to the container along with their instruments

As part of this work, I'd also like to see better integration with OTEL itself, so that one doesn't need to pass strings around to indicate the meters to track etc. Maybe add some method in the OTEL extensions that just "honors all meters defined in the container automatically" (that's what would make the most sense in my use cases).

Lastly, I just want to emphasize again how hard and convoluted it is to create some of these container-related extensions, and that is mostly due to restrictions on what the MS container keeps track of. It would be really nice if MS reconsidered the feature-set it provides out of the native container, so that things such as "named/keyed registrations" and "target-based injection decisions" were possible natively (especially the latter).

For reference, I'll drop one of our more advanced usages here (should help people understand where I'm coming from). Things have been renamed/simplified from the actual code:

    public static MeterProviderBuilder AddCustomStuffInstrumentation(
        this MeterProviderBuilder meterProviderBuilder)
    {
        ArgumentNullException.ThrowIfNull(meterProviderBuilder);

        const string meterName = "Company.Product";

        return meterProviderBuilder
            .ConfigureServices(
                services => services
                    .AddSingleton<CustomStuffAvailabilityReportingContext>()
                    .AddSingleton<ICustomStuffAvailabilityReportingContext>(p => p.GetRequiredService<CustomStuffAvailabilityReportingContext>())
                    .AddHostedService<CustomStuffAvailabilityReportingService>()
                    .AddHostedService<InstanceInitializerService<ObservableGauge<int>>>()
                    .AddSingleton(_ => new Meter(meterName))
                    .AddSingleton(
                        p =>
                        {
                            var reportingContext = p.GetRequiredService<ICustomStuffAvailabilityReportingContext>();

                            return p
                                .GetRequiredService<Meter>()
                                .CreateObservableGauge(
                                    "available-stuff",
                                    observeValue: () => reportingContext.LastStuffCountReading,
                                    unit: "Stuff",
                                    description: "Tracks how much stuff is available.");
                        })

                    .AddNamedSingleton(
                        p => p
                            .GetRequiredService<Meter>()
                            .CreateUpDownCounterOfInt(
                                "pending-stuff",
                                unit: "Stuff",
                                description: "Tracks how much stuff is pending."),
                        name: CounterRegistrationNames.Pending)

                    .AddNamedSingleton(
                        p => p
                            .GetRequiredService<Meter>()
                            .CreateUpDownCounterOfInt(
                                "available-things",
                                unit: "Thing",
                                description: "Tracks how many things are available."),
                        name: CounterRegistrationNames.Available)

                    .AddNamedSingleton(
                        p => p
                            .GetRequiredService<Meter>()
                            .CreateCounter<int>(
                                "confirmed-things",
                                unit: "CodeRanges",
                                description: "Tracks how many things have been confirmed.")
                            .AdaptToICounter(),
                        name: CounterRegistrationNames.Confirmed)

                    .AddTransient<IStuffTracker>(p => new StuffTracker(
                        pendingStuffCounter: p.GetRequiredNamedService<ICounter<int>>(CounterRegistrationNames.Pending),
                        availableThingsCounter: p.GetRequiredNamedService<ICounter<int>>(CounterRegistrationNames.Available),
                        confirmedThingsCounter: p.GetRequiredNamedService<ICounter<int>>(CounterRegistrationNames.Confirmed)))

                    .Decorate<IRequestHandler<IngestStuffRequest, IngestStuffResponse>, IngestStuffMetricsHandler>()
                    .Decorate<IRequestHandler<CancelThingRequest, CancelThingResponse>, ThingCancellationMetricsHandler>()
                    .Decorate<IRequestHandler<ConfirmThingRequest, ConfirmThingResponse>, ThingConfirmationMetricsHandler>()
                    .Decorate<IRequestHandler<ReserveStuffRequest, ReserveStuffResponse>, StuffReservationMetricsHandler>())
            .AddMeter(meterName);
    }

Notes about this implementation:

  • CustomStuffAvailabilityReportingContext is used as an "ambient" storage to communicate a hosted service with an ObservableGauge.
  • The InstanceInitializerService is there to "force" initialization of the gauge (I wish I didn't have to do this, but the gauge only starts when it is first resolved from the container of course...)
  • The AddNamedSingleton is our custom extension to register named instances in the container (using Lazy<T, TMetadata> internally). Obviously wish this had a native counterpart but there is none
  • The CreateUpDownCounterOfInt is just a .NET6 workaround for UpDownCounter support (creates a custom ICounter implementation that uses 2 counters internally: one to represent increments, and another to represent decrements). This is going away now that we've migrated to .NET7 and can use the actual UpDownCounter instrument
  • AdaptToICounter is a custom extension that returns an adapter that implements our custom ICounter interface (abstraction to enable proper testing)
  • StuffTracker relies on 3 counters and updates them depending on some methods that are called on it. This is where being able to inject multiple instruments in the same target service would be useful
  • Decorate is just using Scrutor to add a few decorators to some of our existing command handlers (MediatR), so that we can update the 3 counters depending on what actions are performed

Our initial implementation only contained the 3 counters. The ObservableGauge + Hosted Service were added later as another workaround: they will be removed soon.

Of note as well, is how unfriendly to DI the ObservableGauge API is... the whole thing about passing a container-controlled state object from the gauge to the service was really hard to get right, especially with having to force initialization of the gauge separately.

@tekian
Copy link

tekian commented Jan 27, 2023

This is very problematic.

image

Whenever a new meter is created, a static (i.e., application-wide) meter list under lock is modified.

  1. It significantly deteriorates testability. In fact, virtually forces serialization of test case run if multiple testcases would like to assert a single meter/metric value.
  2. Is not dependency-injection friendly and goes against .NET's own design principles.

It should be noted that metering is typically being used to create business counters and custom metrics across services. Being able to test the counters - values and dimensions - is paramount.

I don't think counters should be declared upfront as per @julealgon's example because why should a consumer of a library need to know how counters are named, aggregated or what's the unit. And if I own the feature, why would I want them to unfold into DI? Quite contrary. Whoever writes the feature has all the knowledge to make the right call on how should the metrics be named, structured, aggregated and used.

I believe it would be enough to provide an abstract MeterFactory with CreateMeter() method. Inside my class, I would then create all the counters that I need, details of which are cohesively part of the class that uses them. But the major difference is that now I can test this. I can provide MockMeterFactory and assert any values I need.

The real implementation of MeterFactory would new-up Meter, registering it with the static collection (still yuck!).

public class MyClass
{
    public MyClass(MeterFactory factory)
    {
        var meter = factory.CreateMeter("throttling");
        var requestsTotal = meter.CreateCounter<long>("requests-total");
    }
}

@julealgon
Copy link

I don't think counters should be declared upfront as per @julealgon's example because why should a consumer of a library need to know how counters are named, aggregated or what's the unit.

I have no idea what scenario you are alluding to. Certainly not mine, as that is not library code. Of course there would be zero expectation for a library consumer to known which counters a library needs, that's the concern of the library itself. It just does not apply at all to my example however as all the code above is self-contained and not consumed anywhere.

And if I own the feature, why would I want them to unfold into DI? Quite contrary. Whoever writes the feature has all the knowledge to make the right call on how should the metrics be named, structured, aggregated and used.

That setup code above is part of "whoever owns the feature". Configuring a meter, and using a meter, are 2 completely distinct concerns. Much like configuring a logger, or an httpclient, are. The similarity is very fitting, particularly when comparing to HttpClient.

What the consumer of the counter does is increment/decrement it, those are the behaviors they are interested in. Setting them up is a configuration/aggregation root concern.

Mocking a factory that returns concrete classes is practically useless for testing purposes, since I'd still not be able to test the main interaction with the counters: the increment/decrement triggers.

Thus, I strongly disagree with your take on this point.

The rest of your post, regarding the poor practices on how static meters are maintained inside the class, I 100% agree with however.

@noahfalk
Copy link
Member

noahfalk commented Feb 2, 2023

@julealgon - I' m not confident I captured all your constraints in the example above, but I'm curious if something like this feels like it is a step in the right direction to simplify your scenario:

    class CustomStuffInstrumentation : IDisposable
    {
        static string MeterName = "Company.Product";
        Meter _meter = new Meter(MeterName);
        public ObservableGauge<int> _availableThings = new ObservableGauge<int>("available-things", () => LastStuffCountReading);
        public UpDownCounter<int> PendingStuff { get; } = _meter.CreateUpDownCounter<int>(...);
        public UpDownCounter<int> ConfirmedThings { get; } = _meter.CreateUpDownCounter<int>(...);
        public int LastStuffCountReading { get; set; }
        
        public void Dispose()
        {
            _meter.Dispose();
        }
    }

    class StuffTracker : IStuffTracker
    {
        CustomStuffInstrumentation _instrumentation;

        // code in this type can access the properties of CustomStuffInstrumentation to 
        // manipulate the counters.
        public StuffTracker(CustomStuffInstrumentation inst) { _instrumentation = inst; }
    }

    public static MeterProviderBuilder AddCustomStuffInstrumentation(
        this MeterProviderBuilder meterProviderBuilder)
    {
        ArgumentNullException.ThrowIfNull(meterProviderBuilder);
        return meterProviderBuilder
            .ConfigureServices(
                services => services
                    .AddSingleton<CustomStuffInstrumentation>()
                    .AddTransient<IStuffTracker,StuffTracker>();
                    // any other types that need to manipulate the values of the counters
                    // could follow a similar pattern as StuffTracker
            )
            .AddMeter(CustomStuffInstrumentation.MeterName);
    }

Let me know if I missed constraints and there are things this wouldn't handle, or if you have an idea of what a further simplification would look like. I think new .NET APIs relating to Meter can certainly be on the table but for now I would assume solutions should still be scoped within the existing capabilities of .NET's DI system.

[EDIT]: Added a missing AddMeter() invocation and made CustomStuffInstrumentation implement IDisposable

@noahfalk
Copy link
Member

noahfalk commented Feb 2, 2023

@tekian

It significantly deteriorates testability. In fact, virtually forces serialization of test case run if multiple testcases would like to assert a single meter/metric value.

Do you have an example of a test you would like to write, but you feel that you can't because of this constraint?

Also I'm not sure why different test cases need to be sharing the same Meter object. Is there something that encourages or forces tests to be written that way?

@CodeBlanch
Copy link
Contributor

The one thing I don't think we've really hit on here is disposal. Meter implements IDisposable. So to do things "correctly" and pass analysis today we need some boilerplate:

sealed class MyClass : IDisposable
{
    private readonly Meter meter;

    public MyClass()
    {
        meter = new("MyMeter");
    }

    public void Dispose()
    {
         meter.Dispose();
    }
}

I think that's where IMeter<T> becomes useful.

sealed class MyClass
{
    private readonly IMeter<MyClass> meter;

    public MyClass(IMeter<MyClass> meter)
    {
        this.meter = meter;
    }
}

Lets the IServiceProvider/host manages the lifetime for us.

Not saying it is a must-have just felt it was worth calling out 😄

@noahfalk
Copy link
Member

noahfalk commented Feb 3, 2023

Sorry @dpk83, I know this has been waiting a while but now I am digging into these :) Right now it feels like there are a number of different goals and I am trying to tease it apart. Let me go through your text and show where I have some questions and then I'm hoping together we can connect the dots.

Currently the Meter is a class which doesn't work well with DI (Dependency Injection) based model.

Can we dig in on what it means for Meter not to work well in DI? We can apply a standard transformation which can do DI with any type, including Meter. Its totally fine if you want to enumerate everything you don't like about the outcome it gives, I'm just trying to get specific on what aspects we are trying to improve. This is my transformation:

// Step 0: starting from a class that doesn't use DI
class Worker
{
    static Meter s_meter = new Meter("Worker");
    public void DoSomething()
    {
        Console.WriteLine(s_meter.Name);
    }
}

// Step 1: extract the dependency via constructor parameterization and put it in the container
class Worker
{
    Meter _meter;
    public Worker(Meter meter) { _meter = meter; }
    public void DoSomething()
    {
        Console.WriteLine(_meter.Name);
    }
}

// Step 2: Different components might want to put different Meters in the container so we need
// to wrap it in a containing class to prevent collisions
class WorkerInstrumentation
{
    public Meter {get;} = new Meter("Worker");
}

class Worker
{
    Meter _meter;
    public Worker(WorkerInstrumentation instrumentation) { _meter = instrumentation.Meter; }
    public void DoSomething()
    {
        Console.WriteLine(_meter.Name);
    }
}

// Step 3: Meter implements IDisposable, previously the service collection would have disposed it
// automatically but now that a container is present we have to do it
class WorkerInstrumentation : IDisposable
{
    public Meter {get;} = new Meter("Worker");
    public void Dispose() { Meter.Dispose(); }
}

DI would improve the testability of the code

Do you have a particular example in mind of how you want to test the code?

it will also allow for better management of creation of Meter object in a consistent fashion

Assuming we implemented either of the suggested APIs above, how do you envision that developers would add a Meter to the DI container?

With DI based approach a meter can be consistently created with the type name of the class where the meter is injected, which opens up a lot of possibilities for further extensions that could simplify the programming model.

Auto-populating a name from the type parameter seemed like syntatic sugar. I'm not saying syntatic sugar is bad or not worthwhile, but it wasn't clear to me how the presence of the syntatic sugar in one place was going to unlock benefits elsewhere. You mentioned R9 configuring enablement and destinations by name, but I assume all those capabilties would still work just as well if the name had been initialized using new Meter("My.Meter.Name")? If so it suggested that all those features can be built on the current Meter API and they are orthogonal from whether we do this feature.

A last thought on this part, what do you think in general about the policy of naming Meters based on type names? For example if one day a dev decided to refactor some code and change a class name, should that also change the name of the Meter? For logs I think its expected that most library authors aren't going to guarantee back-compat on their exact logging output and most logs will be interpreted by humans. For metrics I would have guessed developers will have stronger expectations of compat, and they will hard-code Meter/Instrument names into monitoring systems and expect them to remain constant over time. Although I have a little skepticism here, mostly I am just admitting ignorance. I should probably try to find more real world examples of how developers treated metric naming conventions over time. I could imagine there might be strong differences between metrics in widely used 3rd party libraries vs. metrics in code that only runs in the context of one service.

Based on our experience with IMeter API

One important thing we'll need to reckon with is that .NET's Meter.CreateXXX APIs always allocate a new Instrument whereas it sounds like the R9 implementation had some form of GetOrCreate semantics. This means that scenarios like this which presumably work fine in R9's impl would be memory leaks in .NET's impl if we left it as-is:

class MyHomeController
{
    Counter<int> _counter;
    MyHomeController(IMeter<MyHomeController> meter)
    {
        _counter = meter.CreateCounter<int>("Foo"); // this is allocating a new object on every
                                                    // request
    }
}

I'm not super-excited about having API called Create to also act as a Lookup, but I certainly consider it on the table as part of what we do here.

Thanks @dpk83!

@davidfowl
Copy link
Member

I haven't caught up here but I plan to and will drop some feedback. This is very important for ASP.NET Core adoption of the meter APIs as well.

@dpk83
Copy link
Author

dpk83 commented Feb 3, 2023

We can apply a standard transformation which can do DI with any type, including Meter.

While we can do this, it will require us to write a wrapper in our libraries. This means that our customers will have to have dependencies on our libraries even for these core abstractions. This dependency then start causing problems. To elaborate further

We have a library which provide a functionality X. This library emit some metrics so it uses the Meter API. Now this library has 2 options

  1. Write the wrapper to perform this transformation to do DI: This doesn't scale as we have 10s of libraries. If we were to take this approach every library will need to write these wrappers and then ensuring that every library is using the exact same pattern to do that becomes challenging.
  2. Use a wrapper provided by the our own core Metering abstractions library: This solution scales very well across all our libraries but it brings the dependency of the core metering abstractions library to all these other libraries that need to emit metrics.

Why is option 2 a problem? Consider this case:

  • The core abstractions package which provides the required wrapper for DI is package A
  • There are 2 libraries B and C which provides certain functionalities. Both these libraries also emit metrics and thus depends on A.
  • One of our customer produces a library X which uses library B
  • Another customer uses X and C in their service. So both of them have dependency on A.

Now our release cycle is every month, so if the customer needs to upgrade to the latest version of C and if X is hasn't upgraded to latest version of B, then it will cause the issues due to different versions of A.

Given logging and metering APIs are core APIs and are used by almost all the packages that we deliver this additional dependency definitely hurts. It will be great to get this core support directly from .NET so we can avoid this dependency hell to certain extent.

Do you have a particular example in mind of how you want to test the code?

The lack of ability to mock the meter is what hurts testability. In our metering implementation we have an IMeterProvider which is responsible for providing IMeter objects. Everything here is DI injected so we can easily mock it in our tests whereas we can't mock the Meter object as it's new'ed up inside the user's code.

Assuming we implemented either of the suggested APIs above, how do you envision that developers would add a Meter to the DI container?

With our current implementation we register both IMeter and IMeter, developers then add meters by injecting them directly into the class

class MyClass
{
    public IMeter _meter;
    public MyClass(IMeter meter)
    {
         _meter = meter;
    }

   // OR
   public MyClass(IMeter<MyClass> meter)
   {
         _meter = meter;
   }
}

I assume all those capabilties would still work just as well if the name had been initialized using new Meter("My.Meter.Name")

Yes, they will work even if it's created this way. The problem however is to educate the teams and to enforce the consistent naming across our own libraries as well as for customers. The consistent naming pattern provides the advantage that customers can quickly learn and use it consistently, so if they are using 10 different libraries, they can just configure the meter configuration same way for all of them without needing to search through documentation (and/ore code) for each library to find the meter name that the library is using.

A last thought on this part, what do you think in general about the policy of naming Meters based on type names? For example if one day a dev decided to refactor some code and change a class name, should that also change the name of the Meter? For logs I think its expected that most library authors aren't going to guarantee back-compat on their exact logging output and most logs will be interpreted by humans. For metrics I would have guessed developers will have stronger expectations of compat, and they will hard-code Meter/Instrument names into monitoring systems and expect them to remain constant over time.

Generally metric backends work on instrument name, not so much on metric name so the accidental refactoring is less of a concern. However, I agree that it still is a concern because it will break whatever functionality the metric name is used for e.g. ILogger has the same pattern and runs the same risks.

Regardless, meter's using the name of the class is not that much of a concern as long as there is a good DI way to inject meter. Currently we followed the model that ILogger has along with the IMeterProvider and IMeter interfaces and that served our DI needs well enough, so that's what we proposed. But if there is a better model we can have I am all up for it.

@dpk83
Copy link
Author

dpk83 commented Feb 3, 2023

While this issue is opened for DI for Meter, it will be great to apply that to Activity as well

@noahfalk
Copy link
Member

noahfalk commented Feb 4, 2023

Thanks @dpk83!

Write the wrapper to perform this transformation to do DI: This doesn't scale as we have 10s of libraries

By 'doesn't scale well' does that mean the existing pattern uses too many lines of code and we want a pattern that can be written with fewer lines of code? I hope this doesn't seem pedantic, I'm just trying to ensure its clear what we are trying to improve.

The lack of ability to mock the meter is what hurts testability

In the API suggestion above you were proposing either IMeter<T> or Meter<T>. My understanding is that IMeter<T> would allow mocking, but Meter<T> is no easier than Meter. Is there a good way to mock Meter<T> that I'm not thinking of, or we shouldn't treat those solutions as being equally viable because one allows mocking and the other one doesn't? For the IMeter part of the solution did you mean that all the CreateXXX APIs are part of the IMeter<T> interface, and they return ICounter, IGauge, IHistogram, etc so that the entire closure of the API surface has been wrapped with interfaces?

@dpk83
Copy link
Author

dpk83 commented Feb 6, 2023

By 'doesn't scale well' does that mean the existing pattern uses too many lines of code and we want a pattern that can be written with fewer lines of code? I hope this doesn't seem pedantic, I'm just trying to ensure its clear what we are trying to improve.

It's not about too many lines of code but it's about repeating those lines in 50 different packages and if we are repeating it then one need to also ensure that everyone is following the right pattern or writing the code in the same way etc.

In the API suggestion above you were proposing either IMeter or Meter. My understanding is that IMeter would allow mocking, but Meter is no easier than Meter. Is there a good way to mock Meter that I'm not thinking of, or we shouldn't treat those solutions as being equally viable because one allows mocking and the other one doesn't? For the IMeter part of the solution did you mean that all the CreateXXX APIs are part of the IMeter interface, and they return ICounter, IGauge, IHistogram, etc so that the entire closure of the API surface has been wrapped with interfaces?

No, there is no good way to mock Meter. IMeter with IMeterProvider is what works best here.

For the IMeter part of the solution did you mean that all the CreateXXX APIs are part of the IMeter interface, and they return ICounter, IGauge, IHistogram, etc so that the entire closure of the API surface has been wrapped with interfaces?

This is what we have which was written before .NET Meter API came into existence so we had ICounter, IHistogram, IGauge interfaces and all CreateXXX APIs returned those interfaces.

@noahfalk
Copy link
Member

It's not about too many lines of code but it's about repeating those lines in 50 different packages and if we are repeating it then one need to also ensure that everyone is following the right pattern or writing the code in the same way etc.

It sounds like you are worried about the odds that devs will make a mistake when implementing the pattern. I know lines of code doesn't capture this fully, but there is at least a rough correlation there. Users are much more likely to mess up a pattern that requires them to write 50 lines vs. a pattern that requires 1 line. In any case, I think I've got me a better understanding of what your concern is when you say a given design doesn't scale well. Thanks!

No, there is no good way to mock Meter. IMeter with IMeterProvider is what works best here.

Rats, I was hoping you were going to point out a cool technique I was unaware of :) But I do still have a trick up my sleeve. Even though Meter and Instrument may not be directly mockable they can be unit tested: https://gist.github.com/noahfalk/0e10f4a091dbec68595ff0e2ec0d3260

I think very few people have discovered that you can author that InstrumentRecorder helper which records all the inputs received by an Instrument, but if we publicized it better (and perhaps included it in some assembly) I hope you'll agree it makes testing an instrument pretty straightforward?

I've been exploring if we could use the Meter<T> or IMeter<T> pattern but it hasn't been as straightforward as I had hoped. Let me share snags I am hitting thus far.

I think users would naturally expect this code not to assert:

public MyController(Meter<MyController> meter)
{
    var counter = meter.AddCounter<int>("foo");
    Debug.Assert(counter.Meter == meter);
}

In order for that assert to pass I can't use the trick that Logger<T> or GenevaMeter<T> uses where it defines Meter<T> as a wrapper that forwards all calls to a 2nd non-generic instance. Instead Meter<T> needs to derive directly from Meter so that object identity is preserved. That is doable so far, but then there is a second constraint. The only way the DI container can create arbitrary generic instantiations is by registering the open generic implementation type and invoking its constructor. Logger<T> and GenevaMeter<T> define a constructor that takes an IWhateverFactory parameter and they use that factory to create the non-generic instance. However because Meter<T> is the only instance and it was already created by the DI container there is no opportunity to use a factory. We might be able to say that anyone who wants to set the Meter version or set a Meter name that doesn't come from the type name shouldn't use this pattern, but they are still going to want something that works with DI. For example devs can an inject an ILoggerFactory if they want more control over the ILogger name but I wouldn't want an IMeterFactory to exist if Meter<T> isn't going to be using it.

I'm still exploring but just wanted to give an update.

@julealgon
Copy link

I think users would naturally expect this code not to assert:

public MyController(Meter<MyController> meter)
{
    var counter = meter.AddCounter<int>("foo");
    Debug.Assert(counter.Meter == meter);
}

In order for that assert to pass I can't use the trick that Logger or GenevaMeter uses where it defines Meter as a wrapper that forwards all calls to a 2nd non-generic instance. Instead Meter needs to derive directly from Meter so that object identity is preserved. That is doable so far, but then there is a second constraint. The only way the DI container can create arbitrary generic instantiations is by registering the open generic implementation type and invoking its constructor. Logger and GenevaMeter define a constructor that takes an IWhateverFactory parameter and they use that factory to create the non-generic instance. However because Meter is the only instance and it was already created by the DI container there is no opportunity to use a factory.

Please... please don't make the same mistake as what was done with ILogger here: that's a dependency injection abomination that only exists to workaround a container limitation where you don't have target type information during the injection process (which some other containers have).

I'd be incredibly sad to see that terrible pattern continuing for Meter.

@CodeBlanch
Copy link
Contributor

Off the top of the head, "state" could work?

@tarekgh
Copy link
Member

tarekgh commented May 16, 2023

Yes state was suggested too, context preferred more because state pattern is an object passed to the API and then will be passed back to the callbacks while context is attaching some context to the API. Considering the Activity point, I think the state will be reasonable here.

@davidfowl
Copy link
Member

Seems like IMeterFactory is marked as "needs work", is that because of the impending configuration APIs that are paired with this?

@tarekgh
Copy link
Member

tarekgh commented May 16, 2023

Seems like IMeterFactory is marked as "needs work", is that because of the impending configuration APIs that are paired with this?

This is about the comment We ran out of time before finishing some issues in IMeterFactory (should the name be Create, GetOrCreate, etc; the ownership and disposal semantics)

@tarekgh
Copy link
Member

tarekgh commented May 17, 2023

After careful consideration and offline discussions, we have addressed the remaining open issues related to the proposal. Here is the resolution:

  • Regarding the naming of IMeterFactory.Create(), we have decided to keep it as it is, without any changes. This decision ensures consistency with the ILoggerFactory pattern, which also uses the term Create.
  • For the issue related to Meter disposing, we have decided to clarify the behavior in the documentation. It will state that the factory owns the lifetime of the Meter, and users should refrain from calling Dispose() explicitly. Considering that meter users typically require the meters throughout the entire lifespan of the process, there is currently no need for them to explicitly dispose of the meters.

In addition to these resolutions, we have a small change in the approved APIs:

  • The new Meter constructor currently uses the parameter and property named Context However, we have decided to change it to Scope. This modification aims to avoid any confusion with the term Context in the context of diagnostics, such as trace context. It is especially important since we plan to introduce a similar pattern for tracing in the future (e.g., IActivitySourceFactory).
  • The InstrumentRecorder.Instrument property may return a nullable instrument. This change allows for creating the recorder before the instrument itself, accommodating such scenarios.

If there are no further concerns or feedback, we kindly request marking the issue as approved.

@tarekgh tarekgh added the api-ready-for-review API is ready for review, it is NOT ready for implementation label May 17, 2023
@JamesNK
Copy link
Member

JamesNK commented May 18, 2023

In my hacked-together version of InstrumentRecorder I stored all raw measurements in an in-memory list. I assume that stays true based on the proposed API.

Is there any concern about adding a type with unconstrained memory growth like this? It's ok in tests with a short lifetime, and memory isn't a huge concern, but what about people who decide to use it in production code?

@tarekgh
Copy link
Member

tarekgh commented May 18, 2023

In my hacked-together version of InstrumentRecorder I stored all raw measurements in an in-memory list. I assume that stays true based on the proposed API.

Yes, this stays true.

Is there any concern about adding a type with unconstrained memory growth like this? It's ok in tests with a short lifetime, and memory isn't a huge concern, but what about people who decide to use it in production code?

Note that we added the flag bool clear to the GetMeasurement which allows the callers to clear the previously collected data. This can help the callers control the growth of the memory. If we find any concern later, we can still update the implementation to use file system as back store or similar idea. As you mentioned this type is mostly for testing purposes.

@JamesNK
Copy link
Member

JamesNK commented May 18, 2023

Yeah, I saw the flag. That's a good improvement.

If you think docs are enough to explain the potential danger of this type then ok. Just want to double check it's been thought about.

@JamesNK
Copy link
Member

JamesNK commented May 18, 2023

namespace System.Diagnostics.Metrics
{
    public interface IMeterFactory : IDisposable
    {
        Meter Create(
                string name,
                string? version = null,
                IEnumerable<KeyValuePair<string, object?>>? tags = null);
    }
}

I worry about future new parameters that a Meter might take. We've just added tags. There may be new parameters in the future. An interface is difficult to version.

Have you considered the Create method taking an options class? For example:

public interface IMeterFactory : IDisposable
{
    Meter Create(MeterOptions options);
}
public sealed class MeterOptions
{
    // name, version, tags, etc
}

Future parameters can easily be added to the options class. Then either have an overload with these parameters or an extension method that calls the IMeterFactory.Create(MeterOptions options) method. Meters aren't created often so the overhead of allocating MeterOptions isn't important.

@noahfalk
Copy link
Member

@tarekgh - Is the door still open to incorporate James' MeterOptions feedback? It feels pretty reasonable to me and it was something I was worrying about recently as in the future OpenTelemetry might want to add another constructor parameter like SchemaUrl. Perhaps specifically:

public class Meter
{
    public Meter(MeterOptions options) // in addition to the existing ctors
}

public class MeterOptions
{
    public string Name {get;set;}
    public string? Version {get;set;}
    public IEnumerable<KeyValuePair<string,object?>>? Tags {get;set;}
}
public interface IMeterFactory
{
    Meter Create(MeterOptions options);
}
public static class MeterFactoryExtensions
{
    public static Meter Create(this IMeterFactory, string name, string? version = null, IEnumerable<KeyValuePair<string,object?>> tags = null)
}

The extension method could preserve the current suggested use in code as-is but still leaves the door open to non-breaking additions in the future.

@tarekgh
Copy link
Member

tarekgh commented May 18, 2023

@noahfalk @JamesNK

The suggestions sound reasonable to me. One thing I'll add to the MeterOptions is the Scope. I'll edit @noahfalk proposal to accommodate that too.

public class MeterOptions
{
    public string Name { get; set;}
    public string? Version { get; set;}
    public IEnumerable<KeyValuePair<string,object?>>? Tags { get; set; }
    public object? Scope { get; set; }
}

public class Meter
{
        public Meter(string name, string? version, IEnumerable<KeyValuePair<string, object?>>? tags, object? scope = null);    // Was in the original proposal and will stay.
        public Meter(MeterOptions options) // adding new constructor
}

public interface IMeterFactory
{
        // removing: Meter Create(string name, string? version = null, IEnumerable<KeyValuePair<string, object?>>? tags = null);   
        Meter Create(MeterOptions options); // replaced the one in the original proposal
}

public static class MeterFactoryExtensions
{
    // Adding extra extension method helper for creating the meter with flat parameters. 
    public static Meter Create(this IMeterFactory, string name, string? version = null, IEnumerable<KeyValuePair<string,object?>> tags = null,  object? scope=null);
}
  • Delta changes from last approved

  • We are changing Context name to Scope.

  • InstrumentRecorder.Instrument to return nullable value. e.g., Instrument?.

@bartonjs
Copy link
Member

bartonjs commented May 18, 2023

Video

  • As MeterOptions.Name is required, we added a ctor to specify it.
  • We're OK with the name Create for the factory, but need to ensure that it is OK to call Dispose on the returned Meter.
    • During the discussion we realized that Meter (an unsealed type) did not do the Basic Dispose Pattern, so we're adding the protected virtual Dispose to facilitate this.
namespace System.Diagnostics.Metrics
{
    public abstract partial class Instrument
    {
        protected Instrument(
            Meter meter,
            string name,
            string? unit,
            string? description,
            IEnumerable<KeyValuePair<string, object?>>? tags);

         public IEnumerable<KeyValuePair<string, object?>>? Tags { get; }
    }

    public abstract partial class Instrument<T> : Instrument where T : struct
    {
        protected Instrument(
            Meter meter,
            string name,
            string? unit,
            string? description,
            IEnumerable<KeyValuePair<string, object?>>? tags);
    }

    public abstract partial class ObservableInstrument<T> : Instrument where T : struct
    {
        protected ObservableInstrument(
            Meter meter,
            string name,
            string? unit,
            string? description,
            IEnumerable<KeyValuePair<string, object?>> tags);
    }

    public class MeterOptions
    {
        public string Name { get; set; }
        public string? Version { get; set; }
        public IEnumerable<KeyValuePair<string,object?>>? Tags { get; set; }
        public object? Scope { get; set; }

        public MeterOptions(string name);
    }

    public partial class Meter : IDisposable
    {
        public Meter(
            string name,
            string? version,
            IEnumerable<KeyValuePair<string, object?>>? tags,
            object? scope = null);

        public Meter(MeterOptions options);

        public IEnumerable<KeyValuePair<string, object?>>? Tags { get ; }

        public object? Scope { get; }

        protected virtual void Dispose(bool disposing);

        public Counter<T> CreateCounter<T>(
            string name,
            string? unit,
            string? description,
            IEnumerable<KeyValuePair<string, object?>> tags) where T : struct;

        public UpDownCounter<T> CreateUpDownCounter<T>(
            string name,
            string? unit,
            string? description,
            IEnumerable<KeyValuePair<string, object?>> tags) where T : struct;

        public Histogram<T> CreateHistogram<T>(
            string name,
            string? unit,
            string? description,
            IEnumerable<KeyValuePair<string, object?>> tags) where T : struct;

        public ObservableCounter<T> CreateObservableCounter<T>(
            string name,
            Func<T> observeValue,
            string? unit,
            string? description,
            IEnumerable<KeyValuePair<string, object?>> tags) where T : struct;

        public ObservableCounter<T> CreateObservableCounter<T>(
            string name,
            Func<Measurement<T>> observeValue,
            string? unit,
            string? description,
            IEnumerable<KeyValuePair<string, object?>> tags) where T : struct;

        public ObservableCounter<T> CreateObservableCounter<T>(
            string name,
            Func<IEnumerable<Measurement<T>>> observeValues,
            string? unit,
            string? description,
            IEnumerable<KeyValuePair<string, object?>> tags) where T : struct;

        public ObservableUpDownCounter<T> CreateObservableUpDownCounter<T>(
            string name,
            Func<T> observeValue,
            string? unit,
            string? description,
            IEnumerable<KeyValuePair<string, object?>> tags) where T : struct;

        public ObservableUpDownCounter<T> CreateObservableUpDownCounter<T>(
            string name,
            Func<Measurement<T>> observeValue,
            string? unit,
            string? description,
            IEnumerable<KeyValuePair<string, object?>> tags) where T : struct;

        public ObservableUpDownCounter<T> CreateObservableUpDownCounter<T>(
            string name,
            Func<IEnumerable<Measurement<T>>> observeValues,
            string? unit,
            string? description,
            IEnumerable<KeyValuePair<string, object?>> tags) where T : struct;

        public ObservableGauge<T> CreateObservableGauge<T>(
            string name,
            Func<T> observeValue,
            string? unit,
            string? description,
            IEnumerable<KeyValuePair<string, object?>> tags) where T : struct;

        public ObservableGauge<T> CreateObservableGauge<T>(
            string name,
            Func<Measurement<T>> observeValue,
            string? unit,
            string? description,
            IEnumerable<KeyValuePair<string, object?>> tags) where T : struct;

        public ObservableGauge<T> CreateObservableGauge<T>(
            string name,
            Func<IEnumerable<Measurement<T>>> observeValues,
            string? unit,
            string? description,
            IEnumerable<KeyValuePair<string, object?>> tags) where T : struct;
    }

    public sealed class InstrumentRecorder<T> : IDisposable where T : struct
    {
        public InstrumentRecorder(Instrument instrument);
        public InstrumentRecorder(
            object? scopeFilter,
            string meterName,
            string instrumentName);
        public InstrumentRecorder(
            Meter meter,
            string instrumentName);
    
        public Instrument? Instrument { get; }

        public IEnumerable<Measurement<T>> GetMeasurements(bool clear = false);
        public void Dispose();
    }
}

namespace Microsoft.Extensions.Metrics
{
    public interface IMeterFactory : IDisposable
    {
        Meter Create(MeterOptions options);
    }

    public static class MetricsServiceExtensions
    {
        public static IServiceCollection AddMetrics(this IServiceCollection services);        
    }

    public static class MeterFactoryExtensions
    {
        // Adding extra extension method helper for creating the meter with flat parameters. 
        public static Meter Create(this IMeterFactory, string name, string? version = null, IEnumerable<KeyValuePair<string,object?>> tags = null,  object? scope = null);
    }
}

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed api-needs-work API needs work before it is approved, it is NOT ready for implementation blocking Marks issues that we want to fast track in order to unblock other important work api-ready-for-review API is ready for review, it is NOT ready for implementation labels May 18, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label May 22, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label May 23, 2023
@vandre
Copy link

vandre commented Jun 17, 2023

How would I use IMetricsFactory with Azure Monitor? Is there a reference implementation?

@tarekgh
Copy link
Member

tarekgh commented Jun 17, 2023

The new .NET 8.0 preview libraries having the new APIs are not released yet to NuGet but should happen soon. You may look at dotnet/core#8436 (comment) for info. The new APIs are going to be released in libraries called Microsoft.Extensions.Diagnostics.Abstractions and Microsoft.Extensions.Diagnostics.

@ghost ghost locked as resolved and limited conversation to collaborators Jul 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Diagnostics.Metric enhancement Product code improvement that does NOT require public API changes/additions partner-impact This issue impacts a partner who needs to be kept updated
Projects
None yet
Development

Successfully merging a pull request may close this issue.