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]: Meter Configuration and Pipeline #85684

Closed
noahfalk opened this issue May 2, 2023 · 11 comments
Closed

[API Proposal]: Meter Configuration and Pipeline #85684

noahfalk opened this issue May 2, 2023 · 11 comments
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Diagnostics.Metric blocking Marks issues that we want to fast track in order to unblock other important work 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

@noahfalk
Copy link
Member

noahfalk commented May 2, 2023

Background and motivation

In the current runtime APIs for metrics we have the ability to create and use Meters, and in-progress we have work to make it easier to consume those APIs from DI-centered workloads. However if an app developer wants to configure a set of Meters to enable and where to export that data that still requires 3rd party APIs such as OpenTelemetry, AppMetrics, or Prometheus.NET. This feature is to support a built-in option for configuring which Meters are enabled and one or more sinks to send that data to. The configuration should support being done both programmatically and via external configuration sources. For sinks I'd expect a neutral interface that any 3rd party component could use but we should look most closely at supporting the already existing 3rd party libraries.

Design is still very uncertain at this point but the initial thought is to follow the lead of the Logging APIs which have LoggingBuilder, LoggingBuilder.AddFilter(), and LoggingBuilder.AddProvider().

API Proposal

Microsoft.Extensions.Diagnostics assembly

namespace Microsoft.Extensions.DependencyInjection;

public static class MetricsServiceCollectionExtensions
{
    public static IServiceCollection AddMetrics(this IServiceCollection);
    public static IServiceCollection AddMetrics(this IServiceCollection, Action<IMetricsBuilder> configure);
}

Microsoft.Extensions.Diagnostics.Abstractions assembly

namespace Microsoft.Extensions.Diagnostics.Metrics;

public interface IMetricsBuilder
{
    IServiceCollection Services { get; }
}

public static class MetricsBuilderExtensions
{
    public static IMetricsBuilder AddListener(this IMetricsBuilder builder, IMetricsListener listener);
    public static IMetricsBuilder AddListener<T>(this IMetricsBuilder builder) where T : class, IMetricsListener
    public static IMetricsBuilder ClearListeners(this IMetricsBuilder builder);
}

public interface IMetricsSource
{
    public void RecordObservableInstruments();
}

public interface IMetricsListener
{
    public string Name { get; }
    public void SetSource(IMetricsSource source);
    public bool InstrumentPublished(Instrument instrument, out object? userState);
    public void MeasurementsCompleted(Instrument instrument, object? userState);
    public MeasurementCallback<T> GetMeasurementHandler<T>();
}

public class MetricsEnableOptions
{
    public IList<InstrumentEnableRule> Rules { get; }
}

public class InstrumentEnableRule
{
    public InstrumentEnableRule(string? listenerName, string? meterName, MeterScope scopes, string? instrumentName, bool enable);
    public string? ListenerName { get; }
    public string? MeterName { get; }
    public MeterScope Scopes { get; }
    public string? InstrumentName { get; }
    public bool Enable { get; }
}

[Flags]
public enum MeterScope
{
    Global,
    Local
}

public interface IMetricsSubscriptionManager
{
    public void Start();
}

public static MetricsBuilderEnableExtensions
{
    public static IMetricsBuilder EnableMetrics(this IMetricsBuilder builder) => throw null!;
    public static IMetricsBuilder EnableMetrics(this IMetricsBuilder builder, string? meterName) => throw null!;
    public static IMetricsBuilder EnableMetrics(this IMetricsBuilder builder, string? meterName, string? instrumentName) => throw null!;
    public static IMetricsBuilder EnableMetrics(this IMetricsBuilder builder, string? meterName, string? instrumentName, string? listenerName) => throw null!;
    public static IMetricsBuilder EnableMetrics(this IMetricsBuilder builder, string? meterName, string? instrumentName, string? listenerName, MeterScope scopes) => throw null!;

    public static MetricsEnableOptions EnableMetrics(this MetricsEnableOptions options) => throw null!;
    public static MetricsEnableOptions EnableMetrics(this MetricsEnableOptions options, string? meterName) => throw null!;
    public static MetricsEnableOptions EnableMetrics(this MetricsEnableOptions options, string? meterName, string? instrumentName) => throw null!;
    public static MetricsEnableOptions EnableMetrics(this MetricsEnableOptions options, string? meterName, string? instrumentName, string? listenerName) => throw null!;
    public static MetricsEnableOptions EnableMetrics(this MetricsEnableOptions options, string? meterName, string? instrumentName, string? listenerName, MeterScope scopes) => throw null!;

    // Which overloads of this do we want?
    public static MetricsEnableOptions DisableMetrics(this MetricsEnableOptions options, string? meterName, string? instrumentName, string? listenerName, MeterScope scopes) => throw null!;
}

