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

Invalid summary/histogram labels when the metric has an empty label set ({}) #78

Open
mkuratczyk opened this issue Sep 3, 2024 · 0 comments
Assignees
Labels
bug Something isn't working

Comments

@mkuratczyk
Copy link

If a summary or histogram is registered with an empty, but present, label set, it is returned with an incorrect
label set because the quantile/vmrange labels are appended with a comma, but there are no labels
before the comma.

I ran into this because I allow user-set custom labels, which may be empty. If I use promutils.NewLabelsFromString or NewLabelsFromMap, and just appended the result to the metric name, I will hit this issue if the user didn't provide custom labels.
Of course I can check if the string/map is empty, or remove the {} in the example below, but it feels like this should be handled.
If a metric name like below (test_summary{}) is considered invalid, it should be rejected during registration, since there's already validation happening at that time.

Here's an executable way to reproduce the issue:

  1. Register a summary/histogram with an empty label set:
package main

import (
	"net/http"

	vm "github.com/VictoriaMetrics/metrics"
)

func main() {
	summary := vm.NewSummary(`test_summary{}`)
	summary.Update(1)

	histogram := vm.NewHistogram(`test_histogram{}`)
	histogram.Update(1)

	http.HandleFunc("/metrics", func(w http.ResponseWriter, req *http.Request) {
		vm.WritePrometheus(w, false)
	})
	http.ListenAndServe("127.0.0.1:8080", nil)

}
  1. Scrape the endpoint:
$ curl -s localhost:8080/metrics
test_histogram_bucket{,vmrange="8.799e-01...1.000e+00"} 1
test_histogram_sum{} 1
test_histogram_count{} 1
test_summary{,quantile="0.5"} 1
test_summary{,quantile="0.9"} 1
test_summary{,quantile="0.97"} 1
test_summary{,quantile="0.99"} 1
test_summary{,quantile="1"} 1
test_summary_sum{} 1
test_summary_count{} 1
  1. test_histogram_bucket a test_summary have incorrect label sets starting with a comma.
  2. If you scrape this with Prometheus, you'll get an error:
expected label name, got ",v" ("COMMA") while parsing: "test_histogram_bucket{,v"
@hagen1778 hagen1778 added the bug Something isn't working label Jan 20, 2025
@f41gh7 f41gh7 self-assigned this Jan 31, 2025
f41gh7 added a commit that referenced this issue Jan 31, 2025
Metric name with explicitly set empty labels set is valid syntax, for example:
`metric_name{}`.

 This case works fine for all metric types except histogram and summary.
Since it adds own tags via addTag function.

 addTag function incorrectly handles empty labels set and adds extra comma:
`metric_name{,quantile="0.1"}`. It violites prometheus text exposition format.

 This commit adds extra check for addTag function and fixes this issue.

Related issue:
#78

Signed-off-by: f41gh7 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants