Skip to content
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

stats/prometheus: normalize labels for single-label implementations #12057

Merged
merged 2 commits into from
Jan 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 4 additions & 0 deletions doc/releasenotes/16_0_0_summary.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ The default value for `remote_operation_timeout` has also changed from 30 second
During upgrades, if the users want to preserve the same behaviour as previous releases, then they should provide the `remote_operation_timeout` flag explicitly before upgrading.
After the upgrade, they should then alter their configuration to also specify `lock-timeout` explicitly.

#### Normalized labels in the Prometheus Exporter

The Prometheus metrics exporter now properly normalizes _all_ label names into their `snake_case` form, as it is idiomatic for Prometheus metrics. Previously, Vitess instances were emitting inconsistent labels for their metrics, with some of them being `CamelCase` and others being `snake_case`.

### New command line flags and behavior

#### VTGate: Support query timeout --query-timeout
Expand Down
6 changes: 3 additions & 3 deletions go/stats/prometheusbackend/collectors.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func newCountersWithSingleLabelCollector(c *stats.CountersWithSingleLabel, name
desc: prometheus.NewDesc(
name,
c.Help(),
[]string{labelName},
[]string{normalizeMetric(labelName)},
nil),
vt: vt}

Expand Down Expand Up @@ -111,7 +111,7 @@ func newGaugesWithSingleLabelCollector(g *stats.GaugesWithSingleLabel, name stri
desc: prometheus.NewDesc(
name,
g.Help(),
[]string{labelName},
[]string{normalizeMetric(labelName)},
nil),
vt: vt}

Expand Down Expand Up @@ -266,7 +266,7 @@ func newTimingsCollector(t *stats.Timings, name string) {
desc: prometheus.NewDesc(
name,
t.Help(),
[]string{t.Label()},
[]string{normalizeMetric(t.Label())},
nil),
}

Expand Down
20 changes: 20 additions & 0 deletions go/stats/prometheusbackend/prometheusbackend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,26 @@ func testMetricsHandler(t *testing.T) *httptest.ResponseRecorder {
return response
}

func TestPrometheusLabels(t *testing.T) {
m1 := stats.NewCountersWithSingleLabel("ThisIsMetric1", "helpstring1", "ThisIsALabel")
m1.Add("labelvalue1", 420)

m2 := stats.NewCountersWithMultiLabels("ThisIsMetric2", "helpstring2", []string{"ThisIsALabel"})
m2.Add([]string{"labelvalue2"}, 420)

response := testMetricsHandler(t)

expect := []string{
"namespace_this_is_metric1{this_is_a_label=\"labelvalue1\"} 420",
"namespace_this_is_metric2{this_is_a_label=\"labelvalue2\"} 420",
}
for _, line := range expect {
if !strings.Contains(response.Body.String(), line) {
t.Fatalf("Expected result to contain %s, got %s", line, response.Body.String())
Copy link
Member

Choose a reason for hiding this comment

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

Ideally we would use assert/require but I see that the whole file is currently using t.Fatalf 😞

}
}
}

func TestMain(m *testing.M) {
Init(namespace)
os.Exit(m.Run())
Expand Down