From 803855df6e72bd29daa33be79acd46c6103d6e9f Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Tue, 4 Oct 2022 16:46:43 -0700 Subject: [PATCH 1/9] Runtime metrics support for Go-1.20 --- lightstep/instrumentation/runtime/builtin.go | 17 +++++---- .../runtime/builtin_119_test.go | 2 +- .../runtime/builtin_120_test.go | 35 +++++++++++++++++++ lightstep/instrumentation/runtime/doc.go | 1 + 4 files changed, 47 insertions(+), 8 deletions(-) create mode 100644 lightstep/instrumentation/runtime/builtin_120_test.go diff --git a/lightstep/instrumentation/runtime/builtin.go b/lightstep/instrumentation/runtime/builtin.go index b700d1af..f9409dd1 100644 --- a/lightstep/instrumentation/runtime/builtin.go +++ b/lightstep/instrumentation/runtime/builtin.go @@ -114,7 +114,7 @@ func getTotalizedAttributeName(n string) string { switch x[len(x)-1] { case "cycles": return "cycle" - case "usage": + case "usage", "time": return "class" } panic(fmt.Sprint("unrecognized attribute name: ", n)) @@ -133,10 +133,10 @@ func getTotalizedMetricName(n, u string) string { switch u { case "bytes": return s + "usage" - case "cpu-seconds": + case "{cpu-seconds}": return s + "time" default: - panic("unrecognized metric suffix") + panic(fmt.Sprint("unrecognized metric suffix: ", u)) } } @@ -184,16 +184,19 @@ func (r *builtinRuntime) register() error { // Remove any ".total" suffix, this is redundant for Prometheus. var totalAttrVal string + var totalPrefix string for totalize := range totals { - if strings.HasPrefix(n, totalize) { + if strings.HasPrefix(n, totalize) && len(n)-len(totalize) > len(totalAttrVal) { // Units is unchanged. - // Name becomes the overall prefix. + // Name becomes the shortest prefix. // Remember which attribute to use. totalAttrVal = n[len(totalize):] - n = getTotalizedMetricName(totalize[:len(totalize)-1], u) - break + totalPrefix = totalize } } + if totalPrefix != "" { + n = getTotalizedMetricName(totalPrefix[:len(totalPrefix)-1], u) + } if counts[n] > 1 { if totalAttrVal != "" { diff --git a/lightstep/instrumentation/runtime/builtin_119_test.go b/lightstep/instrumentation/runtime/builtin_119_test.go index 886c911f..2d7388cf 100644 --- a/lightstep/instrumentation/runtime/builtin_119_test.go +++ b/lightstep/instrumentation/runtime/builtin_119_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -//go:build go1.19 +//go:build go1.19 && !go1.20 package runtime diff --git a/lightstep/instrumentation/runtime/builtin_120_test.go b/lightstep/instrumentation/runtime/builtin_120_test.go new file mode 100644 index 00000000..31f1751b --- /dev/null +++ b/lightstep/instrumentation/runtime/builtin_120_test.go @@ -0,0 +1,35 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build go1.20 + +package runtime + +var expectRuntimeMetrics = map[string]int{ + "cgo.go-to-c-calls": 1, + "cpu.time": 8, + "gc.cycles": 2, + "gc.heap.allocs": 1, + "gc.heap.allocs.objects": 1, + "gc.heap.frees": 1, + "gc.heap.frees.objects": 1, + "gc.heap.goal": 1, + "gc.heap.objects": 1, + "gc.heap.tiny.allocs": 1, + "gc.limiter.last-enabled": 1, + "gc.stack.starting-size": 1, + "memory.usage": 13, + "sched.gomaxprocs": 1, + "sched.goroutines": 1, +} diff --git a/lightstep/instrumentation/runtime/doc.go b/lightstep/instrumentation/runtime/doc.go index 8adfa436..c8d13e04 100644 --- a/lightstep/instrumentation/runtime/doc.go +++ b/lightstep/instrumentation/runtime/doc.go @@ -35,6 +35,7 @@ // Name Unit Instrument // ------------------------------------------------------------------------------------ // process.runtime.go.cgo.go-to-c-calls {calls} Counter[int64] +// process.runtime.go.cpu.time{class=...} {cpu-seconds} Counter[float64] // process.runtime.go.gc.cycles{cycle=forced,automatic} {gc-cycles} Counter[int64] // process.runtime.go.gc.heap.allocs bytes (*) Counter[int64] // process.runtime.go.gc.heap.allocs.objects {objects} (*) Counter[int64] From 2667da95e6f092051041eb753f76a1488298b0bf Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Mon, 19 Dec 2022 10:15:46 -0800 Subject: [PATCH 2/9] throw away --- lightstep/instrumentation/runtime/builtin.go | 77 +++-- .../runtime/builtin_119_test.go | 37 ++- .../instrumentation/runtime/builtin_test.go | 298 ++++++++++++++---- 3 files changed, 319 insertions(+), 93 deletions(-) diff --git a/lightstep/instrumentation/runtime/builtin.go b/lightstep/instrumentation/runtime/builtin.go index f9409dd1..be2e9900 100644 --- a/lightstep/instrumentation/runtime/builtin.go +++ b/lightstep/instrumentation/runtime/builtin.go @@ -108,16 +108,13 @@ func newBuiltinRuntime(meter metric.Meter, af allFunc, rf readFunc) *builtinRunt } } -func getTotalizedAttributeName(n string) string { - x := strings.Split(n, ".") +func getTotalizedAttributeName(order int) string { // It's a plural, make it singular. - switch x[len(x)-1] { - case "cycles": - return "cycle" - case "usage", "time": - return "class" + base := "class" + for ; order > 0; order-- { + base = "sub" + base } - panic(fmt.Sprint("unrecognized attribute name: ", n)) + return base } func getTotalizedMetricName(n, u string) string { @@ -131,11 +128,17 @@ func getTotalizedMetricName(n, u string) string { // suffix, while ".cycles" is an exception. // The ideal name depends on what we know. switch u { - case "bytes": + case "By": + // OTel has similar conventions for memory usage, disk usage, etc, so + // for metrics with /classes/*:bytes we create a .usage metric. return s + "usage" case "{cpu-seconds}": + // Same argument above, except OTel uses .time for + // cpu-timing metrics instead of .usage. return s + "time" default: + // There are no other conventions in the + // runtime/metrics that we know of. panic(fmt.Sprint("unrecognized metric suffix: ", u)) } } @@ -166,16 +169,26 @@ func (r *builtinRuntime) register() error { var totalAttrs [][]attribute.KeyValue for _, m := range all { + // n is the output name, which has '/' replaced with + // '.' and a prefix this value may be modified below, + // based on conventions in the runtime/metrics + // package. n, statedUnits := toName(m.Name) + originalName, _, _ := strings.Cut(m.Name, ":") + + // We skip all total metrics because they are implied + // as sums and can be configured as OTel views. if strings.HasSuffix(n, ".total") { continue } + // Get the units, real or pseudo. var u string switch statedUnits { - case "bytes", "seconds": - // Real units + case "bytes": + u = string(unit.Bytes) + case "seconds": u = statedUnits default: // Pseudo-units @@ -183,23 +196,30 @@ func (r *builtinRuntime) register() error { } // Remove any ".total" suffix, this is redundant for Prometheus. - var totalAttrVal string + var totalAttrVals []string + var totalAttrSubstring string var totalPrefix string for totalize := range totals { - if strings.HasPrefix(n, totalize) && len(n)-len(totalize) > len(totalAttrVal) { + // For any totals, find the longest substring. + if strings.HasPrefix(n, totalize) && len(n)-len(totalize) > len(totalAttrSubstring) { // Units is unchanged. // Name becomes the shortest prefix. // Remember which attribute to use. - totalAttrVal = n[len(totalize):] + totalAttrSubstring = n[len(totalize):] + totalAttrVals = strings.Split(totalAttrSubstring, ".") totalPrefix = totalize } } if totalPrefix != "" { n = getTotalizedMetricName(totalPrefix[:len(totalPrefix)-1], u) + originalName = originalName[:len(originalName)-len(totalAttrSubstring)-1] } + // Next branch helps identify the objects/bytes special case, + // which correctly removes "bytes" (a proper unit) + // from appearing as a suffix. if counts[n] > 1 { - if totalAttrVal != "" { + if totalAttrVals != nil { // This has not happened, hopefully never will. // Indicates the special case for objects/bytes // overlaps with the special case for total. @@ -225,9 +245,21 @@ func (r *builtinRuntime) register() error { } } + // We need a fixed description, which depends which + // convention is used. + var description string + if totalAttrVals != nil { + // This case is an aggregation of known values + description = fmt.Sprintf("runtime/metrics: %v/*:%s", originalName, statedUnits) + } else { + // This includes the counts[n] > 1 case, both + // use an exact name. + description = fmt.Sprintf("runtime/metrics: %v", m.Name) + } + opts := []instrument.Option{ instrument.WithUnit(unit.Unit(u)), - instrument.WithDescription(m.Description), + instrument.WithDescription(description), } var inst instrument.Asynchronous var err error @@ -262,13 +294,16 @@ func (r *builtinRuntime) register() error { } samples = append(samples, samp) instruments = append(instruments, inst) - if totalAttrVal == "" { + + if totalAttrVals == nil { totalAttrs = append(totalAttrs, nil) } else { - // Append a singleton list. - totalAttrs = append(totalAttrs, []attribute.KeyValue{ - attribute.String(getTotalizedAttributeName(n), totalAttrVal), - }) + // Form a list of attributes to use in the callback. + var attrs []attribute.KeyValue + for i, val := range totalAttrVals { + attrs = append(attrs, attribute.Key(getTotalizedAttributeName(i)).String(val)) + } + totalAttrs = append(totalAttrs, attrs) } } diff --git a/lightstep/instrumentation/runtime/builtin_119_test.go b/lightstep/instrumentation/runtime/builtin_119_test.go index 2d7388cf..de151db9 100644 --- a/lightstep/instrumentation/runtime/builtin_119_test.go +++ b/lightstep/instrumentation/runtime/builtin_119_test.go @@ -16,19 +16,26 @@ package runtime -var expectRuntimeMetrics = map[string]int{ - "cgo.go-to-c-calls": 1, - "gc.cycles": 2, - "gc.heap.allocs": 1, - "gc.heap.allocs.objects": 1, - "gc.heap.frees": 1, - "gc.heap.frees.objects": 1, - "gc.heap.goal": 1, - "gc.heap.objects": 1, - "gc.heap.tiny.allocs": 1, - "gc.limiter.last-enabled": 1, - "gc.stack.starting-size": 1, - "memory.usage": 13, - "sched.gomaxprocs": 1, - "sched.goroutines": 1, +import "go.opentelemetry.io/otel/attribute" + +var expectRuntimeMetrics = map[builtinNameExpected]map[attribute.Set]bool{ + expectCounter("cgo.go-to-c-calls"): expectSingleton, + expectCounter("gc.cycles"): map[attribute.Set]bool{ + attribute.NewSet(classKey.String("automatic")): true, + attribute.NewSet(classKey.String("forced")): true, + }, + expectCounter("gc.heap.allocs"): nil, + expectCounter("gc.heap.allocs.objects"): nil, + expectCounter("gc.heap.frees"): nil, + expectCounter("gc.heap.frees.objects"): nil, + expectUpDownCounter("gc.heap.goal"): nil, + expectCounter("gc.heap.objects"): nil, + expectCounter("gc.heap.tiny.allocs"): nil, + expectUpDownCounter("gc.limiter.last-enabled"): nil, + expectUpDownCounter("gc.stack.starting-size"): nil, + expectUpDownCounter("memory.usage"): map[attribute.Set]bool{ + attribute.NewSet(classKey.String("heap"), subclassKey.String("free")): true, + }, + expectUpDownCounter("sched.gomaxprocs"): nil, + expectUpDownCounter("sched.goroutines"): nil, } diff --git a/lightstep/instrumentation/runtime/builtin_test.go b/lightstep/instrumentation/runtime/builtin_test.go index 7955d0b3..8a2a193a 100644 --- a/lightstep/instrumentation/runtime/builtin_test.go +++ b/lightstep/instrumentation/runtime/builtin_test.go @@ -22,6 +22,7 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/metric/unit" "go.opentelemetry.io/otel/sdk/instrumentation" "go.opentelemetry.io/otel/sdk/metric" "go.opentelemetry.io/otel/sdk/metric/metricdata" @@ -30,10 +31,55 @@ import ( // prefix is mandatory for this library, however the "go." part is not. const expectPrefix = "process.runtime.go." -var expectScope = instrumentation.Scope{ - Name: "otel-launcher-go/runtime", +type builtinExpected int + +var expectSingleton = map[attribute.Set]bool{ + emptySet: true, +} + +const ( + builtinCounter builtinExpected = iota + builtinUpDownCounter + builtinGauge +) + +type builtinNameExpected struct { + name string + expect builtinExpected +} + +func expectCounter(name string) builtinNameExpected { + return builtinNameExpected{ + name: name, + expect: builtinCounter, + } +} + +func expectUpDownCounter(name string) builtinNameExpected { + return builtinNameExpected{ + name: name, + expect: builtinCounter, + } +} + +func expectGauge(name string) builtinNameExpected { + return builtinNameExpected{ + name: name, + expect: builtinGauge, + } } +var ( + expectScope = instrumentation.Scope{ + Name: "otel-launcher-go/runtime", + } + + emptySet = attribute.NewSet() + + classKey = attribute.Key("class") + subclassKey = attribute.Key("subclass") +) + // TestBuiltinRuntimeMetrics tests the real output of the library to // ensure expected prefix, instrumentation scope, and empty // attributes. @@ -75,8 +121,11 @@ func TestBuiltinRuntimeMetrics(t *testing.T) { attrs = dt.DataPoints[0].Attributes } - if expect[name] > 1 { - require.Equal(t, 1, attrs.Len()) + lookup, ok := expect[name] + require.True(t, ok, "lookup %v", name) + + if lookup == nil { + require.Equal(t, 0, attrs.Len()) } else { require.Equal(t, 1, expect[name], "for %v", inst.Name) require.Equal(t, 0, attrs.Len()) @@ -87,7 +136,7 @@ func TestBuiltinRuntimeMetrics(t *testing.T) { require.Equal(t, expect, allNames) } -func makeTestCase() (allFunc, readFunc, map[string]map[string]metrics.Value) { +func makeAllInts() []metrics.Value { // Note: the library provides no way to generate values, so use the // builtin library to get some. Since we can't generate a Float64 value // we can't even test the Gauge logic in this package. @@ -115,6 +164,32 @@ func makeTestCase() (allFunc, readFunc, map[string]map[string]metrics.Value) { for iv := range ints { allInts = append(allInts, iv) } + return allInts +} + +type testMapping map[string]metrics.Value + +func (m testMapping) read(samples []metrics.Sample) { + for i := range samples { + v, ok := m[samples[i].Name] + if ok { + samples[i].Value = v + } else { + panic("outcome uncertain") + } + } +} + +type testExpectation map[string]*testExpectMetric + +type testExpectMetric struct { + desc string + unit unit.Unit + vals map[attribute.Set]metrics.Value +} + +func makeTestCase1() (allFunc, readFunc, testExpectation) { + allInts := makeAllInts() af := func() []metrics.Description { return []metrics.Description{ @@ -142,9 +217,45 @@ func makeTestCase() (allFunc, readFunc, map[string]map[string]metrics.Value) { Kind: metrics.KindUint64, Cumulative: true, }, + { + Name: "/waste/cycles/ocean:cycles", + Description: "blah blah", + Kind: metrics.KindUint64, + Cumulative: true, + }, + { + Name: "/waste/cycles/sea:cycles", + Description: "blah blah", + Kind: metrics.KindUint64, + Cumulative: true, + }, + { + Name: "/waste/cycles/lake:cycles", + Description: "blah blah", + Kind: metrics.KindUint64, + Cumulative: true, + }, + { + Name: "/waste/cycles/pond:cycles", + Description: "blah blah", + Kind: metrics.KindUint64, + Cumulative: true, + }, + { + Name: "/waste/cycles/puddle:cycles", + Description: "blah blah", + Kind: metrics.KindUint64, + Cumulative: true, + }, + { + Name: "/waste/cycles/total:cycles", + Description: "blah blah", + Kind: metrics.KindUint64, + Cumulative: true, + }, } } - mapping := map[string]metrics.Value{ + mapping := testMapping{ "/cntr/things:things": allInts[0], "/updowncntr/things:things": allInts[1], "/process/count:objects": allInts[2], @@ -156,37 +267,126 @@ func makeTestCase() (allFunc, readFunc, map[string]map[string]metrics.Value) { "/waste/cycles/puddle:cycles": allInts[8], "/waste/cycles/total:cycles": allInts[9], } - rf := func(samples []metrics.Sample) { - for i := range samples { - v, ok := mapping[samples[i].Name] - if ok { - samples[i].Value = v - } else { - panic("outcome uncertain") - } + return af, mapping.read, testExpectation{ + "cntr.things": &testExpectMetric{ + unit: "{things}", + desc: "runtime/metrics: /cntr/things:things", + vals: map[attribute.Set]metrics.Value{ + emptySet: allInts[0], + }, + }, + "updowncntr.things": &testExpectMetric{ + unit: "{things}", + desc: "runtime/metrics: /updowncntr/things:things", + vals: map[attribute.Set]metrics.Value{ + emptySet: allInts[1], + }, + }, + "process.count.objects": &testExpectMetric{ + unit: "{objects}", + desc: "runtime/metrics: /process/count:objects", + vals: map[attribute.Set]metrics.Value{ + emptySet: allInts[2], + }, + }, + "process.count": &testExpectMetric{ + unit: unit.Bytes, + desc: "runtime/metrics: /process/count:bytes", + vals: map[attribute.Set]metrics.Value{ + emptySet: allInts[3], + }, + }, + "waste.cycles": &testExpectMetric{ + unit: "{cycles}", + desc: "runtime/metrics: /waste/cycles/*:cycles", + vals: map[attribute.Set]metrics.Value{ + attribute.NewSet(classKey.String("ocean")): allInts[4], + attribute.NewSet(classKey.String("sea")): allInts[5], + attribute.NewSet(classKey.String("lake")): allInts[6], + attribute.NewSet(classKey.String("pond")): allInts[7], + attribute.NewSet(classKey.String("puddle")): allInts[8], + }, + }, + } +} + +func makeTestCase2() (allFunc, readFunc, testExpectation) { + allInts := makeAllInts() + + af := func() []metrics.Description { + return []metrics.Description{ + { + Name: "/objsize/classes/presos:bytes", + Description: "a counter of presos bytes", + Kind: metrics.KindUint64, + Cumulative: true, + }, + { + Name: "/objsize/classes/sheets:bytes", + Description: "a counter of sheets bytes", + Kind: metrics.KindUint64, + Cumulative: true, + }, + { + Name: "/objsize/classes/docs/word:bytes", + Description: "a counter of word doc bytes", + Kind: metrics.KindUint64, + Cumulative: true, + }, + { + Name: "/objsize/classes/docs/pdf:bytes", + Description: "a counter of word docs", + Kind: metrics.KindUint64, + Cumulative: true, + }, + { + Name: "/objsize/classes/docs/total:bytes", + Description: "a counter of all docs bytes", + Kind: metrics.KindUint64, + Cumulative: true, + }, + { + Name: "/objsize/classes/total:bytes", + Description: "a counter of all kinds things bytes", + Kind: metrics.KindUint64, + Cumulative: true, + }, } } - return af, rf, map[string]map[string]metrics.Value{ - "cntr.things": {"": allInts[0]}, - "updowncntr.things": {"": allInts[1]}, - "process.count.objects": {"": allInts[2]}, - "process.count": {"": allInts[3]}, - - // This uses "cycles", one of the two known - // multi-variate metrics as of go-1.19. - "waste.cycles": { - "ocean": allInts[4], - "sea": allInts[5], - "lake": allInts[6], - "pond": allInts[7], - "puddle": allInts[8], + mapping := testMapping{ + "/objsize/classes/presos:bytes": allInts[0], + "/objsize/classes/sheets:bytes": allInts[1], + "/objsize/classes/docs/word:bytes": allInts[2], + "/objsize/classes/docs/pdf:bytes": allInts[3], + "/objsize/classes/docs/total:bytes": allInts[4], + "/objsize/classes/total:bytes": allInts[5], + } + return af, mapping.read, testExpectation{ + "objsize.usage": &testExpectMetric{ + unit: unit.Bytes, + desc: "runtime/metrics: /objsize/classes/*:bytes", + vals: map[attribute.Set]metrics.Value{ + attribute.NewSet(classKey.String("presos")): allInts[0], + attribute.NewSet(classKey.String("sheets")): allInts[1], + attribute.NewSet(classKey.String("docs"), subclassKey.String("word")): allInts[2], + attribute.NewSet(classKey.String("docs"), subclassKey.String("pdf")): allInts[3], + }, }, } } -// TestMetricTranslation validates the translation logic using +// TestMetricTranslation1 validates the translation logic using // synthetic metric names and values. -func TestMetricTranslation(t *testing.T) { +func TestMetricTranslation1(t *testing.T) { + testMetricTranslation(t, makeTestCase1) +} + +// TestMetricTranslation2 is a more complex test than the first. +func TestMetricTranslation2(t *testing.T) { + testMetricTranslation(t, makeTestCase2) +} + +func testMetricTranslation(t *testing.T, makeTestCase func() (allFunc, readFunc, testExpectation)) { reader := metric.NewManualReader() provider := metric.NewMeterProvider(metric.WithReader(reader)) @@ -195,48 +395,32 @@ func TestMetricTranslation(t *testing.T) { err := br.register() require.NoError(t, err) - expectRecords := 0 - for _, values := range mapping { - expectRecords += len(values) - if len(values) > 1 { - // Counts the total - expectRecords++ - } - } - data, err := reader.Collect(context.Background()) require.NoError(t, err) - require.Equal(t, 10, expectRecords) require.Equal(t, 1, len(data.ScopeMetrics)) require.Equal(t, "test", data.ScopeMetrics[0].Scope.Name) + require.Equal(t, len(mapping), len(data.ScopeMetrics[0].Metrics), "metrics count: %v", data.ScopeMetrics[0].Metrics) for _, inst := range data.ScopeMetrics[0].Metrics { // Test the special cases are present always: - require.True(t, strings.HasPrefix(inst.Name, expectPrefix), "%s", inst.Name) name := inst.Name[len(expectPrefix):] - require.Equal(t, 1, len(inst.Data.(metricdata.Sum[int64]).DataPoints)) - - sum := inst.Data.(metricdata.Sum[int64]).DataPoints[0].Value - attrs := inst.Data.(metricdata.Sum[int64]).DataPoints[0].Attributes - // Note: only int64 is tested, we have no way to // generate Float64 values and Float64Hist values are // not implemented for testing. - m := mapping[name] - if len(m) == 1 { - require.Equal(t, mapping[name][""].Uint64(), uint64(sum)) + exm := mapping[name] - // no attributes - require.Equal(t, 0, attrs.Len()) - } else { - require.Equal(t, 5, len(m)) - require.Equal(t, 1, attrs.Len()) - require.Equal(t, attrs.ToSlice()[0].Key, "class") - feature := attrs.ToSlice()[0].Value.AsString() - require.Equal(t, mapping[name][feature].Uint64(), uint64(sum)) + require.Equal(t, exm.desc, inst.Description) + require.Equal(t, exm.unit, inst.Unit) + + require.Equal(t, len(exm.vals), len(inst.Data.(metricdata.Sum[int64]).DataPoints), "points count: %v != %v", inst.Data.(metricdata.Sum[int64]).DataPoints, exm.vals) + + for _, point := range inst.Data.(metricdata.Sum[int64]).DataPoints { + lookup, ok := exm.vals[point.Attributes] + require.True(t, ok, "lookup failed: %v", exm.vals, point.Attributes) + require.Equal(t, lookup.Uint64(), uint64(point.Value)) } } } From 30e2ac7fc7d3b00594bb540e5eed03302cc83bfd Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Mon, 19 Dec 2022 16:29:38 -0800 Subject: [PATCH 3/9] salvage operation v1 --- lightstep/instrumentation/runtime/builtin.go | 111 ++------------ .../{builtin_118_test.go => builtin_118.go} | 0 .../instrumentation/runtime/builtin_119.go | 44 ++++++ .../runtime/builtin_119_test.go | 41 ------ .../{builtin_120_test.go => builtin_120.go} | 0 .../instrumentation/runtime/builtin_test.go | 138 ++---------------- .../instrumentation/runtime/descriptor.go | 121 +++++++++++++++ 7 files changed, 194 insertions(+), 261 deletions(-) rename lightstep/instrumentation/runtime/{builtin_118_test.go => builtin_118.go} (100%) create mode 100644 lightstep/instrumentation/runtime/builtin_119.go delete mode 100644 lightstep/instrumentation/runtime/builtin_119_test.go rename lightstep/instrumentation/runtime/{builtin_120_test.go => builtin_120.go} (100%) create mode 100644 lightstep/instrumentation/runtime/descriptor.go diff --git a/lightstep/instrumentation/runtime/builtin.go b/lightstep/instrumentation/runtime/builtin.go index be2e9900..dfbb882a 100644 --- a/lightstep/instrumentation/runtime/builtin.go +++ b/lightstep/instrumentation/runtime/builtin.go @@ -28,6 +28,12 @@ import ( "go.opentelemetry.io/otel/metric/unit" ) +const ( + namePrefix = "process.runtime.go" + classKey = attribute.Key("class") + subclassKey = attribute.Key("subclass") +) + // LibraryName is the value of instrumentation.Library.Name. const LibraryName = "otel-launcher-go/runtime" @@ -145,112 +151,21 @@ func getTotalizedMetricName(n, u string) string { func (r *builtinRuntime) register() error { all := r.allFunc() - totals := map[string]bool{} - counts := map[string]int{} - toName := func(in string) (string, string) { - n, statedUnits, _ := strings.Cut(in, ":") - n = "process.runtime.go" + strings.ReplaceAll(n, "/", ".") - return n, statedUnits - } + desc := expectRuntimeMetrics() for _, m := range all { - name, _ := toName(m.Name) - - // Totals map includes the '.' suffix. - if strings.HasSuffix(name, ".total") { - totals[name[:len(name)-len("total")]] = true - } - - counts[name]++ - } - - var samples []metrics.Sample - var instruments []instrument.Asynchronous - var totalAttrs [][]attribute.KeyValue - - for _, m := range all { - // n is the output name, which has '/' replaced with - // '.' and a prefix this value may be modified below, - // based on conventions in the runtime/metrics - // package. - n, statedUnits := toName(m.Name) - - originalName, _, _ := strings.Cut(m.Name, ":") - - // We skip all total metrics because they are implied - // as sums and can be configured as OTel views. - if strings.HasSuffix(n, ".total") { - continue - } - - // Get the units, real or pseudo. - var u string - switch statedUnits { - case "bytes": - u = string(unit.Bytes) - case "seconds": - u = statedUnits - default: - // Pseudo-units - u = "{" + statedUnits + "}" - } - - // Remove any ".total" suffix, this is redundant for Prometheus. - var totalAttrVals []string - var totalAttrSubstring string - var totalPrefix string - for totalize := range totals { - // For any totals, find the longest substring. - if strings.HasPrefix(n, totalize) && len(n)-len(totalize) > len(totalAttrSubstring) { - // Units is unchanged. - // Name becomes the shortest prefix. - // Remember which attribute to use. - totalAttrSubstring = n[len(totalize):] - totalAttrVals = strings.Split(totalAttrSubstring, ".") - totalPrefix = totalize - } - } - if totalPrefix != "" { - n = getTotalizedMetricName(totalPrefix[:len(totalPrefix)-1], u) - originalName = originalName[:len(originalName)-len(totalAttrSubstring)-1] - } - - // Next branch helps identify the objects/bytes special case, - // which correctly removes "bytes" (a proper unit) - // from appearing as a suffix. - if counts[n] > 1 { - if totalAttrVals != nil { - // This has not happened, hopefully never will. - // Indicates the special case for objects/bytes - // overlaps with the special case for total. - panic("special case collision") - } - - // This is treated as a special case, we know this happens - // with "objects" and "bytes" in the standard Go 1.19 runtime. - switch statedUnits { - case "objects": - // In this case, use `.objects` suffix. - n = n + ".objects" - u = "{objects}" - case "bytes": - // In this case, use no suffix. In Prometheus this will - // be appended as a suffix. - default: - panic(fmt.Sprint( - "unrecognized duplicate metrics names, ", - "attention required: ", - n, - )) - } + // each should match one + family, err = desc.find(m.Name) + if err != nil { + return err } // We need a fixed description, which depends which // convention is used. var description string - if totalAttrVals != nil { + if len(family.attrs) != 1 { // This case is an aggregation of known values - description = fmt.Sprintf("runtime/metrics: %v/*:%s", originalName, statedUnits) + description = fmt.Sprintf("runtime/metrics: %v/*:%s", m.Name, statedUnits) } else { // This includes the counts[n] > 1 case, both // use an exact name. diff --git a/lightstep/instrumentation/runtime/builtin_118_test.go b/lightstep/instrumentation/runtime/builtin_118.go similarity index 100% rename from lightstep/instrumentation/runtime/builtin_118_test.go rename to lightstep/instrumentation/runtime/builtin_118.go diff --git a/lightstep/instrumentation/runtime/builtin_119.go b/lightstep/instrumentation/runtime/builtin_119.go new file mode 100644 index 00000000..ca4d8d51 --- /dev/null +++ b/lightstep/instrumentation/runtime/builtin_119.go @@ -0,0 +1,44 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build go1.19 && !go1.20 + +package runtime + +import "go.opentelemetry.io/otel/attribute" + +func expectRuntimeMetrics() *builtinDescriptor { + bd := newBuiltinsDescriptor() + bd.singleCounter("/cgo/go-to-c-calls:calls") + bd.classesCounter("/gc/cycles/*:gc-cycles", + attribute.NewSet(classKey.String("automatic")), + attribute.NewSet(classKey.String("forced")), + ) + bd.objectBytesCounter("/gc/heap/allocs:*") + bd.objectBytesCounter("/gc/heap/frees:*") + + bd.singleGauge("/gc/heap/goal:bytes") + + bd.singleUpDownCounter("/gc/heap/objects:objects") + + bd.singleCounter("/gc/heap/tiny/allocs:objects") + bd.singleGauge("/gc/limiter/last-enabled:gc-cycle") + bd.singleGauge("/gc/stack/starting-size:bytes") + bd.classesUpDownCounter("memory.usage", + attribute.NewSet(classKey.String("heap"), subclassKey.String("free")), + ) + bd.singleGauge("sched.gomaxprocs") + bd.singleUpDownCounter("sched.goroutines") + return bd +} diff --git a/lightstep/instrumentation/runtime/builtin_119_test.go b/lightstep/instrumentation/runtime/builtin_119_test.go deleted file mode 100644 index de151db9..00000000 --- a/lightstep/instrumentation/runtime/builtin_119_test.go +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -//go:build go1.19 && !go1.20 - -package runtime - -import "go.opentelemetry.io/otel/attribute" - -var expectRuntimeMetrics = map[builtinNameExpected]map[attribute.Set]bool{ - expectCounter("cgo.go-to-c-calls"): expectSingleton, - expectCounter("gc.cycles"): map[attribute.Set]bool{ - attribute.NewSet(classKey.String("automatic")): true, - attribute.NewSet(classKey.String("forced")): true, - }, - expectCounter("gc.heap.allocs"): nil, - expectCounter("gc.heap.allocs.objects"): nil, - expectCounter("gc.heap.frees"): nil, - expectCounter("gc.heap.frees.objects"): nil, - expectUpDownCounter("gc.heap.goal"): nil, - expectCounter("gc.heap.objects"): nil, - expectCounter("gc.heap.tiny.allocs"): nil, - expectUpDownCounter("gc.limiter.last-enabled"): nil, - expectUpDownCounter("gc.stack.starting-size"): nil, - expectUpDownCounter("memory.usage"): map[attribute.Set]bool{ - attribute.NewSet(classKey.String("heap"), subclassKey.String("free")): true, - }, - expectUpDownCounter("sched.gomaxprocs"): nil, - expectUpDownCounter("sched.goroutines"): nil, -} diff --git a/lightstep/instrumentation/runtime/builtin_120_test.go b/lightstep/instrumentation/runtime/builtin_120.go similarity index 100% rename from lightstep/instrumentation/runtime/builtin_120_test.go rename to lightstep/instrumentation/runtime/builtin_120.go diff --git a/lightstep/instrumentation/runtime/builtin_test.go b/lightstep/instrumentation/runtime/builtin_test.go index 8a2a193a..be0c7ba1 100644 --- a/lightstep/instrumentation/runtime/builtin_test.go +++ b/lightstep/instrumentation/runtime/builtin_test.go @@ -4,7 +4,7 @@ // you may not use this file except in compliance with the License. // You may obtain a copy of the License at // -// http://www.apache.org/licenses/LICENSE-2.0 +// http://www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, software // distributed under the License is distributed on an "AS IS" BASIS, @@ -23,119 +23,10 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric/unit" - "go.opentelemetry.io/otel/sdk/instrumentation" "go.opentelemetry.io/otel/sdk/metric" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) -// prefix is mandatory for this library, however the "go." part is not. -const expectPrefix = "process.runtime.go." - -type builtinExpected int - -var expectSingleton = map[attribute.Set]bool{ - emptySet: true, -} - -const ( - builtinCounter builtinExpected = iota - builtinUpDownCounter - builtinGauge -) - -type builtinNameExpected struct { - name string - expect builtinExpected -} - -func expectCounter(name string) builtinNameExpected { - return builtinNameExpected{ - name: name, - expect: builtinCounter, - } -} - -func expectUpDownCounter(name string) builtinNameExpected { - return builtinNameExpected{ - name: name, - expect: builtinCounter, - } -} - -func expectGauge(name string) builtinNameExpected { - return builtinNameExpected{ - name: name, - expect: builtinGauge, - } -} - -var ( - expectScope = instrumentation.Scope{ - Name: "otel-launcher-go/runtime", - } - - emptySet = attribute.NewSet() - - classKey = attribute.Key("class") - subclassKey = attribute.Key("subclass") -) - -// TestBuiltinRuntimeMetrics tests the real output of the library to -// ensure expected prefix, instrumentation scope, and empty -// attributes. -func TestBuiltinRuntimeMetrics(t *testing.T) { - reader := metric.NewManualReader() - provider := metric.NewMeterProvider(metric.WithReader(reader)) - - err := Start(WithMeterProvider(provider)) - require.NoError(t, err) - - data, err := reader.Collect(context.Background()) - require.NoError(t, err) - - require.Equal(t, 1, len(data.ScopeMetrics)) - require.Equal(t, expectScope, data.ScopeMetrics[0].Scope) - - expect := expectRuntimeMetrics - allNames := map[string]int{} - - // Note: metrictest library lacks a way to distinguish - // monotonic vs not or to test the unit. This will be fixed in - // the new SDK, all the pieces untested here. - for _, inst := range data.ScopeMetrics[0].Metrics { - require.True(t, strings.HasPrefix(inst.Name, expectPrefix), "%s", inst.Name) - name := inst.Name[len(expectPrefix):] - var attrs attribute.Set - switch dt := inst.Data.(type) { - case metricdata.Gauge[int64]: - require.Equal(t, 1, len(dt.DataPoints)) - attrs = dt.DataPoints[0].Attributes - case metricdata.Gauge[float64]: - require.Equal(t, 1, len(dt.DataPoints)) - attrs = dt.DataPoints[0].Attributes - case metricdata.Sum[int64]: - require.Equal(t, 1, len(dt.DataPoints)) - attrs = dt.DataPoints[0].Attributes - case metricdata.Sum[float64]: - require.Equal(t, 1, len(dt.DataPoints)) - attrs = dt.DataPoints[0].Attributes - } - - lookup, ok := expect[name] - require.True(t, ok, "lookup %v", name) - - if lookup == nil { - require.Equal(t, 0, attrs.Len()) - } else { - require.Equal(t, 1, expect[name], "for %v", inst.Name) - require.Equal(t, 0, attrs.Len()) - } - allNames[name]++ - } - - require.Equal(t, expect, allNames) -} - func makeAllInts() []metrics.Value { // Note: the library provides no way to generate values, so use the // builtin library to get some. Since we can't generate a Float64 value @@ -180,7 +71,7 @@ func (m testMapping) read(samples []metrics.Sample) { } } -type testExpectation map[string]*testExpectMetric +type testExpectation map[string]testExpectMetric type testExpectMetric struct { desc string @@ -268,35 +159,35 @@ func makeTestCase1() (allFunc, readFunc, testExpectation) { "/waste/cycles/total:cycles": allInts[9], } return af, mapping.read, testExpectation{ - "cntr.things": &testExpectMetric{ + "cntr.things": testExpectMetric{ unit: "{things}", desc: "runtime/metrics: /cntr/things:things", vals: map[attribute.Set]metrics.Value{ emptySet: allInts[0], }, }, - "updowncntr.things": &testExpectMetric{ + "updowncntr.things": testExpectMetric{ unit: "{things}", desc: "runtime/metrics: /updowncntr/things:things", vals: map[attribute.Set]metrics.Value{ emptySet: allInts[1], }, }, - "process.count.objects": &testExpectMetric{ + "process.count.objects": testExpectMetric{ unit: "{objects}", desc: "runtime/metrics: /process/count:objects", vals: map[attribute.Set]metrics.Value{ emptySet: allInts[2], }, }, - "process.count": &testExpectMetric{ + "process.count": testExpectMetric{ unit: unit.Bytes, desc: "runtime/metrics: /process/count:bytes", vals: map[attribute.Set]metrics.Value{ emptySet: allInts[3], }, }, - "waste.cycles": &testExpectMetric{ + "waste.cycles": testExpectMetric{ unit: "{cycles}", desc: "runtime/metrics: /waste/cycles/*:cycles", vals: map[attribute.Set]metrics.Value{ @@ -362,7 +253,7 @@ func makeTestCase2() (allFunc, readFunc, testExpectation) { "/objsize/classes/total:bytes": allInts[5], } return af, mapping.read, testExpectation{ - "objsize.usage": &testExpectMetric{ + "objsize.usage": testExpectMetric{ unit: unit.Bytes, desc: "runtime/metrics: /objsize/classes/*:bytes", vals: map[attribute.Set]metrics.Value{ @@ -398,18 +289,21 @@ func testMetricTranslation(t *testing.T, makeTestCase func() (allFunc, readFunc, data, err := reader.Collect(context.Background()) require.NoError(t, err) + require.Equal(t, 1, len(data.ScopeMetrics)) + require.Equal(t, 1, len(data.ScopeMetrics)) require.Equal(t, "test", data.ScopeMetrics[0].Scope.Name) require.Equal(t, len(mapping), len(data.ScopeMetrics[0].Metrics), "metrics count: %v", data.ScopeMetrics[0].Metrics) for _, inst := range data.ScopeMetrics[0].Metrics { // Test the special cases are present always: - require.True(t, strings.HasPrefix(inst.Name, expectPrefix), "%s", inst.Name) - name := inst.Name[len(expectPrefix):] + require.True(t, strings.HasPrefix(inst.Name, namePrefix+"."), "%s", inst.Name) + name := inst.Name[len(namePrefix)+1:] - // Note: only int64 is tested, we have no way to - // generate Float64 values and Float64Hist values are - // not implemented for testing. + // Note: only int64 values are tested, as we have no + // way to generate float64 values w/o an + // runtime/metrics supporting a metric to generate + // these values. exm := mapping[name] require.Equal(t, exm.desc, inst.Description) diff --git a/lightstep/instrumentation/runtime/descriptor.go b/lightstep/instrumentation/runtime/descriptor.go new file mode 100644 index 00000000..9555f356 --- /dev/null +++ b/lightstep/instrumentation/runtime/descriptor.go @@ -0,0 +1,121 @@ +package runtime + +import ( + "fmt" + "path" + + "go.opentelemetry.io/otel/attribute" +) + +type builtinKind int + +var ( + singleton = []attribute.Set{emptySet} + + emptySet = attribute.NewSet() + + ErrUnmatchedBuiltin = fmt.Errorf("builtin unmatched") + ErrOvermatchedBuiltin = fmt.Errorf("builtin overmatched") +) + +const ( + builtinCounter builtinKind = iota + builtinObjectBytesCounter + builtinUpDownCounter + builtinGauge +) + +type builtinMetricFamily struct { + pattern string + kind builtinKind + attrs []attribute.Set +} + +type builtinDescriptor struct { + families []builtinMetricFamily +} + +func newBuiltinsDescriptor() *builtinDescriptor { + return &builtinDescriptor{} +} + +func (bd *builtinDescriptor) add(pattern string, kind builtinKind, attrs ...attribute.Set) { + + bd.families = append(bd.families, builtinMetricFamily{ + pattern: pattern, + kind: kind, + attrs: attrs, + }) +} + +// func toOTelName(n string) string { +// n, _, _ = strings.Cut(n, ":") +// n = namePrefix + strings.ReplaceAll(n, "/", ".") +// return n +// } + +// func toOTelUnit(n string) string { +// _, u, _ := strings.Cut(n, ":") +// return u +// } + +func (bd *builtinDescriptor) singleCounter(pattern string) { + bd.add(pattern, builtinCounter) +} + +func (bd *builtinDescriptor) classesCounter(pattern string, attrs ...attribute.Set) { + if len(attrs) < 2 { + panic("must have >1 attrs") + } + bd.add(pattern, builtinCounter, attrs...) +} + +func (bd *builtinDescriptor) classesUpDownCounter(pattern string, attrs ...attribute.Set) { + if len(attrs) < 2 { + panic("must have >1 attrs") + } + bd.add(pattern, builtinUpDownCounter, attrs...) +} + +func (bd *builtinDescriptor) objectBytesCounter(pattern string) { + bd.add(pattern, builtinObjectBytesCounter) +} + +func (bd *builtinDescriptor) singleUpDownCounter(pattern string) { + bd.add(pattern, builtinUpDownCounter) +} + +func (bd *builtinDescriptor) singleGauge(pattern string) { + bd.add(pattern, builtinGauge) +} + +func (bd *builtinDescriptor) find(name string) error { + fam, err := bd.findFamily(name) + if err != nil { + + } + + return +} + +func (bd *builtinDescriptor) findFamily(name string) (builtinMetricFamily, error) { + var first builtinMetricFamily + matches := 0 + + for _, f := range bd.families { + matched, err := path.Match(f.pattern, name) + if err != nil { + return first, err + } + if matched { + matches++ + } + } + if matches == 0 { + return first, fmt.Errorf("%s: %w", name, ErrUnmatchedBuiltin) + } + if matches > 1 { + return first, fmt.Errorf("%s: %w", name, ErrOvermatchedBuiltin) + } + return first, nil +} From 9642dd1e1be5973d1d4e4816eb7ca7194be3e83a Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Tue, 20 Dec 2022 00:19:15 -0800 Subject: [PATCH 4/9] salvage operation v2 --- lightstep/instrumentation/runtime/builtin.go | 95 ++------ .../instrumentation/runtime/builtin_119.go | 13 +- .../instrumentation/runtime/builtin_test.go | 206 +++++++++--------- .../instrumentation/runtime/descriptor.go | 137 +++++++++--- 4 files changed, 249 insertions(+), 202 deletions(-) diff --git a/lightstep/instrumentation/runtime/builtin.go b/lightstep/instrumentation/runtime/builtin.go index dfbb882a..2b43faff 100644 --- a/lightstep/instrumentation/runtime/builtin.go +++ b/lightstep/instrumentation/runtime/builtin.go @@ -18,7 +18,6 @@ import ( "context" "fmt" "runtime/metrics" - "strings" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" @@ -29,9 +28,10 @@ import ( ) const ( - namePrefix = "process.runtime.go" - classKey = attribute.Key("class") - subclassKey = attribute.Key("subclass") + namePrefix = "process.runtime.go" + classKey = attribute.Key("class") + subclassKey = attribute.Key("subclass") + subsubclassKey = attribute.Key("subsubclass") ) // LibraryName is the value of instrumentation.Library.Name. @@ -86,7 +86,7 @@ func Start(opts ...Option) error { ) r := newBuiltinRuntime(meter, metrics.All, metrics.Read) - return r.register() + return r.register(expectRuntimeMetrics()) } type allFunc = func() []metrics.Description @@ -114,76 +114,35 @@ func newBuiltinRuntime(meter metric.Meter, af allFunc, rf readFunc) *builtinRunt } } -func getTotalizedAttributeName(order int) string { - // It's a plural, make it singular. - base := "class" - for ; order > 0; order-- { - base = "sub" + base - } - return base -} - -func getTotalizedMetricName(n, u string) string { - if !strings.HasSuffix(n, ".classes") { - return n - } - - s := n[:len(n)-len("classes")] - - // Note that ".classes" is (apparently) intended as a generic - // suffix, while ".cycles" is an exception. - // The ideal name depends on what we know. - switch u { - case "By": - // OTel has similar conventions for memory usage, disk usage, etc, so - // for metrics with /classes/*:bytes we create a .usage metric. - return s + "usage" - case "{cpu-seconds}": - // Same argument above, except OTel uses .time for - // cpu-timing metrics instead of .usage. - return s + "time" - default: - // There are no other conventions in the - // runtime/metrics that we know of. - panic(fmt.Sprint("unrecognized metric suffix: ", u)) - } -} - -func (r *builtinRuntime) register() error { +func (r *builtinRuntime) register(desc *builtinDescriptor) error { all := r.allFunc() - desc := expectRuntimeMetrics() + + var instruments []instrument.Asynchronous + var samples []metrics.Sample + var instAttrs [][]attribute.KeyValue for _, m := range all { // each should match one - family, err = desc.find(m.Name) + mname, munit, pattern, attrs, err := desc.findMatch(m.Name) if err != nil { return err } - - // We need a fixed description, which depends which - // convention is used. - var description string - if len(family.attrs) != 1 { - // This case is an aggregation of known values - description = fmt.Sprintf("runtime/metrics: %v/*:%s", m.Name, statedUnits) - } else { - // This includes the counts[n] > 1 case, both - // use an exact name. - description = fmt.Sprintf("runtime/metrics: %v", m.Name) + if mname == "" { + continue } + description := fmt.Sprintf("%s from runtime/metrics", pattern) opts := []instrument.Option{ - instrument.WithUnit(unit.Unit(u)), + instrument.WithUnit(unit.Unit(munit)), instrument.WithDescription(description), } var inst instrument.Asynchronous - var err error if m.Cumulative { switch m.Kind { case metrics.KindUint64: - inst, err = r.meter.AsyncInt64().Counter(n, opts...) + inst, err = r.meter.AsyncInt64().Counter(mname, opts...) case metrics.KindFloat64: - inst, err = r.meter.AsyncFloat64().Counter(n, opts...) + inst, err = r.meter.AsyncFloat64().Counter(mname, opts...) case metrics.KindFloat64Histogram: // Not implemented Histogram[float64]. continue @@ -191,10 +150,10 @@ func (r *builtinRuntime) register() error { } else { switch m.Kind { case metrics.KindUint64: - inst, err = r.meter.AsyncInt64().UpDownCounter(n, opts...) + inst, err = r.meter.AsyncInt64().UpDownCounter(mname, opts...) case metrics.KindFloat64: // Note: this has never been used. - inst, err = r.meter.AsyncFloat64().Gauge(n, opts...) + inst, err = r.meter.AsyncFloat64().Gauge(mname, opts...) case metrics.KindFloat64Histogram: // Not implemented GaugeHistogram[float64]. continue @@ -209,17 +168,7 @@ func (r *builtinRuntime) register() error { } samples = append(samples, samp) instruments = append(instruments, inst) - - if totalAttrVals == nil { - totalAttrs = append(totalAttrs, nil) - } else { - // Form a list of attributes to use in the callback. - var attrs []attribute.KeyValue - for i, val := range totalAttrVals { - attrs = append(attrs, attribute.Key(getTotalizedAttributeName(i)).String(val)) - } - totalAttrs = append(totalAttrs, attrs) - } + instAttrs = append(instAttrs, attrs) } if err := r.meter.RegisterCallback(instruments, func(ctx context.Context) { @@ -229,9 +178,9 @@ func (r *builtinRuntime) register() error { switch samp.Value.Kind() { case metrics.KindUint64: - instruments[idx].(int64Observer).Observe(ctx, int64(samp.Value.Uint64()), totalAttrs[idx]...) + instruments[idx].(int64Observer).Observe(ctx, int64(samp.Value.Uint64()), instAttrs[idx]...) case metrics.KindFloat64: - instruments[idx].(float64Observer).Observe(ctx, samp.Value.Float64(), totalAttrs[idx]...) + instruments[idx].(float64Observer).Observe(ctx, samp.Value.Float64(), instAttrs[idx]...) default: // KindFloat64Histogram (unsupported in OTel) and KindBad // (unsupported by runtime/metrics). Neither should happen diff --git a/lightstep/instrumentation/runtime/builtin_119.go b/lightstep/instrumentation/runtime/builtin_119.go index ca4d8d51..e5453be6 100644 --- a/lightstep/instrumentation/runtime/builtin_119.go +++ b/lightstep/instrumentation/runtime/builtin_119.go @@ -19,7 +19,7 @@ package runtime import "go.opentelemetry.io/otel/attribute" func expectRuntimeMetrics() *builtinDescriptor { - bd := newBuiltinsDescriptor() + bd := newBuiltinDescriptor() bd.singleCounter("/cgo/go-to-c-calls:calls") bd.classesCounter("/gc/cycles/*:gc-cycles", attribute.NewSet(classKey.String("automatic")), @@ -37,6 +37,17 @@ func expectRuntimeMetrics() *builtinDescriptor { bd.singleGauge("/gc/stack/starting-size:bytes") bd.classesUpDownCounter("memory.usage", attribute.NewSet(classKey.String("heap"), subclassKey.String("free")), + attribute.NewSet(classKey.String("heap"), subclassKey.String("objects")), + attribute.NewSet(classKey.String("heap"), subclassKey.String("released")), + attribute.NewSet(classKey.String("heap"), subclassKey.String("stacks")), + attribute.NewSet(classKey.String("heap"), subclassKey.String("unused")), + attribute.NewSet(classKey.String("metadata"), subclassKey.String("mcache"), subsubclassKey.String("free")), + attribute.NewSet(classKey.String("metadata"), subclassKey.String("mcache"), subsubclassKey.String("inuse")), + attribute.NewSet(classKey.String("metadata"), subclassKey.String("mspan"), subsubclassKey.String("free")), + attribute.NewSet(classKey.String("metadata"), subclassKey.String("mspan"), subsubclassKey.String("inuse")), + attribute.NewSet(classKey.String("metadata"), subclassKey.String("other")), + attribute.NewSet(classKey.String("os-stacks")), + attribute.NewSet(classKey.String("other")), ) bd.singleGauge("sched.gomaxprocs") bd.singleUpDownCounter("sched.goroutines") diff --git a/lightstep/instrumentation/runtime/builtin_test.go b/lightstep/instrumentation/runtime/builtin_test.go index be0c7ba1..6761acaf 100644 --- a/lightstep/instrumentation/runtime/builtin_test.go +++ b/lightstep/instrumentation/runtime/builtin_test.go @@ -15,6 +15,7 @@ package runtime import ( "context" + "fmt" "runtime/metrics" "strings" "testing" @@ -27,6 +28,17 @@ import ( "go.opentelemetry.io/otel/sdk/metric/metricdata" ) +// TestMetricTranslation1 validates the translation logic using +// synthetic metric names and values. +func TestMetricTranslation1(t *testing.T) { + testMetricTranslation(t, makeTestCase1) +} + +// TestMetricTranslation2 is a more complex test than the first. +func TestMetricTranslation2(t *testing.T) { + testMetricTranslation(t, makeTestCase2) +} + func makeAllInts() []metrics.Value { // Note: the library provides no way to generate values, so use the // builtin library to get some. Since we can't generate a Float64 value @@ -79,117 +91,118 @@ type testExpectMetric struct { vals map[attribute.Set]metrics.Value } -func makeTestCase1() (allFunc, readFunc, testExpectation) { +func makeTestCase1() (allFunc, readFunc, *builtinDescriptor, testExpectation) { allInts := makeAllInts() af := func() []metrics.Description { return []metrics.Description{ { - Name: "/cntr/things:things", - Description: "a counter of things", - Kind: metrics.KindUint64, - Cumulative: true, + Name: "/cntr/things:things", + Kind: metrics.KindUint64, + Cumulative: true, }, { - Name: "/updowncntr/things:things", - Description: "an updowncounter of things", - Kind: metrics.KindUint64, - Cumulative: false, + Name: "/updowncntr/things:things", + Kind: metrics.KindUint64, + Cumulative: false, }, { - Name: "/process/count:objects", - Description: "a process counter of objects", - Kind: metrics.KindUint64, - Cumulative: true, + Name: "/process/count:objects", + Kind: metrics.KindUint64, + Cumulative: true, }, { - Name: "/process/count:bytes", - Description: "a process counter of bytes", - Kind: metrics.KindUint64, - Cumulative: true, + Name: "/process/count:bytes", + Kind: metrics.KindUint64, + Cumulative: true, }, { - Name: "/waste/cycles/ocean:cycles", - Description: "blah blah", - Kind: metrics.KindUint64, - Cumulative: true, + Name: "/waste/cycles/ocean:gc-cycles", + Kind: metrics.KindUint64, + Cumulative: true, }, { - Name: "/waste/cycles/sea:cycles", - Description: "blah blah", - Kind: metrics.KindUint64, - Cumulative: true, + Name: "/waste/cycles/sea:gc-cycles", + Kind: metrics.KindUint64, + Cumulative: true, }, { - Name: "/waste/cycles/lake:cycles", - Description: "blah blah", - Kind: metrics.KindUint64, - Cumulative: true, + Name: "/waste/cycles/lake:gc-cycles", + Kind: metrics.KindUint64, + Cumulative: true, }, { - Name: "/waste/cycles/pond:cycles", - Description: "blah blah", - Kind: metrics.KindUint64, - Cumulative: true, + Name: "/waste/cycles/pond:gc-cycles", + Kind: metrics.KindUint64, + Cumulative: true, }, { - Name: "/waste/cycles/puddle:cycles", - Description: "blah blah", - Kind: metrics.KindUint64, - Cumulative: true, + Name: "/waste/cycles/puddle:gc-cycles", + Kind: metrics.KindUint64, + Cumulative: true, }, { - Name: "/waste/cycles/total:cycles", - Description: "blah blah", - Kind: metrics.KindUint64, - Cumulative: true, + Name: "/waste/cycles/total:gc-cycles", + Kind: metrics.KindUint64, + Cumulative: true, }, } } mapping := testMapping{ - "/cntr/things:things": allInts[0], - "/updowncntr/things:things": allInts[1], - "/process/count:objects": allInts[2], - "/process/count:bytes": allInts[3], - "/waste/cycles/ocean:cycles": allInts[4], - "/waste/cycles/sea:cycles": allInts[5], - "/waste/cycles/lake:cycles": allInts[6], - "/waste/cycles/pond:cycles": allInts[7], - "/waste/cycles/puddle:cycles": allInts[8], - "/waste/cycles/total:cycles": allInts[9], + "/cntr/things:things": allInts[0], + "/updowncntr/things:things": allInts[1], + "/process/count:objects": allInts[2], + "/process/count:bytes": allInts[3], + "/waste/cycles/ocean:gc-cycles": allInts[4], + "/waste/cycles/sea:gc-cycles": allInts[5], + "/waste/cycles/lake:gc-cycles": allInts[6], + "/waste/cycles/pond:gc-cycles": allInts[7], + "/waste/cycles/puddle:gc-cycles": allInts[8], + "/waste/cycles/total:gc-cycles": allInts[9], } - return af, mapping.read, testExpectation{ + bd := newBuiltinDescriptor() + bd.singleCounter("/cntr/things:things") + bd.singleUpDownCounter("/updowncntr/things:things") + bd.objectBytesCounter("/process/count:*") + bd.classesCounter("/waste/cycles/*:gc-cycles", + attribute.NewSet(classKey.String("ocean")), + attribute.NewSet(classKey.String("sea")), + attribute.NewSet(classKey.String("lake")), + attribute.NewSet(classKey.String("pond")), + attribute.NewSet(classKey.String("puddle")), + ) + return af, mapping.read, bd, testExpectation{ "cntr.things": testExpectMetric{ unit: "{things}", - desc: "runtime/metrics: /cntr/things:things", + desc: "/cntr/things:things from runtime/metrics", vals: map[attribute.Set]metrics.Value{ emptySet: allInts[0], }, }, "updowncntr.things": testExpectMetric{ unit: "{things}", - desc: "runtime/metrics: /updowncntr/things:things", + desc: "/updowncntr/things:things from runtime/metrics", vals: map[attribute.Set]metrics.Value{ emptySet: allInts[1], }, }, "process.count.objects": testExpectMetric{ - unit: "{objects}", - desc: "runtime/metrics: /process/count:objects", + unit: "", + desc: "/process/count:objects from runtime/metrics", vals: map[attribute.Set]metrics.Value{ emptySet: allInts[2], }, }, "process.count": testExpectMetric{ unit: unit.Bytes, - desc: "runtime/metrics: /process/count:bytes", + desc: "/process/count:bytes from runtime/metrics", vals: map[attribute.Set]metrics.Value{ emptySet: allInts[3], }, }, "waste.cycles": testExpectMetric{ - unit: "{cycles}", - desc: "runtime/metrics: /waste/cycles/*:cycles", + unit: "{gc-cycles}", + desc: "/waste/cycles/*:gc-cycles from runtime/metrics", vals: map[attribute.Set]metrics.Value{ attribute.NewSet(classKey.String("ocean")): allInts[4], attribute.NewSet(classKey.String("sea")): allInts[5], @@ -201,46 +214,40 @@ func makeTestCase1() (allFunc, readFunc, testExpectation) { } } -func makeTestCase2() (allFunc, readFunc, testExpectation) { +func makeTestCase2() (allFunc, readFunc, *builtinDescriptor, testExpectation) { allInts := makeAllInts() af := func() []metrics.Description { return []metrics.Description{ { - Name: "/objsize/classes/presos:bytes", - Description: "a counter of presos bytes", - Kind: metrics.KindUint64, - Cumulative: true, + Name: "/objsize/classes/presos:bytes", + Kind: metrics.KindUint64, + Cumulative: true, }, { - Name: "/objsize/classes/sheets:bytes", - Description: "a counter of sheets bytes", - Kind: metrics.KindUint64, - Cumulative: true, + Name: "/objsize/classes/sheets:bytes", + Kind: metrics.KindUint64, + Cumulative: true, }, { - Name: "/objsize/classes/docs/word:bytes", - Description: "a counter of word doc bytes", - Kind: metrics.KindUint64, - Cumulative: true, + Name: "/objsize/classes/docs/word:bytes", + Kind: metrics.KindUint64, + Cumulative: true, }, { - Name: "/objsize/classes/docs/pdf:bytes", - Description: "a counter of word docs", - Kind: metrics.KindUint64, - Cumulative: true, + Name: "/objsize/classes/docs/pdf:bytes", + Kind: metrics.KindUint64, + Cumulative: true, }, { - Name: "/objsize/classes/docs/total:bytes", - Description: "a counter of all docs bytes", - Kind: metrics.KindUint64, - Cumulative: true, + Name: "/objsize/classes/docs/total:bytes", + Kind: metrics.KindUint64, + Cumulative: true, }, { - Name: "/objsize/classes/total:bytes", - Description: "a counter of all kinds things bytes", - Kind: metrics.KindUint64, - Cumulative: true, + Name: "/objsize/classes/total:bytes", + Kind: metrics.KindUint64, + Cumulative: true, }, } } @@ -252,10 +259,17 @@ func makeTestCase2() (allFunc, readFunc, testExpectation) { "/objsize/classes/docs/total:bytes": allInts[4], "/objsize/classes/total:bytes": allInts[5], } - return af, mapping.read, testExpectation{ + bd := newBuiltinDescriptor() + bd.classesUpDownCounter("/objsize/classes/*:bytes", + attribute.NewSet(classKey.String("presos")), + attribute.NewSet(classKey.String("sheets")), + attribute.NewSet(classKey.String("docs"), subclassKey.String("word")), + attribute.NewSet(classKey.String("docs"), subclassKey.String("pdf")), + ) + return af, mapping.read, bd, testExpectation{ "objsize.usage": testExpectMetric{ unit: unit.Bytes, - desc: "runtime/metrics: /objsize/classes/*:bytes", + desc: "/objsize/classes/*:bytes from runtime/metrics", vals: map[attribute.Set]metrics.Value{ attribute.NewSet(classKey.String("presos")): allInts[0], attribute.NewSet(classKey.String("sheets")): allInts[1], @@ -266,24 +280,13 @@ func makeTestCase2() (allFunc, readFunc, testExpectation) { } } -// TestMetricTranslation1 validates the translation logic using -// synthetic metric names and values. -func TestMetricTranslation1(t *testing.T) { - testMetricTranslation(t, makeTestCase1) -} - -// TestMetricTranslation2 is a more complex test than the first. -func TestMetricTranslation2(t *testing.T) { - testMetricTranslation(t, makeTestCase2) -} - -func testMetricTranslation(t *testing.T, makeTestCase func() (allFunc, readFunc, testExpectation)) { +func testMetricTranslation(t *testing.T, makeTestCase func() (allFunc, readFunc, *builtinDescriptor, testExpectation)) { reader := metric.NewManualReader() provider := metric.NewMeterProvider(metric.WithReader(reader)) - af, rf, mapping := makeTestCase() + af, rf, desc, mapping := makeTestCase() br := newBuiltinRuntime(provider.Meter("test"), af, rf) - err := br.register() + err := br.register(desc) require.NoError(t, err) data, err := reader.Collect(context.Background()) @@ -293,6 +296,9 @@ func testMetricTranslation(t *testing.T, makeTestCase func() (allFunc, readFunc, require.Equal(t, 1, len(data.ScopeMetrics)) require.Equal(t, "test", data.ScopeMetrics[0].Scope.Name) + for _, m := range data.ScopeMetrics[0].Metrics { + fmt.Println(m) + } require.Equal(t, len(mapping), len(data.ScopeMetrics[0].Metrics), "metrics count: %v", data.ScopeMetrics[0].Metrics) for _, inst := range data.ScopeMetrics[0].Metrics { diff --git a/lightstep/instrumentation/runtime/descriptor.go b/lightstep/instrumentation/runtime/descriptor.go index 9555f356..bb23c653 100644 --- a/lightstep/instrumentation/runtime/descriptor.go +++ b/lightstep/instrumentation/runtime/descriptor.go @@ -2,9 +2,10 @@ package runtime import ( "fmt" - "path" + "strings" "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/metric/unit" ) type builtinKind int @@ -27,37 +28,44 @@ const ( type builtinMetricFamily struct { pattern string + matches int kind builtinKind attrs []attribute.Set } type builtinDescriptor struct { - families []builtinMetricFamily + families []*builtinMetricFamily } -func newBuiltinsDescriptor() *builtinDescriptor { +func newBuiltinDescriptor() *builtinDescriptor { return &builtinDescriptor{} } func (bd *builtinDescriptor) add(pattern string, kind builtinKind, attrs ...attribute.Set) { - - bd.families = append(bd.families, builtinMetricFamily{ + bd.families = append(bd.families, &builtinMetricFamily{ pattern: pattern, kind: kind, attrs: attrs, }) } -// func toOTelName(n string) string { -// n, _, _ = strings.Cut(n, ":") -// n = namePrefix + strings.ReplaceAll(n, "/", ".") -// return n -// } +func toOTelNameAndStatedUnit(nameAndUnit string) (on, un string) { + on, un, _ = strings.Cut(nameAndUnit, ":") + return toOTelName(on), un +} -// func toOTelUnit(n string) string { -// _, u, _ := strings.Cut(n, ":") -// return u -// } +func toOTelName(name string) string { + return namePrefix + strings.ReplaceAll(name, "/", ".") +} + +func attributeName(order int) string { + base := "class" + for ; order > 0; order-- { + + base = "sub" + base + } + return base +} func (bd *builtinDescriptor) singleCounter(pattern string) { bd.add(pattern, builtinCounter) @@ -65,14 +73,14 @@ func (bd *builtinDescriptor) singleCounter(pattern string) { func (bd *builtinDescriptor) classesCounter(pattern string, attrs ...attribute.Set) { if len(attrs) < 2 { - panic("must have >1 attrs") + panic(fmt.Sprintf("must have >1 attrs: %s", attrs)) } bd.add(pattern, builtinCounter, attrs...) } func (bd *builtinDescriptor) classesUpDownCounter(pattern string, attrs ...attribute.Set) { if len(attrs) < 2 { - panic("must have >1 attrs") + panic(fmt.Sprintf("must have >1 attrs: %s", attrs)) } bd.add(pattern, builtinUpDownCounter, attrs...) } @@ -89,33 +97,106 @@ func (bd *builtinDescriptor) singleGauge(pattern string) { bd.add(pattern, builtinGauge) } -func (bd *builtinDescriptor) find(name string) error { - fam, err := bd.findFamily(name) +func (bd *builtinDescriptor) findMatch(goname string) (mname, munit, descPattern string, attrs []attribute.KeyValue, _ error) { + fam, err := bd.findFamily(goname) if err != nil { + return "", "", "", nil, err + } + fam.matches++ + + // Set the name, unit and pattern. + if wildCnt := strings.Count(fam.pattern, "*"); wildCnt == 0 { + mname, munit = toOTelNameAndStatedUnit(goname) + descPattern = goname + } else if strings.HasSuffix(fam.pattern, ":*") { + // Special case for bytes/objects w/ same prefix. + mname, munit = toOTelNameAndStatedUnit(goname) + descPattern = goname + + if munit == "objects" { + mname += "." + munit + munit = "" + } + } else { + pfx, sfx, _ := strings.Cut(fam.pattern, "/*:") + mname = toOTelName(pfx) + munit = sfx + asubstr := goname[len(pfx):] + asubstr = asubstr[1 : len(asubstr)-len(sfx)-1] + splitVals := strings.Split(asubstr, "/") + for order, val := range splitVals { + attrs = append(attrs, attribute.Key(attributeName(order)).String(val)) + } + // Ignore subtotals + if splitVals[len(splitVals)-1] == "total" { + return "", "", "", nil, nil + } + descPattern = fam.pattern + } + // Fix the units for UCUM. + switch munit { + case "bytes": + munit = string(unit.Bytes) + case "seconds": + munit = "s" + case "": + default: + // Pseudo-units + munit = "{" + munit + "}" } - return + // Fix the name if it ends in ".classes" + if strings.HasSuffix(mname, ".classes") { + + s := mname[:len(mname)-len("classes")] + + // Note that ".classes" is (apparently) intended as a generic + // suffix, while ".cycles" is an exception. + // The ideal name depends on what we know. + switch munit { + case "By": + // OTel has similar conventions for memory usage, disk usage, etc, so + // for metrics with /classes/*:bytes we create a .usage metric. + mname = s + "usage" + case "{cpu-seconds}": + // Same argument above, except OTel uses .time for + // cpu-timing metrics instead of .usage. + mname = s + "time" + } + } + + return mname, munit, descPattern, attrs, nil } -func (bd *builtinDescriptor) findFamily(name string) (builtinMetricFamily, error) { - var first builtinMetricFamily +func (bd *builtinDescriptor) findFamily(name string) (family *builtinMetricFamily, _ error) { matches := 0 for _, f := range bd.families { - matched, err := path.Match(f.pattern, name) - if err != nil { - return first, err + pat := f.pattern + wilds := strings.Count(pat, "*") + if wilds > 1 { + return nil, fmt.Errorf("too many wildcards: %s", pat) + } + if wilds == 0 && name == pat { + matches++ + family = f + continue } - if matched { + pfx, sfx, _ := strings.Cut(pat, "*") + + if len(name) > len(pat) && strings.HasPrefix(name, pfx) && strings.HasSuffix(name, sfx) { matches++ + family = f + continue } } if matches == 0 { - return first, fmt.Errorf("%s: %w", name, ErrUnmatchedBuiltin) + return nil, fmt.Errorf("%s: %w", name, ErrUnmatchedBuiltin) } if matches > 1 { - return first, fmt.Errorf("%s: %w", name, ErrOvermatchedBuiltin) + return nil, fmt.Errorf("%s: %w", name, ErrOvermatchedBuiltin) } - return first, nil + family.matches++ + return family, nil } From b490cd58acea60726ad858d27e9de0037131fbd5 Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Tue, 20 Dec 2022 12:07:12 -0800 Subject: [PATCH 5/9] make test version agnostic --- lightstep/instrumentation/runtime/builtin.go | 53 ++- .../instrumentation/runtime/builtin_118.go | 30 -- .../instrumentation/runtime/builtin_119.go | 55 --- .../instrumentation/runtime/builtin_120.go | 35 -- .../instrumentation/runtime/builtin_test.go | 317 ++++++++++++++---- lightstep/instrumentation/runtime/defs.go | 40 +++ .../instrumentation/runtime/descriptor.go | 88 +++-- 7 files changed, 385 insertions(+), 233 deletions(-) delete mode 100644 lightstep/instrumentation/runtime/builtin_118.go delete mode 100644 lightstep/instrumentation/runtime/builtin_119.go delete mode 100644 lightstep/instrumentation/runtime/builtin_120.go create mode 100644 lightstep/instrumentation/runtime/defs.go diff --git a/lightstep/instrumentation/runtime/builtin.go b/lightstep/instrumentation/runtime/builtin.go index 2b43faff..e63dee72 100644 --- a/lightstep/instrumentation/runtime/builtin.go +++ b/lightstep/instrumentation/runtime/builtin.go @@ -27,12 +27,7 @@ import ( "go.opentelemetry.io/otel/metric/unit" ) -const ( - namePrefix = "process.runtime.go" - classKey = attribute.Key("class") - subclassKey = attribute.Key("subclass") - subsubclassKey = attribute.Key("subsubclass") -) +const namePrefix = "process.runtime.go" // LibraryName is the value of instrumentation.Library.Name. const LibraryName = "otel-launcher-go/runtime" @@ -123,13 +118,25 @@ func (r *builtinRuntime) register(desc *builtinDescriptor) error { for _, m := range all { // each should match one - mname, munit, pattern, attrs, err := desc.findMatch(m.Name) + mname, munit, pattern, attrs, kind, err := desc.findMatch(m.Name) if err != nil { - return err + // skip unrecognized metric names + otel.Handle(fmt.Errorf("unrecognized runtime/metrics name: %s", m.Name)) + continue } - if mname == "" { + if kind == builtinSkip { + // skip e.g., totalized metrics + continue + } + + if kind == builtinHistogram { + // skip unsupported data types + if m.Kind != metrics.KindFloat64Histogram { + otel.Handle(fmt.Errorf("expected histogram runtime/metrics: %s", mname)) + } continue } + description := fmt.Sprintf("%s from runtime/metrics", pattern) opts := []instrument.Option{ @@ -137,31 +144,41 @@ func (r *builtinRuntime) register(desc *builtinDescriptor) error { instrument.WithDescription(description), } var inst instrument.Asynchronous - if m.Cumulative { + switch kind { + case builtinCounter: switch m.Kind { case metrics.KindUint64: + // e.g., alloc bytes inst, err = r.meter.AsyncInt64().Counter(mname, opts...) case metrics.KindFloat64: + // e.g., cpu time (1.20) inst, err = r.meter.AsyncFloat64().Counter(mname, opts...) - case metrics.KindFloat64Histogram: - // Not implemented Histogram[float64]. - continue } - } else { + case builtinUpDownCounter: switch m.Kind { case metrics.KindUint64: + // e.g., memory size inst, err = r.meter.AsyncInt64().UpDownCounter(mname, opts...) case metrics.KindFloat64: - // Note: this has never been used. + // not used through 1.20 + inst, err = r.meter.AsyncFloat64().UpDownCounter(mname, opts...) + } + case builtinGauge: + switch m.Kind { + case metrics.KindUint64: + inst, err = r.meter.AsyncInt64().Gauge(mname, opts...) + case metrics.KindFloat64: + // not used through 1.20 inst, err = r.meter.AsyncFloat64().Gauge(mname, opts...) - case metrics.KindFloat64Histogram: - // Not implemented GaugeHistogram[float64]. - continue } } if err != nil { return err } + if inst == nil { + otel.Handle(fmt.Errorf("unexpected runtime/metrics %v: %s", kind, mname)) + continue + } samp := metrics.Sample{ Name: m.Name, diff --git a/lightstep/instrumentation/runtime/builtin_118.go b/lightstep/instrumentation/runtime/builtin_118.go deleted file mode 100644 index 2e5e9858..00000000 --- a/lightstep/instrumentation/runtime/builtin_118.go +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -//go:build go1.18 && !go1.19 - -package runtime - -var expectRuntimeMetrics = map[string]int{ - "gc.cycles": 2, - "gc.heap.allocs": 1, - "gc.heap.allocs.objects": 1, - "gc.heap.frees": 1, - "gc.heap.frees.objects": 1, - "gc.heap.goal": 1, - "gc.heap.objects": 1, - "gc.heap.tiny.allocs": 1, - "memory.usage": 13, - "sched.goroutines": 1, -} diff --git a/lightstep/instrumentation/runtime/builtin_119.go b/lightstep/instrumentation/runtime/builtin_119.go deleted file mode 100644 index e5453be6..00000000 --- a/lightstep/instrumentation/runtime/builtin_119.go +++ /dev/null @@ -1,55 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -//go:build go1.19 && !go1.20 - -package runtime - -import "go.opentelemetry.io/otel/attribute" - -func expectRuntimeMetrics() *builtinDescriptor { - bd := newBuiltinDescriptor() - bd.singleCounter("/cgo/go-to-c-calls:calls") - bd.classesCounter("/gc/cycles/*:gc-cycles", - attribute.NewSet(classKey.String("automatic")), - attribute.NewSet(classKey.String("forced")), - ) - bd.objectBytesCounter("/gc/heap/allocs:*") - bd.objectBytesCounter("/gc/heap/frees:*") - - bd.singleGauge("/gc/heap/goal:bytes") - - bd.singleUpDownCounter("/gc/heap/objects:objects") - - bd.singleCounter("/gc/heap/tiny/allocs:objects") - bd.singleGauge("/gc/limiter/last-enabled:gc-cycle") - bd.singleGauge("/gc/stack/starting-size:bytes") - bd.classesUpDownCounter("memory.usage", - attribute.NewSet(classKey.String("heap"), subclassKey.String("free")), - attribute.NewSet(classKey.String("heap"), subclassKey.String("objects")), - attribute.NewSet(classKey.String("heap"), subclassKey.String("released")), - attribute.NewSet(classKey.String("heap"), subclassKey.String("stacks")), - attribute.NewSet(classKey.String("heap"), subclassKey.String("unused")), - attribute.NewSet(classKey.String("metadata"), subclassKey.String("mcache"), subsubclassKey.String("free")), - attribute.NewSet(classKey.String("metadata"), subclassKey.String("mcache"), subsubclassKey.String("inuse")), - attribute.NewSet(classKey.String("metadata"), subclassKey.String("mspan"), subsubclassKey.String("free")), - attribute.NewSet(classKey.String("metadata"), subclassKey.String("mspan"), subsubclassKey.String("inuse")), - attribute.NewSet(classKey.String("metadata"), subclassKey.String("other")), - attribute.NewSet(classKey.String("os-stacks")), - attribute.NewSet(classKey.String("other")), - ) - bd.singleGauge("sched.gomaxprocs") - bd.singleUpDownCounter("sched.goroutines") - return bd -} diff --git a/lightstep/instrumentation/runtime/builtin_120.go b/lightstep/instrumentation/runtime/builtin_120.go deleted file mode 100644 index 31f1751b..00000000 --- a/lightstep/instrumentation/runtime/builtin_120.go +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -//go:build go1.20 - -package runtime - -var expectRuntimeMetrics = map[string]int{ - "cgo.go-to-c-calls": 1, - "cpu.time": 8, - "gc.cycles": 2, - "gc.heap.allocs": 1, - "gc.heap.allocs.objects": 1, - "gc.heap.frees": 1, - "gc.heap.frees.objects": 1, - "gc.heap.goal": 1, - "gc.heap.objects": 1, - "gc.heap.tiny.allocs": 1, - "gc.limiter.last-enabled": 1, - "gc.stack.starting-size": 1, - "memory.usage": 13, - "sched.gomaxprocs": 1, - "sched.goroutines": 1, -} diff --git a/lightstep/instrumentation/runtime/builtin_test.go b/lightstep/instrumentation/runtime/builtin_test.go index 6761acaf..ee53db25 100644 --- a/lightstep/instrumentation/runtime/builtin_test.go +++ b/lightstep/instrumentation/runtime/builtin_test.go @@ -28,22 +28,38 @@ import ( "go.opentelemetry.io/otel/sdk/metric/metricdata" ) -// TestMetricTranslation1 validates the translation logic using -// synthetic metric names and values. +const ( + classKey = attribute.Key("class") + subclassKey = attribute.Key("class2") + subsubclassKey = attribute.Key("class3") +) + func TestMetricTranslation1(t *testing.T) { testMetricTranslation(t, makeTestCase1) } -// TestMetricTranslation2 is a more complex test than the first. func TestMetricTranslation2(t *testing.T) { testMetricTranslation(t, makeTestCase2) } -func makeAllInts() []metrics.Value { +func TestMetricTranslationBuiltin(t *testing.T) { + testMetricTranslation(t, makeTestCaseBuiltin) +} + +// makeAllInts retrieves real metric.Values. We use real +// runtime/metrics values b/c these can't be constructed outside the +// library. +// +// Note that all current metrics through go-1.20 are either histogram +// or integer valued, so although Float64 values are supported in +// theory, they are not tested _and can't be tested because the +// library never produces them_. +func makeAllInts() (allInts, allFloats []metrics.Value) { // Note: the library provides no way to generate values, so use the // builtin library to get some. Since we can't generate a Float64 value // we can't even test the Gauge logic in this package. ints := map[metrics.Value]bool{} + floats := map[metrics.Value]bool{} real := metrics.All() realSamples := make([]metrics.Sample, len(real)) @@ -55,23 +71,25 @@ func makeAllInts() []metrics.Value { switch real[i].Kind { case metrics.KindUint64: ints[rs.Value] = true + case metrics.KindFloat64: + floats[rs.Value] = true default: - // Histograms and Floats are not tested. - // The 1.19 runtime generates no Floats and - // exports no test constructors. + // Histograms are not tested. } } - - var allInts []metrics.Value - for iv := range ints { allInts = append(allInts, iv) } - return allInts + for fv := range floats { + allFloats = append(allFloats, fv) + } + return allInts, allFloats } +// testMapping implements a synthetic metrics reader. type testMapping map[string]metrics.Value +// read is like metrics.Read w/ synthetic data. func (m testMapping) read(samples []metrics.Sample) { for i := range samples { v, ok := m[samples[i].Name] @@ -83,16 +101,43 @@ func (m testMapping) read(samples []metrics.Sample) { } } -type testExpectation map[string]testExpectMetric +// readFrom turns real runtime/metrics data into a test expectation. +func (m testMapping) readFrom(af allFunc, rf readFunc) { + all := af() + samples := make([]metrics.Sample, len(all)) + for i := range all { + switch all[i].Kind { + case metrics.KindUint64, metrics.KindFloat64: + default: + continue + } + samples[i].Name = all[i].Name + } + rf(samples) + for i := range samples { + m[samples[i].Name] = samples[i].Value + } +} + +// testExpectation allows validating the behavior using +// hand-constructed test cases. +type testExpectation map[string]*testExpectMetric +// testExpectMetric sets a test expectation consisting of name, unit, +// and known cardinal values. type testExpectMetric struct { desc string unit unit.Unit + kind builtinKind vals map[attribute.Set]metrics.Value } -func makeTestCase1() (allFunc, readFunc, *builtinDescriptor, testExpectation) { - allInts := makeAllInts() +// makeTestCase1 covers the following cases: +// - single counter, updowncounter, gauge +// - bytes/objects counter +// - classes counter (gc-cycles) +func makeTestCase1(t *testing.T) (allFunc, readFunc, *builtinDescriptor, testExpectation) { + allInts, _ := makeAllInts() af := func() []metrics.Description { return []metrics.Description{ @@ -111,6 +156,11 @@ func makeTestCase1() (allFunc, readFunc, *builtinDescriptor, testExpectation) { Kind: metrics.KindUint64, Cumulative: true, }, + { + Name: "/cpu/temp:C", + Kind: metrics.KindUint64, // TODO: post Go-1.20 make this Float64 + Cumulative: false, + }, { Name: "/process/count:bytes", Kind: metrics.KindUint64, @@ -159,50 +209,55 @@ func makeTestCase1() (allFunc, readFunc, *builtinDescriptor, testExpectation) { "/waste/cycles/pond:gc-cycles": allInts[7], "/waste/cycles/puddle:gc-cycles": allInts[8], "/waste/cycles/total:gc-cycles": allInts[9], + + // Note: this would be a nice float test, but 1.19 doesn't have + // any, so we wait for this repo's min Go version to support a + // metrics.KindFloat64 value for testing. + "/cpu/temp:C": allInts[10], } bd := newBuiltinDescriptor() bd.singleCounter("/cntr/things:things") bd.singleUpDownCounter("/updowncntr/things:things") + bd.singleGauge("/cpu/temp:C") bd.objectBytesCounter("/process/count:*") - bd.classesCounter("/waste/cycles/*:gc-cycles", - attribute.NewSet(classKey.String("ocean")), - attribute.NewSet(classKey.String("sea")), - attribute.NewSet(classKey.String("lake")), - attribute.NewSet(classKey.String("pond")), - attribute.NewSet(classKey.String("puddle")), - ) + bd.classesCounter("/waste/cycles/*:gc-cycles") return af, mapping.read, bd, testExpectation{ - "cntr.things": testExpectMetric{ + "cntr.things": &testExpectMetric{ unit: "{things}", desc: "/cntr/things:things from runtime/metrics", + kind: builtinCounter, vals: map[attribute.Set]metrics.Value{ emptySet: allInts[0], }, }, - "updowncntr.things": testExpectMetric{ + "updowncntr.things": &testExpectMetric{ unit: "{things}", desc: "/updowncntr/things:things from runtime/metrics", + kind: builtinUpDownCounter, vals: map[attribute.Set]metrics.Value{ emptySet: allInts[1], }, }, - "process.count.objects": testExpectMetric{ + "process.count.objects": &testExpectMetric{ unit: "", desc: "/process/count:objects from runtime/metrics", + kind: builtinCounter, vals: map[attribute.Set]metrics.Value{ emptySet: allInts[2], }, }, - "process.count": testExpectMetric{ + "process.count": &testExpectMetric{ unit: unit.Bytes, + kind: builtinCounter, desc: "/process/count:bytes from runtime/metrics", vals: map[attribute.Set]metrics.Value{ emptySet: allInts[3], }, }, - "waste.cycles": testExpectMetric{ + "waste.cycles": &testExpectMetric{ unit: "{gc-cycles}", desc: "/waste/cycles/*:gc-cycles from runtime/metrics", + kind: builtinCounter, vals: map[attribute.Set]metrics.Value{ attribute.NewSet(classKey.String("ocean")): allInts[4], attribute.NewSet(classKey.String("sea")): allInts[5], @@ -211,65 +266,95 @@ func makeTestCase1() (allFunc, readFunc, *builtinDescriptor, testExpectation) { attribute.NewSet(classKey.String("puddle")): allInts[8], }, }, + "cpu.temp": &testExpectMetric{ + // This is made-up. We don't recognize this + // unit, code defaults to pseudo-units. + unit: "{C}", + kind: builtinGauge, + desc: "/cpu/temp:C from runtime/metrics", + vals: map[attribute.Set]metrics.Value{ + emptySet: allInts[10], + }, + }, } } -func makeTestCase2() (allFunc, readFunc, *builtinDescriptor, testExpectation) { - allInts := makeAllInts() +// makeTestCase2 covers the following cases: +// - classes counter (bytes) +// - classes counter (cpu-seconds) +func makeTestCase2(t *testing.T) (allFunc, readFunc, *builtinDescriptor, testExpectation) { + allInts, _ := makeAllInts() af := func() []metrics.Description { return []metrics.Description{ + // classes (bytes) { Name: "/objsize/classes/presos:bytes", Kind: metrics.KindUint64, - Cumulative: true, + Cumulative: false, }, { Name: "/objsize/classes/sheets:bytes", Kind: metrics.KindUint64, - Cumulative: true, + Cumulative: false, }, { Name: "/objsize/classes/docs/word:bytes", Kind: metrics.KindUint64, - Cumulative: true, + Cumulative: false, }, { Name: "/objsize/classes/docs/pdf:bytes", Kind: metrics.KindUint64, - Cumulative: true, + Cumulative: false, }, { Name: "/objsize/classes/docs/total:bytes", Kind: metrics.KindUint64, - Cumulative: true, + Cumulative: false, }, { Name: "/objsize/classes/total:bytes", Kind: metrics.KindUint64, + Cumulative: false, + }, + // classes (time) + { + Name: "/socchip/classes/pru:cpu-seconds", + Kind: metrics.KindUint64, + Cumulative: true, + }, + { + Name: "/socchip/classes/dsp:cpu-seconds", + Kind: metrics.KindUint64, + Cumulative: true, + }, + { + Name: "/socchip/classes/total:cpu-seconds", + Kind: metrics.KindUint64, Cumulative: true, }, } } mapping := testMapping{ - "/objsize/classes/presos:bytes": allInts[0], - "/objsize/classes/sheets:bytes": allInts[1], - "/objsize/classes/docs/word:bytes": allInts[2], - "/objsize/classes/docs/pdf:bytes": allInts[3], - "/objsize/classes/docs/total:bytes": allInts[4], - "/objsize/classes/total:bytes": allInts[5], + "/objsize/classes/presos:bytes": allInts[0], + "/objsize/classes/sheets:bytes": allInts[1], + "/objsize/classes/docs/word:bytes": allInts[2], + "/objsize/classes/docs/pdf:bytes": allInts[3], + "/objsize/classes/docs/total:bytes": allInts[4], + "/objsize/classes/total:bytes": allInts[5], + "/socchip/classes/pru:cpu-seconds": allInts[6], + "/socchip/classes/dsp:cpu-seconds": allInts[7], + "/socchip/classes/total:cpu-seconds": allInts[8], } bd := newBuiltinDescriptor() - bd.classesUpDownCounter("/objsize/classes/*:bytes", - attribute.NewSet(classKey.String("presos")), - attribute.NewSet(classKey.String("sheets")), - attribute.NewSet(classKey.String("docs"), subclassKey.String("word")), - attribute.NewSet(classKey.String("docs"), subclassKey.String("pdf")), - ) + bd.classesUpDownCounter("/objsize/classes/*:bytes") + bd.classesCounter("/socchip/classes/*:cpu-seconds") return af, mapping.read, bd, testExpectation{ - "objsize.usage": testExpectMetric{ + "objsize.usage": &testExpectMetric{ unit: unit.Bytes, desc: "/objsize/classes/*:bytes from runtime/metrics", + kind: builtinUpDownCounter, vals: map[attribute.Set]metrics.Value{ attribute.NewSet(classKey.String("presos")): allInts[0], attribute.NewSet(classKey.String("sheets")): allInts[1], @@ -277,14 +362,58 @@ func makeTestCase2() (allFunc, readFunc, *builtinDescriptor, testExpectation) { attribute.NewSet(classKey.String("docs"), subclassKey.String("pdf")): allInts[3], }, }, + "socchip.time": &testExpectMetric{ + unit: "{cpu-seconds}", + desc: "/socchip/classes/*:cpu-seconds from runtime/metrics", + kind: builtinCounter, + vals: map[attribute.Set]metrics.Value{ + attribute.NewSet(classKey.String("pru")): allInts[6], + attribute.NewSet(classKey.String("dsp")): allInts[7], + }, + }, } } -func testMetricTranslation(t *testing.T, makeTestCase func() (allFunc, readFunc, *builtinDescriptor, testExpectation)) { +// makeTestCaseBuiltin fabricates a test expectation from the +// version-specific portion of the descriptor, synthesizes values and +// checks the result for a match. +func makeTestCaseBuiltin(t *testing.T) (allFunc, readFunc, *builtinDescriptor, testExpectation) { + testMap := testMapping{} + testMap.readFrom(metrics.All, metrics.Read) + + realDesc := expectRuntimeMetrics() + + expect := testExpectation{} + + for goname, realval := range testMap { + mname, munit, descPat, attrs, kind, err := realDesc.findMatch(goname) + if err != nil || mname == "" { + continue // e.g., async histogram data, totalized metrics + } + noprefix := mname[len(namePrefix)+1:] + te, ok := expect[noprefix] + if !ok { + te = &testExpectMetric{ + desc: fmt.Sprint(descPat, " from runtime/metrics"), + unit: unit.Unit(munit), + kind: kind, + vals: map[attribute.Set]metrics.Value{}, + } + expect[noprefix] = te + } + te.vals[attribute.NewSet(attrs...)] = realval + } + + return metrics.All, testMap.read, realDesc, expect +} + +// testMetricTranslation registers the metrics allFunc and readFunc +// functions using the descriptor and validates the test expectation. +func testMetricTranslation(t *testing.T, makeTestCase func(t *testing.T) (allFunc, readFunc, *builtinDescriptor, testExpectation)) { reader := metric.NewManualReader() provider := metric.NewMeterProvider(metric.WithReader(reader)) - af, rf, desc, mapping := makeTestCase() + af, rf, desc, expectation := makeTestCase(t) br := newBuiltinRuntime(provider.Meter("test"), af, rf) err := br.register(desc) require.NoError(t, err) @@ -296,31 +425,95 @@ func testMetricTranslation(t *testing.T, makeTestCase func() (allFunc, readFunc, require.Equal(t, 1, len(data.ScopeMetrics)) require.Equal(t, "test", data.ScopeMetrics[0].Scope.Name) + + // Compare the name sets, to make the test output readable. + haveNames := map[string]bool{} + expectNames := map[string]bool{} + for _, m := range data.ScopeMetrics[0].Metrics { - fmt.Println(m) + require.True(t, strings.HasPrefix(m.Name, namePrefix)) + haveNames[m.Name[len(namePrefix)+1:]] = true + } + for n := range expectation { + expectNames[n] = true } - require.Equal(t, len(mapping), len(data.ScopeMetrics[0].Metrics), "metrics count: %v", data.ScopeMetrics[0].Metrics) + require.Equal(t, expectNames, haveNames) for _, inst := range data.ScopeMetrics[0].Metrics { - // Test the special cases are present always: + // Test name, description, and unit. require.True(t, strings.HasPrefix(inst.Name, namePrefix+"."), "%s", inst.Name) - name := inst.Name[len(namePrefix)+1:] - // Note: only int64 values are tested, as we have no - // way to generate float64 values w/o an - // runtime/metrics supporting a metric to generate - // these values. - exm := mapping[name] + name := inst.Name[len(namePrefix)+1:] + exm := expectation[name] require.Equal(t, exm.desc, inst.Description) require.Equal(t, exm.unit, inst.Unit) - require.Equal(t, len(exm.vals), len(inst.Data.(metricdata.Sum[int64]).DataPoints), "points count: %v != %v", inst.Data.(metricdata.Sum[int64]).DataPoints, exm.vals) + // The counter and gauge branches do make the same + // checks, just have to be split b/c the underlying + // types are different. + switch exm.kind { + case builtinCounter, builtinUpDownCounter: + // Handle both int/float cases. Note: If we could write + // in-line generic code, this could be less repetitive. + if _, isInt := inst.Data.(metricdata.Sum[int64]); isInt { + // Integer + + _, isSum := inst.Data.(metricdata.Sum[int64]) + // Expect a sum data point w/ correct monotonicity. + require.True(t, isSum, "%v", exm) + require.Equal(t, exm.kind == builtinCounter, inst.Data.(metricdata.Sum[int64]).IsMonotonic, "%v", exm) + require.Equal(t, metricdata.CumulativeTemporality, inst.Data.(metricdata.Sum[int64]).Temporality, "%v", exm) + + // Check expected values. + for _, point := range inst.Data.(metricdata.Sum[int64]).DataPoints { + lookup, ok := exm.vals[point.Attributes] + require.True(t, ok, "lookup failed: %v: %v", exm.vals, point.Attributes) + require.Equal(t, lookup.Uint64(), uint64(point.Value)) + } + } else { + // Floating point - for _, point := range inst.Data.(metricdata.Sum[int64]).DataPoints { - lookup, ok := exm.vals[point.Attributes] - require.True(t, ok, "lookup failed: %v", exm.vals, point.Attributes) - require.Equal(t, lookup.Uint64(), uint64(point.Value)) + _, isSum := inst.Data.(metricdata.Sum[float64]) + // Expect a sum data point w/ correct monotonicity. + require.True(t, isSum, "%v", exm) + require.Equal(t, exm.kind == builtinCounter, inst.Data.(metricdata.Sum[float64]).IsMonotonic, "%v", exm) + require.Equal(t, metricdata.CumulativeTemporality, inst.Data.(metricdata.Sum[float64]).Temporality, "%v", exm) + + // Check expected values. + for _, point := range inst.Data.(metricdata.Sum[float64]).DataPoints { + lookup, ok := exm.vals[point.Attributes] + require.True(t, ok, "lookup failed: %v: %v", exm.vals, point.Attributes) + require.Equal(t, lookup.Float64(), float64(point.Value)) + } + } + case builtinGauge: + if _, isInt := inst.Data.(metricdata.Gauge[int64]); isInt { + // Integer + _, isGauge := inst.Data.(metricdata.Gauge[int64]) + require.True(t, isGauge, "%v", exm) + + // Check expected values. + for _, point := range inst.Data.(metricdata.Gauge[int64]).DataPoints { + lookup, ok := exm.vals[point.Attributes] + require.True(t, ok, "lookup failed: %v: %v", exm.vals, point.Attributes) + require.Equal(t, lookup.Uint64(), uint64(point.Value)) + } + } else { + // Floating point + _, isGauge := inst.Data.(metricdata.Gauge[float64]) + require.True(t, isGauge, "%v", exm) + + // Check expected values. + for _, point := range inst.Data.(metricdata.Gauge[float64]).DataPoints { + lookup, ok := exm.vals[point.Attributes] + require.True(t, ok, "lookup failed: %v: %v", exm.vals, point.Attributes) + require.Equal(t, lookup.Float64(), float64(point.Value)) + } + } + default: + t.Errorf("unexpected runtimes/metric test case: %v", exm) + continue } } } diff --git a/lightstep/instrumentation/runtime/defs.go b/lightstep/instrumentation/runtime/defs.go new file mode 100644 index 00000000..3fc01848 --- /dev/null +++ b/lightstep/instrumentation/runtime/defs.go @@ -0,0 +1,40 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build go1.20 + +package runtime + +func expectRuntimeMetrics() *builtinDescriptor { + bd := newBuiltinDescriptor() + bd.classesCounter("/cpu/classes/*:cpu-seconds") + bd.classesCounter("/gc/cycles/*:gc-cycles") + bd.classesUpDownCounter("/memory/classes/*:bytes") + bd.ignoreHistogram("/gc/heap/allocs-by-size:bytes") + bd.ignoreHistogram("/gc/heap/frees-by-size:bytes") + bd.ignoreHistogram("/gc/pauses:seconds") + bd.ignoreHistogram("/sched/latencies:seconds") + bd.objectBytesCounter("/gc/heap/allocs:*") + bd.objectBytesCounter("/gc/heap/frees:*") + bd.singleCounter("/cgo/go-to-c-calls:calls") + bd.singleCounter("/gc/heap/tiny/allocs:objects") + bd.singleCounter("/sync/mutex/wait/total:seconds") + bd.singleGauge("/gc/heap/goal:bytes") + bd.singleGauge("/gc/limiter/last-enabled:gc-cycle") + bd.singleGauge("/gc/stack/starting-size:bytes") + bd.singleGauge("/sched/gomaxprocs:threads") + bd.singleUpDownCounter("/gc/heap/objects:objects") + bd.singleUpDownCounter("/sched/goroutines:goroutines") + return bd +} diff --git a/lightstep/instrumentation/runtime/descriptor.go b/lightstep/instrumentation/runtime/descriptor.go index bb23c653..6749e4a2 100644 --- a/lightstep/instrumentation/runtime/descriptor.go +++ b/lightstep/instrumentation/runtime/descriptor.go @@ -20,33 +20,39 @@ var ( ) const ( - builtinCounter builtinKind = iota + builtinSkip builtinKind = iota + builtinCounter builtinObjectBytesCounter builtinUpDownCounter builtinGauge + builtinHistogram ) type builtinMetricFamily struct { pattern string matches int kind builtinKind - attrs []attribute.Set } type builtinDescriptor struct { families []*builtinMetricFamily } -func newBuiltinDescriptor() *builtinDescriptor { - return &builtinDescriptor{} -} - -func (bd *builtinDescriptor) add(pattern string, kind builtinKind, attrs ...attribute.Set) { - bd.families = append(bd.families, &builtinMetricFamily{ - pattern: pattern, - kind: kind, - attrs: attrs, - }) +func (k builtinKind) String() string { + switch k { + case builtinCounter: + return "counter" + case builtinObjectBytesCounter: + return "object/bytes counter" + case builtinUpDownCounter: + return "up/down counter" + case builtinGauge: + return "gauge" + case builtinHistogram: + return "histogram" + case builtinSkip: + } + return "skipped" } func toOTelNameAndStatedUnit(nameAndUnit string) (on, un string) { @@ -58,31 +64,35 @@ func toOTelName(name string) string { return namePrefix + strings.ReplaceAll(name, "/", ".") } +// attributeName returns "class", "class2", "class3", ... func attributeName(order int) string { - base := "class" - for ; order > 0; order-- { - - base = "sub" + base + if order == 0 { + return "class" } - return base + return fmt.Sprintf("class%d", order+1) +} + +func newBuiltinDescriptor() *builtinDescriptor { + return &builtinDescriptor{} +} + +func (bd *builtinDescriptor) add(pattern string, kind builtinKind) { + bd.families = append(bd.families, &builtinMetricFamily{ + pattern: pattern, + kind: kind, + }) } func (bd *builtinDescriptor) singleCounter(pattern string) { bd.add(pattern, builtinCounter) } -func (bd *builtinDescriptor) classesCounter(pattern string, attrs ...attribute.Set) { - if len(attrs) < 2 { - panic(fmt.Sprintf("must have >1 attrs: %s", attrs)) - } - bd.add(pattern, builtinCounter, attrs...) +func (bd *builtinDescriptor) classesCounter(pattern string) { + bd.add(pattern, builtinCounter) } -func (bd *builtinDescriptor) classesUpDownCounter(pattern string, attrs ...attribute.Set) { - if len(attrs) < 2 { - panic(fmt.Sprintf("must have >1 attrs: %s", attrs)) - } - bd.add(pattern, builtinUpDownCounter, attrs...) +func (bd *builtinDescriptor) classesUpDownCounter(pattern string) { + bd.add(pattern, builtinUpDownCounter) } func (bd *builtinDescriptor) objectBytesCounter(pattern string) { @@ -97,22 +107,32 @@ func (bd *builtinDescriptor) singleGauge(pattern string) { bd.add(pattern, builtinGauge) } -func (bd *builtinDescriptor) findMatch(goname string) (mname, munit, descPattern string, attrs []attribute.KeyValue, _ error) { +func (bd *builtinDescriptor) ignoreHistogram(pattern string) { + bd.add(pattern, builtinHistogram) +} + +func (bd *builtinDescriptor) findMatch(goname string) (mname, munit, descPattern string, attrs []attribute.KeyValue, kind builtinKind, _ error) { fam, err := bd.findFamily(goname) if err != nil { - return "", "", "", nil, err + return "", "", "", nil, builtinSkip, err } fam.matches++ + kind = fam.kind + // Set the name, unit and pattern. if wildCnt := strings.Count(fam.pattern, "*"); wildCnt == 0 { mname, munit = toOTelNameAndStatedUnit(goname) descPattern = goname } else if strings.HasSuffix(fam.pattern, ":*") { - // Special case for bytes/objects w/ same prefix. + // Special case for bytes/objects w/ same prefix: two + // counters, different names. One has "By" (UCUM for + // "bytes") units and no suffix. One has no units and + // a ".objects" suffix. (In Prometheus, this becomes + // _objects and _bytes as you would expect.) mname, munit = toOTelNameAndStatedUnit(goname) descPattern = goname - + kind = builtinCounter if munit == "objects" { mname += "." + munit munit = "" @@ -129,7 +149,7 @@ func (bd *builtinDescriptor) findMatch(goname string) (mname, munit, descPattern } // Ignore subtotals if splitVals[len(splitVals)-1] == "total" { - return "", "", "", nil, nil + return "", "", "", nil, builtinSkip, nil } descPattern = fam.pattern } @@ -166,7 +186,9 @@ func (bd *builtinDescriptor) findMatch(goname string) (mname, munit, descPattern } } - return mname, munit, descPattern, attrs, nil + // Note: we may be returning the special builtinObjectBytes. + // if it was not fixed for patterns w/ trailing wildcard (see above). + return mname, munit, descPattern, attrs, kind, err } func (bd *builtinDescriptor) findFamily(name string) (family *builtinMetricFamily, _ error) { From 06ef13fe77800250143d419eef66bde3e7988810 Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Tue, 20 Dec 2022 12:19:48 -0800 Subject: [PATCH 6/9] docs --- lightstep/instrumentation/runtime/builtin.go | 9 +++ lightstep/instrumentation/runtime/defs.go | 2 - lightstep/instrumentation/runtime/doc.go | 65 ++++++-------------- 3 files changed, 28 insertions(+), 48 deletions(-) diff --git a/lightstep/instrumentation/runtime/builtin.go b/lightstep/instrumentation/runtime/builtin.go index e63dee72..ba6c1fec 100644 --- a/lightstep/instrumentation/runtime/builtin.go +++ b/lightstep/instrumentation/runtime/builtin.go @@ -27,6 +27,7 @@ import ( "go.opentelemetry.io/otel/metric/unit" ) +// namePrefix is prefixed onto OTel instrument names. const namePrefix = "process.runtime.go" // LibraryName is the value of instrumentation.Library.Name. @@ -84,19 +85,25 @@ func Start(opts ...Option) error { return r.register(expectRuntimeMetrics()) } +// allFunc is the function signature of metrics.All() type allFunc = func() []metrics.Description + +// allFunc is the function signature of metrics.Read() type readFunc = func([]metrics.Sample) +// builtinRuntime instruments all supported kinds of runtime/metrics. type builtinRuntime struct { meter metric.Meter allFunc allFunc readFunc readFunc } +// int64Observer is any async int64 instrument. type int64Observer interface { Observe(ctx context.Context, x int64, attrs ...attribute.KeyValue) } +// float64Observer is any async float64 instrument. type float64Observer interface { Observe(ctx context.Context, x float64, attrs ...attribute.KeyValue) } @@ -109,6 +116,8 @@ func newBuiltinRuntime(meter metric.Meter, af allFunc, rf readFunc) *builtinRunt } } +// register parses each name and registers metric instruments for all +// the recognized instruments. func (r *builtinRuntime) register(desc *builtinDescriptor) error { all := r.allFunc() diff --git a/lightstep/instrumentation/runtime/defs.go b/lightstep/instrumentation/runtime/defs.go index 3fc01848..5767f05f 100644 --- a/lightstep/instrumentation/runtime/defs.go +++ b/lightstep/instrumentation/runtime/defs.go @@ -12,8 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -//go:build go1.20 - package runtime func expectRuntimeMetrics() *builtinDescriptor { diff --git a/lightstep/instrumentation/runtime/doc.go b/lightstep/instrumentation/runtime/doc.go index c8d13e04..fea65e64 100644 --- a/lightstep/instrumentation/runtime/doc.go +++ b/lightstep/instrumentation/runtime/doc.go @@ -12,52 +12,25 @@ // See the License for the specific language governing permissions and // limitations under the License. -// package runtime geneartes metrics run the Golang runtime/metrics package. -// -// There are two special policies that are used to translate these -// metrics into the OpenTelemetry model. -// -// 1. The runtime/metrics name is split into its name and unit part; -// when there are two metrics with the same name and different -// units, the only known case is where "objects" and "bytes" are -// present. In this case, the outputs are a unitless metric (with -// suffix, e.g., ending `gc.heap.allocs.objects`) and a unitful -// metric with no suffix (e.g., ending `gc.heap.allocs` having -// bytes units). -// 2. When there are >= 2 metrics with the same prefix and one -// matching `prefix.total`, the total is skipped and the other -// members are assembled into a single Counter or UpDownCounter -// metric with multiple attribute values. The supported cases -// are for `class` and `cycle` attributes. -// -// The following metrics are generated in go-1.19. -// -// Name Unit Instrument -// ------------------------------------------------------------------------------------ -// process.runtime.go.cgo.go-to-c-calls {calls} Counter[int64] -// process.runtime.go.cpu.time{class=...} {cpu-seconds} Counter[float64] -// process.runtime.go.gc.cycles{cycle=forced,automatic} {gc-cycles} Counter[int64] -// process.runtime.go.gc.heap.allocs bytes (*) Counter[int64] -// process.runtime.go.gc.heap.allocs.objects {objects} (*) Counter[int64] -// process.runtime.go.gc.heap.allocs-by-size bytes Histogram[float64] (**) -// process.runtime.go.gc.heap.frees bytes (*) Counter[int64] -// process.runtime.go.gc.heap.frees.objects {objects} (*) Counter[int64] -// process.runtime.go.gc.heap.frees-by-size bytes Histogram[float64] (**) -// process.runtime.go.gc.heap.goal bytes UpDownCounter[int64] -// process.runtime.go.gc.heap.objects {objects} UpDownCounter[int64] -// process.runtime.go.gc.heap.tiny.allocs {objects} Counter[int64] -// process.runtime.go.gc.limiter.last-enabled {gc-cycle} UpDownCounter[int64] -// process.runtime.go.gc.pauses seconds Histogram[float64] (**) -// process.runtime.go.gc.stack.starting-size bytes UpDownCounter[int64] -// process.runtime.go.memory.usage{class=...} bytes UpDownCounter[int64] -// process.runtime.go.sched.gomaxprocs {threads} UpDownCounter[int64] -// process.runtime.go.sched.goroutines {goroutines} UpDownCounter[int64] -// process.runtime.go.sched.latencies seconds GaugeHistogram[float64] (**) -// -// (*) Empty unit strings are cases where runtime/metric produces -// duplicate names ignoring the unit string (see policy #1). -// (**) Histograms are not currently implemented, see the related -// issues for an explanation: +// package runtime generates metrics from the Golang runtime/metrics package. +// +// There are several conventions used to translate these metrics into +// the OpenTelemetry model. Builtin metrics are defined in terms of +// the expected OpenTelemetry instrument kind in defs.go. +// +// 1. Single Counter, UpDownCounter, and Gauge instruments. No +// wildcards are used. E.g., /cgo/go-to-c-calls:calls becomes +// process.runtime.go.cgo.go-to-c-calls with unit {calls}. +// 2. Objects/Bytes Counter. There are two runtime/metrics with the +// same name and different units. The objects counter has a suffix, +// the bytes counter has a unit, to disambiguate. +// 3. Multi-dimensional Counter/UpDownCounter (generally), ignore any +// "total" elements to avoid double-counting. 4. Multi-dimensional +// Counter/UpDownCounter (named ".classes"), map to "usage" for bytes +// and "time" for cpu-seconds. +// +// Histograms are not currently implemented, see the related issues +// for an explanation: // https://github.com/open-telemetry/opentelemetry-specification/issues/2713 // https://github.com/open-telemetry/opentelemetry-specification/issues/2714 From e235888c435275728b573a6741ea0b87fc75db0e Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Tue, 20 Dec 2022 16:04:27 -0800 Subject: [PATCH 7/9] Update lightstep/instrumentation/runtime/builtin.go Co-authored-by: Kristina Pathak --- lightstep/instrumentation/runtime/builtin.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lightstep/instrumentation/runtime/builtin.go b/lightstep/instrumentation/runtime/builtin.go index ba6c1fec..37c30f04 100644 --- a/lightstep/instrumentation/runtime/builtin.go +++ b/lightstep/instrumentation/runtime/builtin.go @@ -88,7 +88,7 @@ func Start(opts ...Option) error { // allFunc is the function signature of metrics.All() type allFunc = func() []metrics.Description -// allFunc is the function signature of metrics.Read() +// readFunc is the function signature of metrics.Read() type readFunc = func([]metrics.Sample) // builtinRuntime instruments all supported kinds of runtime/metrics. From 53d7fd57411684f4a3571a345901268864f032b9 Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Wed, 21 Dec 2022 10:27:10 -0800 Subject: [PATCH 8/9] update docs --- lightstep/instrumentation/go.mod | 2 +- .../instrumentation/runtime/descriptor.go | 2 - lightstep/instrumentation/runtime/doc.go | 55 +++++++++++++++++-- 3 files changed, 50 insertions(+), 9 deletions(-) diff --git a/lightstep/instrumentation/go.mod b/lightstep/instrumentation/go.mod index b081809d..f3cd2889 100644 --- a/lightstep/instrumentation/go.mod +++ b/lightstep/instrumentation/go.mod @@ -7,7 +7,6 @@ require ( github.com/stretchr/testify v1.8.1 go.opentelemetry.io/otel v1.11.2 go.opentelemetry.io/otel/metric v0.34.0 - go.opentelemetry.io/otel/sdk v1.11.2 go.opentelemetry.io/otel/sdk/metric v0.34.0 ) @@ -22,6 +21,7 @@ require ( github.com/tklauser/go-sysconf v0.3.10 // indirect github.com/tklauser/numcpus v0.4.0 // indirect github.com/yusufpapurcu/wmi v1.2.2 // indirect + go.opentelemetry.io/otel/sdk v1.11.2 // indirect go.opentelemetry.io/otel/trace v1.11.2 // indirect golang.org/x/sys v0.0.0-20220919091848-fb04ddd9f9c8 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/lightstep/instrumentation/runtime/descriptor.go b/lightstep/instrumentation/runtime/descriptor.go index 6749e4a2..f17a6b20 100644 --- a/lightstep/instrumentation/runtime/descriptor.go +++ b/lightstep/instrumentation/runtime/descriptor.go @@ -11,8 +11,6 @@ import ( type builtinKind int var ( - singleton = []attribute.Set{emptySet} - emptySet = attribute.NewSet() ErrUnmatchedBuiltin = fmt.Errorf("builtin unmatched") diff --git a/lightstep/instrumentation/runtime/doc.go b/lightstep/instrumentation/runtime/doc.go index fea65e64..be144bdb 100644 --- a/lightstep/instrumentation/runtime/doc.go +++ b/lightstep/instrumentation/runtime/doc.go @@ -19,15 +19,58 @@ // the expected OpenTelemetry instrument kind in defs.go. // // 1. Single Counter, UpDownCounter, and Gauge instruments. No -// wildcards are used. E.g., /cgo/go-to-c-calls:calls becomes -// process.runtime.go.cgo.go-to-c-calls with unit {calls}. +// wildcards are used. For example: +// +// /cgo/go-to-c-calls:calls +// +// becomes: +// +// process.runtime.go.cgo.go-to-c-calls (unit: {calls}) +// // 2. Objects/Bytes Counter. There are two runtime/metrics with the // same name and different units. The objects counter has a suffix, -// the bytes counter has a unit, to disambiguate. +// the bytes counter has a unit, to disambiguate. For example: +// +// /gc/heap/allocs:* +// +// becomes: +// +// process.runtime.go.gc.heap.allocs (unit: bytes) +// process.runtime.go.gc.heap.allocs.objects (unitless) +// // 3. Multi-dimensional Counter/UpDownCounter (generally), ignore any -// "total" elements to avoid double-counting. 4. Multi-dimensional -// Counter/UpDownCounter (named ".classes"), map to "usage" for bytes -// and "time" for cpu-seconds. +// "total" elements to avoid double-counting. For example: +// +// /gc/cycles/*:gc-cycles +// +// becomes: +// +// process.runtime.go.gc.cycles (unit: gc-cycles) +// +// with two attribute setes: +// +// class=automatic +// class=forced +// +// 4. Multi-dimensional Counter/UpDownCounter (named ".classes"), map +// to ".usage" for bytes and ".time" for cpu-seconds. For example: +// +// /cpu/classes/*:cpu-seconds +// +// becomes: +// +// process.runtime.go.cpu.time (unit: cpu-seconds) +// +// with multi-dimensional attributes: +// +// class=gc,class2=mark,class3=assist +// class=gc,class2=mark,class3=dedicated +// class=gc,class2=mark,class3=idle +// class=gc,class2=pause +// class=scavenge,class2=assist +// class=scavenge,class2=background +// class=idle +// class=user // // Histograms are not currently implemented, see the related issues // for an explanation: From 7fa756ad83e7106b243cad34d914c5e3cabbd693 Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Wed, 21 Dec 2022 10:34:40 -0800 Subject: [PATCH 9/9] lint license --- lightstep/instrumentation/runtime/builtin_test.go | 1 + lightstep/instrumentation/runtime/descriptor.go | 14 ++++++++++++++ 2 files changed, 15 insertions(+) diff --git a/lightstep/instrumentation/runtime/builtin_test.go b/lightstep/instrumentation/runtime/builtin_test.go index ee53db25..0da0517c 100644 --- a/lightstep/instrumentation/runtime/builtin_test.go +++ b/lightstep/instrumentation/runtime/builtin_test.go @@ -11,6 +11,7 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. + package runtime import ( diff --git a/lightstep/instrumentation/runtime/descriptor.go b/lightstep/instrumentation/runtime/descriptor.go index f17a6b20..647a4d25 100644 --- a/lightstep/instrumentation/runtime/descriptor.go +++ b/lightstep/instrumentation/runtime/descriptor.go @@ -1,3 +1,17 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package runtime import (