serilog-enrichers-environment icon indicating copy to clipboard operation
serilog-enrichers-environment copied to clipboard

Using hostsettings.json results in incorrect environment name

Open erikmf12 opened this issue 3 years ago • 5 comments

When using a hostsettings.json file to set the environment, only the built-in source context logs come through with the correct environment. Any custom code that calls an ILogger log method enriches the logs with the default value of "Production".

erikmf12 avatar Oct 18 '21 22:10 erikmf12

One issue seems to be that the implementation only looks into environment variables:

// Qualify as uncommon-path
[MethodImpl(MethodImplOptions.NoInlining)]
private static LogEventProperty CreateProperty(ILogEventPropertyFactory propertyFactory)
{
    var environmentName = Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT");

    if (string.IsNullOrWhiteSpace(environmentName))
    {
        environmentName = Environment.GetEnvironmentVariable("DOTNET_ENVIRONMENT");
    }

    if (string.IsNullOrWhiteSpace(environmentName))
    {
        environmentName = "Production";
    }

    return propertyFactory.CreateProperty(EnvironmentNamePropertyName, environmentName);
}

In that regard, the enricher is actually correct, it looks into the environment variable and, if not found, defaults to Production. This behaviour is probably expected in a console application, but not in the context of an asp.net application, where you have multiple built-in ways to control the environment variable, like:

  • Settings files, such as appsettings.json
  • Environment variables
  • Azure Key Vault
  • Azure App Configuration
  • Command-line arguments
  • Custom providers, installed or created
  • Directory files
  • In-memory .NET objects

See: https://docs.microsoft.com/en-us/aspnet/core/fundamentals/configuration/?view=aspnetcore-5.0

Most people, including me, do not expect this behaviour when using an asp.net application, where you use IWebHostEnvironment to obtain the current environment, as it could be set from any source.

One solution would probably be to add another property like WebHostEnvironmentNamePropertyName. A completely new enricher like Serilog.Enrichers.WebHostEnvironment or something in that direction might also be an option.

I would like to hear the opinion of others.

flennic avatar Oct 25 '21 16:10 flennic

I think we need some solution for these different hosting models. I use the Worker Service template extensively for services and even though it's not an asp.net app, it still uses the same hosting model functions. We sometimes run dev and prod on the same machine for smaller services and the env vars are always set the same for both, so we use a hostsettings.json file to set the environment.

I think a good alternative would be adding another property named something like EnvironmentNameFromHost that specifies that it's getting the environment from the running IHost, whether it's a web host or a generic host.

erikmf12 avatar Oct 25 '21 17:10 erikmf12

Probably a good idea to have it as generic as possible, so I like your suggestion. The question is just how, or rather where, to implement it. I would also like a solution where it tries to get the IHost implementation and if that is not possible, it defaults to the current implementation.

flennic avatar Oct 25 '21 17:10 flennic

Would it be best to add this environment enricher into the serilog-extensions-hosting repo? I know most of these add-on enrichers come in their own repo but it might make sense to add this enricher directly in there since that will be the only time it's needed.

EDIT: I suggested this because if we add it there, we'll have access to the underlying hosting environment.

erikmf12 avatar Oct 25 '21 18:10 erikmf12

Definitely seems like this method is in the wrong place 👍

Not certain how an implementation in Serilog.Extensions.Hosting would look, but interested to check it out if anyone can sketch one up.

nblumhardt avatar Nov 08 '21 07:11 nblumhardt