You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Based on comments in #3051 and code comments, it seems like it'd be interesting to get a discussion going about how to handle metrics options for instrumentation.
Current Handling
At present, while metrics exporter libraries handle options, metrics instrumentation libraries do not. In more concrete terms:
builder.Services.Configure<PrometheusExporterOptions>(opt =>{ opt.StartHttpListener =false;});
builder.Services.AddOpenTelemetryMetrics(options =>{// AddHttpClientInstrumentation does NOT take options. options.AddHttpClientInstrumentation();// AddPrometheusExporter will use the previously registered options. options.AddPrometheusExporter();});
In OpenTelemetry.Instrumentation.Http as an example, it appears the HttpHandlerMetricsDiagnosticListener is a separate entity that generates metrics but does not support any sort of options. The related HttpHandlerDiagnosticListener is where trace is handled and that does support options.
Questions to Address
What does the OpenTelemetry SDK spec provide for options on metrics? I didn't see anything specific to metrics in the way of filtering or enrichment other than views which the MeterProviderBuilder already supports. This concept appears to be a more formalized filter, where such a thing doesn't appear to be addressed in trace.
Is filtering interesting? Trace has a Filter option for dropping traces before they get recorded. Given the "view" support required for metrics, is a Filter also interesting?
Is enrichment interesting? Trace has an Enrich option for adding to a given Activity as it gets recorded. Should there be something similar for metrics? Maybe, since it could add flexibility, though I know most of the tags I might add to scraped Prometheus metrics would likely be done as part of the Prometheus scrape process (e.g., add the k8s deployment name to the metric) rather than something the application would put there. I'm not sure what the use case would be specifically and it seems like something that could be added later based on need.
Should metrics options be the same object as tracing options? On one hand, having a single set of options for both tracing and metrics on the same source makes it easy; on the other hand, it's going to lock you in design-wise and probably be confusing to set up. If the object is shared, you'd need to make sure all the members are named clearly, like MetricsEnricher vs TraceEnricher to make it clear how they differ. Filter might be used for both, but then, that subverts the notion of "views" mentioned earlier. Probably should not share options objects.
Can we add Services to IDeferredMetricsProviderBuilder? One of the challenges with using options, particularly Options<T>, has been that the consuming dev needs to services.AddOptions<T>() themselves. It would be nice if that call was made during things like .AddHttpClientInstrumentation(), but in order to do that, we'd need to expose the Services property. This is not uncommon in builders - see the various builder implementations in the dotnet/aspnetcore repo.
My Take
I have to admit, I'm not as neck-deep in this as others that have been contributing for far longer than me so I'm not sure if I've missed something.
My gut says the simplest possible thing to do is:
Use views for the metrics filtering. Using the first-class mechanism for this seems reasonable, and the filtering should be properly applied across tags and everything else.
Skip options for metrics for now. Enrichment seems like the only truly interesting bit here.
Add Services to the IDeferredMetricsProviderBuilder and IDeferredTracerProviderBuilder interfaces. This will open up a lot of capabilities for later.
In the TracerProviderBuilderExtensions, call builder.Services.AddOptions<T>() for any options you expect to be configured. This will set a pattern up for metrics if/when that time comes.
Consider name changes to indicate trace vs. metrics instrumentation. For example, AspNetCoreTraceOptions rather than AspNetCoreInstrumentationOptions. The idea here is that you're going to need different options for metrics - the Enrich signature, for example, is going to be different.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
-
Based on comments in #3051 and code comments, it seems like it'd be interesting to get a discussion going about how to handle metrics options for instrumentation.
Current Handling
At present, while metrics exporter libraries handle options, metrics instrumentation libraries do not. In more concrete terms:
In
OpenTelemetry.Instrumentation.Http
as an example, it appears theHttpHandlerMetricsDiagnosticListener
is a separate entity that generates metrics but does not support any sort of options. The relatedHttpHandlerDiagnosticListener
is where trace is handled and that does support options.Questions to Address
MeterProviderBuilder
already supports. This concept appears to be a more formalized filter, where such a thing doesn't appear to be addressed in trace.Filter
option for dropping traces before they get recorded. Given the "view" support required for metrics, is aFilter
also interesting?Enrich
option for adding to a givenActivity
as it gets recorded. Should there be something similar for metrics? Maybe, since it could add flexibility, though I know most of the tags I might add to scraped Prometheus metrics would likely be done as part of the Prometheus scrape process (e.g., add the k8sdeployment
name to the metric) rather than something the application would put there. I'm not sure what the use case would be specifically and it seems like something that could be added later based on need.MetricsEnricher
vsTraceEnricher
to make it clear how they differ.Filter
might be used for both, but then, that subverts the notion of "views" mentioned earlier. Probably should not share options objects.Services
toIDeferredMetricsProviderBuilder
? One of the challenges with using options, particularlyOptions<T>
, has been that the consuming dev needs toservices.AddOptions<T>()
themselves. It would be nice if that call was made during things like.AddHttpClientInstrumentation()
, but in order to do that, we'd need to expose theServices
property. This is not uncommon in builders - see the various builder implementations in the dotnet/aspnetcore repo.My Take
I have to admit, I'm not as neck-deep in this as others that have been contributing for far longer than me so I'm not sure if I've missed something.
My gut says the simplest possible thing to do is:
Services
to theIDeferredMetricsProviderBuilder
andIDeferredTracerProviderBuilder
interfaces. This will open up a lot of capabilities for later.TracerProviderBuilderExtensions
, callbuilder.Services.AddOptions<T>()
for any options you expect to be configured. This will set a pattern up for metrics if/when that time comes.AspNetCoreTraceOptions
rather thanAspNetCoreInstrumentationOptions
. The idea here is that you're going to need different options for metrics - theEnrich
signature, for example, is going to be different.Beta Was this translation helpful? Give feedback.
All reactions