serilog / serilog-enrichers-environment

Enrich Serilog log events with properties from System.Environment.
Apache License 2.0
84 stars 32 forks source link

Azure Functions Environment #49

Open realSergiy opened 3 years ago

realSergiy commented 3 years ago

Please capture Azure Functions Environment as well:

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

here

https://github.com/serilog/serilog-enrichers-environment/blob/77ea4c40a8ba3bcd793296f78b7e3fe2a4b2b6d4/src/Serilog.Enrichers.Environment/Enrichers/EnvironmentNameEnricher.cs#L56

as per

https://docs.microsoft.com/en-us/azure/azure-functions/functions-app-settings#azure_functions_environment

Thanks!

henrikrxn commented 12 months ago

@nblumhardt I would like to take this on, maybe in combination with #48 ?

My suggestion would be to add several overload:

  1. Add the code above to handle Azure Functions correctly
  2. Add overload along lines of .Enrich.WithEnvironmentName(IHostEnvironment hostEnvironment) to handle the simple cases IHostEnvironment / IWebHostEnvironment from #48
    Should this live in Serilog.Extensions.Hosting ? Seems like a bad idea to spread Enrich.WithEnvironmentName out over several NuGet packages, but the next point shows that there is no obvious choice (IMHO)
  3. Maybe add another overload along lines of .Enrich.WithEnvironmentName(IConfiguration configuration) to handle other sources listed in #48, but where should that "live" ? Serilog.Extensions.Configuration ? Serilog.Extensions.Hosting ?
nblumhardt commented 12 months ago

Hi! I think Enrich.WithEnvironmentVariable("AZURE_FUNCTIONS_ENVIRONMENT", "Environment") already does this - ?

henrikrxn commented 12 months ago

True, but I would see it as a matter of convenience, because your solution will not handle the case where the environment variable is not set and the environment defaults to Production

You could do something like var environmentName = Environment.GetEnvironmentVariable("AZURE_FUNCTIONS_ENVIRONMENT") ?? "Production"; and then Enrich.WithProperty("EnvironmentName", environmentName) in the setup, but less convenient.

Or in the library add another optional parameter to Enrich.WithEnvironmentVariable, e.g. string fallbackValueWhenNotSet = null so that you get the signature WithEnvironmentVariable(this LoggerEnrichmentConfiguration enrichmentConfiguration, string environmentVariableName, string propertyName = null, string fallbackValueWhenNotSet = null) which I do not like because it is such a niche case.

Similarly the simple (IHostEnvironment) extra possibilities requested in #48 could be handled with Enrich.WithProperty("EnvironmentName", context.HostingEnvironment.EnvironmentName) and other uses of Enrich.withProperty(...) where developers do the "gymnatics" of finding the values in the config. But again not very convinient.

I have thought a little about it and think all implementations of .Enrich.WithEnvironmentName(...), including the current, should live in Serilog.Extensions.Hosting, so a first step could be

So what is your take on 1. + 2. + 3. ?

Leave as is because it is possible for developers to do their own "stuff" or I try and come up with an implementation ?

Numpsy commented 12 months ago

Obsoleting the current implementation in this repo

fwiw, I don't think this repo should depend on external things like Microsoft.Extensions.* and such, and a self contained 'simple' setup could still be useful (I don't know how niche the 'fallbackValueWhenNotSet' idea might really be though)

henrikrxn commented 11 months ago

@Numpsy If you are depending on the exact environment variables used in https://medium.com/event-driven-utopia/an-essential-guide-to-webhooks-1ebeeae5f990, then you are probably using either Microsoft.Extensions.Hosting or ASP.NET Core