Remove structured logs exp flag #2113
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We've been using structured-logs for months and it's working well at this point. Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
When I removed the last experimental feature flag, that caused a bunch of tests to not compile that verify that our experimental feature flags are working. I've added a NoopFeature flag that we can use to always have at least one flag for testing. It's not used in the codebase to enable any functionality. Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
The structure-logs feature flag was being used to indicate not only that the logs and telemetry config sections should be used, but also that the format of the logs printed to the console should include the timestamp and log level. I've added a property to the logs config section, structured, to replace that now that we aren't using the feature flag anymore. Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
I realized that the logs.enabled config setting is confusing. It's actually used to toggle logging to a file. I've renamed it to log-to-file so that it's more clear what the setting controls. Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
VinozzZ
approved these changes
May 31, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this change
Remove structured-logs experimental flag
We've been using structured-logs for months and it's working well at this point.
Add NoopFeature flag for testing
When I removed the last experimental feature flag, that caused a bunch of tests to not compile that verify that our experimental feature flags are working.
I've added a NoopFeature flag that we can use to always have at least one flag for testing. It's not used in the codebase to enable any functionality.
Add setting for the console log format
The structure-logs feature flag was being used to indicate not only that the logs and telemetry config sections should be used, but also that the format of the logs printed to the console should include the timestamp and log level.
I've added a property to the logs config section, structured, to replace that now that we aren't using the feature flag anymore.
Rename logs.enabled to logs.log-to-file
I realized that the logs.enabled config setting is confusing. It's actually used to toggle logging to a file. I've renamed it to
log-to-file so that it's more clear what the setting controls.
What issue does it fix
Closes #2075
Notes for the reviewer
N/A
Checklist
Reviewer Checklist