Using Func for enrichment is a bad idea: use event properties or IObservable instead

[julealgon]

The Options configuration mechanism relies on the fact that the underlying options object is created once and can be configured additively multiple times. This is why the Configure methods exist the way they do, and why you can implement multiple IConfigureOptions classes, as well as have the PostConfigure approach.

When an options model has a Func property for attaching event handlers, it goes against this principle as it is exceedingly hard to "add" extra handlers. Here is an example of an extension method we created to support this behavior:

(PS: this is on top of a fairly old version of the AspNetCore instrumentation package, so it might not match exactly with the newer interface)

public static void AddStopActivityEnrichment(
    this AspNetCoreInstrumentationOptions options,
    Action<Activity, HttpResponse> enrichmentAction)
{
    _ = options ?? throw new ArgumentNullException(nameof(options));
    _ = enrichmentAction ?? throw new ArgumentNullException(nameof(enrichmentAction));

    var previousEnrichmentAction = options.Enrich;
    options.Enrich = (activity, eventName, context) =>
    {
        previousEnrichmentAction?.Invoke(activity, eventName, context);
        if (eventName is "OnStopActivity")
        {
            // The 'OnStopActivity' event passes a 'HttpResponse' object so this is a safe cast.
            enrichmentAction(activity, (HttpResponse)context);
        }
    };
}

This allows us to call AddStopActivityEnrichment multiple times and "accumulate" the various handlers.

The main reason we came up with this was because it is extremely common for us to have multiple enrichments being done from libraries etc, where each can implement their own logic and be used together.

Here is an example of how we've used this in an implementation of IConfigureOptions:

public sealed class LegacyRoutingTracingConfigurator :
    IConfigureOptions<AspNetCoreInstrumentationOptions>
{
    private readonly IServiceProvider serviceProvider;

    public LegacyRoutingTracingConfigurator(IServiceProvider serviceProvider)
    {
        this.serviceProvider = serviceProvider;
    }

    void IConfigureOptions<AspNetCoreInstrumentationOptions>.Configure(AspNetCoreInstrumentationOptions options)
    {
        options.AddStopActivityEnrichment((activity, response) =>
        {
            if (activity.HasHttpRouteTag())
            {
                return;
            }

            var routeFinder = this.serviceProvider.GetRequiredService<IRouteFinder>();
            var routeTemplate = routeFinder.FindRoute(response.HttpContext.Request.Path);
            if (routeTemplate != null)
            {
                activity.SetHttpRouteTag(routeTemplate.TemplateText);
            }
        });
    }
}

This is then added via an extension method on TracerProviderBuilder:

public static class LegacyRoutingTracingExtensions
{
    public static TracerProviderBuilder AddLegacyRoutingTracingSupport(this TracerProviderBuilder tracerProviderBuilder)
    {
        _ = tracerProviderBuilder ?? throw new ArgumentNullException(nameof(tracerProviderBuilder));

        return tracerProviderBuilder.ConfigureServices(
            services => services.ConfigureOptions<LegacyRoutingTracingConfigurator>());
    }
}

It allows us to extract enrichment logic into isolated implementations like this and reuse them across many projects.

I believe the way instrumentation packages expose enrichment today is bad because it doesn't cleanly allow one to "add" enrichment handlers. You either have to do something like we did here (a custom method that uses closures to capture and accumulate func calls), or a completely custom abstraction that can itself be composed and used inside of the Func only once.

I suggest the team reconsider this design for all instrumentation libraries and go with something more robust for this purpose, like IObservable. I mentioned using events in the title but those are mostly considered legacy at this point since the IObserable interface tends to be superior to it in every way.