Unify the enrichment API surface for Log/Metric/Trace

[dariusclay]

Background and motivation

This would migrate the changes originally introduced on Otel-dotnet-contrib side into dotnet/extensions, changing from interfaces to an abstract BaseEnricher class.

original proposal:
open-telemetry/opentelemetry-dotnet#4097

implementation:
open-telemetry/opentelemetry-dotnet-contrib#949

API Proposal

public abstract class BaseEnricher<T>
{
    public abstract void Enrich(T enrichmentBag);
}

public abstract class TraceEnricher : BaseEnricher<TraceEnrichmentBag>
{
    public virtual void Enrich(ref TraceEnrichmentBag traceEnrichmentBag) {}
}

public abstract class EnrichmentBag
{
    public void Add(string key, object? value);
}

public abstract class TraceEnrichmentBag : EnrichmentBag {}
public abstract class LogEnrichmentBag : EnrichmentBag {}
public abstract class MetricEnrichmentBag : EnrichmentBag {}

API Usage

public class MyTraceEnricher : Tracenricher
{
    public override void Enrich(LogEnrichmentBag enrichmentBag)
    {...}
}

// Register all required enrichers
services.AddTraceEnricher<MyTraceEnricher>();

Alternative Designs

No response

Risks

No response

[geeknoid]

What's the motivation for this change?

[geeknoid]

And what's the value in BaseEnricher? What code would ever be written that would use such a type?

[dariusclay]

And what's the value in BaseEnricher? What code would ever be written that would use such a type?

I feel it should follow the conventions of dotnet and if interfaces are preferred we'll go that direction. OpenTelemetry typically builds on top of abstract base classes, where this was originally added.

[dariusclay]

What's the motivation for this change?

Currently, there's a scattered set of conceptually but otherwise unrelated interfaces for enrichment. The goal here would be to unify and standardize the enrichment layer so that developers can build on top of a consistent framework.

[xakep139]

What's the motivation for this change?

@geeknoid this will also allow us to extend the functionality later on - even for older frameworks. Currently, the approach with having interface methods with default implementation is not supported on .NET FX.

[geeknoid]

I understand the small incremental benefit of changing ITagCollector to be a base class. I did this for IRedactor a few months back to give us a Redactor base class instead.

But what does having BaseEnricher as a base class do? Why is this useful? When would this type ever be used? When would there ever be something polymorphic in nature using BaseEnricher?

[xakep139]

@geeknoid, do you propose eliminating BaseEnricher<T> type and have only TraceEnricher?

@dariusclay BTW why do we need ref modifier in TraceEnricher.Enrich() method signature?

[geeknoid]

Yes, just don't have BaseEnricher. The common base type is only useful if we expect multiple enricher objects for different kinds of enrichment to be used in a polymorphic way.

[geeknoid]

OK, time is passed for this. We're locked in for .NET 8.