Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Azure Functions Environment #49

Open
realSergiy opened this issue Nov 11, 2021 · 5 comments
Open

Azure Functions Environment #49

realSergiy opened this issue Nov 11, 2021 · 5 comments

Comments

@realSergiy
Copy link

realSergiy commented Nov 11, 2021

Please capture Azure Functions Environment as well:

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

here

private static LogEventProperty CreateProperty(ILogEventPropertyFactory propertyFactory)

as per

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

Thanks!

@henrikrxn
Copy link

henrikrxn commented Dec 7, 2023

@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 Using hostsettings.json results in incorrect environment name #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 Using hostsettings.json results in incorrect environment name #48, but where should that "live" ?
    Serilog.Extensions.Configuration ?
    Serilog.Extensions.Hosting ?

@nblumhardt
Copy link
Member

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

@henrikrxn
Copy link

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

  • Obsoleting the current implementation in this repo
  • Moving the current implementation to Serilog.Extensions.Hosting

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
Copy link
Member

Numpsy commented Dec 8, 2023

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
Copy link

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants