Skip to content

Commit

Permalink
fix(pyroscope): allow slashes in tag name (#2172)
Browse files Browse the repository at this point in the history
  • Loading branch information
marcsanmi authored Dec 2, 2024
1 parent d82f44b commit 2b16ff0
Show file tree
Hide file tree
Showing 3 changed files with 152 additions and 2 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ Main (unreleased)
- Add perf_schema quantile columns to collector

### Bugfixes

- Fixed an issue in the `pyroscope.write` component to allow slashes in application names in the same way it is done in the Pyroscope push API (@marcsanmi)
- Fixed an issue in the `prometheus.exporter.postgres` component that would leak goroutines when the target was not reachable (@dehaansa)

- Fixed an issue in the `otelcol.exporter.prometheus` component that would set series value incorrectly for stale metrics (@YusifAghalar)
Expand Down
2 changes: 1 addition & 1 deletion internal/component/pyroscope/write/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ func validateAppName(n string) error {
}

func isAppNameRuneAllowed(r rune) bool {
return r == '-' || r == '.' || isTagKeyRuneAllowed(r)
return r == '-' || r == '.' || r == '/' || isTagKeyRuneAllowed(r)
}

func isTagKeyReserved(k string) bool {
Expand Down
150 changes: 150 additions & 0 deletions internal/component/pyroscope/write/parser_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
package write

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestParseKey(t *testing.T) {
tests := []struct {
name string
input string
expected *Key
wantErr bool
}{
{
name: "basic app name",
input: "simple-app",
expected: &Key{
labels: map[string]string{
"__name__": "simple-app",
},
},
},
{
name: "app name with slashes and tags",
input: "my/service/name{environment=prod,version=1.0}",
expected: &Key{
labels: map[string]string{
"__name__": "my/service/name",
"environment": "prod",
"version": "1.0",
},
},
},
{
name: "multiple slashes and special characters",
input: "app/service/v1.0-beta/component{region=us-west}",
expected: &Key{
labels: map[string]string{
"__name__": "app/service/v1.0-beta/component",
"region": "us-west",
},
},
},
{
name: "empty app name",
input: "{}",
wantErr: true,
},
{
name: "invalid characters in tag key",
input: "my/service/name{invalid@key=value}",
wantErr: true,
},
{
name: "whitespace handling",
input: "my/service/name{ tag1 = value1 , tag2 = value2 }",
expected: &Key{
labels: map[string]string{
"__name__": "my/service/name",
"tag1": "value1",
"tag2": "value2",
},
},
},
{
name: "dots in service name",
input: "my/service.name/v1.0{environment=prod}",
expected: &Key{
labels: map[string]string{
"__name__": "my/service.name/v1.0",
"environment": "prod",
},
},
},
{
name: "app name with slashes",
input: "my/service/name{}",
expected: &Key{
labels: map[string]string{
"__name__": "my/service/name",
},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := ParseKey(tt.input)

if tt.wantErr {
assert.Error(t, err)
return
}

require.NoError(t, err)
assert.Equal(t, tt.expected, got)
})
}
}

func TestKey_Normalized(t *testing.T) {
tests := []struct {
name string
key *Key
expected string
}{
{
name: "simple normalization",
key: &Key{
labels: map[string]string{
"__name__": "my/service/name",
},
},
expected: "my/service/name{}",
},
{
name: "normalization with tags",
key: &Key{
labels: map[string]string{
"__name__": "my/service/name",
"environment": "prod",
"version": "1.0",
},
},
expected: "my/service/name{environment=prod,version=1.0}",
},
{
name: "tags should be sorted",
key: &Key{
labels: map[string]string{
"__name__": "my/service/name",
"c": "3",
"b": "2",
"a": "1",
},
},
expected: "my/service/name{a=1,b=2,c=3}",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := tt.key.Normalized()
assert.Equal(t, tt.expected, got)
})
}
}

0 comments on commit 2b16ff0

Please sign in to comment.