Better Dev Experience for UseOpenTelemetryPrometheusScrapingEndpoint() Extension Method

[hannahchan]

In the Prometheus exporter, the current .UseOpenTelemetryPrometheusScrapingEndpoint() extension method under the hood branches the middleware pipeline by using the .Map() and .MapWhen() methods. It may be more appropriate to rename this method to .MapOpenTelemetryPrometheusScrapingEndpoint() or just MapPrometheusScrapingEndpoint().

In addition, it might better to update the signature of that method from;

public static IApplicationBuilder UseOpenTelemetryPrometheusScrapingEndpoint(this IApplicationBuilder app, string path)

to;

public static IEndpointConventionBuilder MapPrometheusScrapingEndpoint(this IEndpointRouteBuilder builder, string path)

These changes would make the extension method more consistent with other Endpoint Enabled middleware that branch the middleware pipeline such as the ASP.NET Core HealthCheck middleware .MapHealthChecks() extension method or even the out of the box .MapControllers() method. Returning an IEndpointConventionBuilder would allow the caller to configure the endpoint further. For example;

app.MapPrometheusScrapingEndpoint("/metrics").RequireHost("*:9090");

I'm happy to make the changes myself and submit a PR if someone is willing to accept my contribution. As long as there is an agreement that this is important enough to get fixed.

[cijothomas]

tagging @CodeBlanch @tillig to get thoughts, who previously worked on improvements to Prometheus exporter.

[tillig]

My use case was basically that exact thing - app.MapPrometheusScrapingEndpoint("/metrics").RequireHost("*:9090"); - but I also have something that's basically app.MapHealthChecks("/healthz").RequireHost("*:9090"); and getting the two to work together in the previous incarnation of the extensions was challenging. The new ones have a lot of flexibility that fixed it up for me.

I noticed that, given the health checks example, it actually supports both ways:

• Using IEndpointRouteBuilder
• Using IApplicationBuilder

I think if the extensions proposed could be additive, such that we could actually have it both ways - just like the health checks middleware - it'd be the best of both worlds.

[hannahchan]

This is literally the extension method I am thinking of adding. Before I do is there any concerns or anything else you think should be covered. This extensions is additive of course.

/// <summary>
/// Adds OpenTelemetry Prometheus scraping endpoint middleware to an
/// <see cref="IEndpointRouteBuilder"/> instance.
/// </summary>
/// <param name="endpoints">The <see cref="IEndpointRouteBuilder"/> to add
/// middleware to.</param>
/// <param name="path">Optional path to use for the branched pipeline.</param>
/// <param name="meterProvider">Optional <see cref="MeterProvider"/>
/// containing a <see cref="PrometheusExporter"/> otherwise the primary
/// SDK provider will be resolved using application services.</param>
/// <param name="configureBranchedPipeline">Optional callback to
/// configure the branched pipeline. Called before registration of the
/// Prometheus middleware.</param>
/// <returns>A convention routes for the Prometheus scraping endpoint.</returns>
public static IEndpointConventionBuilder MapPrometheusScrapingEndpoint(
    this IEndpointRouteBuilder endpoints,
    string path = null,
    MeterProvider meterProvider = null,
    Action<IApplicationBuilder> configureBranchedPipeline = null)
{
    var builder = endpoints.CreateApplicationBuilder();

    // Note: Order is important here. MeterProvider is accessed before
    // GetOptions<PrometheusExporterOptions> so that any changes made to
    // PrometheusExporterOptions in deferred AddPrometheusExporter
    // configure actions are reflected.
    meterProvider ??= builder.ApplicationServices.GetRequiredService<MeterProvider>();

    if (path == null)
    {
        var options = builder.ApplicationServices.GetOptions<PrometheusExporterOptions>();
        path = options.ScrapeEndpointPath ?? PrometheusExporterOptions.DefaultScrapeEndpointPath;
    }

    if (!path.StartsWith("/"))
    {
        path = $"/{path}";
    }

    configureBranchedPipeline?.Invoke(builder);

    builder.UseMiddleware<PrometheusExporterMiddleware>(meterProvider);

    return endpoints.Map(new PathString(path), builder.Build());
}

[CodeBlanch]

I'm good with adding this new extension if it will be helpful. I noticed when I was working on these extensions that Prometheus .NET also has a "Use" and a "Map" extension. We just weren't sure why and figured we could add it if anyone asked for it 🤣

@hannahchan Go ahead and open PR mate (translated to Australian) if you have the time.

[hannahchan]

Hi all,

Sorry for the delayed response. Life got busy. I've never contributed to this repo before but just opened a PR for my change. Please let me know if anything is missing. I'm keen to be a more regular contributor.

[Alexander-Bartosh]

Thx @hannahchan !
This also fixes #2869. Any chance this will be merged/released in the new future ?
It looks like this is a blocker for all people who do not want to expose metric endpoints publicly and are using ASP .net core

[tillig]

While it will be nice to see it released, I'm not sure I agree it's a "blocker." You have full pipeline config control through app.UseOpenTelemetryPrometheusScrapingEndpoint(...) overloads right now and could add any security middleware, branching to a different port, etc. using those.

[Alexander-Bartosh]

Oh I might have missed that.
@tillig could you please help us with this one ?
How can we configure endpoint with anonymous authentication exposed on different port, say :8081/metrics ?
Of course one would like to use ASP .net routing for best performance.
For Health checks it is done using
endpoints.MapHealthChecks("/health/live", new HealthCheckOptions(...) ).RequireHost($":8081").WithMetadata(new AllowAnonymousAttribute());

[tillig]

@Alexander-Bartosh I'm on my mobile and on holiday, so maybe give it a shot yourself first. That's literally the exact use case I have, too, which is why it's there. Use the overload that takes a lambda, that's where you can map path and port. Put it before you put your authentication middleware so it will be anonymous. It's just like any other app.MapWhen sort of thing, there's no magic.