Integration with configuration

We want users to be able to configure which metrics to capture for different listeners using appsettings.json or other
IConfiguration sources

Data format

Metrics is the top level section under which "EnabledMetrics" configures which metrics are enabled for all listeners and
a section named with a listener name adds additional rules specific to that listener. The key under "EnabledMetrics" is a
Meter name prefix and the value is true if the Meter should be enabled or false if disabled.

{
    "Metrics": {
        "EnabledMetrics": {
            "System.Runtime": true
        },
        "OpenTelemetry": {
            "EnabledMetrics": {
                "Microsoft.AspNetCore": true
            }
        }
    }
}

Similar to logging, "Default" acts as a zero-length meter prefix creating a default rule applying to all Meters if no
longer prefix rule matches. Logically this allows creating opt-in or opt-out policies. If no rule matches a Meter it will
not be enabled, so omitting "Default" is the same behavior as including Default=false.

{
    "Metrics": {
        "EnabledMetrics": {
            "Default": true
            "NoisyLibrary": false
        },
    }
}

To enable individual instruments within a Meter replace the boolean with object syntax where the keys are instrument names or prefixes
and the value is true iff enabled. "Default" is also honored at this scope as the zero-length instrument name prefix. Not specifying a
"Default" is the same as "Default": false.

{
    "Metrics": {
        "EnabledMetrics": {
            "System.Runtime": {
                "gc": true,
                "jit": true
            }
            "Microsoft.AspNet.Core": {
                "Default": true,
                "connection-duration": false
            }
        }
    }
}

To make rules apply only to Meters at a specific Global or Local scope instead of "EnabledMetrics" you can use "EnabledGlobalMetrics" or
"EnabledLocalMetrics". Rules in a "EnabledMetrics" section apply to Meters in both scopes. The expectation is that using this local/global config is rare and maybe we should just remove it?

{
    "Metrics": {
        "EnabledGlobalMetrics": {
            "System.Runtime": true
        }
        "EnabledLocalMetrics": {
            "System.Net.Http": true
        }
        "EnabledMetrics": {
            "Meter1": true,
            "Meter2": true
        }
    }
}

API

Microsoft.Extensions.Diagnostics.Configuration.dll:

namespace Microsoft.Extensions.Diagnostics.Metrics
{
    public static class MetricsBuilderExtensions
    {
        public static IMetricsBuilder AddConfiguration(this IMetricsBuilder builder, IConfiguration configuration);
    }
}

namespace Microsoft.Extensions.Diagnostics.Metrics.Configuration
{
    public interface IMetricListenerConfigurationFactory
    {
        IConfiguration GetConfiguration(Type listenerType);
    }
    
    public interface IMetricListenerConfiguration<T>
    {
        IConfiguration Configuration { get; }
    }
}

Integration with Hosting APIs

We should include a Metrics property in parallel where existing hosting APIs expose Logging

public class HostApplicationBuilder //pre-existing
{
    public IMetricsBuilder Metrics { get; }
}

public class WebApplicationBuilder //pre-existing
{
    public IMetricsBuilder Metrics { get; }
}

public static class HostingHostBuilderExtensions //pre-existing
{
    public static IHostBuilder ConfigureMetrics(this IHostBuilder hostBuilder, Action<IMetricsBuilder> configureMetrics);
    public static IHostBuilder ConfigureMetrics(this IHostBuilder hostBuilder, Action<HostBuilderContext, IMetricsBuilder> configureMetrics);
}

Default sinks

We should include a simple console output of the metric measurements intended for debugging purposes.
This doesn't have any of the customization that the ILogger console does because it isn't intended to be left
enabled in production code. Recording raw measurements to text is very inefficient.

// All in Microsoft.Extensions.Diagnostics.dll

namespace Microsoft.Extensions.Diagnostics.Metrics

