Skip to content

Delay logging config warnings until the logger has been initialized#2645

Merged
yvrhdn merged 2 commits intografana:mainfrom
yvrhdn:kvrhdn/print-config-warnings
Jul 13, 2023
Merged

Delay logging config warnings until the logger has been initialized#2645
yvrhdn merged 2 commits intografana:mainfrom
yvrhdn:kvrhdn/print-config-warnings

Conversation

@yvrhdn
Copy link
Copy Markdown
Contributor

@yvrhdn yvrhdn commented Jul 12, 2023

What this PR does:
While adding config warnings in #2543 I noticed the warnings never got printed. Because we print the warnings while loading the configuration, we log them using the default NoopLogger...

This PR holds on to the warnings until we initialized the logger and can correctly log them.

End result:

level=warn ts=2023-07-12T16:30:04.59588Z caller=main.go:78 msg="-- CONFIGURATION WARNINGS --"
level=warn ts=2023-07-12T16:30:04.595906Z caller=main.go:78 msg="c.Distributor.LogReceivedTraces is deprecated. The new flag is c.Distributor.log_received_spans.enabled"
level=warn ts=2023-07-12T16:30:04.595911Z caller=main.go:78 msg="Trace storage conflict with user-configuirable overrides storage"
level=info ts=2023-07-12T16:30:04.595916Z caller=main.go:222 msg="initialising OpenTracing tracer"
...

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@yvrhdn
Copy link
Copy Markdown
Contributor Author

yvrhdn commented Jul 12, 2023

Alternative solutions:

  • just print warnings to stdout
  • do a second pass over the config after we initialized the logger to print warnings

Copy link
Copy Markdown
Collaborator

@joe-elliott joe-elliott left a comment

Choose a reason for hiding this comment

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

i swear i've seen it correctly print warnings before. /shrug

@yvrhdn yvrhdn merged commit 88b859f into grafana:main Jul 13, 2023
@yvrhdn yvrhdn deleted the kvrhdn/print-config-warnings branch July 13, 2023 14:44
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.

3 participants