diff --git a/semconv/engine.go b/semconv/engine.go index ec0cf9973e..52dd0e1748 100644 --- a/semconv/engine.go +++ b/semconv/engine.go @@ -369,7 +369,7 @@ func (s *transformingSeries) Labels() labels.Labels { builder.Del(l.Name) case "__unit__": if s.result.to.Unit != "" { - builder.Set(l.Name, strings.Trim(s.result.to.Unit, "{}")) + builder.Set(l.Name, s.result.to.DirectUnit()) } case "le": if typ == model.MetricTypeHistogram { diff --git a/semconv/engine_test.go b/semconv/engine_test.go index 959124f8fe..6b638c78e8 100644 --- a/semconv/engine_test.go +++ b/semconv/engine_test.go @@ -6,9 +6,8 @@ import ( "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/require" - "github.com/prometheus/prometheus/promql/parser" - "github.com/prometheus/prometheus/model/labels" + "github.com/prometheus/prometheus/promql/parser" "github.com/prometheus/prometheus/util/testutil" ) @@ -47,7 +46,7 @@ func TestEngine_FindVariants(t *testing.T) { { matchers: []*labels.Matcher{ labels.MustNewMatcher(labels.MatchEqual, labels.MetricName, "my_app_latency_milliseconds"), - labels.MustNewMatcher(labels.MatchEqual, "__unit__", "millisecond"), + labels.MustNewMatcher(labels.MatchEqual, "__unit__", "milliseconds"), labels.MustNewMatcher(labels.MatchEqual, "foo", "bar"), }, result: resultTransform{ @@ -75,7 +74,7 @@ func TestEngine_FindVariants(t *testing.T) { { matchers: []*labels.Matcher{ labels.MustNewMatcher(labels.MatchEqual, labels.MetricName, "my_app_latency_seconds"), - labels.MustNewMatcher(labels.MatchEqual, "__unit__", "second"), + labels.MustNewMatcher(labels.MatchEqual, "__unit__", "seconds"), labels.MustNewMatcher(labels.MatchEqual, "foo", "bar"), }, result: resultTransform{ diff --git a/semconv/gen_files.go b/semconv/gen_files.go index 1c33b04f14..d587cf85fc 100644 --- a/semconv/gen_files.go +++ b/semconv/gen_files.go @@ -53,8 +53,11 @@ type metricGroupChange struct { } func (m metricGroupChange) DirectUnit() string { + if strings.HasPrefix(m.Unit, "{") { + return strings.Trim(m.Unit, "{}") + "s" + } // TODO(bwplotka): Yolo, fix it. - return strings.Trim(m.Unit, "{}") + return m.Unit } type attribute struct { diff --git a/semconv/gen_files_test.go b/semconv/gen_files_test.go index 445250b63f..5db9a15b96 100644 --- a/semconv/gen_files_test.go +++ b/semconv/gen_files_test.go @@ -53,29 +53,29 @@ func TestFetchIDs(t *testing.T) { expected := &ids{ Version: 1, MetricsIDs: map[string][]versionedID{ - "my_app_latency_seconds~second.histogram": { + "my_app_latency_seconds~seconds.histogram": { {ID: "my_app_latency.2", IntroVersion: "1.1.0"}, }, - "my_app_custom_elements_changed_total~element.counter": { + "my_app_custom_elements_changed_total~elements.counter": { { ID: "my_app_custom_elements.2", IntroVersion: "1.1.0", }, }, - "my_app_latency_milliseconds~millisecond.histogram": { + "my_app_latency_milliseconds~milliseconds.histogram": { { ID: "my_app_latency", IntroVersion: "1.0.0", }, }, - "my_app_custom_elements_total~element.counter": { + "my_app_custom_elements_total~elements.counter": { { ID: "my_app_custom_elements", IntroVersion: "1.0.0", }, }, - "my_app_some_elements~element.gauge": { + "my_app_some_elements~elements.gauge": { { ID: "my_app_some_elements", IntroVersion: "1.0.0", @@ -83,18 +83,18 @@ func TestFetchIDs(t *testing.T) { }, }, uniqueNameToIdentity: map[string]string{ - "my_app_custom_elements_changed_total": "my_app_custom_elements_changed_total~element.counter", - "my_app_custom_elements_total": "my_app_custom_elements_total~element.counter", - "my_app_latency_milliseconds": "my_app_latency_milliseconds~millisecond.histogram", - "my_app_latency_seconds": "my_app_latency_seconds~second.histogram", - "my_app_some_elements": "my_app_some_elements~element.gauge", + "my_app_custom_elements_changed_total": "my_app_custom_elements_changed_total~elements.counter", + "my_app_custom_elements_total": "my_app_custom_elements_total~elements.counter", + "my_app_latency_milliseconds": "my_app_latency_milliseconds~milliseconds.histogram", + "my_app_latency_seconds": "my_app_latency_seconds~seconds.histogram", + "my_app_some_elements": "my_app_some_elements~elements.gauge", }, uniqueNameTypeToIdentity: map[string]string{ - "my_app_custom_elements_changed_total.counter": "my_app_custom_elements_changed_total~element.counter", - "my_app_custom_elements_total.counter": "my_app_custom_elements_total~element.counter", - "my_app_latency_milliseconds.histogram": "my_app_latency_milliseconds~millisecond.histogram", - "my_app_latency_seconds.histogram": "my_app_latency_seconds~second.histogram", - "my_app_some_elements.gauge": "my_app_some_elements~element.gauge", + "my_app_custom_elements_changed_total.counter": "my_app_custom_elements_changed_total~elements.counter", + "my_app_custom_elements_total.counter": "my_app_custom_elements_total~elements.counter", + "my_app_latency_milliseconds.histogram": "my_app_latency_milliseconds~milliseconds.histogram", + "my_app_latency_seconds.histogram": "my_app_latency_seconds~seconds.histogram", + "my_app_some_elements.gauge": "my_app_some_elements~elements.gauge", }, } diff --git a/semconv/storage_test.go b/semconv/storage_test.go index 15a42445f5..238666a4bd 100644 --- a/semconv/storage_test.go +++ b/semconv/storage_test.go @@ -38,11 +38,12 @@ var ( "fraction", "1.243", "test", "new", ) + testdataLatencySeriesOld = labels.FromStrings( "__name__", "my_app_latency_milliseconds", "__schema_url__", testSchemaURL("1.0.0"), "__type__", "histogram", - "__unit__", "millisecond", + "__unit__", "milliseconds", "code", "200", "test", "old", ) @@ -50,7 +51,7 @@ var ( "__name__", "my_app_latency_seconds", "__schema_url__", testSchemaURL("1.1.0"), "__type__", "histogram", - "__unit__", "second", + "__unit__", "seconds", "code", "200", "test", "new", ) @@ -347,6 +348,7 @@ func TestAwareStorage(t *testing.T) { }) t.Run("classic histogram", func(t *testing.T) { var a []appendSeries + for _, m := range []labels.Labels{testdataLatencySeriesNew, testdataLatencySeriesOld} { b := labels.NewBuilder(m) if m.Get("test") == "new" { @@ -385,7 +387,7 @@ func TestAwareStorage(t *testing.T) { labels.MustNewMatcher(labels.MatchEqual, "code", "200"), ) require.Equal(t, map[string][]chunks.Sample{ - `{__name__="my_app_latency_seconds_bucket", __schema_url__="` + testSchemaURL("1.1.0") + `", __type__="histogram", __unit__="second", code="200", le="10", test="new"}`: testFSamples, + `{__name__="my_app_latency_seconds_bucket", __schema_url__="` + testSchemaURL("1.1.0") + `", __type__="histogram", __unit__="seconds", code="200", le="10", test="new"}`: testFSamples, }, onlyNewResult) got := query(t, aware, // Without schema selector, semconv aware storage should have no effect. @@ -400,8 +402,8 @@ func TestAwareStorage(t *testing.T) { labels.MustNewMatcher(labels.MatchEqual, "code", "200"), ) require.Equal(t, map[string][]chunks.Sample{ - `{__name__="my_app_latency_seconds_bucket", __type__="histogram", __unit__="second", code="200", le="10", test="new"}`: testFSamples, - `{__name__="my_app_latency_seconds_bucket", __type__="histogram", __unit__="second", code="200", le="10", test="old"}`: testFSamples, + `{__name__="my_app_latency_seconds_bucket", __type__="histogram", __unit__="seconds", code="200", le="10", test="new"}`: testFSamples, + `{__name__="my_app_latency_seconds_bucket", __type__="histogram", __unit__="seconds", code="200", le="10", test="old"}`: testFSamples, }, compatibleResult) }) t.Run("_count", func(t *testing.T) { @@ -411,7 +413,7 @@ func TestAwareStorage(t *testing.T) { labels.MustNewMatcher(labels.MatchEqual, "code", "200"), ) require.Equal(t, map[string][]chunks.Sample{ - `{__name__="my_app_latency_seconds_count", __schema_url__="` + testSchemaURL("1.1.0") + `", __type__="histogram", __unit__="second", code="200", test="new"}`: testFSamples, // TODO(bwplotka): Type and unit proposal is not really consistent with count/sum + `{__name__="my_app_latency_seconds_count", __schema_url__="` + testSchemaURL("1.1.0") + `", __type__="histogram", __unit__="seconds", code="200", test="new"}`: testFSamples, // TODO(bwplotka): Type and unit proposal is not really consistent with count/sum }, onlyNewResult) got := query(t, aware, // Without schema selector, semconv aware storage should have no effect. @@ -426,8 +428,8 @@ func TestAwareStorage(t *testing.T) { labels.MustNewMatcher(labels.MatchEqual, "code", "200"), ) require.Equal(t, map[string][]chunks.Sample{ - `{__name__="my_app_latency_seconds_count", __type__="histogram", __unit__="second", code="200", test="new"}`: testFSamples, - `{__name__="my_app_latency_seconds_count", __type__="histogram", __unit__="second", code="200", test="old"}`: testFSamples, + `{__name__="my_app_latency_seconds_count", __type__="histogram", __unit__="seconds", code="200", test="new"}`: testFSamples, + `{__name__="my_app_latency_seconds_count", __type__="histogram", __unit__="seconds", code="200", test="old"}`: testFSamples, }, compatibleResult) }) t.Run("_sum", func(t *testing.T) { @@ -437,7 +439,7 @@ func TestAwareStorage(t *testing.T) { labels.MustNewMatcher(labels.MatchEqual, "code", "200"), ) require.Equal(t, map[string][]chunks.Sample{ - `{__name__="my_app_latency_seconds_sum", __schema_url__="` + testSchemaURL("1.1.0") + `", __type__="histogram", __unit__="second", code="200", test="new"}`: testFSamples, // TODO(bwplotka): Type and unit proposal is not really consistent with count/sum + `{__name__="my_app_latency_seconds_sum", __schema_url__="` + testSchemaURL("1.1.0") + `", __type__="histogram", __unit__="seconds", code="200", test="new"}`: testFSamples, // TODO(bwplotka): Type and unit proposal is not really consistent with count/sum }, onlyNewResult) got := query(t, aware, // Without schema selector, semconv aware storage should have no effect. @@ -452,8 +454,8 @@ func TestAwareStorage(t *testing.T) { labels.MustNewMatcher(labels.MatchEqual, "code", "200"), ) require.Equal(t, map[string][]chunks.Sample{ - `{__name__="my_app_latency_seconds_sum", __type__="histogram", __unit__="second", code="200", test="new"}`: testFSamples, - `{__name__="my_app_latency_seconds_sum", __type__="histogram", __unit__="second", code="200", test="old"}`: scaleSamples(testFSamples, false, 1000), + `{__name__="my_app_latency_seconds_sum", __type__="histogram", __unit__="seconds", code="200", test="new"}`: testFSamples, + `{__name__="my_app_latency_seconds_sum", __type__="histogram", __unit__="seconds", code="200", test="old"}`: scaleSamples(testFSamples, false, 1000), }, compatibleResult) }) }) @@ -465,7 +467,7 @@ func TestAwareStorage(t *testing.T) { labels.MustNewMatcher(labels.MatchEqual, "code", "200"), ) require.Equal(t, map[string][]chunks.Sample{ - `{__name__="my_app_latency_milliseconds_bucket", __schema_url__="` + testSchemaURL("1.0.0") + `", __type__="histogram", __unit__="millisecond", code="200", le="10000", test="old"}`: testFSamples, + `{__name__="my_app_latency_milliseconds_bucket", __schema_url__="` + testSchemaURL("1.0.0") + `", __type__="histogram", __unit__="milliseconds", code="200", le="10000", test="old"}`: testFSamples, }, onlyOldResult) got := query(t, aware, // Without schema selector, semconv aware storage should have no effect. @@ -480,8 +482,8 @@ func TestAwareStorage(t *testing.T) { labels.MustNewMatcher(labels.MatchEqual, "code", "200"), ) require.Equal(t, map[string][]chunks.Sample{ - `{__name__="my_app_latency_milliseconds_bucket", __type__="histogram", __unit__="millisecond", code="200", le="10000", test="new"}`: testFSamples, - `{__name__="my_app_latency_milliseconds_bucket", __type__="histogram", __unit__="millisecond", code="200", le="10000", test="old"}`: testFSamples, + `{__name__="my_app_latency_milliseconds_bucket", __type__="histogram", __unit__="milliseconds", code="200", le="10000", test="new"}`: testFSamples, + `{__name__="my_app_latency_milliseconds_bucket", __type__="histogram", __unit__="milliseconds", code="200", le="10000", test="old"}`: testFSamples, }, compatibleResult) }) t.Run("_count", func(t *testing.T) { @@ -491,7 +493,7 @@ func TestAwareStorage(t *testing.T) { labels.MustNewMatcher(labels.MatchEqual, "code", "200"), ) require.Equal(t, map[string][]chunks.Sample{ - `{__name__="my_app_latency_milliseconds_count", __schema_url__="` + testSchemaURL("1.0.0") + `", __type__="histogram", __unit__="millisecond", code="200", test="old"}`: testFSamples, // TODO(bwplotka): Type and unit proposal is not really consistent with count/sum + `{__name__="my_app_latency_milliseconds_count", __schema_url__="` + testSchemaURL("1.0.0") + `", __type__="histogram", __unit__="milliseconds", code="200", test="old"}`: testFSamples, // TODO(bwplotka): Type and unit proposal is not really consistent with count/sum }, onlyOldResult) got := query(t, aware, // Without schema selector, semconv aware storage should have no effect. @@ -506,8 +508,8 @@ func TestAwareStorage(t *testing.T) { labels.MustNewMatcher(labels.MatchEqual, "code", "200"), ) require.Equal(t, map[string][]chunks.Sample{ - `{__name__="my_app_latency_milliseconds_count", __type__="histogram", __unit__="millisecond", code="200", test="new"}`: testFSamples, - `{__name__="my_app_latency_milliseconds_count", __type__="histogram", __unit__="millisecond", code="200", test="old"}`: testFSamples, + `{__name__="my_app_latency_milliseconds_count", __type__="histogram", __unit__="milliseconds", code="200", test="new"}`: testFSamples, + `{__name__="my_app_latency_milliseconds_count", __type__="histogram", __unit__="milliseconds", code="200", test="old"}`: testFSamples, }, compatibleResult) }) t.Run("_sum", func(t *testing.T) { @@ -517,7 +519,7 @@ func TestAwareStorage(t *testing.T) { labels.MustNewMatcher(labels.MatchEqual, "code", "200"), ) require.Equal(t, map[string][]chunks.Sample{ - `{__name__="my_app_latency_milliseconds_sum", __schema_url__="` + testSchemaURL("1.0.0") + `", __type__="histogram", __unit__="millisecond", code="200", test="old"}`: testFSamples, // TODO(bwplotka): Type and unit proposal is not really consistent with count/sum + `{__name__="my_app_latency_milliseconds_sum", __schema_url__="` + testSchemaURL("1.0.0") + `", __type__="histogram", __unit__="milliseconds", code="200", test="old"}`: testFSamples, // TODO(bwplotka): Type and unit proposal is not really consistent with count/sum }, onlyOldResult) got := query(t, aware, // Without schema selector, semconv aware storage should have no effect. @@ -532,8 +534,8 @@ func TestAwareStorage(t *testing.T) { labels.MustNewMatcher(labels.MatchEqual, "code", "200"), ) require.Equal(t, map[string][]chunks.Sample{ - `{__name__="my_app_latency_milliseconds_sum", __type__="histogram", __unit__="millisecond", code="200", test="old"}`: testFSamples, - `{__name__="my_app_latency_milliseconds_sum", __type__="histogram", __unit__="millisecond", code="200", test="new"}`: scaleSamples(testFSamples, true, 1000), + `{__name__="my_app_latency_milliseconds_sum", __type__="histogram", __unit__="milliseconds", code="200", test="old"}`: testFSamples, + `{__name__="my_app_latency_milliseconds_sum", __type__="histogram", __unit__="milliseconds", code="200", test="new"}`: scaleSamples(testFSamples, true, 1000), }, compatibleResult) }) }) @@ -562,7 +564,7 @@ func TestAwareStorage(t *testing.T) { labels.MustNewMatcher(labels.MatchEqual, "code", testdataLatencySeriesNew.Get("code")), ) require.Equal(t, map[string][]chunks.Sample{ - `{__name__="my_app_latency_seconds", __schema_url__="` + testSchemaURL("1.1.0") + `", __type__="histogram", __unit__="second", code="200", test="new"}`: testNHCBSamples, + `{__name__="my_app_latency_seconds", __schema_url__="` + testSchemaURL("1.1.0") + `", __type__="histogram", __unit__="seconds", code="200", test="new"}`: testNHCBSamples, }, onlyNewResult) got := query(t, aware, // Without schema selector, semconv aware storage should have no effect. @@ -577,8 +579,8 @@ func TestAwareStorage(t *testing.T) { labels.MustNewMatcher(labels.MatchEqual, "code", testdataLatencySeriesNew.Get("code")), ) require.Equal(t, map[string][]chunks.Sample{ - `{__name__="my_app_latency_seconds", __type__="histogram", __unit__="second", code="200", test="new"}`: testNHCBSamples, - `{__name__="my_app_latency_seconds", __type__="histogram", __unit__="second", code="200", test="old"}`: scaleSamples(testNHCBSamples, false, 1000), + `{__name__="my_app_latency_seconds", __type__="histogram", __unit__="seconds", code="200", test="new"}`: testNHCBSamples, + `{__name__="my_app_latency_seconds", __type__="histogram", __unit__="seconds", code="200", test="old"}`: scaleSamples(testNHCBSamples, false, 1000), }, compatibleResult) }) t.Run("forward", func(t *testing.T) { @@ -588,7 +590,7 @@ func TestAwareStorage(t *testing.T) { labels.MustNewMatcher(labels.MatchEqual, "code", testdataLatencySeriesOld.Get("code")), ) require.Equal(t, map[string][]chunks.Sample{ - `{__name__="my_app_latency_milliseconds", __schema_url__="` + testSchemaURL("1.0.0") + `", __type__="histogram", __unit__="millisecond", code="200", test="old"}`: testNHCBSamples, + `{__name__="my_app_latency_milliseconds", __schema_url__="` + testSchemaURL("1.0.0") + `", __type__="histogram", __unit__="milliseconds", code="200", test="old"}`: testNHCBSamples, }, onlyOldResult) got := query(t, aware, // Without schema selector, semconv aware storage should have no effect. @@ -603,8 +605,8 @@ func TestAwareStorage(t *testing.T) { labels.MustNewMatcher(labels.MatchEqual, "code", testdataLatencySeriesOld.Get("code")), ) require.Equal(t, map[string][]chunks.Sample{ - `{__name__="my_app_latency_milliseconds", __type__="histogram", __unit__="millisecond", code="200", test="new"}`: scaleSamples(testNHCBSamples, true, 1000), - `{__name__="my_app_latency_milliseconds", __type__="histogram", __unit__="millisecond", code="200", test="old"}`: testNHCBSamples, + `{__name__="my_app_latency_milliseconds", __type__="histogram", __unit__="milliseconds", code="200", test="new"}`: scaleSamples(testNHCBSamples, true, 1000), + `{__name__="my_app_latency_milliseconds", __type__="histogram", __unit__="milliseconds", code="200", test="old"}`: testNHCBSamples, }, compatibleResult) }) }) diff --git a/semconv/testdata/ids.yaml b/semconv/testdata/ids.yaml index ba7fcc4819..cb93dba2a1 100644 --- a/semconv/testdata/ids.yaml +++ b/semconv/testdata/ids.yaml @@ -3,18 +3,18 @@ version: 1 # map from identity of an element to its id(s). metrics_ids: - my_app_latency_seconds~second.histogram: + my_app_latency_seconds~seconds.histogram: - id: "my_app_latency.2" intro_version: "1.1.0" # When introduced. - my_app_custom_elements_changed_total~element.counter: + my_app_custom_elements_changed_total~elements.counter: - id: "my_app_custom_elements.2" intro_version: "1.1.0" - my_app_latency_milliseconds~millisecond.histogram: + my_app_latency_milliseconds~milliseconds.histogram: - id: "my_app_latency" intro_version: "1.0.0" - my_app_custom_elements_total~element.counter: + my_app_custom_elements_total~elements.counter: - id: "my_app_custom_elements" intro_version: "1.0.0" - my_app_some_elements~element.gauge: + my_app_some_elements~elements.gauge: - id: "my_app_some_elements" intro_version: "1.0.0"