public static class MetricsBuilderConsoleExtensions
{
    public static IMetricsBuilder AddDebugConsole(this IMetricsBuilder builder);
}
public static class ConsoleMetrics
{
    public static string ListenerName => "Console";
}

API Usage

App dev sends the default metrics telemetry to an OLTP endpoint using OpenTelemetry

NOTE: This scenario may be sending metric data that doesn't conform to OpenTelemetry naming conventions
var builder = WebApplication.CreateBuilder(args);
builder.Metrics.AddOpenTelemetry(builder => builder.AddOtlpExporter());

The defaults are controlled from appsettings.json which would contain this in a new web app template:

{
  "Metrics": {
    "EnabledMetrics": {
      "System": true,
      "Microsoft.AspNetCore": true
    }
  }
}

The specific destination for the telemetry is picked up by OTel from the environment variable OTEL_EXPORTER_OTLP_ENDPOINT. Other variations allow the configuration to be passed as a parameter ot AddOltpExporter() or using the service container to configure the
OtlpExporterOptions object.

The AddOpenTelemetry() call here is a hypothetical extension method implemented by the OTel.NET project similar to their current
ILoggingBuilder.AddOpenTelemetry() API.

App dev wants to send default metrics via Prometheus.NET

var builder = WebApplication.CreateBuilder(args);
builder.Metrics.AddPrometheus();

[omitting standard webapp code here]

app.MapMetrics();

Prometheus requires Kestrel to host its scrape endpoint which is why it integrates again in app.UseEndpoints(). The same appsettings.json as above (not shown) is controlling the default metrics being sent to Prometheus.

App dev sends the default metrics telemetry, named with OTel conventions, to an OLTP endpoint using OpenTelemetry

Option #1: Use instrumentation libraries provided by OpenTelemetry

var builder = WebApplication.CreateBuilder(args);
builder.Metrics.AddRuntimeMetrics();
builder.Metrics.AddAspNetCoreMetrics();
builder.Metrics.AddOpenTelemetry(builder => builder.AddOtlpExporter());

The AddXXXMetrics() APIs are new hypothetical extension methods provided by OpenTelemetry instrumentation libraries. These calls produce
metrics under a Meter name defined by the OTel instrumentation libraries such as "OpenTelemetry.Instrumentation.AspNetCore". If the
metrics are left enabled in appsettings.json then both OpenTelmetry and built-in metrics would be transmitted. The app developer
should exlucde built-in metrics from the list if they don't want both.

Option #2: Use APIs provided by runtime libraries (Optional future development, does not ship in .NET 8)

var builder = WebApplication.CreateBuilder(args);
builder.Metrics.AddRuntimeMetrics(RuntimeMetricNaming.OpenTelemetry1_0);
builder.Metrics.AddAspNetCoreMetrics(AspNetCoreMetricNaming.OpenTelemetry1_0);
builder.Metrics.AddOpenTelemetry(builder => builder.AddOtlpExporter());

Once OpenTelemetry has stable conventions it is possible for the libraries that produce metrics to also include extension methods that
apply OpenTelemetry naming rules. The enumeration would likely start with two options, the default metric names shipped originally and
OpenTelemetry's names. Over time more options might be added to the enumeration for updated versions of OpenTelemetry's conventions or
other standards that have gained a broad interest. Unlike option #1, these metrics are produced with the default Meter name for the
library such as "Microsoft.AspNetCore" and it replaces the default metric rather than adding new metrics in parallel.

App dev wants to enable metrics from certain Meters to get sent (config-based approach)

{
  "Metrics": {
    "EnabledMetrics": {
      "System": true,
      "Microsoft.AspNetCore": true
      "Microsoft.Extensions.Caching": true // hypothetical runtime provided Meter
      "StackExchange.Redis": true          // hypothetical Meter in a 3P library
    }
  }
}

App dev wants to enable metrics from certain Meters to get sent (programmatic approach)

var builder = WebApplication.CreateBuilder(args);
builder.Metrics
  .EnableMetrics("Microsoft.Extensions.Caching")
  .EnableMetrics("StackExchange.Redis")
  .AddOpenTelemetry(builder => builder.AddOtlpExporter());

App dev wants to enable metrics from certain Meters to get sent (custom-instrumentation approach)

Meters need to be created in order for turning them on to have any effect. In some cases it may be convenient to both create them
and turn them on as a single step using a strongly typed extension method:

