logconfig: update file sink log validation config #132742
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
|
This change is |
aa-joshi
added
backport-24.2.x
dhartunian
reviewed
Oct 16, 2024
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @aa-joshi and @arjunmahishi)
-- commits line 8 at r1:
Missing release note describing the change, this is required here because docs need to be updated and it's user-facing.
pkg/util/log/logconfig/validate.go line 377 at r1 (raw file):
if *fc.Auditable { return errors.Newf(`File-based audit logging cannot coexist with buffered writes. ` + `Disable either the buffered-writes or auditable log configuration.`)
nit: should we put buffered-writes or auditable in quotes like above to signify that they're literal values in the yaml?
aa-joshi
force-pushed
the
CRDB-39467_validate_sinks
branch
from
3c731b6
to
6513cfc
Compare
Previously, you can configure auditable & buffered sink at once in file sink log config. These configuration shouldn't co-exist. To address this, we have added a validation that both these configuration shouldn't co-exist. Epic: CRDB-37534 Release note (ops change): Added log file sink validation for co-existing of audit logging & buffering configurations. This validation makes sure that only one of them should present.
aa-joshi
force-pushed
the
CRDB-39467_validate_sinks
branch
from
6513cfc
to
56c0f1e
Compare
aa-joshi
commented
Oct 16, 2024
There was a problem hiding this comment.
Reviewable status:
Previously, dhartunian (David Hartunian) wrote…
Missing release note describing the change, this is required here because docs need to be updated and it's user-facing.
Done.
dhartunian
approved these changes
Oct 16, 2024
There was a problem hiding this comment.
Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status:
aa-joshi
removed
the
branch-release-24.3
|
TFTR! bors r+ |
aa-joshi
added a commit
to aa-joshi/cockroach
that referenced
this pull request
Oct 18, 2024
Previously we added a file sink log validation (cockroachdb#132742). This change updates message so that message is clear to the user. Epic: CRDB-37534 Release note (ops change): Updated the file sink log validation message. This would give clear indication to user about the expected valid configuration.
aa-joshi
added a commit
to aa-joshi/cockroach
that referenced
this pull request
Oct 18, 2024
Previously we added a file sink log validation (cockroachdb#132742). This change updates message so that message is clear to the user. Epic: CRDB-37534 Release note (ops change): Updated the file sink log validation message. This would give clear indication to user about the expected valid configuration.
aa-joshi
added a commit
to aa-joshi/cockroach
that referenced
this pull request
Oct 18, 2024
Previously we added a file sink log validation (cockroachdb#132742). This change updates message so that message is clear to the user. Epic: CRDB-37534 Release note (ops change): Updated the file sink log validation message. This would give clear indication to user about the expected valid configuration.
craig bot
pushed a commit
that referenced
this pull request
Oct 18, 2024
132899: logconfig: update file sink log validation message r=aa-joshi a=aa-joshi Previously we added a file sink log validation (#132742). This change updates message so that message is clear to the user. Epic: CRDB-37534 Release note (ops change): Updated the file sink log validation message. This would give clear indication to user about the expected valid configuration. Co-authored-by: Akshay Joshi <akshay@cockroachlabs.com>
jbowens
added a commit
to jbowens/cockroach
that referenced
this pull request
Oct 21, 2024
Update the default logging configuration used for roachprod clusters to disable auditable logs on logs going to file sinks. Some roachtests use the buffered:true configuration to withstand disk stall events. This setting is incompatible with auditable logs on file sinks and recently introduced validation (cockroachdb#132742) prohibits the settings from being used together. Release note: none Informs cockroachdb#129922. Informs cockroachdb#132988. Epic: none
craig bot
pushed a commit
that referenced
this pull request
Oct 21, 2024
132916: kvserver: clear rac2 token metrics prior to integration testing r=sumeerbhola a=kvoli `TestFlowControl.*V2` tests assert on exact counters. This can be problematic if benign deltas occur while setting up the test, such a send queue forming when adding a new learner, but being quickly resolved. Clear the token metrics prior to commencing these tests, in order to prevent flakes that result from such deltas in setup. Fixes: #132642 Release note: None 133089: roachprod: update default CockroachDB logging configuration r=dhartunian a=jbowens Update the default logging configuration used for roachprod clusters to disable auditable logs on logs going to file sinks. Some roachtests use the buffered:true configuration to withstand disk stall events. This setting is incompatible with auditable logs on file sinks and recently introduced validation (#132742) prohibits the settings from being used together. Release note: none Informs #129922. Informs #132988. Epic: none Co-authored-by: Austen McClernon <austen@cockroachlabs.com> Co-authored-by: Jackson Owens <jackson@cockroachlabs.com>
blathers-crl bot
pushed a commit
that referenced
this pull request
Oct 21, 2024
Update the default logging configuration used for roachprod clusters to disable auditable logs on logs going to file sinks. Some roachtests use the buffered:true configuration to withstand disk stall events. This setting is incompatible with auditable logs on file sinks and recently introduced validation (#132742) prohibits the settings from being used together. Release note: none Informs #129922. Informs #132988. Epic: none
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.
Previously, you can configure auditable & buffered sink at once in file sink log config. These configuration shouldn't co-exist. To address this, we have added a validation that both these configuration shouldn't co-exist.
Epic: CRDB-37534
Sample onfiguration & error message:
file-defaults: dir: cockroach-data/logs max-file-size: 10MiB max-group-size: 100MiB buffered-writes: false filter: INFO format: crdb-v2 redact: false redactable: true exit-on-error: true auditable: true buffering: max-staleness: 1s flush-trigger-size: 256KiB max-buffer-size: 50MiB