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

Logger factory #356

Merged
merged 1 commit into from
Mar 10, 2021
Merged

Logger factory #356

merged 1 commit into from
Mar 10, 2021

Conversation

kuritka
Copy link
Collaborator

@kuritka kuritka commented Mar 8, 2021

  • logger factory implementation
  • Move client.Client argument from depresolver NewDependencyResolver() to ResolveSpec(client.Client).
    Thanks to that we can resolve configuration when main function starts instead of
    after manager is is instantiated. Now, immediately after obtaining the configuration,
    we can create a logger instance. Just at the beginning of the program.
    Together with moving client argument, I had to fix tests.
  • move resolving configuration on the top of main and initialize logger just after that.
  • remove setupLog from main. The single zero logger instance will be used across whole app.
  • Change LOG_FORMAT resolving, in case variable is not provided, default value is json instead of simple.

Related to #331

Screenshot 2021-03-08 at 18 12 58
Logger is configured, third line is from the second logger which is still present in reconciliation

Screenshot 2021-03-08 at 18 15 57
Config.Log was incorrectly configured, fail fast and print messages with default logger

Screenshot 2021-03-08 at 18 17 35
Config.Log was not resolved so default logger was returned [json, info]

k0da
k0da previously approved these changes Mar 10, 2021
Copy link
Contributor

@somaritane somaritane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kuritka looks good, just a small comment

 - logger factory implementation
 - Move client attribute from depresolver NewDependencyResolver() to ResolveSpec(clien.Client).
   Thanks to that we can resolve configuration when main function starts instead of
   after manager is is instantiated. Now, immediately after obtaining the configuration,
   we can create a logger instance. Just on the beginning of the program.
   Together with moving attribute I had to fix tetsts
 - move resolving configuration on the top of main and initialize logger just after that
 - remove setupLog from main. new logger takes it's his role. the new logger will be used across whole app.
 - Change resolving `LOG_FORMAT`, in case is not given, default value is json instead of simple.

Related to #331
Copy link
Contributor

@somaritane somaritane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 👍

@kuritka kuritka merged commit 5770d46 into master Mar 10, 2021
@kuritka kuritka deleted the logger-factory branch March 10, 2021 14:40
@somaritane somaritane linked an issue Mar 24, 2021 that may be closed by this pull request
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

Successfully merging this pull request may close these issues.

Logger Enhancements
3 participants