var builder = WebApplication.CreateBuilder(args);
builder.Metrics
  .AddSqlClientInstrumentation()
  .AddOpenTelemetry(builder => builder.AddOtlpExporter());

This AddSqlClientInstrumentation method is a hypothetical extension method that could be offered by an OpenTelemetry instrumentation package. Note that some instrumentation packages are likely to be duplicative with built-in metrics. If the app developer doesn't want
both then they should ensure the built-in metrics aren't included in their configuration.

App dev wants to disable sending metrics for some instruments on a Meter (config-based approach)

{
  "Metrics": {
    "EnabledMetrics": {
      "System": true,
      "Microsoft.AspNet.Core": {
        "Default": true,               // enable default metrics for all the instruments
        "connection-duration": false   // but exclude the connection-duration metric
      }
    }
  }
}

App dev wants to only enable specific instruments on a Meter (config-based approach)

{
  "Metrics": {
    "EnabledMetrics": {
      "System": true,
      "Microsoft.AspNet.Core": {
        "connection-duration": true   // enable only the connection-duration metric
      }
    }
  }
}

App dev wants to only enable specific instruments on a Meter (API-based approach)

var builder = WebApplication.CreateBuilder(args);
builder.Metrics
  // enable only the duration-counter metric
  .EnableMetrics("Microsoft.AspNetCore", "duration-counter");
  .AddOpenTelemetry(builder => builder.AddOtlpExporter());

App dev wants to disable sending metrics for some instruments on a Meter (API-based approach)

var builder = WebApplication.CreateBuilder(args);
builder.Metrics
  // enable all default metrics for all instruments on this Meter except duration-counter
  .EnableMetrics("Microsoft.AspNetCore", instrument => instrument.Name != "duration-counter");
  .AddOpenTelemetry(builder => builder.AddOtlpExporter());

The idea above is that the HostBuilder.Metrics property is a new MetricsBuilder type, the host defaults automatically constructed MetricsBuilder to load configuration from appsettings.json, and the config file specified which Meters to enable. The hypothetical AddOpenTelemetry() API is an extension method that would be provided by the OpenTelemetry NuGet package, very similar to the extension method they provide for LoggingBuilder.

@noahfalk noahfalk added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label May 2, 2023
@noahfalk noahfalk added this to the 8.0.0 milestone May 2, 2023
@noahfalk noahfalk self-assigned this May 2, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 2, 2023
@tarekgh tarekgh added area-System.Diagnostics.Metric partner-impact This issue impacts a partner who needs to be kept updated and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels May 2, 2023
@samsp-msft
Copy link
Member

samsp-msft commented May 3, 2023

Should this be global, or on a per-sink basis? It feels like it would be better for each sink to have its own configuration. For example, if OTel had the ability to add transforms or additional dimensions, those may be enabled via configuration, and that would need to be specific to OTel. Another sink may have completely different capabilities.

@noahfalk
Copy link
Member Author

noahfalk commented May 3, 2023

If we follow the lead from logging, the AddFilter() API affects all sinks by default, but if you need customized configuration that only affects one specific sink you can use an overload that targets a specific sink.

@hoyosjs hoyosjs added the enhancement Product code improvement that does NOT require public API changes/additions label May 8, 2023
@noahfalk
Copy link
Member Author

This is the gist where I've been experimenting with API design and getting some feedback.

@Tratcher Tratcher changed the title [Feature]: Meter Configuration and Pipeline [API Proposal]: Meter Configuration and Pipeline Jul 25, 2023
@tarekgh tarekgh added api-ready-for-review API is ready for review, it is NOT ready for implementation blocking Marks issues that we want to fast track in order to unblock other important work and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Jul 30, 2023
@terrajobst
Copy link
Member

terrajobst commented Aug 1, 2023

Video

  • IMetricsListener.GetMeasurementHandler()
    • This method should return a class that has a callback for all Ts that we expect listeners to support (7 as of today). We should probably have those be nullable. If they are, we'll drop the measurements on the floor.
  • InstrumentEnableRule
    • Should probably just be InstrumentRule
  • MetricsEnableOptions
    • Should probably just MetricsOptions
  • MetricScope
    • Should have a zero bit named None and Global should be non-zero
  • MetricsBuilderEnableExtensions
    • Consider simplifying the overload using default parameters
    • But you want to keep the simplest ones to avoid confusing people
    • DisableMetrics should probably mirror EnableMetrics
  • IMetricsSubscriptionManager
    • Does this need to be public?
