-
Notifications
You must be signed in to change notification settings - Fork 447
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
Allow for the UserLogDefaultLogLevel to be set independent of system logs #4318
base: main
Are you sure you want to change the base?
Allow for the UserLogDefaultLogLevel to be set independent of system logs #4318
Conversation
@liliankasem could you review the changes when have a moment. Thanks |
{ | ||
if (LoggingFilterHelper.ValidUserLogLevels.Contains(UserLogLevel, StringComparer.OrdinalIgnoreCase) == false) | ||
{ | ||
throw new CliException($"The userLogLevel value provided, '{UserLogLevel}', is invalid. Valid values are '{string.Join("', '", LoggingFilterHelper.ValidUserLogLevels)}'."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add that the default level for user logs is Information
Thanks, did you also investigate option (b) in my proposal? It would be good to see if that works? Second, can you validate this works for all language workers? I am still on the fence between a flag or env var for this |
If we set "AzureFunctionsJobHost__Logging__LogLevel__Function" to 'debug' in local.settings.json or also use as an environment variable: Its showing debug logs for dotnet worker (if we set minimum level in program.cs), dotnet, Node, Python without depending on host.json. The changes were made to setup the value of UserLoglevel and systemLogLevel be different by using --userLoglevel flag to show logs based on loglevel is not working. it's still depending on host.json can i set the value of 'AzureFunctionsJobHost__Logging__LogLevel__Function' as UserLogDefaultLogLevel instead of --userLogLevel flag? |
@liliankasem added new changes and tests. could you provide review. |
Sorry I didn't mean for you to remove the implementation for the --userLogLevel flag, just that we should decide which approach. @aishwaryabh @mattchenderson @soninaren - can you share thoughts on what you think here? a) We can provide a flag for users to set on func start => b) We can provide an environment variable that can be set in local.settings.json or in local env => `` c) We can use the existing host.json logging properties. We know that a
"logLevel": {
"default": "Warning",
"Host.Aggregator": "Trace",
"Host.Results": "Information",
"Function": "Information" // use this to set user log level
} I am thinking we do both @VineethReyya lets hold off on more changes until we have consensus with the team, thanks! |
Issue describing the changes in this PR
Added --flag userLogLevel to set UserLogDefaultLogLevel without having to set the default log level, and without having to also set the SystemLogDefaultLogLevel to the same value.
resolves #4295
Pull request checklist