Skip to content

chore: fix missleading errors total grafana #969

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 9 commits into
base: main
Choose a base branch
from
Open

chore: fix missleading errors total grafana #969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
425680d
chore: fix missleading errors total grafana
Oct 31, 2023
381dbde
chore: add missing rate in other places
Nov 8, 2023
a0adc8e
chore: update faling unit tests
Nov 8, 2023
95795b2
chore: update faling unit tests
Nov 8, 2023
346684e
chore: update faling unit tests
Nov 8, 2023
c9812c6
chore: update faling unit tests
Nov 8, 2023
35bd02e
chore: update faling unit tests
Nov 8, 2023
234c7e9
chore: update errors and request rr name
Nov 8, 2023
80a7f11
chore: address comment from review
Dec 10, 2023
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
4 changes: 2 additions & 2 deletions examples/grafana/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ It means that Pyrra generates metrics with the same name for each SLO.
pyrra_objective
pyrra_window
pyrra_availability
pyrra_requests_total
pyrra_errors_total
pyrra_requests:rate5m
pyrra_errors:rate5m
```

The Grafana dashboards rely on these recording rules. You need to enabled generating these on the `filesystem` or `kubernetes` component.
Expand Down
8 changes: 4 additions & 4 deletions examples/grafana/detail.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@
"uid": "${DS_PROMETHEUS}"
},
"editorMode": "code",
"expr": "sum(rate(pyrra_requests_total{slo=\"$slo\"}[$__rate_interval]))",
"expr": "sum(pyrra_requests:rate5m{slo=\"$slo\"}[$__rate_interval])",
"hide": false,
"legendFormat": "__auto",
"range": true,
Expand Down Expand Up @@ -598,7 +598,7 @@
}
]
},
"unit": "percentunit"
Copy link
Member

Choose a reason for hiding this comment

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

To keep the RED (Requests, Error Rate, Duration) type of panels, how about actually using the error rate percentage?

"unit": "reqps"
},
"overrides": [
{
Expand Down Expand Up @@ -644,7 +644,7 @@
"uid": "${DS_PROMETHEUS}"
},
"editorMode": "builder",
"expr": "sum(rate(pyrra_errors_total{slo=\"$slo\"}[$__rate_interval]))",
"expr": "sum(pyrra_errors:rate5m{slo=\"$slo\"})",
"hide": false,
"legendFormat": "__auto",
"range": true,
Expand Down Expand Up @@ -715,4 +715,4 @@
"uid": "ccssRIenz",
"version": 1,
"weekStart": ""
}
}
4 changes: 3 additions & 1 deletion slo/promql.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,9 @@ func (r objectiveReplacer) replace(node parser.Node) {
r.replace(expr)
}
case *parser.MatrixSelector:
n.Range = r.window
if r.window != 0 {
n.Range = r.window
}
r.replace(n.VectorSelector)
case *parser.VectorSelector:
if n.Name == "errorMetric" {
Expand Down
20 changes: 10 additions & 10 deletions slo/rules.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1230,7 +1230,7 @@ func (o Objective) GenericRules() (monitoringv1.RuleGroup, error) {
Labels: ruleLabels,
})

rate, err := parser.ParseExpr(`sum(metric{matchers="total"})`)
rate, err := parser.ParseExpr(`sum(rate(metric{matchers="total"}[5m]))`)
if err != nil {
return monitoringv1.RuleGroup{}, err
}
Expand All @@ -1241,13 +1241,13 @@ func (o Objective) GenericRules() (monitoringv1.RuleGroup, error) {
}.replace(rate)

rules = append(rules, monitoringv1.Rule{
Record: "pyrra_requests_total",
Record: "pyrra_requests:rate5m",
Expr: intstr.FromString(rate.String()),
Labels: ruleLabels,
})

errorsExpr := func() (parser.Expr, error) { // Returns a new instance of Expr with this query each time called
return parser.ParseExpr(`sum(metric{matchers="total"} or vector(0))`)
return parser.ParseExpr(`sum(rate(metric{matchers="total"}[5m])) or vector(0)`)
}
errorsParsedExpr, err := errorsExpr()
if err != nil {
Expand All @@ -1260,7 +1260,7 @@ func (o Objective) GenericRules() (monitoringv1.RuleGroup, error) {
}.replace(errorsParsedExpr)

rules = append(rules, monitoringv1.Rule{
Record: "pyrra_errors_total",
Record: "pyrra_errors:rate5m",
Expr: intstr.FromString(errorsParsedExpr.String()),
Labels: ruleLabels,
})
Expand Down Expand Up @@ -1321,7 +1321,7 @@ func (o Objective) GenericRules() (monitoringv1.RuleGroup, error) {
}
// rate
{
rate, err := parser.ParseExpr(`sum(metric{matchers="total"})`)
rate, err := parser.ParseExpr(`sum(rate(metric{matchers="total"}[5m]))`)
if err != nil {
return monitoringv1.RuleGroup{}, err
}
Expand All @@ -1340,14 +1340,14 @@ func (o Objective) GenericRules() (monitoringv1.RuleGroup, error) {
}.replace(rate)

rules = append(rules, monitoringv1.Rule{
Record: "pyrra_requests_total",
Record: "pyrra_requests:rate5m",
Expr: intstr.FromString(rate.String()),
Labels: ruleLabels,
})
}
// errors
{
errorsExpr, err := parser.ParseExpr(`sum(metric{matchers="total"}) - sum(errorMetric{matchers="errors"})`)
errorsExpr, err := parser.ParseExpr(`sum(rate(metric{matchers="total"}[5m])) - sum(rate(errorMetric{matchers="errors"}[5m]))`)
if err != nil {
return monitoringv1.RuleGroup{}, err
}
Expand Down Expand Up @@ -1378,7 +1378,7 @@ func (o Objective) GenericRules() (monitoringv1.RuleGroup, error) {
}.replace(errorsExpr)

rules = append(rules, monitoringv1.Rule{
Record: "pyrra_errors_total",
Record: "pyrra_errors:rate5m",
Expr: intstr.FromString(errorsExpr.String()),
Labels: ruleLabels,
})
Expand Down Expand Up @@ -1451,7 +1451,7 @@ func (o Objective) GenericRules() (monitoringv1.RuleGroup, error) {
}.replace(rate)

rules = append(rules, monitoringv1.Rule{
Record: "pyrra_requests_total",
Record: "pyrra_requests:rate5m",
Expr: intstr.FromString(rate.String()),
Labels: ruleLabels,
})
Expand All @@ -1472,7 +1472,7 @@ func (o Objective) GenericRules() (monitoringv1.RuleGroup, error) {
}.replace(rate)

rules = append(rules, monitoringv1.Rule{
Record: "pyrra_errors_total",
Record: "pyrra_errors:rate5m",
Expr: intstr.FromString(rate.String()),
Labels: ruleLabels,
})
Expand Down
52 changes: 26 additions & 26 deletions slo/rules_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1640,12 +1640,12 @@ func TestObjective_GrafanaRules(t *testing.T) {
Expr: intstr.FromString(`1 - sum(http_requests:increase4w{code=~"5..",job="thanos-receive-default",slo="monitoring-http-errors"} or vector(0)) / sum(http_requests:increase4w{job="thanos-receive-default",slo="monitoring-http-errors"})`),
Labels: map[string]string{"slo": "monitoring-http-errors"},
}, {
Record: "pyrra_requests_total",
Expr: intstr.FromString(`sum(http_requests_total{job="thanos-receive-default"})`),
Record: "pyrra_requests:rate5m",
Expr: intstr.FromString(`sum(rate(http_requests_total{job="thanos-receive-default"}[5m]))`),
Labels: map[string]string{"slo": "monitoring-http-errors"},
}, {
Record: "pyrra_errors_total",
Expr: intstr.FromString(`sum(http_requests_total{code=~"5..",job="thanos-receive-default"} or vector(0))`),
Record: "pyrra_errors:rate5m",
Expr: intstr.FromString(`sum(rate(http_requests_total{code=~"5..",job="thanos-receive-default"}[5m])) or vector(0)`),
Labels: map[string]string{"slo": "monitoring-http-errors"},
}},
},
Expand Down Expand Up @@ -1676,12 +1676,12 @@ func TestObjective_GrafanaRules(t *testing.T) {
Expr: intstr.FromString(`1 - sum(grpc_server_handled:increase4w{grpc_code=~"Aborted|Unavailable|Internal|Unknown|Unimplemented|DataLoss",grpc_method="Write",grpc_service="conprof.WritableProfileStore",job="api",slo="monitoring-grpc-errors"} or vector(0)) / sum(grpc_server_handled:increase4w{grpc_method="Write",grpc_service="conprof.WritableProfileStore",job="api",slo="monitoring-grpc-errors"})`),
Labels: map[string]string{"slo": "monitoring-grpc-errors"},
}, {
Record: "pyrra_requests_total",
Expr: intstr.FromString(`sum(grpc_server_handled_total{grpc_method="Write",grpc_service="conprof.WritableProfileStore",job="api"})`),
Record: "pyrra_requests:rate5m",
Expr: intstr.FromString(`sum(rate(grpc_server_handled_total{grpc_method="Write",grpc_service="conprof.WritableProfileStore",job="api"}[5m]))`),
Labels: map[string]string{"slo": "monitoring-grpc-errors"},
}, {
Record: "pyrra_errors_total",
Expr: intstr.FromString(`sum(grpc_server_handled_total{grpc_code=~"Aborted|Unavailable|Internal|Unknown|Unimplemented|DataLoss",grpc_method="Write",grpc_service="conprof.WritableProfileStore",job="api"} or vector(0))`),
Record: "pyrra_errors:rate5m",
Expr: intstr.FromString(`sum(rate(grpc_server_handled_total{grpc_code=~"Aborted|Unavailable|Internal|Unknown|Unimplemented|DataLoss",grpc_method="Write",grpc_service="conprof.WritableProfileStore",job="api"}[5m])) or vector(0)`),
Labels: map[string]string{"slo": "monitoring-grpc-errors"},
}},
},
Expand All @@ -1708,12 +1708,12 @@ func TestObjective_GrafanaRules(t *testing.T) {
Expr: intstr.FromString(`sum(http_request_duration_seconds:increase4w{code=~"2..",job="metrics-service-thanos-receive-default",le="1",slo="monitoring-http-latency"} or vector(0)) / sum(http_request_duration_seconds:increase4w{code=~"2..",job="metrics-service-thanos-receive-default",le="",slo="monitoring-http-latency"})`),
Labels: map[string]string{"slo": "monitoring-http-latency"},
}, {
Record: "pyrra_requests_total",
Expr: intstr.FromString(`sum(http_request_duration_seconds_count{code=~"2..",job="metrics-service-thanos-receive-default"})`),
Record: "pyrra_requests:rate5m",
Expr: intstr.FromString(`sum(rate(http_request_duration_seconds_count{code=~"2..",job="metrics-service-thanos-receive-default"}[5m]))`),
Labels: map[string]string{"slo": "monitoring-http-latency"},
}, {
Record: "pyrra_errors_total",
Expr: intstr.FromString(`sum(http_request_duration_seconds_count{code=~"2..",job="metrics-service-thanos-receive-default"}) - sum(http_request_duration_seconds_bucket{code=~"2..",job="metrics-service-thanos-receive-default",le="1"})`),
Record: "pyrra_errors:rate5m",
Expr: intstr.FromString(`sum(rate(http_request_duration_seconds_count{code=~"2..",job="metrics-service-thanos-receive-default"}[5m])) - sum(rate(http_request_duration_seconds_bucket{code=~"2..",job="metrics-service-thanos-receive-default",le="1"}[5m]))`),
Labels: map[string]string{"slo": "monitoring-http-latency"},
}},
},
Expand Down Expand Up @@ -1744,12 +1744,12 @@ func TestObjective_GrafanaRules(t *testing.T) {
Expr: intstr.FromString(`sum(grpc_server_handling_seconds:increase1w{grpc_method="Write",grpc_service="conprof.WritableProfileStore",job="api",le="0.6",slo="monitoring-grpc-latency"} or vector(0)) / sum(grpc_server_handling_seconds:increase1w{grpc_method="Write",grpc_service="conprof.WritableProfileStore",job="api",le="",slo="monitoring-grpc-latency"})`),
Labels: map[string]string{"slo": "monitoring-grpc-latency"},
}, {
Record: "pyrra_requests_total",
Expr: intstr.FromString(`sum(grpc_server_handling_seconds_count{grpc_method="Write",grpc_service="conprof.WritableProfileStore",job="api"})`),
Record: "pyrra_requests:rate5m",
Expr: intstr.FromString(`sum(rate(grpc_server_handling_seconds_count{grpc_method="Write",grpc_service="conprof.WritableProfileStore",job="api"}[5m]))`),
Labels: map[string]string{"slo": "monitoring-grpc-latency"},
}, {
Record: "pyrra_errors_total",
Expr: intstr.FromString(`sum(grpc_server_handling_seconds_count{grpc_method="Write",grpc_service="conprof.WritableProfileStore",job="api"}) - sum(grpc_server_handling_seconds_bucket{grpc_method="Write",grpc_service="conprof.WritableProfileStore",job="api",le="0.6"})`),
Record: "pyrra_errors:rate5m",
Expr: intstr.FromString(`sum(rate(grpc_server_handling_seconds_count{grpc_method="Write",grpc_service="conprof.WritableProfileStore",job="api"}[5m])) - sum(rate(grpc_server_handling_seconds_bucket{grpc_method="Write",grpc_service="conprof.WritableProfileStore",job="api",le="0.6"}[5m]))`),
Labels: map[string]string{"slo": "monitoring-grpc-latency"},
}},
},
Expand All @@ -1776,12 +1776,12 @@ func TestObjective_GrafanaRules(t *testing.T) {
Expr: intstr.FromString(`1 - sum(prometheus_operator_reconcile_errors:increase2w{slo="monitoring-prometheus-operator-errors"} or vector(0)) / sum(prometheus_operator_reconcile_operations:increase2w{slo="monitoring-prometheus-operator-errors"})`),
Labels: map[string]string{"slo": "monitoring-prometheus-operator-errors"},
}, {
Record: "pyrra_requests_total",
Expr: intstr.FromString(`sum(prometheus_operator_reconcile_operations_total)`),
Record: "pyrra_requests:rate5m",
Expr: intstr.FromString(`sum(rate(prometheus_operator_reconcile_operations_total[5m]))`),
Labels: map[string]string{"slo": "monitoring-prometheus-operator-errors"},
}, {
Record: "pyrra_errors_total",
Expr: intstr.FromString(`sum(prometheus_operator_reconcile_errors_total or vector(0))`),
Record: "pyrra_errors:rate5m",
Expr: intstr.FromString(`sum(rate(prometheus_operator_reconcile_errors_total[5m])) or vector(0)`),
Labels: map[string]string{"slo": "monitoring-prometheus-operator-errors"},
}},
},
Expand All @@ -1808,12 +1808,12 @@ func TestObjective_GrafanaRules(t *testing.T) {
Expr: intstr.FromString(`1 - sum(apiserver_request:increase2w{code=~"5..",job="apiserver",slo="apiserver-write-response-errors",verb=~"POST|PUT|PATCH|DELETE"} or vector(0)) / sum(apiserver_request:increase2w{job="apiserver",slo="apiserver-write-response-errors",verb=~"POST|PUT|PATCH|DELETE"})`),
Labels: map[string]string{"slo": "apiserver-write-response-errors"},
}, {
Record: "pyrra_requests_total",
Expr: intstr.FromString(`sum(apiserver_request_total{job="apiserver",verb=~"POST|PUT|PATCH|DELETE"})`),
Record: "pyrra_requests:rate5m",
Expr: intstr.FromString(`sum(rate(apiserver_request_total{job="apiserver",verb=~"POST|PUT|PATCH|DELETE"}[5m]))`),
Labels: map[string]string{"slo": "apiserver-write-response-errors"},
}, {
Record: "pyrra_errors_total",
Expr: intstr.FromString(`sum(apiserver_request_total{code=~"5..",job="apiserver",verb=~"POST|PUT|PATCH|DELETE"} or vector(0))`),
Record: "pyrra_errors:rate5m",
Expr: intstr.FromString(`sum(rate(apiserver_request_total{code=~"5..",job="apiserver",verb=~"POST|PUT|PATCH|DELETE"}[5m])) or vector(0)`),
Labels: map[string]string{"slo": "apiserver-write-response-errors"},
}},
},
Expand All @@ -1840,11 +1840,11 @@ func TestObjective_GrafanaRules(t *testing.T) {
Expr: intstr.FromString(`sum(up:sum4w{slo="up-targets"}) / sum(up:count4w{slo="up-targets"})`),
Labels: map[string]string{"slo": "up-targets"},
}, {
Record: "pyrra_requests_total",
Record: "pyrra_requests:rate5m",
Expr: intstr.FromString(`sum(up:count4w{slo="up-targets"})`),
Labels: map[string]string{"slo": "up-targets"},
}, {
Record: "pyrra_errors_total",
Record: "pyrra_errors:rate5m",
Expr: intstr.FromString(`sum(up:count4w{slo="up-targets"}) - sum(up:sum4w{slo="up-targets"})`),
Labels: map[string]string{"slo": "up-targets"},
}},
Expand Down