namespace Microsoft.Extensions.DependencyInjection;

public static class MetricsServiceCollectionExtensions
{
    public static IServiceCollection AddMetrics(this IServiceCollection services);
    public static IServiceCollection AddMetrics(this IServiceCollection services, Action<IMetricsBuilder> configure);
}
namespace Microsoft.Extensions.Diagnostics.Metrics;

public interface IMetricsBuilder
{
    IServiceCollection Services { get; }
}

public static class MetricsBuilderExtensions
{
    public static IMetricsBuilder AddListener(this IMetricsBuilder builder, IMetricsListener listener);
    public static IMetricsBuilder AddListener<T>(this IMetricsBuilder builder) where T : class, IMetricsListener;
    public static IMetricsBuilder ClearListeners(this IMetricsBuilder builder);
}

public interface IMetricsSource
{
    void RecordObservableInstruments();
}

public interface IMetricsListener
{
    string Name { get; }
    void SetSource(IMetricsSource source);
    bool InstrumentPublished(Instrument instrument, out object? userState);
    void MeasurementsCompleted(Instrument instrument, object? userState);
    MeasurementCallback<T> GetMeasurementHandler<T>();
}

public class MetricsEnableOptions
{
    public IList<InstrumentEnableRule> Rules { get; }
}

public class InstrumentEnableRule
{
    public InstrumentEnableRule(string? listenerName, string? meterName, MeterScope scopes, string? instrumentName, bool enable);
    public string? ListenerName { get; }
    public string? MeterName { get; }
    public MeterScope Scopes { get; }
    public string? InstrumentName { get; }
    public bool Enable { get; }
}

[Flags]
public enum MeterScope
{
    None = 0x00,
    Global = 0x01
    Local = 0x02
}

public interface IMetricsSubscriptionManager
{
    void Start();
}

public static class MetricsBuilderEnableExtensions
{
    public static IMetricsBuilder EnableMetrics(this IMetricsBuilder builder);
    public static IMetricsBuilder EnableMetrics(this IMetricsBuilder builder, string? meterName);
    public static IMetricsBuilder EnableMetrics(this IMetricsBuilder builder, string? meterName, string? instrumentName);
    public static IMetricsBuilder EnableMetrics(this IMetricsBuilder builder, string? meterName, string? instrumentName, string? listenerName);
    public static IMetricsBuilder EnableMetrics(this IMetricsBuilder builder, string? meterName, string? instrumentName, string? listenerName, MeterScope scopes);

    public static MetricsEnableOptions EnableMetrics(this MetricsEnableOptions options);
    public static MetricsEnableOptions EnableMetrics(this MetricsEnableOptions options, string? meterName);
    public static MetricsEnableOptions EnableMetrics(this MetricsEnableOptions options, string? meterName, string? instrumentName);
    public static MetricsEnableOptions EnableMetrics(this MetricsEnableOptions options, string? meterName, string? instrumentName, string? listenerName);
    public static MetricsEnableOptions EnableMetrics(this MetricsEnableOptions options, string? meterName, string? instrumentName, string? listenerName, MeterScope scopes);

    // Which overloads of this do we want?
    public static MetricsEnableOptions DisableMetrics(this MetricsEnableOptions options, string? meterName, string? instrumentName, string? listenerName, MeterScope scopes);
}

public static class MetricsBuilderExtensions
{
    public static IMetricsBuilder AddConfiguration(this IMetricsBuilder builder, IConfiguration configuration);
}
namespace Microsoft.Extensions.Hosting
{
    public partial class HostApplicationBuilder
    {
        public IMetricsBuilder Metrics { get; }
    }
    public partial static class HostingHostBuilderExtensions
    {
        public static IHostBuilder ConfigureMetrics(this IHostBuilder hostBuilder, Action<IMetricsBuilder> configureMetrics);
        public static IHostBuilder ConfigureMetrics(this IHostBuilder hostBuilder, Action<HostBuilderContext, IMetricsBuilder> configureMetrics);
    }
}

namespace Microsoft.AspNetCore.Builder
{
    public partial class WebApplicationBuilder
    {
        public IMetricsBuilder Metrics { get; }
    }
}
Unreviewed
namespace Microsoft.Extensions.Diagnostics.Metrics.Configuration
{
    public interface IMetricListenerConfigurationFactory
    {
        IConfiguration GetConfiguration(Type listenerType);
    }

