-
Notifications
You must be signed in to change notification settings - Fork 5
test(compose): gate the sched log topic in mixed clusters #586
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,6 +32,9 @@ | |
| cargo-machete | ||
| protobuf | ||
| oas3-gen | ||
| go | ||
| gopls | ||
| delve | ||
| ]; | ||
|
|
||
| shellHook = '' | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -67,14 +67,11 @@ func TestWriteAlertRulesExcludesDegradedJobs(t *testing.T) { | |
| require.Contains(t, content, "expr: up == 0") | ||
| } | ||
|
|
||
| // TestWriteAlertRulesWarnTopicExtension asserts scenario-scoped warn-topic | ||
| // exclusions append to the built-in mock-noise list. | ||
| func TestWriteAlertRulesWarnTopicExtension(t *testing.T) { | ||
| conf := NewDefaultConfig() | ||
| conf.AlertWarnExcludeTopics = []string{"sched"} | ||
|
|
||
| content := writeRules(t, conf) | ||
| require.Contains(t, content, `increase(app_log_warn_total{topic!~"vmock|tracker|sched"}[30s]) > 2`) | ||
| // TestWriteAlertRulesWarnTopics asserts the Warn Log Rate gate excludes exactly | ||
| // the two charon mock-noise topics. | ||
| func TestWriteAlertRulesWarnTopics(t *testing.T) { | ||
| content := writeRules(t, NewDefaultConfig()) | ||
| require.Contains(t, content, `increase(app_log_warn_total{topic!~"vmock|tracker"}[30s]) > 2`) | ||
| } | ||
|
Comment on lines
+70
to
75
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Deliberately a stricter test than the one it replaces. The previous case asserted the knob's append behaviour; with the knob gone this pins the exclusion list exactly, so adding a third topic has to be an explicit edit here rather than a scenario-local escape hatch. Given that #583 exists because a topic was excluded on a rationale nobody re-checked, making the list hard to extend quietly is the point. |
||
|
|
||
| // TestWriteAlertRulesDropsOutstandingDuty pins the removal of charon's dead | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -236,12 +236,6 @@ func smokeScenarios() []smokeScenario { | |
| } | ||
| // `pluto run` fails fast on --synthetic-block-proposals. | ||
| conf.SyntheticBlockProposals = false | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Heads up if you run this locally. The scenario fails today, before and after this change, on #585 — the inclusion checker rejects charon beaconmock's block responses and re-warns every tick, so both pluto nodes trip |
||
| // Charon triggers infosync (/charon/priority/2.0.0) every | ||
| // epoch; pluto does not serve the protocol yet (#402B), so | ||
| // charon nodes warn "P2P sending failing" under topic=sched | ||
| // twice per epoch. Exempt that topic in mixed clusters until | ||
| // the protocol lands; drop this with #402B. | ||
| conf.AlertWarnExcludeTopics = []string{"sched"} | ||
| }, | ||
| }, | ||
| { | ||
|
|
||
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.
Why removing
schedis safe, and the trap to avoid. Pluto attaches a logtopicin exactly one live code path (the health checker'shealthspan). Everything else it warns about is recorded with no topic label, and Prometheus drops empty label values at ingest, so those series carry notopickey at all. A negative regex matcher selects a series whose label is absent, which meanstopic!~"vmock|tracker"andtopic!~"vmock|tracker|sched"gate pluto's warnings identically — verified withpromtool test ruleson the pinned Prometheus version. That is why droppingschedcannot alter the pluto side of the verdict, and why the control run failed the same way as the un-excluded one.The trap: both remaining topics are named after charon components, and the matcher itself is not scoped by job. If pluto ever adopts charon's topic convention, a pluto node emitting
topic=trackerwould start being excluded silently. Worth scoping this to charon jobs at that point rather than discovering it the way #583 was discovered.(Charon labels its own untopiced warnings
topic="unknown"rather than leaving the label empty — a divergence that happens not to matter here, since both forms are gated.)