Skip to content

Commit

Permalink
{summary,histogram}: properly addTags to empty labels set
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
f41gh7 committed Jan 31, 2025
1 parent d783709 commit e449781
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 1 deletion.
14 changes: 14 additions & 0 deletions histogram_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,20 @@ func TestHistogramWithTags(t *testing.T) {
}
}

func TestHistogramWithEmptyTags(t *testing.T) {
name := `TestHistogram{}`
h := NewHistogram(name)
h.Update(123)

var bb bytes.Buffer
WritePrometheus(&bb, false)
result := bb.String()
namePrefixWithTag := `TestHistogram_bucket{vmrange="1.136e+02...1.292e+02"} 1` + "\n"
if !strings.Contains(result, namePrefixWithTag) {
t.Fatalf("missing histogram %s in the WritePrometheus output; got\n%s", namePrefixWithTag, result)
}
}

func TestGetOrCreateHistogramSerial(t *testing.T) {
name := "GetOrCreateHistogramSerial"
if err := testGetOrCreateHistogram(name); err != nil {
Expand Down
10 changes: 9 additions & 1 deletion summary.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,15 @@ func addTag(name, tag string) string {
if len(name) == 0 || name[len(name)-1] != '}' {
return fmt.Sprintf("%s{%s}", name, tag)
}
return fmt.Sprintf("%s,%s}", name[:len(name)-1], tag)
name = name[:len(name)-1]
if len(name) == 0 {
panic(fmt.Errorf("BUG: metric name cannot be empty"))
}
if name[len(name)-1] == '{' {
// case for empty labels set metric_name{}
return fmt.Sprintf("%s%s}", name, tag)
}
return fmt.Sprintf("%s,%s}", name, tag)
}

func registerSummaryLocked(sm *Summary) {
Expand Down
14 changes: 14 additions & 0 deletions summary_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,20 @@ func TestSummaryWithTags(t *testing.T) {
}
}

func TestSummaryWithEmptyTags(t *testing.T) {
name := `TestSummary{}`
s := NewSummary(name)
s.Update(123)

var bb bytes.Buffer
WritePrometheus(&bb, false)
result := bb.String()
namePrefixWithTag := `TestSummary{quantile="`
if !strings.Contains(result, namePrefixWithTag) {
t.Fatalf("missing summary prefix %s in the WritePrometheus output; got\n%s", namePrefixWithTag, result)
}
}

func TestSummaryInvalidQuantiles(t *testing.T) {
name := "SummaryInvalidQuantiles"
expectPanic(t, name, func() {
Expand Down

0 comments on commit e449781

Please sign in to comment.