    public interface IMetricListenerConfiguration<T>
    {
        IConfiguration Configuration { get; }
    }
}

namespace Microsoft.Extensions.Diagnostics.Metrics
{
    public static class MetricsBuilderExtensions
    {
        public static IMetricsBuilder AddConfiguration(this IMetricsBuilder builder, IConfiguration configuration);
    }
}

namespace Microsoft.Extensions.Diagnostics.Metrics.Configuration
{
    public interface IMetricListenerConfigurationFactory
    {
        IConfiguration GetConfiguration(Type listenerType);
    }
    
    public interface IMetricListenerConfiguration<T>
    {
        IConfiguration Configuration { get; }
    }
}

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-ready-for-review API is ready for review, it is NOT ready for implementation api-approved API was approved in API review, it can be implemented labels Aug 1, 2023
@terrajobst
Copy link
Member

(marked ready for review again so we tackle the unreviewed portion)

@Tratcher
Copy link
Member

Tratcher commented Aug 1, 2023

Updated:

namespace Microsoft.Extensions.DependencyInjection;

public static class MetricsServiceCollectionExtensions
{
    public static IServiceCollection AddMetrics(this IServiceCollection services); // existing
    public static IServiceCollection AddMetrics(this IServiceCollection services, Action<IMetricsBuilder> configure);
}
namespace Microsoft.Extensions.Diagnostics.Metrics;

public interface IMetricsBuilder
{
    IServiceCollection Services { get; }
}

public static class MetricsBuilderExtensions
{
    public static IMetricsBuilder AddListener(this IMetricsBuilder builder, IMetricsListener listener);
    public static IMetricsBuilder AddListener<T>(this IMetricsBuilder builder) where T : class, IMetricsListener;
    public static IMetricsBuilder ClearListeners(this IMetricsBuilder builder);

    public static IMetricsBuilder EnableMetrics(this IMetricsBuilder builder, string? meterName);
    public static IMetricsBuilder EnableMetrics(this IMetricsBuilder builder, string? meterName, string? instrumentName = null, string? listenerName = null, MeterScope scopes = MeterScope.Global | MeterScope.Local);

    public static MetricsEnableOptions EnableMetrics(this MetricsOptions options, string? meterName);
    public static MetricsEnableOptions EnableMetrics(this MetricsOptions options, string? meterName, string?  instrumentName = null, string? listenerName = null, MeterScope scopes = MeterScope.Global | MeterScope.Local);

    public static IMetricsBuilder DisableMetrics(this IMetricsBuilder builder, string? meterName);
    public static IMetricsBuilder DisableMetrics(this IMetricsBuilder builder, string? meterName, string? instrumentName = null, string? listenerName = null, MeterScope scopes = MeterScope.Global | MeterScope.Local);

    public static MetricsEnableOptions DisableMetrics(this MetricsOptions options, string? meterName);
    public static MetricsEnableOptions DisableMetrics(this MetricsOptions options, string? meterName, string?  instrumentName = null, string? listenerName = null, MeterScope scopes = MeterScope.Global | MeterScope.Local);
}

public interface IObservableInstrumentsSource
{
    void RecordObservableInstruments();
}

public interface IMetricsListener
{
    string Name { get; }
    public void Initialize(IObservableInstrumentsSource source);
    bool InstrumentPublished(Instrument instrument, out object? userState);
    void MeasurementsCompleted(Instrument instrument, object? userState);
    public MeasurementHandlers GetMeasurementHandlers();
}

public class MetricsOptions
{
    public IList<InstrumentRule> Rules { get; }
}

public class InstrumentRule
{
    public InstrumentEnableRule(string? meterName, string? instrumentName, string? listenerName, MeterScope scopes, bool enable);
    public string? MeterName { get; }
    public string? InstrumentName { get; }
    public string? ListenerName { get; }
    public MeterScope Scopes { get; }
    public bool Enable { get; }
}

