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

[Feature Request] Add a replacement for dot in EnvironmentVariablesConfigurationProvider #35989

Open
Tracked by #77390
0x53A opened this issue Oct 29, 2019 · 21 comments
Open
Tracked by #77390

Comments

@0x53A
Copy link

0x53A commented Oct 29, 2019

Currently EnvironmentVariablesConfigurationProvider replaces __ with :.

https://github.com/aspnet/Extensions/blob/192abfdf3e73106e40d7651eecfb621e4f78c344/src/Configuration/Config.EnvironmentVariables/src/EnvironmentVariablesConfigurationProvider.cs#L67-L70

I propose to do something similar for dot.

Use case:

I need to configure logging in an asp.net core app hosted in Azure Container Instance.

I want to set one specific logger to Trace, so I need to set the configuration key Logging:LogLevel:Company.Namespace.MyCustomMiddleware = Trace.

Locally on my windows PC I can run
set Logging__LogLevel__Company.Namespace.MyCustomMiddleware=Trace
and everything works.

When I try to do it in Azure App Service, it doesn't allow me to create the container, even though it is a windows container:

image

Request: Please add a replacement for ., similar to :.

Workaround: I can do a code change to change the name of the logger, this works as long as it is first party code, not third party.

@0x53A 0x53A changed the title Add a replacement for dot in EnvironmentVariablesConfigurationProvider [Feature Request] Add a replacement for dot in EnvironmentVariablesConfigurationProvider Oct 29, 2019
@analogrelay
Copy link
Contributor

Putting in the list of possible 5.0 candidates (no guarantees!). The fact that logging can't be fully configured via environment variables because of this is a compelling reason to consider this.

@analogrelay
Copy link
Contributor

Triage: We think the right approach here is to see if App Service can resolve this blockage. Adding a new replacement pattern would be extremely breaking.

@analogrelay
Copy link
Contributor

