[BUG] Unwanted creation of `HttpContextAccessor` for enrichment

[tjpeel-ee]

ECS integration/library project(s) (e.g. Elastic.CommonSchema.Serilog): Elastic.Serilog.Enrichers.Web

ECS schema version (e.g. 1.4.0): N/A

ECS .NET assembly version (e.g. 1.4.2): N/A

Elasticsearch version (if applicable): N/A

.NET framework / OS: All

Description of the problem, including expected versus actual behavior:

Your docs at https://github.com/elastic/ecs-dotnet/blob/main/src/Elastic.Serilog.Enrichers.Web/README.md suggest the following:

.UseSerilog((ctx, config) =>
{
  // Ensure HttpContextAccessor is accessible
  var httpAccessor = ctx.Configuration.Get<HttpContextAccessor>();

  config
    .Enrich.WithEcsHttpContext(httpAccessor);

Yet ctx.Configuration.Get<T>() is intended for binding configuration. HttpContextAccessor is not config.

In other samples, and implied via the // Ensure HttpContextAccessor is accessible comment, the use of the following is suggested so HttpContextAccessor can be resolved via your configuration get call:

builder.Services.AddHttpContextAccessor();

These are two completely different things and your recommended code is misleading and causes unwanted behaviour.

Using the AddHttpContextAccessor helper to register a singleton instance of IHttpContextAccessor means the service container will be in control of the instance that is instantiated. Therefore, it will also dispose cleanly if needed (I appreciate the interface does not implement IDisposable in this case).

Accessing the HttpContextAccessor in the way you are recommending it be done leads to an additional instance being instantiated. It is logically equivalent to suggesting to use new HttpContextAccessor() instead as all that happens is Activator.CreateInstance is called.

Is there a reason you're doing it in this way that I have missed?

Otherwise I would suggest using your main UseSerilog overload instead that would look like as follows:

.UseSerilog((ctx, services, config) =>
{
  // Ensure HttpContextAccessor is accessible
  var httpAccessor = services.GetRequiredService<IHttpContextAccessor>();

  config
    .Enrich.WithEcsHttpContext(httpAccessor);

Here you are resolving the IHttpContextAccessor instances correctly.