[Flags]
public enum MeterScope
{
    None = 0x00,
    Global = 0x01,
    Local = 0x02,
}
namespace Microsoft.Extensions.Hosting
{
    public partial class HostApplicationBuilder
    {
        public IMetricsBuilder Metrics { get; }
    }
    public partial static class HostingHostBuilderExtensions
    {
        public static IHostBuilder ConfigureMetrics(this IHostBuilder hostBuilder, Action<IMetricsBuilder> configureMetrics);
        public static IHostBuilder ConfigureMetrics(this IHostBuilder hostBuilder, Action<HostBuilderContext, IMetricsBuilder> configureMetrics);
    }
}

namespace Microsoft.AspNetCore.Builder
{
    public partial class WebApplicationBuilder
    {
        public IMetricsBuilder Metrics { get; }
    }
}
Unreviewed
namespace Microsoft.Extensions.Diagnostics.Metrics
{
    public static class MetricsBuilderExtensions
    {
        public static IMetricsBuilder AddConfiguration(this IMetricsBuilder builder, IConfiguration configuration);
    }
}

namespace Microsoft.Extensions.Diagnostics.Metrics.Configuration
{
    public interface IMetricListenerConfigurationFactory
    {
        IConfiguration GetConfiguration(Type listenerType);
    }
    
    public interface IMetricListenerConfiguration<T>
    {
        IConfiguration Configuration { get; }
    }
}

// All in Microsoft.Extensions.Diagnostics.dll

namespace Microsoft.Extensions.Diagnostics.Metrics

public static class MetricsBuilderConsoleExtensions
{
    public static IMetricsBuilder AddDebugConsole(this IMetricsBuilder builder);
}
public static class ConsoleMetrics
{
    public static string ListenerName => "Console";
}

@msschl
Copy link

msschl commented Aug 2, 2023

Will there be an equivalent API surface for adding tracing? Then all three major parts of open telemetry are covered: logging, metrics, and tracing.

@tarekgh
Copy link
Member

tarekgh commented Aug 2, 2023

Will there be an equivalent API surface for adding tracing?

We'll try to address this in future releases, but this will not be part of .NET 8.0.

@terrajobst
Copy link
Member

  • Looks good as proposed
namespace Microsoft.Extensions.DependencyInjection;

public static partial class MetricsServiceCollectionExtensions
{
    // Existing
    // public static IServiceCollection AddMetrics(this IServiceCollection services);
    public static IServiceCollection AddMetrics(this IServiceCollection services, Action<IMetricsBuilder> configure);
}
namespace Microsoft.Extensions.Diagnostics.Metrics;

public interface IMetricsBuilder
{
    IServiceCollection Services { get; }
}

public static class MetricsBuilderExtensions
{
    public static IMetricsBuilder AddListener(this IMetricsBuilder builder, IMetricsListener listener);
    public static IMetricsBuilder AddListener<T>(this IMetricsBuilder builder) where T : class, IMetricsListener;
    public static IMetricsBuilder ClearListeners(this IMetricsBuilder builder);

    public static IMetricsBuilder EnableMetrics(this IMetricsBuilder builder, string? meterName);
    public static IMetricsBuilder EnableMetrics(this IMetricsBuilder builder, string? meterName, string? instrumentName = null, string? listenerName = null, MeterScope scopes = MeterScope.Global | MeterScope.Local);

    public static MetricsEnableOptions EnableMetrics(this MetricsOptions options, string? meterName);
    public static MetricsEnableOptions EnableMetrics(this MetricsOptions options, string? meterName, string?  instrumentName = null, string? listenerName = null, MeterScope scopes = MeterScope.Global | MeterScope.Local);

    public static IMetricsBuilder DisableMetrics(this IMetricsBuilder builder, string? meterName);
    public static IMetricsBuilder DisableMetrics(this IMetricsBuilder builder, string? meterName, string? instrumentName = null, string? listenerName = null, MeterScope scopes = MeterScope.Global | MeterScope.Local);

    public static MetricsEnableOptions DisableMetrics(this MetricsOptions options, string? meterName);
    public static MetricsEnableOptions DisableMetrics(this MetricsOptions options, string? meterName, string?  instrumentName = null, string? listenerName = null, MeterScope scopes = MeterScope.Global | MeterScope.Local);
}

public interface IObservableInstrumentsSource
{
    void RecordObservableInstruments();
}

public interface IMetricsListener
{
    string Name { get; }
    public void Initialize(IObservableInstrumentsSource source);
    bool InstrumentPublished(Instrument instrument, out object? userState);
    void MeasurementsCompleted(Instrument instrument, object? userState);
    public MeasurementHandlers GetMeasurementHandlers();
}

