-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
pkg/server/structlogging: support hot ranges stats with diagnostic reporting disabled #124425
pkg/server/structlogging: support hot ranges stats with diagnostic reporting disabled #124425
Conversation
057fcce
to
a68a18d
Compare
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.
Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @abarganier, @kyle-a-wong, and @xinhaoz)
-- commits
line 2 at r1:
nit: just structlogging
is fine here. this line is probably too long also.
-- commits
line 10 at r1:
nit: grammar "while without"
-- commits
line 14 at r1:
nit: no need to repeat the ticket that 122977 is already linked to. They're identical and once the github ticket is closed, the Jira ticket will get closed automatically.
-- commits
line 16 at r1:
This change should have a release note explaining the modified behavior.
29c82e3
to
8db9740
Compare
…abled Previously, enabling hot ranges stats also required the enabling of diagnostic reporting. Hot ranges stats doesn't need to be dependent on diagnostic reporting and someone might want to enable hot ranges stats without enabling diagnostic reporting. Now, `server.telemetry.hot_ranges_stats.enabled` can be set to true without setting `diagnostics.reporting.enabled`. Epic: none Fixes: cockroachdb#122977 Release note (ops change): Enabling the `server.telemetry.hot_ranges_stats.enabled` setting no longer requires the `diagnostics.reporting.enabled` setting to be set to true. Because `server.telemetry.hot_ranges_stats.enabled` is enabled by default, those who have `diagnostics.reporting.enabled` set to false and no explicit setting for `server.telemetry.hot_ranges_stats.enabled` will now see hot ranges stats in their telemetry logs.
8db9740
to
fd57c1b
Compare
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.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @abarganier, @dhartunian, and @xinhaoz)
Previously, dhartunian (David Hartunian) wrote…
This change should have a release note explaining the modified behavior.
Done.
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.
Reviewed all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @abarganier and @xinhaoz)
bors r+ |
Build failed (retrying...): |
Previously, enable hot ranges stats also required the enabling of diagnostic reporting. Hot ranges stats doesn't need to be dependent on diagnostic reporting and someone might want to enable hot ranges stats without enabling diagnostic reporting.
Now, server.telemetry.hot_ranges_stats.enabled can be set to true while without setting diagnostics.reporting.enabled
Epic: none
Fixes: #122977
Part of: https://cockroachlabs.atlassian.net/browse/CRDB-38152
Release note: None