Turns out macOS and Linux don't support . in env vars either though, so we may need to find a deeper solution for the Logging problem specifically. The problem is it's also breaking to use a replacement token in Logging because logger names can be any string (they tend to be based on type names but don't have to be).

@ericstj
Copy link
Member

ericstj commented Apr 24, 2020

There appear to be lots of characters that aren't allowed here. Here are some that I tried: #^+&|;, does this require some generic escaping rule?

@analogrelay analogrelay transferred this issue from dotnet/extensions May 7, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Runtime untriaged New issue has not been triaged by the area owner labels May 7, 2020
@analogrelay analogrelay added this to the 5.0 milestone May 7, 2020
@ericstj ericstj removed the untriaged New issue has not been triaged by the area owner label Jun 18, 2020
@maryamariyan maryamariyan modified the milestones: 5.0.0, 6.0.0 Aug 12, 2020
@maryamariyan maryamariyan modified the milestones: 6.0.0, 7.0.0 Jul 26, 2021
@thohng
Copy link

thohng commented Sep 10, 2021

I dont see any wrong if input to Azure AppService this configuration:

Logging__LogLevel__Company.Namespace.MyCustomMiddleware
Trace

image

In the screenshot, I can see that the configuration is not valid because the name contains special char :
image

When I try to do it in Azure App Service, it doesn't allow me to create the container, even though it is a windows container:

image

@WernerMairl
Copy link
Contributor

Some analysis about this in October 2021:

I'm using a (alpine) docker container inside Azure AppServices.

YES: it is possible to use the dot inside Azure UI:
image

BUT internally it remains "_" (using IConfigurationRoot.DebugView)

image

and with this, the LogFiltering basically is not working :-(

@WernerMairl
Copy link
Contributor

Hi

I have implemented my own solution and that may be a idea for solving this without breaking changes (implement a customizing for NormalizeKey inside EnvironmentVariablesConfigurationProvider without modification on the current behavior.

I have re-written current (net 6) EnvironmentVariablesConfigurationProvider (copy and rename).

the only changes that i have implemented are:

before:
image

after:
image

Full source code here

if someone likes to use this workaround in the meantime, use the gist plus this snippet.

It replaces the current EnvironmentVariablesProvider with my new workaround implementation and shows how we can inject special cases into the key normalization process.

image

I can provide PR for the runtime sourcecode if we agree on strategy and api details
(assuming we need some other naming there)

regards
Werner

@eerhardt eerhardt modified the milestones: Future, 7.0.0 Jul 14, 2022
@eerhardt
Copy link
Member

We will consider this in a future release. Moving this issue out of the 7.0 milestone.

@eerhardt eerhardt modified the milestones: 7.0.0, Future Jul 14, 2022
@maryamariyan maryamariyan modified the milestones: Future, 8.0.0 Sep 28, 2022
@drothmaler
Copy link

I think this could be done using: #61169

@jez9999
Copy link

jez9999 commented Jan 7, 2023

Sign me up for this. One of the reasons I was always uneasy about using env. variables for config is how limited they tend to be because they're being set in a shell environment. Linux env var names are incredibly limited, being apparently limited to letters, numbers, and underscore. There should indeed be a generic escape mechanism (which kinda sucks because it'll make env var names necessarily ugly) implemented in .NET to cater for Linux's limited env var names.

I just tried doing this in Linux and unsurprisingly it doesn't work (not a valid identifier): export Logging__LogLevel__Microsoft.Hosting.Lifetime="Warning"

As of the time of writing it would seem there's literally no solution to this. Environment variables. Ugh.

@jez9999
Copy link

jez9999 commented Feb 26, 2023

Instead of escaping stuff in the environment variable name, would it make more sense to shift everything to the env var value, where you can put far more characters? I propose that env vars whose name matches a specific pattern - say DOTNETVAR[integer] - have their values parsed specially for a name=value format. Then you could say:

DOTNETVAR1='Logging:LogLevel:Microsoft.Hosting.Lifetime="Debug"'
DOTNETVAR2='Some:Config:Section="It'\''s even possible to mix quotes"'

@layomia
Copy link
Contributor

layomia commented Jul 21, 2023

Triage - moving to future; no strong feedback to prioritize this relative to other work for .NET 8.

@layomia layomia modified the milestones: 8.0.0, Future Jul 21, 2023
@jez9999
Copy link

jez9999 commented Jul 21, 2023

Triage - moving to future; no strong feedback to prioritize this relative to other work for .NET 8.

Seriously? I'm giving my strong feedback. It's ludicrous that there's still no mechanism to configure log levels in environment variables on non-Windows platforms! Plenty of people in this bug have expressed a wish for it. I think it should absolutely be in .NET 8.

@thohng
Copy link

thohng commented Jul 22, 2023

We really rely on this update, current we have to inject the appsettings.Production.json for those need update configuration with dot (.) on key.

Such as below test on Azure AppService:

"Serilog": {
  "MinimumLevel": {
    "Override": {
      "Microsoft.AspNetCore.Authentication": "Information"
    }
  }
}

Should become

Serilog__MinimumLevel__Override__Microsoft.AspNetCore.Authentication=Information

But on the application receive Microsoft_AspNetCore_Authentication instead of Microsoft.AspNetCore.Authentication:

Serilog__MinimumLevel__Override__Microsoft_AspNetCore_Authentication=Information

@layomia
Copy link
Contributor

layomia commented Jul 24, 2023

In the meantime see this workaround from @tarekgh in #87130 (comment)


We are currently tracking the same issue under #35989. I will proceed to close this particular issue, but please feel free to respond here or on the linked issue if you have any further questions or require assistance.

Please note that this is a feature request that can only be implemented in a future release. However, I can provide you with a workaround code that you can utilize in your application to temporarily resolve the issue until it is officially supported by the framework. The workaround involves creating a custom configuration provider to handle the replacement of dots in the environment variable. This workaround serves as an extension to the environment variable configuration provider, making it straightforward to implement by replacing the call to IConfigurationBuilder.AddEnvironmentVariables() with the new custom method, IConfigurationBuilder.AddCustomEnvironmentVariables().

public class CustomEnvironmentVariablesConfigurationProvider : EnvironmentVariablesConfigurationProvider
{
    internal const string DefaultDotReplacement = ":_";
    private string _dotReplacement; 
    public CustomEnvironmentVariablesConfigurationProvider(string? dotReplacement = DefaultDotReplacement) : base()
    {
        _dotReplacement = dotReplacement ?? DefaultDotReplacement;
    }

    public CustomEnvironmentVariablesConfigurationProvider(string? prefix, string? dotReplacment = DefaultDotReplacement) : base(prefix)
    {
        _dotReplacement = dotReplacment ?? DefaultDotReplacement;
    }

    public override void Load()
    {
        base.Load();

        Dictionary<string, string?> data = new Dictionary<string, string?>();

        foreach (KeyValuePair<string, string?> kvp in Data)
        {
            if (kvp.Key.Contains(_dotReplacement))
            {
                data.Add(kvp.Key.Replace(_dotReplacement, ".", StringComparison.OrdinalIgnoreCase), kvp.Value);
            }
            else
            {
                data.Add(kvp.Key, kvp.Value);
            }
        }

        Data = data;
    }
}

public class CustomEnvironmentVariablesConfigurationSource : IConfigurationSource
{
    public string? Prefix { get; set; }
    public string? DotReplacement { get; set; }

    public IConfigurationProvider Build(IConfigurationBuilder builder)
    {
        return new CustomEnvironmentVariablesConfigurationProvider(Prefix, DotReplacement);
    }
}

public static class CustomEnvironmentVariablesExtensions
{
    public static IConfigurationBuilder AddCustomEnvironmentVariables(this IConfigurationBuilder configurationBuilder)
    {
        configurationBuilder.Add(new CustomEnvironmentVariablesConfigurationSource());
        return configurationBuilder;
    }

    public static IConfigurationBuilder AddCustomEnvironmentVariables(this IConfigurationBuilder configurationBuilder, string? prefix, string? dotReplacement = CustomEnvironmentVariablesConfigurationProvider.DefaultDotReplacement)
    {
        configurationBuilder.Add(new CustomEnvironmentVariablesConfigurationSource { Prefix = prefix, DotReplacement = dotReplacement });
        return configurationBuilder;
    }

    public static IConfigurationBuilder AddCustomEnvironmentVariables(this IConfigurationBuilder builder, Action<CustomEnvironmentVariablesConfigurationSource>? configureSource) => builder.Add(configureSource);
}

Using that, now you can define the environment variable with three underscores ___ in the places you want dot.

Logging__LogLevel__Microsoft___Hosting___Lifetime=Information    <-- Logging:LogLevel:Microsoft.Hosting.Lifetime=Information

@worldspawn
Copy link

Its not only logging where this comes up. In migrating older apps from windows servers to nix containers this comes up quite a bit. People have named configuration values with periods in them because they could and now... they can't.

@tarekgh tarekgh modified the milestones: Future, 9.0.0 Nov 20, 2023
@bt-Knodel
Copy link

@layomia Small bug to note in that workaround, it changes the environment variable provider to be case sensitive. Data dictionary is normally initialized with StringComparer.OrdinalIgnoreCase. Might be worth updating that workaround to follow the ignore casing requirement of the provider it inherits.

@Angelinsky7
Copy link

Hello, any news about this ? Is the workaround of @layomia the current way to go ? Thanks for any answer :-)

@bt-Knodel
Copy link

@Angelinsky7 it worked for us if it helps. Do note the comment I made though, the workaround makes all environment variable settings case-sensitive, which isn't good. Line 19 of the workaround should be switched to this to have 1-to-1 functionality with its base class:

Dictionary<string, string> data = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);

@Angelinsky7
Copy link

@bt-Knodel thanks for your answer !!!

@ericstj ericstj modified the milestones: 9.0.0, Future Aug 6, 2024
@colinblaise
Copy link

Wait, has this really been an unresolved issue for 5 years? We just dropped serilog in favor of using Open Telemetry with the .NET logger. Couldn't figure out why are log verbosities in production were not being honored. In disbelief this isn't causing everyone problems.

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

No branches or pull requests