public class MetricsOptions
{
    public IList<InstrumentRule> Rules { get; }
}

public class InstrumentRule
{
    public InstrumentEnableRule(string? meterName, string? instrumentName, string? listenerName, MeterScope scopes, bool enable);
    public string? MeterName { get; }
    public string? InstrumentName { get; }
    public string? ListenerName { get; }
    public MeterScope Scopes { get; }
    public bool Enable { get; }
}

[Flags]
public enum MeterScope
{
    None = 0x00,
    Global = 0x01,
    Local = 0x02
}
namespace Microsoft.Extensions.Hosting
{
    public partial class HostApplicationBuilder
    {
        public IMetricsBuilder Metrics { get; }
    }
    public partial static class HostingHostBuilderExtensions
    {
        public static IHostBuilder ConfigureMetrics(this IHostBuilder hostBuilder, Action<IMetricsBuilder> configureMetrics);
        public static IHostBuilder ConfigureMetrics(this IHostBuilder hostBuilder, Action<HostBuilderContext, IMetricsBuilder> configureMetrics);
    }
}

namespace Microsoft.AspNetCore.Builder
{
    public partial class WebApplicationBuilder
    {
        public IMetricsBuilder Metrics { get; }
    }
}
namespace Microsoft.Extensions.Diagnostics.Metrics;

public static class MetricsBuilderConsoleExtensions
{
    public static IMetricsBuilder AddDebugConsole(this IMetricsBuilder builder);
}

public static class ConsoleMetrics
{
    public static const string DebugListenerName = "DebugConsole";
}
namespace Microsoft.Extensions.Diagnostics.Metrics
{
    public static class MetricsBuilderExtensions
    {
        public static IMetricsBuilder AddConfiguration(this IMetricsBuilder builder, IConfiguration configuration);
    }
}
namespace Microsoft.Extensions.Diagnostics.Metrics.Configuration
{
    public interface IMetricListenerConfigurationFactory
    {
        IConfiguration GetConfiguration(Type listenerType);
    }
    
    public interface IMetricListenerConfiguration<T>
    {
        IConfiguration Configuration { get; }
    }
}

@Tratcher Tratcher added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Aug 8, 2023
@Tratcher
Copy link
Member

Tratcher commented Aug 9, 2023

When implementing the configuration interfaces, I found they didn't work with other parts of the system and needed to be changed.

Before:

namespace Microsoft.Extensions.Diagnostics.Metrics.Configuration
{
    public interface IMetricListenerConfigurationFactory
    {
        IConfiguration GetConfiguration(Type listenerType);
    }
    
    public interface IMetricListenerConfiguration<T>
    {
        IConfiguration Configuration { get; }
    }
}

Issue: LoggerFactory used full type names or a special attribute to get the alias name, so the type T input was ok. We've chosen not to follow that pattern for metrics, and instead use IMetricsListener.Name to identify instances. This however doesn't work with DI.

After:

namespace Microsoft.Extensions.Diagnostics.Metrics.Configuration
{
    public interface IMetricListenerConfigurationFactory
    {
        IConfiguration GetConfiguration(string listenerName);
    }
}

A provider like OpenTelemetry that wanted to pull out custom config sections for itself would inject IMetricListenerConfigurationFactory and call GetConfiguration(ListenerName). This is simpler to understand and implement.

@terrajobst terrajobst added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-approved API was approved in API review, it can be implemented labels Aug 10, 2023
@terrajobst
Copy link
Member

terrajobst commented Aug 10, 2023

Video

  • Looks good as proposed.
 namespace Microsoft.Extensions.Diagnostics.Metrics.Configuration
 {
-    public interface IMetricListenerConfigurationFactory
-    {
-        IConfiguration GetConfiguration(Type listenerType);
-    }
-    public interface IMetricListenerConfiguration<T>
-    {
-        IConfiguration Configuration { get; }
-    }
+    public interface IMetricListenerConfigurationFactory
+    {
+        IConfiguration GetConfiguration(string listenerName);
+    }
 }

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Aug 10, 2023
@tarekgh tarekgh closed this as completed Aug 11, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Sep 10, 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 blocking Marks issues that we want to fast track in order to unblock other important work 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

No branches or pull requests

7 participants