feat(api): add receiver labels and receiver_matchers filter#5152
feat(api): add receiver labels and receiver_matchers filter#5152cxdy wants to merge 1 commit intoprometheus:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds receiver labels to config and API models; surfaces labels in API responses; supports a multi-valued Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as API Handler
participant Config as Config Store
participant Matcher as Matcher Engine
participant Encoder as JSON Encoder
Client->>API: GET /api/v2/receivers?receiver_matchers=owner="team-a"
activate API
API->>API: parseFilter(receiver_matchers)
API->>Config: read receivers (includes Labels)
Config-->>API: receivers + labels
API->>Matcher: receiversMatchLabels(matchers, receiverLabels)
Matcher-->>API: matched receiver names
API->>API: map matched via configReceiverToAPIReceiver
API->>Encoder: encode matched receivers (include labels)
Encoder-->>Client: 200 OK JSON
deactivate API
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
c64c08e to
e713404
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (4)
config/config_test.go (1)
249-269: Test name slightly misleading - only tests hyphens, not UTF-8.The test
TestReceiverLabelsAllowsHyphensAndUTF8only tests hyphen characters in label keys/values. Consider either renaming toTestReceiverLabelsAllowsHyphensor adding actual UTF-8 test cases (e.g.,"owner": "équipe-α").🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/config_test.go` around lines 249 - 269, The test named TestReceiverLabelsAllowsHyphensAndUTF8 only verifies hyphens; either rename the test to TestReceiverLabelsAllowsHyphens or extend it to include a UTF‑8 case: update TestReceiverLabelsAllowsHyphensAndUTF8 to add a receiver label and/or value with UTF‑8 characters (e.g., "owner": "équipe-α") and assert via cfg := Load(in) and rcv := cfg.Receivers[0] that rcv.Labels contains the expected UTF‑8 key/value, or simply rename the function to TestReceiverLabelsAllowsHyphens if you prefer not to add UTF‑8 content.api/v2/api_test.go (2)
685-714: Add a matcher case for the injectednamelabel.This table only exercises user-defined labels. A
receiverMatchers: []string{name="team-X"}case would lock the filter path to the same invariant the response assertions rely on.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/v2/api_test.go` around lines 685 - 714, Add a test row to the table-driven test (the slice of structs iterated as tc with fields receiverMatchers and expectedNames) that exercises the injected name label by setting receiverMatchers: []string{`name="team-X"`} and expectedNames: []string{"team-X"}; this ensures the filter path that handles the injected `name` label is exercised alongside the existing owner/kind cases (look for the receiverMatchers / expectedNames entries in the test table).
658-916: Please cover the other tworeceiver_matchershandlers end-to-end.This file now exercises
/api/v2/receiversplus helper functions, but the new request-parsing and filtering branches ingetAlertsHandlerandgetAlertGroupsHandlerstill are not hit here. A minimal happy-path and bad-matcher case for each endpoint would catch wiring regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/v2/api_test.go` around lines 658 - 916, Add end-to-end tests that exercise the new request-parsing and filtering branches for getAlertsHandler and getAlertGroupsHandler: create minimal config and alerts/alert-groups fixtures, then call api.getAlertsHandler and api.getAlertGroupsHandler with valid ReceiverMatchers (e.g. owner="team-x") and assert the filtered results match expected names/IDs, and add companion tests that pass an invalid matcher string (like "not-a-valid-matcher!!!") and assert a 400 response; reference the handler functions getAlertsHandler and getAlertGroupsHandler and reuse the pattern from TestGetReceiversHandlerWithReceiverMatchers and TestGetReceiversHandlerBadReceiverMatchers to wire HTTP request creation, responder.WriteResponse, and response assertions.api/v2/api.go (1)
250-256: Derive receiver labels once and reuse them.
getReceiversHandlerfilters on rawrcv.Labels, while the response and alert/group paths build separate label copies. A single normalization helper keepsreceiver_matchersaligned with the payload you serialize and gives you one place to preserve the injectednamelabel contract.♻️ Possible consolidation
+func normalizedReceiverLabels(rcv *config.Receiver) open_api_models.LabelSet { + apiLabels := make(open_api_models.LabelSet, len(rcv.Labels)+1) + maps.Copy(apiLabels, rcv.Labels) + if _, ok := apiLabels["name"]; !ok { + apiLabels["name"] = rcv.Name + } + return apiLabels +} + func (api *API) getReceiversHandler(params receiver_ops.GetReceiversParams) middleware.Responder { // ... receivers := make([]*open_api_models.Receiver, 0, len(api.alertmanagerConfig.Receivers)) for i := range api.alertmanagerConfig.Receivers { rcv := &api.alertmanagerConfig.Receivers[i] - if len(receiverMatchers) > 0 && !matchFilterLabels(receiverMatchers, rcv.Labels) { + apiRcv := configReceiverToAPIReceiver(rcv) + if len(receiverMatchers) > 0 && !matchFilterLabels(receiverMatchers, apiRcv.Labels) { continue } - receivers = append(receivers, configReceiverToAPIReceiver(rcv)) + receivers = append(receivers, apiRcv) } } func configReceiverToAPIReceiver(rcv *config.Receiver) *open_api_models.Receiver { - apiLabels := make(open_api_models.LabelSet, len(rcv.Labels)) - maps.Copy(apiLabels, rcv.Labels) return &open_api_models.Receiver{ Name: &rcv.Name, - Labels: apiLabels, + Labels: normalizedReceiverLabels(rcv), } } func (api *API) receiverLabelsMap() map[string]open_api_models.LabelSet { m := make(map[string]open_api_models.LabelSet, len(api.alertmanagerConfig.Receivers)) for i := range api.alertmanagerConfig.Receivers { rcv := &api.alertmanagerConfig.Receivers[i] - apiLabels := make(open_api_models.LabelSet, len(rcv.Labels)) - maps.Copy(apiLabels, rcv.Labels) - m[rcv.Name] = apiLabels + m[rcv.Name] = normalizedReceiverLabels(rcv) } return m }Also applies to: 601-621
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/v2/api.go` around lines 250 - 256, getReceiversHandler currently filters using raw rcv.Labels but later builds separate label copies for response and alert/group paths, causing inconsistency; introduce a single normalization helper (e.g., normalizeReceiverLabels) and call it once per receiver to produce the canonical label map (including the injected "name" label) and then use that same map for matchFilterLabels(receiverMatchers, ...), for passing into configReceiverToAPIReceiver, and anywhere else receiver labels are constructed (also apply same change to the similar block at lines ~601-621) so all code reuses the derived labels rather than duplicating raw rcv.Labels.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@api/v2/api_test.go`:
- Around line 685-714: Add a test row to the table-driven test (the slice of
structs iterated as tc with fields receiverMatchers and expectedNames) that
exercises the injected name label by setting receiverMatchers:
[]string{`name="team-X"`} and expectedNames: []string{"team-X"}; this ensures
the filter path that handles the injected `name` label is exercised alongside
the existing owner/kind cases (look for the receiverMatchers / expectedNames
entries in the test table).
- Around line 658-916: Add end-to-end tests that exercise the new
request-parsing and filtering branches for getAlertsHandler and
getAlertGroupsHandler: create minimal config and alerts/alert-groups fixtures,
then call api.getAlertsHandler and api.getAlertGroupsHandler with valid
ReceiverMatchers (e.g. owner="team-x") and assert the filtered results match
expected names/IDs, and add companion tests that pass an invalid matcher string
(like "not-a-valid-matcher!!!") and assert a 400 response; reference the handler
functions getAlertsHandler and getAlertGroupsHandler and reuse the pattern from
TestGetReceiversHandlerWithReceiverMatchers and
TestGetReceiversHandlerBadReceiverMatchers to wire HTTP request creation,
responder.WriteResponse, and response assertions.
In `@api/v2/api.go`:
- Around line 250-256: getReceiversHandler currently filters using raw
rcv.Labels but later builds separate label copies for response and alert/group
paths, causing inconsistency; introduce a single normalization helper (e.g.,
normalizeReceiverLabels) and call it once per receiver to produce the canonical
label map (including the injected "name" label) and then use that same map for
matchFilterLabels(receiverMatchers, ...), for passing into
configReceiverToAPIReceiver, and anywhere else receiver labels are constructed
(also apply same change to the similar block at lines ~601-621) so all code
reuses the derived labels rather than duplicating raw rcv.Labels.
In `@config/config_test.go`:
- Around line 249-269: The test named TestReceiverLabelsAllowsHyphensAndUTF8
only verifies hyphens; either rename the test to TestReceiverLabelsAllowsHyphens
or extend it to include a UTF‑8 case: update
TestReceiverLabelsAllowsHyphensAndUTF8 to add a receiver label and/or value with
UTF‑8 characters (e.g., "owner": "équipe-α") and assert via cfg := Load(in) and
rcv := cfg.Receivers[0] that rcv.Labels contains the expected UTF‑8 key/value,
or simply rename the function to TestReceiverLabelsAllowsHyphens if you prefer
not to add UTF‑8 content.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 274c1e3d-c599-4f0a-922b-db5d40e6aba4
📒 Files selected for processing (19)
api/v2/api.goapi/v2/api_test.goapi/v2/client/alert/get_alerts_parameters.goapi/v2/client/alertgroup/get_alert_groups_parameters.goapi/v2/client/receiver/get_receivers_parameters.goapi/v2/client/receiver/get_receivers_responses.goapi/v2/compat.goapi/v2/models/receiver.goapi/v2/openapi.yamlapi/v2/restapi/embedded_spec.goapi/v2/restapi/operations/alert/get_alerts_parameters.goapi/v2/restapi/operations/alert/get_alerts_urlbuilder.goapi/v2/restapi/operations/alertgroup/get_alert_groups_parameters.goapi/v2/restapi/operations/alertgroup/get_alert_groups_urlbuilder.goapi/v2/restapi/operations/receiver/get_receivers_parameters.goapi/v2/restapi/operations/receiver/get_receivers_responses.goapi/v2/restapi/operations/receiver/get_receivers_urlbuilder.goconfig/config.goconfig/config_test.go
There was a problem hiding this comment.
🧹 Nitpick comments (1)
config/config_test.go (1)
249-269: Test name is misleading - no UTF-8 characters are actually tested.The test name
TestReceiverLabelsAllowsHyphensAndUTF8implies UTF-8 character validation, but the test only verifies hyphens in label keys and values. Consider either renaming the test or adding actual UTF-8 test cases.📝 Option 1: Rename to reflect actual behavior
-func TestReceiverLabelsAllowsHyphensAndUTF8(t *testing.T) { +func TestReceiverLabelsAllowsHyphens(t *testing.T) {📝 Option 2: Add actual UTF-8 test cases
func TestReceiverLabelsAllowsHyphensAndUTF8(t *testing.T) { in := ` route: - receiver: team-X-slack + receiver: team-X-日本語 receivers: -- name: 'team-X-slack' +- name: 'team-X-日本語' labels: - owning-team: team-X-slack + owning-team: équipe-α kind: heartbeat `🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/config_test.go` around lines 249 - 269, The test TestReceiverLabelsAllowsHyphensAndUTF8 is misleading because it only asserts hyphens; either rename the test to TestReceiverLabelsAllowsHyphens (update the function name) or extend the YAML input string `in` and assertions to include real UTF-8 label keys/values (e.g., add a receiver label like `团队: "团队-slack"` or an emoji key/value) and add assertions on cfg.Receivers[0].Labels for those UTF-8 entries so the test actually verifies UTF-8 handling; update the test function name and assertions accordingly (referencing the TestReceiverLabelsAllowsHyphensAndUTF8 function and the `in` variable and cfg.Receivers[0].Labels lookup).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@config/config_test.go`:
- Around line 249-269: The test TestReceiverLabelsAllowsHyphensAndUTF8 is
misleading because it only asserts hyphens; either rename the test to
TestReceiverLabelsAllowsHyphens (update the function name) or extend the YAML
input string `in` and assertions to include real UTF-8 label keys/values (e.g.,
add a receiver label like `团队: "团队-slack"` or an emoji key/value) and add
assertions on cfg.Receivers[0].Labels for those UTF-8 entries so the test
actually verifies UTF-8 handling; update the test function name and assertions
accordingly (referencing the TestReceiverLabelsAllowsHyphensAndUTF8 function and
the `in` variable and cfg.Receivers[0].Labels lookup).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a3a886ef-46e8-44d5-acc5-800cce3178b3
📒 Files selected for processing (21)
api/v2/api.goapi/v2/api_test.goapi/v2/client/alert/get_alerts_parameters.goapi/v2/client/alertgroup/get_alert_groups_parameters.goapi/v2/client/receiver/get_receivers_parameters.goapi/v2/client/receiver/get_receivers_responses.goapi/v2/compat.goapi/v2/models/receiver.goapi/v2/openapi.yamlapi/v2/restapi/embedded_spec.goapi/v2/restapi/operations/alert/get_alerts_parameters.goapi/v2/restapi/operations/alert/get_alerts_urlbuilder.goapi/v2/restapi/operations/alertgroup/get_alert_groups_parameters.goapi/v2/restapi/operations/alertgroup/get_alert_groups_urlbuilder.goapi/v2/restapi/operations/receiver/get_receivers_parameters.goapi/v2/restapi/operations/receiver/get_receivers_responses.goapi/v2/restapi/operations/receiver/get_receivers_urlbuilder.goconfig/config.goconfig/config_test.goui/app/src/Data/Receiver.elmui/app/src/Views/AlertList/Updates.elm
✅ Files skipped from review due to trivial changes (2)
- api/v2/restapi/operations/receiver/get_receivers_urlbuilder.go
- api/v2/restapi/operations/receiver/get_receivers_responses.go
🚧 Files skipped from review as they are similar to previous changes (7)
- api/v2/restapi/operations/receiver/get_receivers_parameters.go
- api/v2/restapi/operations/alertgroup/get_alert_groups_parameters.go
- api/v2/client/alert/get_alerts_parameters.go
- api/v2/compat.go
- api/v2/models/receiver.go
- api/v2/api_test.go
- api/v2/api.go
|
This looks awesome, thanks for the contribution! I am away for a little while, but I will do a code review as soon as I'm able. Perhaps the other maintainers will get to to first 🙂 |
siavashs
left a comment
There was a problem hiding this comment.
LGTM, Thank you for your contribution.
siavashs
left a comment
There was a problem hiding this comment.
There is a frontend test error:
Compiling (80)-- TYPE MISMATCH ------------------------------- src/Views/AlertList/Updates.elm
The 2nd argument to `AlertGroup` is not what I expect:
47| AlertGroup (Dict.fromList labels) { name = "unknown", labels = Dict.empty } alerts_
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This argument is a record of type:
{ labels : Dict.Dict k v, name : String }
But `AlertGroup` needs the 2nd argument to be:
Data.Receiver.ReceiverSee the CI job for details.
e20d804 to
03caa8a
Compare
Add support for arbitrary key-value labels on receivers, enabling structured identification for querying and filtering. The receiver name is auto-injected as the "name" label for uniform matcher access. Expose labels in all API receiver responses (GET /receivers, GET /alerts, GET /alerts/groups) and add a receiver_matchers query parameter to all three endpoints for filtering by receiver labels using PromQL-style matchers. Ref: prometheus#4963 Signed-off-by: Cody Kaczynski <ckaczyns@akamai.com>
03caa8a to
1f1073f
Compare
|
Ah thanks @siavashs! I got it passing, just rebased and letting it run again. |
SoloJacobs
left a comment
There was a problem hiding this comment.
Two issues we should discuss:
-
The name auto-injection makes every user opt into this feature silently on upgrade. Since receiver names are already unique identifiers, re-expressing them as a label feels like a category error. I'd like to discuss whether this is the right default before it ships.
-
Returning labels on every
GettableAlertduplicates static receiver configuration across every alert in the response. There are also an open proposal to remove the receiver field from /alerts altogether #4727 . Expanding it now moves in the opposite direction. The right place for receiver metadata is GET /receivers. Callers who need the mapping can join on receiver.name client-side.
Suggested direction: keep labels and receiver_matchers on GET /receivers only, and drop them from the alert/alert-group responses.
| '400': | ||
| $ref: '#/responses/BadRequest' | ||
| '500': | ||
| $ref: '#/responses/InternalServerError' |
There was a problem hiding this comment.
Why is there a 500 error code here? Does this implemenation make it so the receiver endpoint no longer works correctly in all circumstances?
| description: Get list of all receivers (name of notification integrations) | ||
| parameters: |
There was a problem hiding this comment.
It seems like receiver_matchers should not be duplicated? Can't we use a $ref here?
| // Labels attached to this receiver for querying and filtering. | ||
| Labels map[string]string `yaml:"labels,omitempty" json:"labels,omitempty"` |
There was a problem hiding this comment.
This needs to be documented in docs/configuration.md.
Add support for arbitrary key-value labels on receivers, enabling structured identification for querying and filtering. The receiver name is auto-injected as the "name" label for uniform matcher access.
Expose labels in all API receiver responses (GET /receivers, GET /alerts, GET /alerts/groups) and add a receiver_matchers query parameter to all three endpoints for filtering by receiver labels using PromQL-style matchers.
Pull Request Checklist
Which user-facing changes does this PR introduce?
labelstoreceiversreceiver.labelsin/api/v2/alerts,/api/v2/alerts/groups, and/api/v2/receiversGood config + response
Bad config + error
time=2026-04-03T12:59:36.559Z level=ERROR source=coordinator.go:116 msg="Loading configuration file failed" component=configuration file=/path/to/alertmanager.yml err="receiver label \"name\" must match receiver name \"this-is-my-receiver\", got \"this-is-not-my-receiver-name\""I'm sort of torn on the error handling though, I currently have it failing hard (seems like that's the general convention in Alertmanager for this sort of thing) if
receiver[i].name != receiver[i].labels.name, but that seems a little dramatic for what this is. Open to feedback on that one, happy to change it if you agree with me or leave it if you don't!Summary by CodeRabbit
New Features
API Changes
Config
Tests
UI