Skip to content

Support matchers in rules API #1843

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Support matchers in rules API #1843

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions api/prometheus/v1/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ type API interface {
// under the TSDB's data directory and returns the directory as response.
Snapshot(ctx context.Context, skipHead bool) (SnapshotResult, error)
// Rules returns a list of alerting and recording rules that are currently loaded.
Rules(ctx context.Context) (RulesResult, error)
Rules(ctx context.Context, matches []string) (RulesResult, error)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a blocker, how about making matches a variadic argument? It would make it neater

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can do it if desired, but I also see some cons:

  • it's less consistent with other API functions that also use matches []string, such as LabelNames
  • if the signature has to change again later to add more arguments, depending on what those new arguments are, variadic may have to be reconsidered (e.g. if another []string-like argument is added)

I don't really have an opinion here, let me know what's best

Copy link
Author

@jotak jotak Jul 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I only proposed to add matches because it's the use-case I'm interested in, but there are a bunch of other arguments that could be relevant, according to the doc. I guess I'm trying to avoid a bigger refactoring, if that makes sense. But a more future-proof change would be to accept a parameters struct.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, never mind. I was lazy, and I hadn't checked the other parameters.
Even though we consider the API package as experimental, let's stick to the existing conventions.

We can add Options if needed.

When it comes to making it future-proof, maybe we can have the type as a first-order parameter as well and make the rest optional.

Any thoughts? cc @bwplotka

// Targets returns an overview of the current state of the Prometheus target discovery.
Targets(ctx context.Context) (TargetsResult, error)
// TargetsMetadata returns metadata about metrics currently scraped by the target.
Expand Down Expand Up @@ -1235,8 +1235,15 @@ func (h *httpAPI) Snapshot(ctx context.Context, skipHead bool) (SnapshotResult,
return res, err
}

func (h *httpAPI) Rules(ctx context.Context) (RulesResult, error) {
func (h *httpAPI) Rules(ctx context.Context, matches []string) (RulesResult, error) {
u := h.client.URL(epRules, nil)
q := u.Query()

for _, m := range matches {
q.Add("match[]", m)
}

u.RawQuery = q.Encode()

req, err := http.NewRequest(http.MethodGet, u.String(), nil)
if err != nil {
Expand Down
66 changes: 61 additions & 5 deletions api/prometheus/v1/api_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,9 +191,9 @@ func TestAPIs(t *testing.T) {
}
}

doRules := func() func() (interface{}, Warnings, error) {
doRules := func(matches []string) func() (interface{}, Warnings, error) {
return func() (interface{}, Warnings, error) {
v, err := promAPI.Rules(context.Background())
v, err := promAPI.Rules(context.Background(), matches)
return v, nil, err
}
}
Expand Down Expand Up @@ -689,7 +689,7 @@ func TestAPIs(t *testing.T) {
},

{
do: doRules(),
do: doRules(nil),
reqMethod: "GET",
reqPath: "/api/v1/rules",
inRes: map[string]interface{}{
Expand Down Expand Up @@ -784,7 +784,7 @@ func TestAPIs(t *testing.T) {

// This has the newer API elements like lastEvaluation, evaluationTime, etc.
{
do: doRules(),
do: doRules(nil),
reqMethod: "GET",
reqPath: "/api/v1/rules",
inRes: map[string]interface{}{
Expand Down Expand Up @@ -888,7 +888,63 @@ func TestAPIs(t *testing.T) {
},

{
do: doRules(),
do: doRules([]string{`severity="info"`}),
reqMethod: "GET",
reqPath: "/api/v1/rules",
inRes: map[string]interface{}{
"groups": []map[string]interface{}{
{
"file": "/rules.yaml",
"interval": 60,
"name": "example",
"rules": []map[string]interface{}{
{
"alerts": []map[string]interface{}{},
"annotations": map[string]interface{}{
"summary": "High request latency",
},
"duration": 600,
"health": "ok",
"labels": map[string]interface{}{
"severity": "info",
},
"name": "HighRequestLatency",
"query": "job:request_latency_seconds:mean5m{job=\"myjob\"} > 0.5",
"type": "alerting",
},
},
},
},
},
res: RulesResult{
Groups: []RuleGroup{
{
Name: "example",
File: "/rules.yaml",
Interval: 60,
Rules: []interface{}{
AlertingRule{
Alerts: []*Alert{},
Annotations: model.LabelSet{
"summary": "High request latency",
},
Labels: model.LabelSet{
"severity": "info",
},
Duration: 600,
Health: RuleHealthGood,
Name: "HighRequestLatency",
Query: "job:request_latency_seconds:mean5m{job=\"myjob\"} > 0.5",
LastError: "",
},
},
},
},
},
},

{
do: doRules(nil),
reqMethod: "GET",
reqPath: "/api/v1/rules",
inErr: errors.New("some error"),
Expand Down
Loading