CNV-80608: PR12 add alerts_effective_active_at_timestamp_seconds metric#16
Open
sradco wants to merge 1 commit into
Open
CNV-80608: PR12 add alerts_effective_active_at_timestamp_seconds metric#16sradco wants to merge 1 commit into
sradco wants to merge 1 commit into
Conversation
machadovilaca
suggested changes
Mar 9, 2026
sradco
changed the title
sradco
force-pushed
the
alert-mgmt-15-orphan-arc-gc
branch
2 times, most recently
from
760bab8 to
f62057b
Compare
sradco
force-pushed
the
alerts-effective-metric
branch
from
5d89003 to
efd5a1a
Compare
sradco
force-pushed
the
alert-mgmt-15-orphan-arc-gc
branch
from
f62057b to
bad1af3
Compare
sradco
force-pushed
the
alerts-effective-metric
branch
2 times, most recently
from
e8fff27 to
56481c7
Compare
Owner
Author
|
Please note, I found a bug with the metric value. It was based on the StartAt label of Alertmanager, which I thought holds the alert start time, but it actually holds the time the alert was collected to Alertmanager, which is nt what we need. |
sradco
force-pushed
the
alert-mgmt-15-orphan-arc-gc
branch
from
bad1af3 to
6ff63c9
Compare
sradco
force-pushed
the
alerts-effective-metric
branch
from
56481c7 to
ba86543
Compare
sradco
force-pushed
the
alert-mgmt-15-orphan-arc-gc
branch
from
6ff63c9 to
6b2d009
Compare
sradco
force-pushed
the
alerts-effective-metric
branch
from
ba86543 to
6cfc7e4
Compare
sradco
force-pushed
the
alert-mgmt-15-orphan-arc-gc
branch
from
6b2d009 to
1c95614
Compare
sradco
force-pushed
the
alerts-effective-metric
branch
from
6cfc7e4 to
5c81f95
Compare
sradco
force-pushed
the
alert-mgmt-15-orphan-arc-gc
branch
from
1c95614 to
6ace03c
Compare
sradco
force-pushed
the
alerts-effective-metric
branch
2 times, most recently
from
6ec1d22 to
8ad2c3a
Compare
machadovilaca
suggested changes
Mar 11, 2026
Comment on lines
+357
to
+358
| enrichActiveAt(alerts, promAlerts) | ||
| return append(alerts, filterAlertsByState(promAlerts, "pending")...), nil |
There was a problem hiding this comment.
why not filter first then enrich?
Comment on lines
299
to
301
| if promErr != nil { | ||
| return nil, promErr | ||
| } |
There was a problem hiding this comment.
this should be done immediately after the failed call
| promAlerts, promErr := pa.getAlertsViaProxy(ctx, namespace, promRouteName, source) | ||
|
|
||
| if amErr == nil { | ||
| if promErr == nil { |
There was a problem hiding this comment.
if we handled the error, this check wouldnt be needed
| if promErr == nil { | ||
| enrichActiveAt(amAlerts, promAlerts) | ||
| } | ||
| pending := filterAlertsByState(promAlerts, "pending") |
There was a problem hiding this comment.
we are only filtering if no amErr
why would we return all promAlerts if no amErr
but return only the pending if amErr
|
|
||
| // alertFingerprint builds a stable identity key from an alert's labels, | ||
| // excluding metadata labels injected by this plugin (source, backend). | ||
| func alertFingerprint(labels map[string]string) string { |
There was a problem hiding this comment.
whats the difference between using this implementation and the one for the id label?
| managementRouter := managementrouter.New(managementClient) | ||
| router.PathPrefix("/api/v1/alerting").Handler(managementRouter) | ||
|
|
||
| metricsHandler, err := managementClient.MetricsHandler(ctx, k8sconfig) |
There was a problem hiding this comment.
maybe pkg/monitoring/management/metrics/alerts_collector.go
or even
pkg/management/metrics/alerts_collector.go
There was a problem hiding this comment.
in pkg/management/management.go
"github.com/openshift/monitoring-plugin/pkg/management/metrics" -> metrics.NewHandler(ctx, c, kubeConfig)
instead of
"github.com/openshift/monitoring-plugin/pkg/metrics" -> metrics.NewHandler(ctx, c, kubeConfig)
| // backend, component, layer) plus "alertstate". Thanos-sourced alerts are | ||
| // filtered out to avoid duplicates. Annotations are excluded because they | ||
| // are available from the alert rule definition. | ||
| type AlertsCollector struct { |
There was a problem hiding this comment.
should only be instantiated if isLeader
and not receive a function to check
There was a problem hiding this comment.
should not belong in metrics package
metrics, specifically the new collector, depends on being the leader
not the other way around
leader election is a server responsability
server should manage the leader election
and then if takes the lead, instantiate the collector
sradco
force-pushed
the
alert-mgmt-15-orphan-arc-gc
branch
from
6ace03c to
53c6cf9
Compare
sradco
force-pushed
the
alerts-effective-metric
branch
6 times, most recently
from
ce1decd to
dc78a65
Compare
sradco
changed the base branch from
alert-mgmt-15-orphan-arc-gc
to
alert-mgmt-restructured-11-orphan-gc
sradco
changed the title
sradco
force-pushed
the
alert-mgmt-restructured-11-orphan-gc
branch
from
f99316a to
2aeb592
Compare
sradco
force-pushed
the
alerts-effective-metric
branch
from
dc78a65 to
0147856
Compare
sradco
force-pushed
the
alert-mgmt-restructured-11-orphan-gc
branch
from
0e3ceaa to
5f87dde
Compare
sradco
force-pushed
the
alerts-effective-metric
branch
from
497e2db to
0398569
Compare
sradco
force-pushed
the
alert-mgmt-restructured-11-orphan-gc
branch
from
5f87dde to
f5bda9f
Compare
sradco
force-pushed
the
alerts-effective-metric
branch
from
0398569 to
cb7aa1d
Compare
sradco
force-pushed
the
alert-mgmt-restructured-11-orphan-gc
branch
from
f5bda9f to
7276833
Compare
sradco
force-pushed
the
alerts-effective-metric
branch
from
cb7aa1d to
cfc4f9f
Compare
sradco
force-pushed
the
alert-mgmt-restructured-11-orphan-gc
branch
from
7276833 to
528338b
Compare
sradco
force-pushed
the
alerts-effective-metric
branch
from
cfc4f9f to
3edd517
Compare
sradco
force-pushed
the
alert-mgmt-restructured-11-orphan-gc
branch
4 times, most recently
from
b6094a8 to
a262f88
Compare
sradco
force-pushed
the
alerts-effective-metric
branch
2 times, most recently
from
db8d155 to
5c7c6d4
Compare
sradco
force-pushed
the
alert-mgmt-restructured-11-orphan-gc
branch
from
a262f88 to
f2f4bbe
Compare
sradco
force-pushed
the
alerts-effective-metric
branch
from
5c7c6d4 to
d24450d
Compare
sradco
force-pushed
the
alert-mgmt-restructured-11-orphan-gc
branch
from
f2f4bbe to
6014450
Compare
sradco
force-pushed
the
alerts-effective-metric
branch
from
d24450d to
8cbd67c
Compare
sradco
force-pushed
the
alert-mgmt-restructured-11-orphan-gc
branch
from
6014450 to
598abd6
Compare
sradco
force-pushed
the
alerts-effective-metric
branch
from
8cbd67c to
a217801
Compare
sradco
force-pushed
the
alert-mgmt-restructured-11-orphan-gc
branch
from
598abd6 to
5b090e4
Compare
sradco
force-pushed
the
alerts-effective-metric
branch
from
a217801 to
59c226e
Compare
sradco
force-pushed
the
alert-mgmt-restructured-11-orphan-gc
branch
2 times, most recently
from
672bc3b to
a24fa21
Compare
sradco
force-pushed
the
alerts-effective-metric
branch
from
59c226e to
31921ea
Compare
sradco
force-pushed
the
alert-mgmt-restructured-11-orphan-gc
branch
from
a24fa21 to
ed02e88
Compare
sradco
force-pushed
the
alerts-effective-metric
branch
from
31921ea to
e2a91c0
Compare
sradco
force-pushed
the
alert-mgmt-restructured-11-orphan-gc
branch
from
ed02e88 to
6a001cd
Compare
sradco
force-pushed
the
alerts-effective-metric
branch
from
e2a91c0 to
66459e8
Compare
Expose a Prometheus gauge metric whose value is the activeAt Unix timestamp for every effective alert (firing, pending, silenced). Labels include all alerts labels after relabeling plus enrichment labels and alertstate. Annotations are excluded since they are available from the alert rule definition. Signed-off-by: Shirly Radco <sradco@redhat.com> Co-authored-by: AI Assistant <noreply@cursor.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Expose a Prometheus gauge metric whose value is the activeAt Unix
timestamp for every effective alert (firing, pending, silenced).
Labels include all alerts labels after relabeling plus enrichment labels
and alertstate.
Annotations are excluded since they are available from the alert rule definition.
Signed-off-by: Shirly Radco sradco@redhat.com
Co-authored-by: AI Assistant noreply@cursor.com