From a845ca641cda9ce28dacdc3810274c49b3fd325d Mon Sep 17 00:00:00 2001 From: Zach Reyes Date: Mon, 24 Jun 2024 20:05:05 -0400 Subject: [PATCH 01/13] Add instrument registry --- .../instrumentregistry/instrument_registry.go | 239 ++++++++++++ .../instrument_registry_test.go | 348 ++++++++++++++++++ 2 files changed, 587 insertions(+) create mode 100644 internal/instrumentregistry/instrument_registry.go create mode 100644 internal/instrumentregistry/instrument_registry_test.go diff --git a/internal/instrumentregistry/instrument_registry.go b/internal/instrumentregistry/instrument_registry.go new file mode 100644 index 000000000000..f2f454b3292c --- /dev/null +++ b/internal/instrumentregistry/instrument_registry.go @@ -0,0 +1,239 @@ +/* + * + * Copyright 2024 gRPC 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 instrumentregistry is a instrument registry that gRPC components can +// use to register instruments (metrics). +package instrumentregistry + +import "log" + +// InstrumentDescriptor is a data of a registered instrument (metric). +type InstrumentDescriptor struct { + // Name is the name of this metric. + Name string + // Description is the description of this metric. + Description string + // Unit is the unit of this metric. + Unit string + // Labels are the required label keys for this metric. + Labels []string + // OptionalLabels are the optional label keys for this + // metric. + OptionalLabels []string + // Default is whether this metric is on by default. + Default bool +} + +// All of these globals are written to at initialization time only, and are read +// only after initialization. + +// Int64CountInsts is information about registered int 64 count instruments in +// order of registration. +var Int64CountInsts []InstrumentDescriptor + +// Float64CountInsts is information about registered float 64 count instruments +// in order of registration. +var Float64CountInsts []InstrumentDescriptor + +// Int64HistoInsts is information about registered int 64 histo instruments in +// order of registration. +var Int64HistoInsts []InstrumentDescriptor + +// Float64HistoInsts is information about registered float 64 histo instruments +// in order of registration. +var Float64HistoInsts []InstrumentDescriptor + +// Int64GaugeInsts is information about registered int 64 gauge instruments in +// order of registration. +var Int64GaugeInsts []InstrumentDescriptor + +// registeredInsts are the registered instrument descriptor names. +var registeredInsts = make(map[string]bool) + +// DefaultNonPerCallMetrics are the instruments registered that are on by +// default. +var DefaultNonPerCallMetrics = make(map[string]bool) + +// ClearInstrumentRegistryForTesting clears the instrument registry for testing +// purposes only. +func ClearInstrumentRegistryForTesting() { + Int64CountInsts = nil + Float64CountInsts = nil + Int64HistoInsts = nil + Float64HistoInsts = nil + Int64GaugeInsts = nil + registeredInsts = make(map[string]bool) + DefaultNonPerCallMetrics = make(map[string]bool) +} + +// Label represents a string attribute/label to attach to metrics. +type Label struct { + // Key is the key of the label. + Key string + // Value is the value of the label. + Value string +} + +func registerInst(name string, def bool) { + if registeredInsts[name] { + log.Panicf("instrument %v already registered", name) + } + registeredInsts[name] = true + if def { + DefaultNonPerCallMetrics[name] = true + } +} + +// Int64CountHandle is a typed handle for a int count instrument. This handle is +// passed at the recording point in order to know which instrument to record on. +type Int64CountHandle struct { + Index int +} + +// RegisterInt64Count registers the int count instrument description onto the +// global registry. It returns a typed handle to use when recording data. +// +// NOTE: this function must only be called during initialization time (i.e. in +// an init() function), and is not thread-safe. If multiple instruments are +// registered with the same name, this function will panic. +func RegisterInt64Count(name string, desc string, unit string, labels []string, optionalLabels []string, def bool) Int64CountHandle { + registerInst(name, def) + Int64CountInsts = append(Int64CountInsts, InstrumentDescriptor{ + Name: name, + Description: desc, + Unit: unit, + Labels: labels, + OptionalLabels: optionalLabels, + Default: def, + }) + return Int64CountHandle{ + Index: len(Int64CountInsts) - 1, + } +} + +// Float64CountHandle is a typed handle for a float count instrument. This handle +// is passed at the recording point in order to know which instrument to record +// on. +type Float64CountHandle struct { + Index int +} + +// RegisterFloat64Count registers the float count instrument description onto the +// global registry. It returns a typed handle to use when recording data. +// +// NOTE: this function must only be called during initialization time (i.e. in +// an init() function), and is not thread-safe. If multiple instruments are +// registered with the same name, this function will panic. +func RegisterFloat64Count(name string, desc string, unit string, labels []string, optionalLabels []string, def bool) Float64CountHandle { + registerInst(name, def) + Float64CountInsts = append(Float64CountInsts, InstrumentDescriptor{ + Name: name, + Description: desc, + Unit: unit, + Labels: labels, + OptionalLabels: optionalLabels, + Default: def, + }) + return Float64CountHandle{ + Index: len(Float64CountInsts) - 1, + } +} + +// Int64HistoHandle is a typed handle for a int histogram instrument. This handle +// is passed at the recording point in order to know which instrument to record +// on. +type Int64HistoHandle struct { + Index int +} + +// RegisterInt64Histo registers the int histogram instrument description onto the +// global registry. It returns a typed handle to use when recording data. +// +// NOTE: this function must only be called during initialization time (i.e. in +// an init() function), and is not thread-safe. If multiple instruments are +// registered with the same name, this function will panic. +func RegisterInt64Histo(name string, desc string, unit string, labels []string, optionalLabels []string, def bool) Int64HistoHandle { + registerInst(name, def) + Int64HistoInsts = append(Int64HistoInsts, InstrumentDescriptor{ + Name: name, + Description: desc, + Unit: unit, + Labels: labels, + OptionalLabels: optionalLabels, + Default: def, + }) + return Int64HistoHandle{ + Index: len(Int64HistoInsts) - 1, + } +} + +// Float64HistoHandle is a typed handle for a float histogram instrument. This +// handle is passed at the recording point in order to know which instrument to +// record on. +type Float64HistoHandle struct { + Index int +} + +// RegisterFloat64Histo registers the float histogram instrument description +// onto the global registry. It returns a typed handle to use when recording +// data. +// +// NOTE: this function must only be called during initialization time (i.e. in +// an init() function), and is not thread-safe. If multiple instruments are +// registered with the same name, this function will panic. +func RegisterFloat64Histo(name string, desc string, unit string, labels []string, optionalLabels []string, def bool) Float64HistoHandle { + registerInst(name, def) + Float64HistoInsts = append(Float64HistoInsts, InstrumentDescriptor{ + Name: name, + Description: desc, + Unit: unit, + Labels: labels, + OptionalLabels: optionalLabels, + Default: def, + }) + return Float64HistoHandle{ + Index: len(Float64HistoInsts) - 1, + } +} + +// Int64GaugeHandle is a typed handle for a int gauge instrument. This handle is +// passed at the recording point in order to know which instrument to record on. +type Int64GaugeHandle struct { + Index int +} + +// RegisterInt64Gauge registers the int gauge instrument description onto the +// global registry. It returns a typed handle to use when recording data. +// +// NOTE: this function must only be called during initialization time (i.e. in +// an init() function), and is not thread-safe. If multiple instruments are +// registered with the same name, this function will panic. +func RegisterInt64Gauge(name string, desc string, unit string, labels []string, optionalLabels []string, def bool) Int64GaugeHandle { + registerInst(name, def) + Int64GaugeInsts = append(Int64GaugeInsts, InstrumentDescriptor{ + Name: name, + Description: desc, + Unit: unit, + Labels: labels, + OptionalLabels: optionalLabels, + Default: def, + }) + return Int64GaugeHandle{ + Index: len(Int64GaugeInsts) - 1, + } +} diff --git a/internal/instrumentregistry/instrument_registry_test.go b/internal/instrumentregistry/instrument_registry_test.go new file mode 100644 index 000000000000..eea88e1910c1 --- /dev/null +++ b/internal/instrumentregistry/instrument_registry_test.go @@ -0,0 +1,348 @@ +/* + * + * Copyright 2024 gRPC 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 instrumentregistry + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "google.golang.org/grpc/internal/grpctest" +) + +type s struct { + grpctest.Tester +} + +func Test(t *testing.T) { + grpctest.RunSubTests(t, s{}) +} + +// TestPanic tests that registering two instruments with the same name across +// any type of instrument triggers a panic. +func (s) TestPanic(t *testing.T) { + defer ClearInstrumentRegistryForTesting() + want := "instrument simple counter already registered" + defer func() { + if r := recover(); r != "instrument simple counter already registered" { + t.Errorf("expected panic %q, got %q", want, r) + } + }() + RegisterInt64Count("simple counter", "number of times recorded on tests", "calls", nil, nil, false) + RegisterInt64Gauge("simple counter", "number of times recorded on tests", "calls", nil, nil, false) +} + +type int64WithLabels struct { + value int64 + labels []string + optionalLabels []string +} + +type float64WithLabels struct { + value float64 + labels []string + optionalLabels []string +} + +// fakeMetricsRecorder persists data and labels based off the global instrument +// registry for use by tests. The data is passed to this recorder from a test +// using the instrument registry's handles. +// +// Do not construct directly; use newFakeMetricsRecorder() instead. +type fakeMetricsRecorder struct { + t *testing.T + + int64counts []int64WithLabels + float64counts []float64WithLabels + int64histos []int64WithLabels + float64histos []float64WithLabels + int64gauges []int64WithLabels +} + +// newFakeMetricsRecorder returns a fake metrics recorder based off the current +// state of global instrument registry. +func newFakeMetricsRecorder(t *testing.T) *fakeMetricsRecorder { + fmr := &fakeMetricsRecorder{t: t} + for _, inst := range Int64CountInsts { + fmr.int64counts = append(fmr.int64counts, int64WithLabels{ + labels: inst.Labels, + optionalLabels: inst.OptionalLabels, + }) + } + for _, inst := range Float64CountInsts { + fmr.float64counts = append(fmr.float64counts, float64WithLabels{ + labels: inst.Labels, + optionalLabels: inst.OptionalLabels, + }) + } + for _, inst := range Int64HistoInsts { + fmr.int64histos = append(fmr.int64histos, int64WithLabels{ + labels: inst.Labels, + optionalLabels: inst.OptionalLabels, + }) + } + for _, inst := range Float64HistoInsts { + fmr.float64histos = append(fmr.float64histos, float64WithLabels{ + labels: inst.Labels, + optionalLabels: inst.OptionalLabels, + }) + } + for _, inst := range Int64GaugeInsts { + fmr.int64gauges = append(fmr.int64gauges, int64WithLabels{ + labels: inst.Labels, + optionalLabels: inst.OptionalLabels, + }) + } + return fmr +} + +// TestInstrumentRegistry tests the instrument registry. It registers testing +// only instruments using the instrument registry, and creates a fake metrics +// recorder which uses these instruments. Using the handles returned from the +// instrument registry, this test records stats using the fake metrics recorder. +// Then, the test verifies the persisted metrics data in the metrics recorder is +// what is expected. Thus, this tests the interactions between the metrics +// recorder and the instruments registry. +func (s) TestInstrumentRegistry(t *testing.T) { + defer ClearInstrumentRegistryForTesting() + intCountHandle1 := RegisterInt64Count("int counter", "number of times recorded on tests", "calls", []string{"int counter label"}, []string{"int counter optional label"}, false) + RegisterInt64Count("int counter 2", "number of times recorded on tests", "calls", []string{"int counter 2 label"}, []string{"int counter 2 optional label"}, false) + + floatCountHandle1 := RegisterFloat64Count("float counter", "number of times recorded on tests", "calls", []string{"float counter label"}, []string{"float counter optional label"}, false) + intHistoHandle1 := RegisterInt64Histo("int histo", "", "calls", []string{"int histo label"}, []string{"int histo optional label"}, false) + floatHistoHandle1 := RegisterFloat64Histo("float histo", "", "calls", []string{"float histo label"}, []string{"float histo optional label"}, false) + intGaugeHandle1 := RegisterInt64Gauge("simple gauge", "the most recent int emitted by test", "int", []string{"int gauge label"}, []string{"int gauge optional label"}, false) + + fmr := newFakeMetricsRecorder(t) + fmr.RecordIntCount(intCountHandle1, []Label{{Key: "int counter label", Value: "some value"}}, []Label{{Key: "int counter optional label", Value: "some value"}}, 1) + + intWithLabelsWant := []int64WithLabels{ + { + value: 1, + labels: []string{"int counter label"}, + optionalLabels: []string{"int counter optional label"}, + }, + { + value: 0, + labels: []string{"int counter 2 label"}, + optionalLabels: []string{"int counter 2 optional label"}, + }, + } + if diff := cmp.Diff(fmr.int64counts, intWithLabelsWant, cmp.AllowUnexported(int64WithLabels{})); diff != "" { + t.Fatalf("fmr.int64counts (-got, +want): %v", diff) + } + + fmr.RecordFloatCount(floatCountHandle1, []Label{{Key: "float counter label", Value: "some value"}}, []Label{{Key: "float counter optional label", Value: "some value"}}, 1.2) + floatWithLabelsWant := []float64WithLabels{ + { + value: 1.2, + labels: []string{"float counter label"}, + optionalLabels: []string{"float counter optional label"}, + }, + } + if diff := cmp.Diff(fmr.float64counts, floatWithLabelsWant, cmp.AllowUnexported(float64WithLabels{})); diff != "" { + t.Fatalf("fmr.float64counts (-got, +want): %v", diff) + } + + fmr.RecordIntHisto(intHistoHandle1, []Label{{Key: "int histo label", Value: "some value"}}, []Label{{Key: "int histo optional label", Value: "some value"}}, 3) + intHistoWithLabelsWant := []int64WithLabels{ + { + value: 3, + labels: []string{"int histo label"}, + optionalLabels: []string{"int histo optional label"}, + }, + } + if diff := cmp.Diff(fmr.int64histos, intHistoWithLabelsWant, cmp.AllowUnexported(int64WithLabels{})); diff != "" { + t.Fatalf("fmr.int64histos (-got, +want): %v", diff) + } + + fmr.RecordFloatHisto(floatHistoHandle1, []Label{{Key: "float histo label", Value: "some value"}}, []Label{{Key: "float histo optional label", Value: "some value"}}, 4) + floatHistoWithLabelsWant := []float64WithLabels{ + { + value: 4, + labels: []string{"float histo label"}, + optionalLabels: []string{"float histo optional label"}, + }, + } + if diff := cmp.Diff(fmr.float64histos, floatHistoWithLabelsWant, cmp.AllowUnexported(float64WithLabels{})); diff != "" { + t.Fatalf("fmr.float64histos (-got, +want): %v", diff) + } + + fmr.RecordIntGauge(intGaugeHandle1, []Label{{Key: "int gauge label", Value: "some value"}}, []Label{{Key: "int gauge optional label", Value: "some value"}}, 7) + intGaugeWithLabelsWant := []int64WithLabels{ + { + value: 7, + labels: []string{"int gauge label"}, + optionalLabels: []string{"int gauge optional label"}, + }, + } + if diff := cmp.Diff(fmr.int64gauges, intGaugeWithLabelsWant, cmp.AllowUnexported(int64WithLabels{})); diff != "" { + t.Fatalf("fmr.int64gauges (-got, +want): %v", diff) + } +} + +// TestNumerousIntCounts tests numerous int count instruments registered onto +// the instrument registry. A component (simulated by test) should be able to +// record on the different registered int count instruments. +func (s) TestNumerousIntCounts(t *testing.T) { + defer ClearInstrumentRegistryForTesting() + intCountHandle1 := RegisterInt64Count("int counter", "number of times recorded on tests", "calls", []string{"int counter label"}, []string{"int counter optional label"}, false) + intCountHandle2 := RegisterInt64Count("int counter 2", "number of times recorded on tests", "calls", []string{"int counter 2 label"}, []string{"int counter 2 optional label"}, false) + intCountHandle3 := RegisterInt64Count("int counter 3", "number of times recorded on tests", "calls", []string{"int counter 3 label"}, []string{"int counter 3 optional label"}, false) + fmr := newFakeMetricsRecorder(t) + + fmr.RecordIntCount(intCountHandle1, []Label{{Key: "int counter label", Value: "some value"}}, []Label{{Key: "int counter optional label", Value: "some value"}}, 1) + intWithLabelsWant := []int64WithLabels{ + { + value: 1, + labels: []string{"int counter label"}, + optionalLabels: []string{"int counter optional label"}, + }, + { + value: 0, + labels: []string{"int counter 2 label"}, + optionalLabels: []string{"int counter 2 optional label"}, + }, + { + value: 0, + labels: []string{"int counter 3 label"}, + optionalLabels: []string{"int counter 3 optional label"}, + }, + } + if diff := cmp.Diff(fmr.int64counts, intWithLabelsWant, cmp.AllowUnexported(int64WithLabels{})); diff != "" { + t.Fatalf("fmr.int64counts (-got, +want): %v", diff) + } + + fmr.RecordIntCount(intCountHandle2, []Label{{Key: "int counter 2 label", Value: "some value"}}, []Label{{Key: "int counter 2 optional label", Value: "some value"}}, 1) + intWithLabelsWant = []int64WithLabels{ + { + value: 1, + labels: []string{"int counter label"}, + optionalLabels: []string{"int counter optional label"}, + }, + { + value: 1, + labels: []string{"int counter 2 label"}, + optionalLabels: []string{"int counter 2 optional label"}, + }, + { + value: 0, + labels: []string{"int counter 3 label"}, + optionalLabels: []string{"int counter 3 optional label"}, + }, + } + if diff := cmp.Diff(fmr.int64counts, intWithLabelsWant, cmp.AllowUnexported(int64WithLabels{})); diff != "" { + t.Fatalf("fmr.int64counts (-got, +want): %v", diff) + } + + fmr.RecordIntCount(intCountHandle3, []Label{{Key: "int counter 3 label", Value: "some value"}}, []Label{{Key: "int counter 3 optional label", Value: "some value"}}, 1) + intWithLabelsWant = []int64WithLabels{ + { + value: 1, + labels: []string{"int counter label"}, + optionalLabels: []string{"int counter optional label"}, + }, + { + value: 1, + labels: []string{"int counter 2 label"}, + optionalLabels: []string{"int counter 2 optional label"}, + }, + { + value: 1, + labels: []string{"int counter 3 label"}, + optionalLabels: []string{"int counter 3 optional label"}, + }, + } + if diff := cmp.Diff(fmr.int64counts, intWithLabelsWant, cmp.AllowUnexported(int64WithLabels{})); diff != "" { + t.Fatalf("fmr.int64counts (-got, +want): %v", diff) + } + + fmr.RecordIntCount(intCountHandle3, []Label{{Key: "int counter 3 label", Value: "some value"}}, []Label{{Key: "int counter 3 optional label", Value: "some value"}}, 1) + intWithLabelsWant = []int64WithLabels{ + { + value: 1, + labels: []string{"int counter label"}, + optionalLabels: []string{"int counter optional label"}, + }, + { + value: 1, + labels: []string{"int counter 2 label"}, + optionalLabels: []string{"int counter 2 optional label"}, + }, + { + value: 2, + labels: []string{"int counter 3 label"}, + optionalLabels: []string{"int counter 3 optional label"}, + }, + } + if diff := cmp.Diff(fmr.int64counts, intWithLabelsWant, cmp.AllowUnexported(int64WithLabels{})); diff != "" { + t.Fatalf("fmr.int64counts (-got, +want): %v", diff) + } +} + +// verifyLabels verifies that all of the labels keys expected are present in the +// labels received. +func verifyLabels(t *testing.T, labelsWant []string, optionalLabelsWant []string, labelsGot []Label, optionalLabelsGot []Label) { + for i, label := range labelsWant { + if labelsGot[i].Key != label { + t.Fatalf("label key at position %v got %v, want %v", i, labelsGot[i].Key, label) + } + } + if len(labelsWant) != len(labelsGot) { + t.Fatalf("length of labels expected did not match got %v, want %v", len(labelsGot), len(optionalLabelsWant)) + } + + for i, label := range optionalLabelsWant { + if optionalLabelsGot[i].Key != label { + t.Fatalf("optional label key at position %v got %v, want %v", i, optionalLabelsGot[i].Key, label) + } + } + if len(optionalLabelsWant) != len(optionalLabelsGot) { + t.Fatalf("length of optional labels expected did not match got %v, want %v", len(optionalLabelsGot), len(optionalLabelsWant)) + } +} + +func (r *fakeMetricsRecorder) RecordIntCount(handle Int64CountHandle, labels []Label, optionalLabels []Label, incr int64) { + ic := r.int64counts[handle.Index] + verifyLabels(r.t, ic.labels, ic.optionalLabels, labels, optionalLabels) + r.int64counts[handle.Index].value += incr +} + +func (r *fakeMetricsRecorder) RecordFloatCount(handle Float64CountHandle, labels []Label, optionalLabels []Label, incr float64) { + fc := r.float64counts[handle.Index] + verifyLabels(r.t, fc.labels, fc.optionalLabels, labels, optionalLabels) + r.float64counts[handle.Index].value += incr +} + +func (r *fakeMetricsRecorder) RecordIntHisto(handle Int64HistoHandle, labels []Label, optionalLabels []Label, incr int64) { + ih := r.int64histos[handle.Index] + verifyLabels(r.t, ih.labels, ih.optionalLabels, labels, optionalLabels) + r.int64histos[handle.Index].value = incr +} + +func (r *fakeMetricsRecorder) RecordFloatHisto(handle Float64HistoHandle, labels []Label, optionalLabels []Label, incr float64) { + fh := r.float64histos[handle.Index] + verifyLabels(r.t, fh.labels, fh.optionalLabels, labels, optionalLabels) + r.float64histos[handle.Index].value = incr +} + +func (r *fakeMetricsRecorder) RecordIntGauge(handle Int64GaugeHandle, labels []Label, optionalLabels []Label, incr int64) { + ig := r.int64gauges[handle.Index] + verifyLabels(r.t, ig.labels, ig.optionalLabels, labels, optionalLabels) + r.int64gauges[handle.Index].value = incr +} From 6920f1d44456e466542ec564cae31df995c305bf Mon Sep 17 00:00:00 2001 From: Zach Reyes Date: Tue, 25 Jun 2024 20:03:45 -0400 Subject: [PATCH 02/13] Move symbols around --- .../instrumentregistry/instrument_registry.go | 61 +++++++++++++++ experimental/stats/stats.go | 49 ++++++++++++ .../instrumentregistry/instrument_registry.go | 76 +++++-------------- .../instrument_registry_test.go | 32 ++++---- stats/opentelemetry/client_metrics.go | 10 +-- stats/opentelemetry/opentelemetry.go | 24 +++--- stats/opentelemetry/server_metrics.go | 8 +- stats/stats.go | 3 + 8 files changed, 168 insertions(+), 95 deletions(-) create mode 100644 experimental/stats/instrumentregistry/instrument_registry.go create mode 100644 experimental/stats/stats.go rename internal/{ => stats}/instrumentregistry/instrument_registry.go (76%) rename internal/{ => stats}/instrumentregistry/instrument_registry_test.go (82%) diff --git a/experimental/stats/instrumentregistry/instrument_registry.go b/experimental/stats/instrumentregistry/instrument_registry.go new file mode 100644 index 000000000000..ed46e590dbec --- /dev/null +++ b/experimental/stats/instrumentregistry/instrument_registry.go @@ -0,0 +1,61 @@ +/* + * + * Copyright 2024 gRPC 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 instrumentregistry contains the experimental instrument registry data +// structures exposed to users. +package instrumentregistry + +import "google.golang.org/grpc/stats" + +// Int64CountHandle is a typed handle for a int count instrument. This handle is +// passed at the recording point in order to know which instrument to record on. +type Int64CountHandle struct { + Index int +} + +// Float64CountHandle is a typed handle for a float count instrument. This handle +// is passed at the recording point in order to know which instrument to record +// on. +type Float64CountHandle struct { + Index int +} + +// Int64HistoHandle is a typed handle for a int histogram instrument. This handle +// is passed at the recording point in order to know which instrument to record +// on. +type Int64HistoHandle struct { + Index int +} + +// Float64HistoHandle is a typed handle for a float histogram instrument. This +// handle is passed at the recording point in order to know which instrument to +// record on. +type Float64HistoHandle struct { + Index int +} + +// Int64GaugeHandle is a typed handle for a int gauge instrument. This handle is +// passed at the recording point in order to know which instrument to record on. +type Int64GaugeHandle struct { + Index int +} + +// DefaultMetrics are the default metrics registered through global instruments +// registry. This is written to at initialization time only, and is read +// only after initialization. +var DefaultMetrics = make(map[stats.Metric]bool) diff --git a/experimental/stats/stats.go b/experimental/stats/stats.go new file mode 100644 index 000000000000..efae8cc5a6ec --- /dev/null +++ b/experimental/stats/stats.go @@ -0,0 +1,49 @@ +/* + * + * Copyright 2024 gRPC 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 stats contains experimental metrics/stats API's. +package stats + +import "google.golang.org/grpc/experimental/stats/instrumentregistry" + +// MetricsRecorder records on metrics derived from instrument registry. +type MetricsRecorder interface { + // RecordIntCount records the measurement alongside labels on the int count + // associated with the provided handle. + RecordIntCount(instrumentregistry.Int64CountHandle, []Label, []Label, int64) + // RecordFloatCount records the measurement alongside labels on the float count + // associated with the provided handle. + RecordFloatCount(instrumentregistry.Float64CountHandle, []Label, []Label, float64) + // RecordIntHisto records the measurement alongside labels on the int histo + // associated with the provided handle. + RecordIntHisto(instrumentregistry.Int64HistoHandle, []Label, []Label, int64) + // RecordFloatHisto records the measurement alongside labels on the float + // histo associated with the provided handle. + RecordFloatHisto(instrumentregistry.Float64CountHandle, []Label, []Label, float64) + // RecordIntGauge records the measurement alongside labels on the int gauge + // associated with the provided handle. + RecordIntGauge(instrumentregistry.Int64GaugeHandle, []Label, []Label, int64) +} + +// Label represents a string attribute/label to attach to metrics. +type Label struct { + // Key is the key of the label. + Key string + // Value is the value of the label. + Value string +} diff --git a/internal/instrumentregistry/instrument_registry.go b/internal/stats/instrumentregistry/instrument_registry.go similarity index 76% rename from internal/instrumentregistry/instrument_registry.go rename to internal/stats/instrumentregistry/instrument_registry.go index f2f454b3292c..6de003b08e59 100644 --- a/internal/instrumentregistry/instrument_registry.go +++ b/internal/stats/instrumentregistry/instrument_registry.go @@ -20,7 +20,12 @@ // use to register instruments (metrics). package instrumentregistry -import "log" +import ( + "log" + + "google.golang.org/grpc/experimental/stats/instrumentregistry" + "google.golang.org/grpc/stats" +) // InstrumentDescriptor is a data of a registered instrument (metric). type InstrumentDescriptor struct { @@ -65,10 +70,6 @@ var Int64GaugeInsts []InstrumentDescriptor // registeredInsts are the registered instrument descriptor names. var registeredInsts = make(map[string]bool) -// DefaultNonPerCallMetrics are the instruments registered that are on by -// default. -var DefaultNonPerCallMetrics = make(map[string]bool) - // ClearInstrumentRegistryForTesting clears the instrument registry for testing // purposes only. func ClearInstrumentRegistryForTesting() { @@ -78,15 +79,7 @@ func ClearInstrumentRegistryForTesting() { Float64HistoInsts = nil Int64GaugeInsts = nil registeredInsts = make(map[string]bool) - DefaultNonPerCallMetrics = make(map[string]bool) -} - -// Label represents a string attribute/label to attach to metrics. -type Label struct { - // Key is the key of the label. - Key string - // Value is the value of the label. - Value string + instrumentregistry.DefaultMetrics = make(map[stats.Metric]bool) } func registerInst(name string, def bool) { @@ -95,23 +88,17 @@ func registerInst(name string, def bool) { } registeredInsts[name] = true if def { - DefaultNonPerCallMetrics[name] = true + instrumentregistry.DefaultMetrics[stats.Metric(name)] = true } } -// Int64CountHandle is a typed handle for a int count instrument. This handle is -// passed at the recording point in order to know which instrument to record on. -type Int64CountHandle struct { - Index int -} - // RegisterInt64Count registers the int count instrument description onto the // global registry. It returns a typed handle to use when recording data. // // NOTE: this function must only be called during initialization time (i.e. in // an init() function), and is not thread-safe. If multiple instruments are // registered with the same name, this function will panic. -func RegisterInt64Count(name string, desc string, unit string, labels []string, optionalLabels []string, def bool) Int64CountHandle { +func RegisterInt64Count(name string, desc string, unit string, labels []string, optionalLabels []string, def bool) instrumentregistry.Int64CountHandle { registerInst(name, def) Int64CountInsts = append(Int64CountInsts, InstrumentDescriptor{ Name: name, @@ -121,25 +108,18 @@ func RegisterInt64Count(name string, desc string, unit string, labels []string, OptionalLabels: optionalLabels, Default: def, }) - return Int64CountHandle{ + return instrumentregistry.Int64CountHandle{ Index: len(Int64CountInsts) - 1, } } -// Float64CountHandle is a typed handle for a float count instrument. This handle -// is passed at the recording point in order to know which instrument to record -// on. -type Float64CountHandle struct { - Index int -} - // RegisterFloat64Count registers the float count instrument description onto the // global registry. It returns a typed handle to use when recording data. // // NOTE: this function must only be called during initialization time (i.e. in // an init() function), and is not thread-safe. If multiple instruments are // registered with the same name, this function will panic. -func RegisterFloat64Count(name string, desc string, unit string, labels []string, optionalLabels []string, def bool) Float64CountHandle { +func RegisterFloat64Count(name string, desc string, unit string, labels []string, optionalLabels []string, def bool) instrumentregistry.Float64CountHandle { registerInst(name, def) Float64CountInsts = append(Float64CountInsts, InstrumentDescriptor{ Name: name, @@ -149,25 +129,18 @@ func RegisterFloat64Count(name string, desc string, unit string, labels []string OptionalLabels: optionalLabels, Default: def, }) - return Float64CountHandle{ + return instrumentregistry.Float64CountHandle{ Index: len(Float64CountInsts) - 1, } } -// Int64HistoHandle is a typed handle for a int histogram instrument. This handle -// is passed at the recording point in order to know which instrument to record -// on. -type Int64HistoHandle struct { - Index int -} - // RegisterInt64Histo registers the int histogram instrument description onto the // global registry. It returns a typed handle to use when recording data. // // NOTE: this function must only be called during initialization time (i.e. in // an init() function), and is not thread-safe. If multiple instruments are // registered with the same name, this function will panic. -func RegisterInt64Histo(name string, desc string, unit string, labels []string, optionalLabels []string, def bool) Int64HistoHandle { +func RegisterInt64Histo(name string, desc string, unit string, labels []string, optionalLabels []string, def bool) instrumentregistry.Int64HistoHandle { registerInst(name, def) Int64HistoInsts = append(Int64HistoInsts, InstrumentDescriptor{ Name: name, @@ -177,18 +150,11 @@ func RegisterInt64Histo(name string, desc string, unit string, labels []string, OptionalLabels: optionalLabels, Default: def, }) - return Int64HistoHandle{ + return instrumentregistry.Int64HistoHandle{ Index: len(Int64HistoInsts) - 1, } } -// Float64HistoHandle is a typed handle for a float histogram instrument. This -// handle is passed at the recording point in order to know which instrument to -// record on. -type Float64HistoHandle struct { - Index int -} - // RegisterFloat64Histo registers the float histogram instrument description // onto the global registry. It returns a typed handle to use when recording // data. @@ -196,7 +162,7 @@ type Float64HistoHandle struct { // NOTE: this function must only be called during initialization time (i.e. in // an init() function), and is not thread-safe. If multiple instruments are // registered with the same name, this function will panic. -func RegisterFloat64Histo(name string, desc string, unit string, labels []string, optionalLabels []string, def bool) Float64HistoHandle { +func RegisterFloat64Histo(name string, desc string, unit string, labels []string, optionalLabels []string, def bool) instrumentregistry.Float64HistoHandle { registerInst(name, def) Float64HistoInsts = append(Float64HistoInsts, InstrumentDescriptor{ Name: name, @@ -206,24 +172,18 @@ func RegisterFloat64Histo(name string, desc string, unit string, labels []string OptionalLabels: optionalLabels, Default: def, }) - return Float64HistoHandle{ + return instrumentregistry.Float64HistoHandle{ Index: len(Float64HistoInsts) - 1, } } -// Int64GaugeHandle is a typed handle for a int gauge instrument. This handle is -// passed at the recording point in order to know which instrument to record on. -type Int64GaugeHandle struct { - Index int -} - // RegisterInt64Gauge registers the int gauge instrument description onto the // global registry. It returns a typed handle to use when recording data. // // NOTE: this function must only be called during initialization time (i.e. in // an init() function), and is not thread-safe. If multiple instruments are // registered with the same name, this function will panic. -func RegisterInt64Gauge(name string, desc string, unit string, labels []string, optionalLabels []string, def bool) Int64GaugeHandle { +func RegisterInt64Gauge(name string, desc string, unit string, labels []string, optionalLabels []string, def bool) instrumentregistry.Int64GaugeHandle { registerInst(name, def) Int64GaugeInsts = append(Int64GaugeInsts, InstrumentDescriptor{ Name: name, @@ -233,7 +193,7 @@ func RegisterInt64Gauge(name string, desc string, unit string, labels []string, OptionalLabels: optionalLabels, Default: def, }) - return Int64GaugeHandle{ + return instrumentregistry.Int64GaugeHandle{ Index: len(Int64GaugeInsts) - 1, } } diff --git a/internal/instrumentregistry/instrument_registry_test.go b/internal/stats/instrumentregistry/instrument_registry_test.go similarity index 82% rename from internal/instrumentregistry/instrument_registry_test.go rename to internal/stats/instrumentregistry/instrument_registry_test.go index eea88e1910c1..362082d151bf 100644 --- a/internal/instrumentregistry/instrument_registry_test.go +++ b/internal/stats/instrumentregistry/instrument_registry_test.go @@ -22,6 +22,8 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "google.golang.org/grpc/experimental/stats" + "google.golang.org/grpc/experimental/stats/instrumentregistry" "google.golang.org/grpc/internal/grpctest" ) @@ -129,7 +131,7 @@ func (s) TestInstrumentRegistry(t *testing.T) { intGaugeHandle1 := RegisterInt64Gauge("simple gauge", "the most recent int emitted by test", "int", []string{"int gauge label"}, []string{"int gauge optional label"}, false) fmr := newFakeMetricsRecorder(t) - fmr.RecordIntCount(intCountHandle1, []Label{{Key: "int counter label", Value: "some value"}}, []Label{{Key: "int counter optional label", Value: "some value"}}, 1) + fmr.RecordIntCount(intCountHandle1, []stats.Label{{Key: "int counter label", Value: "some value"}}, []stats.Label{{Key: "int counter optional label", Value: "some value"}}, 1) intWithLabelsWant := []int64WithLabels{ { @@ -147,7 +149,7 @@ func (s) TestInstrumentRegistry(t *testing.T) { t.Fatalf("fmr.int64counts (-got, +want): %v", diff) } - fmr.RecordFloatCount(floatCountHandle1, []Label{{Key: "float counter label", Value: "some value"}}, []Label{{Key: "float counter optional label", Value: "some value"}}, 1.2) + fmr.RecordFloatCount(floatCountHandle1, []stats.Label{{Key: "float counter label", Value: "some value"}}, []stats.Label{{Key: "float counter optional label", Value: "some value"}}, 1.2) floatWithLabelsWant := []float64WithLabels{ { value: 1.2, @@ -159,7 +161,7 @@ func (s) TestInstrumentRegistry(t *testing.T) { t.Fatalf("fmr.float64counts (-got, +want): %v", diff) } - fmr.RecordIntHisto(intHistoHandle1, []Label{{Key: "int histo label", Value: "some value"}}, []Label{{Key: "int histo optional label", Value: "some value"}}, 3) + fmr.RecordIntHisto(intHistoHandle1, []stats.Label{{Key: "int histo label", Value: "some value"}}, []stats.Label{{Key: "int histo optional label", Value: "some value"}}, 3) intHistoWithLabelsWant := []int64WithLabels{ { value: 3, @@ -171,7 +173,7 @@ func (s) TestInstrumentRegistry(t *testing.T) { t.Fatalf("fmr.int64histos (-got, +want): %v", diff) } - fmr.RecordFloatHisto(floatHistoHandle1, []Label{{Key: "float histo label", Value: "some value"}}, []Label{{Key: "float histo optional label", Value: "some value"}}, 4) + fmr.RecordFloatHisto(floatHistoHandle1, []stats.Label{{Key: "float histo label", Value: "some value"}}, []stats.Label{{Key: "float histo optional label", Value: "some value"}}, 4) floatHistoWithLabelsWant := []float64WithLabels{ { value: 4, @@ -183,7 +185,7 @@ func (s) TestInstrumentRegistry(t *testing.T) { t.Fatalf("fmr.float64histos (-got, +want): %v", diff) } - fmr.RecordIntGauge(intGaugeHandle1, []Label{{Key: "int gauge label", Value: "some value"}}, []Label{{Key: "int gauge optional label", Value: "some value"}}, 7) + fmr.RecordIntGauge(intGaugeHandle1, []stats.Label{{Key: "int gauge label", Value: "some value"}}, []stats.Label{{Key: "int gauge optional label", Value: "some value"}}, 7) intGaugeWithLabelsWant := []int64WithLabels{ { value: 7, @@ -206,7 +208,7 @@ func (s) TestNumerousIntCounts(t *testing.T) { intCountHandle3 := RegisterInt64Count("int counter 3", "number of times recorded on tests", "calls", []string{"int counter 3 label"}, []string{"int counter 3 optional label"}, false) fmr := newFakeMetricsRecorder(t) - fmr.RecordIntCount(intCountHandle1, []Label{{Key: "int counter label", Value: "some value"}}, []Label{{Key: "int counter optional label", Value: "some value"}}, 1) + fmr.RecordIntCount(intCountHandle1, []stats.Label{{Key: "int counter label", Value: "some value"}}, []stats.Label{{Key: "int counter optional label", Value: "some value"}}, 1) intWithLabelsWant := []int64WithLabels{ { value: 1, @@ -228,7 +230,7 @@ func (s) TestNumerousIntCounts(t *testing.T) { t.Fatalf("fmr.int64counts (-got, +want): %v", diff) } - fmr.RecordIntCount(intCountHandle2, []Label{{Key: "int counter 2 label", Value: "some value"}}, []Label{{Key: "int counter 2 optional label", Value: "some value"}}, 1) + fmr.RecordIntCount(intCountHandle2, []stats.Label{{Key: "int counter 2 label", Value: "some value"}}, []stats.Label{{Key: "int counter 2 optional label", Value: "some value"}}, 1) intWithLabelsWant = []int64WithLabels{ { value: 1, @@ -250,7 +252,7 @@ func (s) TestNumerousIntCounts(t *testing.T) { t.Fatalf("fmr.int64counts (-got, +want): %v", diff) } - fmr.RecordIntCount(intCountHandle3, []Label{{Key: "int counter 3 label", Value: "some value"}}, []Label{{Key: "int counter 3 optional label", Value: "some value"}}, 1) + fmr.RecordIntCount(intCountHandle3, []stats.Label{{Key: "int counter 3 label", Value: "some value"}}, []stats.Label{{Key: "int counter 3 optional label", Value: "some value"}}, 1) intWithLabelsWant = []int64WithLabels{ { value: 1, @@ -272,7 +274,7 @@ func (s) TestNumerousIntCounts(t *testing.T) { t.Fatalf("fmr.int64counts (-got, +want): %v", diff) } - fmr.RecordIntCount(intCountHandle3, []Label{{Key: "int counter 3 label", Value: "some value"}}, []Label{{Key: "int counter 3 optional label", Value: "some value"}}, 1) + fmr.RecordIntCount(intCountHandle3, []stats.Label{{Key: "int counter 3 label", Value: "some value"}}, []stats.Label{{Key: "int counter 3 optional label", Value: "some value"}}, 1) intWithLabelsWant = []int64WithLabels{ { value: 1, @@ -297,7 +299,7 @@ func (s) TestNumerousIntCounts(t *testing.T) { // verifyLabels verifies that all of the labels keys expected are present in the // labels received. -func verifyLabels(t *testing.T, labelsWant []string, optionalLabelsWant []string, labelsGot []Label, optionalLabelsGot []Label) { +func verifyLabels(t *testing.T, labelsWant []string, optionalLabelsWant []string, labelsGot []stats.Label, optionalLabelsGot []stats.Label) { for i, label := range labelsWant { if labelsGot[i].Key != label { t.Fatalf("label key at position %v got %v, want %v", i, labelsGot[i].Key, label) @@ -317,31 +319,31 @@ func verifyLabels(t *testing.T, labelsWant []string, optionalLabelsWant []string } } -func (r *fakeMetricsRecorder) RecordIntCount(handle Int64CountHandle, labels []Label, optionalLabels []Label, incr int64) { +func (r *fakeMetricsRecorder) RecordIntCount(handle instrumentregistry.Int64CountHandle, labels []stats.Label, optionalLabels []stats.Label, incr int64) { ic := r.int64counts[handle.Index] verifyLabels(r.t, ic.labels, ic.optionalLabels, labels, optionalLabels) r.int64counts[handle.Index].value += incr } -func (r *fakeMetricsRecorder) RecordFloatCount(handle Float64CountHandle, labels []Label, optionalLabels []Label, incr float64) { +func (r *fakeMetricsRecorder) RecordFloatCount(handle instrumentregistry.Float64CountHandle, labels []stats.Label, optionalLabels []stats.Label, incr float64) { fc := r.float64counts[handle.Index] verifyLabels(r.t, fc.labels, fc.optionalLabels, labels, optionalLabels) r.float64counts[handle.Index].value += incr } -func (r *fakeMetricsRecorder) RecordIntHisto(handle Int64HistoHandle, labels []Label, optionalLabels []Label, incr int64) { +func (r *fakeMetricsRecorder) RecordIntHisto(handle instrumentregistry.Int64HistoHandle, labels []stats.Label, optionalLabels []stats.Label, incr int64) { ih := r.int64histos[handle.Index] verifyLabels(r.t, ih.labels, ih.optionalLabels, labels, optionalLabels) r.int64histos[handle.Index].value = incr } -func (r *fakeMetricsRecorder) RecordFloatHisto(handle Float64HistoHandle, labels []Label, optionalLabels []Label, incr float64) { +func (r *fakeMetricsRecorder) RecordFloatHisto(handle instrumentregistry.Float64HistoHandle, labels []stats.Label, optionalLabels []stats.Label, incr float64) { fh := r.float64histos[handle.Index] verifyLabels(r.t, fh.labels, fh.optionalLabels, labels, optionalLabels) r.float64histos[handle.Index].value = incr } -func (r *fakeMetricsRecorder) RecordIntGauge(handle Int64GaugeHandle, labels []Label, optionalLabels []Label, incr int64) { +func (r *fakeMetricsRecorder) RecordIntGauge(handle instrumentregistry.Int64GaugeHandle, labels []stats.Label, optionalLabels []stats.Label, incr int64) { ig := r.int64gauges[handle.Index] verifyLabels(r.t, ig.labels, ig.optionalLabels, labels, optionalLabels) r.int64gauges[handle.Index].value = incr diff --git a/stats/opentelemetry/client_metrics.go b/stats/opentelemetry/client_metrics.go index 8654132153fd..5f16ce9fb3b7 100644 --- a/stats/opentelemetry/client_metrics.go +++ b/stats/opentelemetry/client_metrics.go @@ -238,17 +238,17 @@ func (h *clientStatsHandler) processRPCEnd(ctx context.Context, ai *attemptInfo, const ( // ClientAttemptStarted is the number of client call attempts started. - ClientAttemptStarted Metric = "grpc.client.attempt.started" + ClientAttemptStarted stats.Metric = "grpc.client.attempt.started" // ClientAttemptDuration is the end-to-end time taken to complete a client // call attempt. - ClientAttemptDuration Metric = "grpc.client.attempt.duration" + ClientAttemptDuration stats.Metric = "grpc.client.attempt.duration" // ClientAttemptSentCompressedTotalMessageSize is the compressed message // bytes sent per client call attempt. - ClientAttemptSentCompressedTotalMessageSize Metric = "grpc.client.attempt.sent_total_compressed_message_size" + ClientAttemptSentCompressedTotalMessageSize stats.Metric = "grpc.client.attempt.sent_total_compressed_message_size" // ClientAttemptRcvdCompressedTotalMessageSize is the compressed message // bytes received per call attempt. - ClientAttemptRcvdCompressedTotalMessageSize Metric = "grpc.client.attempt.rcvd_total_compressed_message_size" + ClientAttemptRcvdCompressedTotalMessageSize stats.Metric = "grpc.client.attempt.rcvd_total_compressed_message_size" // ClientCallDuration is the time taken by gRPC to complete an RPC from // application's perspective. - ClientCallDuration Metric = "grpc.client.call.duration" + ClientCallDuration stats.Metric = "grpc.client.call.duration" ) diff --git a/stats/opentelemetry/opentelemetry.go b/stats/opentelemetry/opentelemetry.go index 4bc195b4e12c..ede6377e6b46 100644 --- a/stats/opentelemetry/opentelemetry.go +++ b/stats/opentelemetry/opentelemetry.go @@ -27,6 +27,7 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/grpclog" "google.golang.org/grpc/internal" + "google.golang.org/grpc/stats" otelinternal "google.golang.org/grpc/stats/opentelemetry/internal" "go.opentelemetry.io/otel/metric" @@ -45,20 +46,17 @@ var canonicalString = internal.CanonicalString.(func(codes.Code) string) var joinDialOptions = internal.JoinDialOptions.(func(...grpc.DialOption) grpc.DialOption) -// Metric is an identifier for a metric provided by this package. -type Metric string - // Metrics is a set of metrics to record. Once created, Metrics is immutable, // however Add and Remove can make copies with specific metrics added or // removed, respectively. type Metrics struct { // metrics are the set of metrics to initialize. - metrics map[Metric]bool + metrics map[stats.Metric]bool } // NewMetrics returns a Metrics containing Metrics. -func NewMetrics(metrics ...Metric) *Metrics { - newMetrics := make(map[Metric]bool) +func NewMetrics(metrics ...stats.Metric) *Metrics { + newMetrics := make(map[stats.Metric]bool) for _, metric := range metrics { newMetrics[metric] = true } @@ -69,8 +67,8 @@ func NewMetrics(metrics ...Metric) *Metrics { // Add adds the metrics to the metrics set and returns a new copy with the // additional metrics. -func (m *Metrics) Add(metrics ...Metric) *Metrics { - newMetrics := make(map[Metric]bool) +func (m *Metrics) Add(metrics ...stats.Metric) *Metrics { + newMetrics := make(map[stats.Metric]bool) for metric := range m.metrics { newMetrics[metric] = true } @@ -85,8 +83,8 @@ func (m *Metrics) Add(metrics ...Metric) *Metrics { // Remove removes the metrics from the metrics set and returns a new copy with // the metrics removed. -func (m *Metrics) Remove(metrics ...Metric) *Metrics { - newMetrics := make(map[Metric]bool) +func (m *Metrics) Remove(metrics ...stats.Metric) *Metrics { + newMetrics := make(map[stats.Metric]bool) for metric := range m.metrics { newMetrics[metric] = true } @@ -260,7 +258,7 @@ type serverMetrics struct { callDuration metric.Float64Histogram } -func createInt64Counter(setOfMetrics map[Metric]bool, metricName Metric, meter metric.Meter, options ...metric.Int64CounterOption) metric.Int64Counter { +func createInt64Counter(setOfMetrics map[stats.Metric]bool, metricName stats.Metric, meter metric.Meter, options ...metric.Int64CounterOption) metric.Int64Counter { if _, ok := setOfMetrics[metricName]; !ok { return noop.Int64Counter{} } @@ -272,7 +270,7 @@ func createInt64Counter(setOfMetrics map[Metric]bool, metricName Metric, meter m return ret } -func createInt64Histogram(setOfMetrics map[Metric]bool, metricName Metric, meter metric.Meter, options ...metric.Int64HistogramOption) metric.Int64Histogram { +func createInt64Histogram(setOfMetrics map[stats.Metric]bool, metricName stats.Metric, meter metric.Meter, options ...metric.Int64HistogramOption) metric.Int64Histogram { if _, ok := setOfMetrics[metricName]; !ok { return noop.Int64Histogram{} } @@ -284,7 +282,7 @@ func createInt64Histogram(setOfMetrics map[Metric]bool, metricName Metric, meter return ret } -func createFloat64Histogram(setOfMetrics map[Metric]bool, metricName Metric, meter metric.Meter, options ...metric.Float64HistogramOption) metric.Float64Histogram { +func createFloat64Histogram(setOfMetrics map[stats.Metric]bool, metricName stats.Metric, meter metric.Meter, options ...metric.Float64HistogramOption) metric.Float64Histogram { if _, ok := setOfMetrics[metricName]; !ok { return noop.Float64Histogram{} } diff --git a/stats/opentelemetry/server_metrics.go b/stats/opentelemetry/server_metrics.go index a20d6a261fc6..3aa874d3d888 100644 --- a/stats/opentelemetry/server_metrics.go +++ b/stats/opentelemetry/server_metrics.go @@ -254,14 +254,14 @@ func (h *serverStatsHandler) processRPCEnd(ctx context.Context, ai *attemptInfo, const ( // ServerCallStarted is the number of server calls started. - ServerCallStarted Metric = "grpc.server.call.started" + ServerCallStarted stats.Metric = "grpc.server.call.started" // ServerCallSentCompressedTotalMessageSize is the compressed message bytes // sent per server call. - ServerCallSentCompressedTotalMessageSize Metric = "grpc.server.call.sent_total_compressed_message_size" + ServerCallSentCompressedTotalMessageSize stats.Metric = "grpc.server.call.sent_total_compressed_message_size" // ServerCallRcvdCompressedTotalMessageSize is the compressed message bytes // received per server call. - ServerCallRcvdCompressedTotalMessageSize Metric = "grpc.server.call.rcvd_total_compressed_message_size" + ServerCallRcvdCompressedTotalMessageSize stats.Metric = "grpc.server.call.rcvd_total_compressed_message_size" // ServerCallDuration is the end-to-end time taken to complete a call from // server transport's perspective. - ServerCallDuration Metric = "grpc.server.call.duration" + ServerCallDuration stats.Metric = "grpc.server.call.duration" ) diff --git a/stats/stats.go b/stats/stats.go index fdb0bd65182c..11c2033f1317 100644 --- a/stats/stats.go +++ b/stats/stats.go @@ -347,3 +347,6 @@ func OutgoingTrace(ctx context.Context) []byte { b, _ := ctx.Value(outgoingTraceKey{}).([]byte) return b } + +// Metric is an identifier for a metric. +type Metric string From e5d3d681c21fbb79b00e68a1a713cc9707bcd504 Mon Sep 17 00:00:00 2001 From: Zach Reyes Date: Wed, 26 Jun 2024 20:45:02 -0400 Subject: [PATCH 03/13] Move Metrics Set to stats and export --- stats/opentelemetry/client_metrics.go | 10 ++--- stats/opentelemetry/example_test.go | 5 ++- stats/opentelemetry/opentelemetry.go | 55 +---------------------- stats/opentelemetry/server_metrics.go | 8 ++-- stats/stats.go | 65 +++++++++++++++++++++++++++ 5 files changed, 79 insertions(+), 64 deletions(-) diff --git a/stats/opentelemetry/client_metrics.go b/stats/opentelemetry/client_metrics.go index 5f16ce9fb3b7..60f786ea6877 100644 --- a/stats/opentelemetry/client_metrics.go +++ b/stats/opentelemetry/client_metrics.go @@ -54,11 +54,11 @@ func (h *clientStatsHandler) initializeMetrics() { metrics = DefaultMetrics } - h.clientMetrics.attemptStarted = createInt64Counter(metrics.metrics, "grpc.client.attempt.started", meter, otelmetric.WithUnit("attempt"), otelmetric.WithDescription("Number of client call attempts started.")) - h.clientMetrics.attemptDuration = createFloat64Histogram(metrics.metrics, "grpc.client.attempt.duration", meter, otelmetric.WithUnit("s"), otelmetric.WithDescription("End-to-end time taken to complete a client call attempt."), otelmetric.WithExplicitBucketBoundaries(DefaultLatencyBounds...)) - h.clientMetrics.attemptSentTotalCompressedMessageSize = createInt64Histogram(metrics.metrics, "grpc.client.attempt.sent_total_compressed_message_size", meter, otelmetric.WithUnit("By"), otelmetric.WithDescription("Compressed message bytes sent per client call attempt."), otelmetric.WithExplicitBucketBoundaries(DefaultSizeBounds...)) - h.clientMetrics.attemptRcvdTotalCompressedMessageSize = createInt64Histogram(metrics.metrics, "grpc.client.attempt.rcvd_total_compressed_message_size", meter, otelmetric.WithUnit("By"), otelmetric.WithDescription("Compressed message bytes received per call attempt."), otelmetric.WithExplicitBucketBoundaries(DefaultSizeBounds...)) - h.clientMetrics.callDuration = createFloat64Histogram(metrics.metrics, "grpc.client.call.duration", meter, otelmetric.WithUnit("s"), otelmetric.WithDescription("Time taken by gRPC to complete an RPC from application's perspective."), otelmetric.WithExplicitBucketBoundaries(DefaultLatencyBounds...)) + h.clientMetrics.attemptStarted = createInt64Counter(metrics.Metrics, "grpc.client.attempt.started", meter, otelmetric.WithUnit("attempt"), otelmetric.WithDescription("Number of client call attempts started.")) + h.clientMetrics.attemptDuration = createFloat64Histogram(metrics.Metrics, "grpc.client.attempt.duration", meter, otelmetric.WithUnit("s"), otelmetric.WithDescription("End-to-end time taken to complete a client call attempt."), otelmetric.WithExplicitBucketBoundaries(DefaultLatencyBounds...)) + h.clientMetrics.attemptSentTotalCompressedMessageSize = createInt64Histogram(metrics.Metrics, "grpc.client.attempt.sent_total_compressed_message_size", meter, otelmetric.WithUnit("By"), otelmetric.WithDescription("Compressed message bytes sent per client call attempt."), otelmetric.WithExplicitBucketBoundaries(DefaultSizeBounds...)) + h.clientMetrics.attemptRcvdTotalCompressedMessageSize = createInt64Histogram(metrics.Metrics, "grpc.client.attempt.rcvd_total_compressed_message_size", meter, otelmetric.WithUnit("By"), otelmetric.WithDescription("Compressed message bytes received per call attempt."), otelmetric.WithExplicitBucketBoundaries(DefaultSizeBounds...)) + h.clientMetrics.callDuration = createFloat64Histogram(metrics.Metrics, "grpc.client.call.duration", meter, otelmetric.WithUnit("s"), otelmetric.WithDescription("Time taken by gRPC to complete an RPC from application's perspective."), otelmetric.WithExplicitBucketBoundaries(DefaultLatencyBounds...)) } func (h *clientStatsHandler) unaryInterceptor(ctx context.Context, method string, req, reply any, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { diff --git a/stats/opentelemetry/example_test.go b/stats/opentelemetry/example_test.go index b4d6755b2299..09ca9af3bfa3 100644 --- a/stats/opentelemetry/example_test.go +++ b/stats/opentelemetry/example_test.go @@ -19,6 +19,7 @@ package opentelemetry_test import ( "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/stats" "google.golang.org/grpc/stats/opentelemetry" "go.opentelemetry.io/otel/sdk/metric" @@ -102,7 +103,7 @@ func ExampleMetrics_disableAll() { // To disable all metrics, initialize Options as follows: opts := opentelemetry.Options{ MetricsOptions: opentelemetry.MetricsOptions{ - Metrics: opentelemetry.NewMetrics(), // Distinct to nil, which creates default metrics. This empty set creates no metrics. + Metrics: stats.NewMetrics(), // Distinct to nil, which creates default metrics. This empty set creates no metrics. }, } do := opentelemetry.DialOption(opts) @@ -117,7 +118,7 @@ func ExampleMetrics_enableSome() { // To only create specific metrics, initialize Options as follows: opts := opentelemetry.Options{ MetricsOptions: opentelemetry.MetricsOptions{ - Metrics: opentelemetry.NewMetrics(opentelemetry.ClientAttemptDuration, opentelemetry.ClientAttemptRcvdCompressedTotalMessageSize), // only create these metrics + Metrics: stats.NewMetrics(opentelemetry.ClientAttemptDuration, opentelemetry.ClientAttemptRcvdCompressedTotalMessageSize), // only create these metrics }, } do := opentelemetry.DialOption(opts) diff --git a/stats/opentelemetry/opentelemetry.go b/stats/opentelemetry/opentelemetry.go index ede6377e6b46..db48bfdf6a93 100644 --- a/stats/opentelemetry/opentelemetry.go +++ b/stats/opentelemetry/opentelemetry.go @@ -46,57 +46,6 @@ var canonicalString = internal.CanonicalString.(func(codes.Code) string) var joinDialOptions = internal.JoinDialOptions.(func(...grpc.DialOption) grpc.DialOption) -// Metrics is a set of metrics to record. Once created, Metrics is immutable, -// however Add and Remove can make copies with specific metrics added or -// removed, respectively. -type Metrics struct { - // metrics are the set of metrics to initialize. - metrics map[stats.Metric]bool -} - -// NewMetrics returns a Metrics containing Metrics. -func NewMetrics(metrics ...stats.Metric) *Metrics { - newMetrics := make(map[stats.Metric]bool) - for _, metric := range metrics { - newMetrics[metric] = true - } - return &Metrics{ - metrics: newMetrics, - } -} - -// Add adds the metrics to the metrics set and returns a new copy with the -// additional metrics. -func (m *Metrics) Add(metrics ...stats.Metric) *Metrics { - newMetrics := make(map[stats.Metric]bool) - for metric := range m.metrics { - newMetrics[metric] = true - } - - for _, metric := range metrics { - newMetrics[metric] = true - } - return &Metrics{ - metrics: newMetrics, - } -} - -// Remove removes the metrics from the metrics set and returns a new copy with -// the metrics removed. -func (m *Metrics) Remove(metrics ...stats.Metric) *Metrics { - newMetrics := make(map[stats.Metric]bool) - for metric := range m.metrics { - newMetrics[metric] = true - } - - for _, metric := range metrics { - delete(newMetrics, metric) - } - return &Metrics{ - metrics: newMetrics, - } -} - // Options are the options for OpenTelemetry instrumentation. type Options struct { // MetricsOptions are the metrics options for OpenTelemetry instrumentation. @@ -116,7 +65,7 @@ type MetricsOptions struct { // for corresponding metric supported by the client and server // instrumentation components if applicable. If not set, the default metrics // will be recorded. - Metrics *Metrics + Metrics *stats.Metrics // MethodAttributeFilter is to record the method name of RPCs handled by // grpc.UnknownServiceHandler, but take care to limit the values allowed, as @@ -305,5 +254,5 @@ var ( // DefaultSizeBounds are the default bounds for metrics which record size. DefaultSizeBounds = []float64{0, 1024, 2048, 4096, 16384, 65536, 262144, 1048576, 4194304, 16777216, 67108864, 268435456, 1073741824, 4294967296} // DefaultMetrics are the default metrics provided by this module. - DefaultMetrics = NewMetrics(ClientAttemptStarted, ClientAttemptDuration, ClientAttemptSentCompressedTotalMessageSize, ClientAttemptRcvdCompressedTotalMessageSize, ClientCallDuration, ServerCallStarted, ServerCallSentCompressedTotalMessageSize, ServerCallRcvdCompressedTotalMessageSize, ServerCallDuration) + DefaultMetrics = stats.NewMetrics(ClientAttemptStarted, ClientAttemptDuration, ClientAttemptSentCompressedTotalMessageSize, ClientAttemptRcvdCompressedTotalMessageSize, ClientCallDuration, ServerCallStarted, ServerCallSentCompressedTotalMessageSize, ServerCallRcvdCompressedTotalMessageSize, ServerCallDuration) ) diff --git a/stats/opentelemetry/server_metrics.go b/stats/opentelemetry/server_metrics.go index 3aa874d3d888..d1f25d229985 100644 --- a/stats/opentelemetry/server_metrics.go +++ b/stats/opentelemetry/server_metrics.go @@ -53,10 +53,10 @@ func (h *serverStatsHandler) initializeMetrics() { metrics = DefaultMetrics } - h.serverMetrics.callStarted = createInt64Counter(metrics.metrics, "grpc.server.call.started", meter, otelmetric.WithUnit("call"), otelmetric.WithDescription("Number of server calls started.")) - h.serverMetrics.callSentTotalCompressedMessageSize = createInt64Histogram(metrics.metrics, "grpc.server.call.sent_total_compressed_message_size", meter, otelmetric.WithUnit("By"), otelmetric.WithDescription("Compressed message bytes sent per server call."), otelmetric.WithExplicitBucketBoundaries(DefaultSizeBounds...)) - h.serverMetrics.callRcvdTotalCompressedMessageSize = createInt64Histogram(metrics.metrics, "grpc.server.call.rcvd_total_compressed_message_size", meter, otelmetric.WithUnit("By"), otelmetric.WithDescription("Compressed message bytes received per server call."), otelmetric.WithExplicitBucketBoundaries(DefaultSizeBounds...)) - h.serverMetrics.callDuration = createFloat64Histogram(metrics.metrics, "grpc.server.call.duration", meter, otelmetric.WithUnit("s"), otelmetric.WithDescription("End-to-end time taken to complete a call from server transport's perspective."), otelmetric.WithExplicitBucketBoundaries(DefaultLatencyBounds...)) + h.serverMetrics.callStarted = createInt64Counter(metrics.Metrics, "grpc.server.call.started", meter, otelmetric.WithUnit("call"), otelmetric.WithDescription("Number of server calls started.")) + h.serverMetrics.callSentTotalCompressedMessageSize = createInt64Histogram(metrics.Metrics, "grpc.server.call.sent_total_compressed_message_size", meter, otelmetric.WithUnit("By"), otelmetric.WithDescription("Compressed message bytes sent per server call."), otelmetric.WithExplicitBucketBoundaries(DefaultSizeBounds...)) + h.serverMetrics.callRcvdTotalCompressedMessageSize = createInt64Histogram(metrics.Metrics, "grpc.server.call.rcvd_total_compressed_message_size", meter, otelmetric.WithUnit("By"), otelmetric.WithDescription("Compressed message bytes received per server call."), otelmetric.WithExplicitBucketBoundaries(DefaultSizeBounds...)) + h.serverMetrics.callDuration = createFloat64Histogram(metrics.Metrics, "grpc.server.call.duration", meter, otelmetric.WithUnit("s"), otelmetric.WithDescription("End-to-end time taken to complete a call from server transport's perspective."), otelmetric.WithExplicitBucketBoundaries(DefaultLatencyBounds...)) } // attachLabelsTransportStream intercepts SetHeader and SendHeader calls of the diff --git a/stats/stats.go b/stats/stats.go index 11c2033f1317..77ec5a3478e7 100644 --- a/stats/stats.go +++ b/stats/stats.go @@ -23,6 +23,7 @@ package stats // import "google.golang.org/grpc/stats" import ( "context" + "maps" "net" "time" @@ -350,3 +351,67 @@ func OutgoingTrace(ctx context.Context) []byte { // Metric is an identifier for a metric. type Metric string + +// Metrics is a set of metrics to record. Once created, Metrics is immutable, +// however Add and Remove can make copies with specific metrics added or +// removed, respectively. +// +// Do not construct directly; use NewMetrics instead. +type Metrics struct { + // Metrics are the set of Metrics to initialize. + Metrics map[Metric]bool +} + +// NewMetrics returns a Metrics containing Metrics. +func NewMetrics(metrics ...Metric) *Metrics { + newMetrics := make(map[Metric]bool) + for _, metric := range metrics { + newMetrics[metric] = true + } + return &Metrics{ + Metrics: newMetrics, + } +} + +// Add adds the metrics to the metrics set and returns a new copy with the +// additional metrics. +func (m *Metrics) Add(metrics ...Metric) *Metrics { + newMetrics := make(map[Metric]bool) + for metric := range m.Metrics { + newMetrics[metric] = true + } + + for _, metric := range metrics { + newMetrics[metric] = true + } + return &Metrics{ + Metrics: newMetrics, + } +} + +// Join joins the metrics passed in with the metrics set, and returns a new copy +// with the merged metrics. +func (m *Metrics) Join(metrics *Metrics) *Metrics { + newMetrics := make(map[Metric]bool) + maps.Copy(newMetrics, m.Metrics) + maps.Copy(newMetrics, metrics.Metrics) + return &Metrics{ + Metrics: newMetrics, + } +} + +// Remove removes the metrics from the metrics set and returns a new copy with +// the metrics removed. +func (m *Metrics) Remove(metrics ...Metric) *Metrics { + newMetrics := make(map[Metric]bool) + for metric := range m.Metrics { + newMetrics[metric] = true + } + + for _, metric := range metrics { + delete(newMetrics, metric) + } + return &Metrics{ + Metrics: newMetrics, + } +} From c5cb64138a45c6908df22146247f94684432cb93 Mon Sep 17 00:00:00 2001 From: Zach Reyes Date: Wed, 3 Jul 2024 17:56:54 -0400 Subject: [PATCH 04/13] Snapshot for 1:1 --- experimental/stats/metricregistry.go | 210 ++++++++++++++++++ .../metric_registry.go} | 23 +- experimental/stats/metricregistry_test.go | 135 +++++++++++ experimental/stats/stats.go | 11 +- 4 files changed, 365 insertions(+), 14 deletions(-) create mode 100644 experimental/stats/metricregistry.go rename experimental/stats/{instrumentregistry/instrument_registry.go => metricregistry/metric_registry.go} (72%) create mode 100644 experimental/stats/metricregistry_test.go diff --git a/experimental/stats/metricregistry.go b/experimental/stats/metricregistry.go new file mode 100644 index 000000000000..932169d4bc53 --- /dev/null +++ b/experimental/stats/metricregistry.go @@ -0,0 +1,210 @@ +/* + * + * Copyright 2024 gRPC 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 stats + +import ( + "log" + + "google.golang.org/grpc/stats" +) + +// DefaultMetrics are the default metrics registered through global instruments +// registry. This is written to at initialization time only, and is read +// only after initialization. +var DefaultMetrics = stats.NewMetrics() + +// MetricDescriptor is the data for a registered metric. +type MetricDescriptor struct { + // Name is the name of this metric. + Name stats.Metric + // Description is the description of this metric. + Description string + // Unit is the unit of this metric. + Unit string + // Labels are the required label keys for this metric. + Labels []string + // OptionalLabels are the optional label keys for this + // metric. + OptionalLabels []string + // Default is whether this metric is on by default. + Default bool +} + +// Int64CountHandle is a typed handle for a float count instrument. This handle +// is passed at the recording point in order to know which instrument to record +// on. +type Int64CountHandle struct { + MetricDescriptor *MetricDescriptor +} + +// RecordInt64Count records the int64 count value on the metrics recorder +// provided. +func (h *Int64CountHandle) RecordInt64Count(recorder MetricsRecorder, labels []Label, optionalLabels []Label, incr int64) { + recorder.RecordIntCount(*h, labels, optionalLabels, incr) +} + +// Float64CountHandle is a typed handle for a float count instrument. This handle +// is passed at the recording point in order to know which instrument to record +// on. +type Float64CountHandle struct { + MetricDescriptor *MetricDescriptor +} + +// RecordFloat64Count records the float64 count value on the metrics recorder +// provided. +func (h *Float64CountHandle) RecordFloat64Count(recorder MetricsRecorder, labels []Label, optionalLabels []Label, incr float64) { + recorder.RecordFloatCount(*h, labels, optionalLabels, incr) +} + +// Int64HistoHandle is a typed handle for an int histogram instrument. This +// handle is passed at the recording point in order to know which instrument to +// record on. +type Int64HistoHandle struct { + MetricDescriptor *MetricDescriptor +} + +// RecordInt64Histo records the int64 histo value on the metrics recorder +// provided. +func (h *Int64HistoHandle) RecordInt64Histo(recorder MetricsRecorder, labels []Label, optionalLabels []Label, incr int64) { + recorder.RecordIntHisto(*h, labels, optionalLabels, incr) +} + +// Float64HistoHandle is a typed handle for a float histogram instrument. This +// handle is passed at the recording point in order to know which instrument to +// record on. +type Float64HistoHandle struct { + MetricDescriptor *MetricDescriptor +} + +// RecordFloat64Histo records the float64 histo value on the metrics recorder +// provided. +func (h *Float64HistoHandle) RecordFloat64Histo(recorder MetricsRecorder, labels []Label, optionalLabels []Label, incr float64) { + recorder.RecordFloatHisto(*h, labels, optionalLabels, incr) +} + +// Int64GaugeHandle is a typed handle for an int gauge instrument. This handle +// is passed at the recording point in order to know which instrument to record +// on. +type Int64GaugeHandle struct { + MetricDescriptor *MetricDescriptor +} + +// RecordInt64Histo records the int64 histo value on the metrics recorder +// provided. +func (h *Int64GaugeHandle) RecordInt64Gauge(recorder MetricsRecorder, labels []Label, optionalLabels []Label, incr int64) { + recorder.RecordIntGauge(*h, labels, optionalLabels, incr) +} + +// registeredInsts are the registered instrument descriptor names. +var registeredInsts = make(map[stats.Metric]bool) + +// MetricsRegistry is all the registered metrics. +// +// This is written to only at init time, and read only after that. +var MetricsRegistry = make(map[stats.Metric]*MetricDescriptor) + +func registerInst(name stats.Metric, def bool) { + if registeredInsts[name] { + log.Panicf("instrument %v already registered", name) + } + registeredInsts[name] = true + if def { + DefaultMetrics = DefaultMetrics.Add(name) + } +} + +// RegisterInt64Count registers the instrument description onto the global +// registry. It returns a typed handle to use to recording data. +// +// NOTE: this function must only be called during initialization time (i.e. in +// an init() function), and is not thread-safe. If multiple instruments are +// registered with the same name, this function will panic. +func RegisterInt64Count(descriptor MetricDescriptor) Int64CountHandle { + registerInst(descriptor.Name, descriptor.Default) + handle := Int64CountHandle{ + MetricDescriptor: &descriptor, + } + MetricsRegistry[descriptor.Name] = &descriptor + return handle +} + +// RegisterFloat64Count registers the instrument description onto the global +// registry. It returns a typed handle to use to recording data. +// +// NOTE: this function must only be called during initialization time (i.e. in +// an init() function), and is not thread-safe. If multiple instruments are +// registered with the same name, this function will panic. +func RegisterFloat64Count(descriptor MetricDescriptor) Float64CountHandle { + registerInst(descriptor.Name, descriptor.Default) + handle := Float64CountHandle{ + MetricDescriptor: &descriptor, + } + MetricsRegistry[descriptor.Name] = &descriptor + return handle +} + +// RegisterInt64Histo registers the instrument description onto the global +// registry. It returns a typed handle to use to recording data. +// +// NOTE: this function must only be called during initialization time (i.e. in +// an init() function), and is not thread-safe. If multiple instruments are +// registered with the same name, this function will panic. +func RegisterInt64Histo(descriptor MetricDescriptor) Int64HistoHandle { + registerInst(descriptor.Name, descriptor.Default) + handle := Int64HistoHandle{ + MetricDescriptor: &descriptor, + } + MetricsRegistry[descriptor.Name] = &descriptor + return handle +} + +// RegisterFloat64Histo registers the instrument description onto the global +// registry. It returns a typed handle to use to recording data. +// +// NOTE: this function must only be called during initialization time (i.e. in +// an init() function), and is not thread-safe. If multiple instruments are +// registered with the same name, this function will panic. +func RegisterFloat64Histo(descriptor MetricDescriptor) Float64HistoHandle { + registerInst(descriptor.Name, descriptor.Default) + handle := Float64HistoHandle{ + MetricDescriptor: &descriptor, + } + MetricsRegistry[descriptor.Name] = &descriptor + return handle +} + +// RegisterInt64Gauge registers the instrument description onto the global +// registry. It returns a typed handle to use to recording data. +// +// NOTE: this function must only be called during initialization time (i.e. in +// an init() function), and is not thread-safe. If multiple instruments are +// registered with the same name, this function will panic. +func RegisterInt64Gauge(descriptor MetricDescriptor) Int64GaugeHandle { + registerInst(descriptor.Name, descriptor.Default) + handle := Int64GaugeHandle{ + MetricDescriptor: &descriptor, + } + MetricsRegistry[descriptor.Name] = &descriptor + return handle +} + +// Will take a list, write comments and rewrite tests/cleanup and then I think ready to send out... +// How do I even test this really? +// Internal only clear...I don't think it's worth it just for default set to do it in internal... + diff --git a/experimental/stats/instrumentregistry/instrument_registry.go b/experimental/stats/metricregistry/metric_registry.go similarity index 72% rename from experimental/stats/instrumentregistry/instrument_registry.go rename to experimental/stats/metricregistry/metric_registry.go index ed46e590dbec..c099de50014e 100644 --- a/experimental/stats/instrumentregistry/instrument_registry.go +++ b/experimental/stats/metricregistry/metric_registry.go @@ -22,8 +22,9 @@ package instrumentregistry import "google.golang.org/grpc/stats" -// Int64CountHandle is a typed handle for a int count instrument. This handle is -// passed at the recording point in order to know which instrument to record on. +// Int64CountHandle is a typed handle for an int count instrument. This handle +// is passed at the recording point in order to know which instrument to record +// on. type Int64CountHandle struct { Index int } @@ -35,9 +36,9 @@ type Float64CountHandle struct { Index int } -// Int64HistoHandle is a typed handle for a int histogram instrument. This handle -// is passed at the recording point in order to know which instrument to record -// on. +// Int64HistoHandle is a typed handle for an int histogram instrument. This +// handle is passed at the recording point in order to know which instrument to +// record on. type Int64HistoHandle struct { Index int } @@ -49,8 +50,9 @@ type Float64HistoHandle struct { Index int } -// Int64GaugeHandle is a typed handle for a int gauge instrument. This handle is -// passed at the recording point in order to know which instrument to record on. +// Int64GaugeHandle is a typed handle for an int gauge instrument. This handle +// is passed at the recording point in order to know which instrument to record +// on. type Int64GaugeHandle struct { Index int } @@ -58,4 +60,9 @@ type Int64GaugeHandle struct { // DefaultMetrics are the default metrics registered through global instruments // registry. This is written to at initialization time only, and is read // only after initialization. -var DefaultMetrics = make(map[stats.Metric]bool) +var DefaultMetrics = stats.NewMetrics() // this I think, can join at runtime... + +// DefaultMetrics() helper in OTel that merges with this... + +// And have operations on the set including Merge... + diff --git a/experimental/stats/metricregistry_test.go b/experimental/stats/metricregistry_test.go new file mode 100644 index 000000000000..0eec803b4b30 --- /dev/null +++ b/experimental/stats/metricregistry_test.go @@ -0,0 +1,135 @@ +/* + * + * Copyright 2024 gRPC 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 stats + +import ( + "google.golang.org/grpc/experimental/stats/metricregistry" + "testing" +) + +func (s) TestMetricRegistry(t *testing.T) { + + // Now there's an implied metrics recorder as part of record calls... + + // Component from client conn will satisfy interface... + + // Same thing create instruments, pass a metrics recorder, instrument is expected to call that metrics recorder + + // Register one of each instrument, verify the metrics recorder works with it... + +} + +type fakeMetricsRecorder struct { + t *testing.T + + // 5 different or for one for ints/floats...? + + // Test the maps built out in OTel (mention to Doug this represents it...) + intValues map[*MetricDescriptor]int64 + floatValues map[*MetricDescriptor]float64 +} + +// MetricsRecorderList layer just looks at labels/optional labels... + +// newFakeMetricsRecorder returns a fake metrics recorder based off the current +// state of global instrument registry. +func newFakeMetricsRecorder(t *testing.T) *fakeMetricsRecorder { + // Access globals, build a map like OTel would + MetricsRegistry // map[stats.Metric]->Pointer to metrics descriptor, yeah let's test this out, make sure pointer can be used as key value... +} + +// verifyLabels verifies that all of the labels keys expected are present in the +// labels received. +func verifyLabels(t *testing.T, labelsWant []string, optionalLabelsWant []string, labelsGot []stats.Label, optionalLabelsGot []stats.Label) { + for i, label := range labelsWant { + if labelsGot[i].Key != label { + t.Fatalf("label key at position %v got %v, want %v", i, labelsGot[i].Key, label) + } + } + if len(labelsWant) != len(labelsGot) { + t.Fatalf("length of labels expected did not match got %v, want %v", len(labelsGot), len(optionalLabelsWant)) + } + + for i, label := range optionalLabelsWant { + if optionalLabelsGot[i].Key != label { + t.Fatalf("optional label key at position %v got %v, want %v", i, optionalLabelsGot[i].Key, label) + } + } + if len(optionalLabelsWant) != len(optionalLabelsGot) { + t.Fatalf("length of optional labels expected did not match got %v, want %v", len(optionalLabelsGot), len(optionalLabelsWant)) + } +} + +// Test 2 for each? 5 different maps...? + +// All the operations will get a handle with pointer above, make sure it can use to record... + +// Need a clear for testing... + +// It still implements these methods but gets called from handle +func (r *fakeMetricsRecorder) RecordIntCount(handle Int64CountHandle, labels []Label, optionalLabels []Label, incr int64) { // Techncialy this owuld eat labels...verifyLabels too? + // Rather than reading from registry/building out data structures, labels come from handle + handle.MetricDescriptor.Labels // []string also makes sure not nil...MetricDescriptor + + handle.MetricDescriptor.OptionalLabels // []string + + verifyLabels(r.t, handle.MetricDescriptor.Labels, handle.MetricDescriptor.OptionalLabels, labels, optionalLabels) + + // Overall data structure of the stats handler... + // map[name]->local would create a string comp + + // map[*MetricDescriptor]->local would just be a pointer comp... + + // record incr against data structure built out maybe map[name]-> + // No it's a map of metricdescriptor... + // How to build this out? + r.intValues[handle.MetricDescriptor] += incr // have the handle in main use that to verify... + +} + +func (r *fakeMetricsRecorder) RecordFloatCount(handle Float64CountHandle, labels []Label, optionalLabels []Label, incr float64) { + verifyLabels(r.t, handle.MetricDescriptor.Labels, handle.MetricDescriptor.OptionalLabels, labels, optionalLabels) + + handle.MetricDescriptor // *MetricDescriptor - use as key to map if not found then fatalf + + r.floatValues[handle.MetricDescriptor] += incr +} + +func (r *fakeMetricsRecorder) RecordIntHisto(handle Int64HistoHandle, labels []Label, optionalLabels []Label, incr int64) { + verifyLabels(r.t, handle.MetricDescriptor.Labels, handle.MetricDescriptor.OptionalLabels, labels, optionalLabels) + + handle.MetricDescriptor // *MetricDescriptor - use as key to map if not found then fatalf + + r.intValues[handle.MetricDescriptor] += incr // after 5 of these, makes sure they don't collide +} + +func (r *fakeMetricsRecorder) RecordFloatHisto(handle Float64HistoHandle, labels []Label, optionalLabels []Label, incr float64) { + verifyLabels(r.t, handle.MetricDescriptor.Labels, handle.MetricDescriptor.OptionalLabels, labels, optionalLabels) + + handle.MetricDescriptor // *MetricDescriptor - use as key to map if not found then fatalf +} + +func (r *fakeMetricsRecorder) RecordIntGauge(handle Int64GaugeHandle, labels []Label, optionalLabels []Label, incr int64) { + verifyLabels(r.t, handle.MetricDescriptor.Labels, handle.MetricDescriptor.OptionalLabels, labels, optionalLabels) + + handle.MetricDescriptor // *MetricDescriptor - use as key to map if not found then fatalf +} + +// If run out of time just push implementation...otel and metrics recorder list still come after I guess... +// just push the extra file.... diff --git a/experimental/stats/stats.go b/experimental/stats/stats.go index efae8cc5a6ec..79a758675cee 100644 --- a/experimental/stats/stats.go +++ b/experimental/stats/stats.go @@ -19,25 +19,24 @@ // Package stats contains experimental metrics/stats API's. package stats -import "google.golang.org/grpc/experimental/stats/instrumentregistry" // MetricsRecorder records on metrics derived from instrument registry. type MetricsRecorder interface { // RecordIntCount records the measurement alongside labels on the int count // associated with the provided handle. - RecordIntCount(instrumentregistry.Int64CountHandle, []Label, []Label, int64) + RecordIntCount(Int64CountHandle, []Label, []Label, int64) // RecordFloatCount records the measurement alongside labels on the float count // associated with the provided handle. - RecordFloatCount(instrumentregistry.Float64CountHandle, []Label, []Label, float64) + RecordFloatCount(Float64CountHandle, []Label, []Label, float64) // RecordIntHisto records the measurement alongside labels on the int histo // associated with the provided handle. - RecordIntHisto(instrumentregistry.Int64HistoHandle, []Label, []Label, int64) + RecordIntHisto(Int64HistoHandle, []Label, []Label, int64) // RecordFloatHisto records the measurement alongside labels on the float // histo associated with the provided handle. - RecordFloatHisto(instrumentregistry.Float64CountHandle, []Label, []Label, float64) + RecordFloatHisto(Float64HistoHandle, []Label, []Label, float64) // RecordIntGauge records the measurement alongside labels on the int gauge // associated with the provided handle. - RecordIntGauge(instrumentregistry.Int64GaugeHandle, []Label, []Label, int64) + RecordIntGauge(Int64GaugeHandle, []Label, []Label, int64) } // Label represents a string attribute/label to attach to metrics. From 7c051c05935b49e256805f0126d5c48553fc8c3f Mon Sep 17 00:00:00 2001 From: Zach Reyes Date: Wed, 3 Jul 2024 20:16:34 -0400 Subject: [PATCH 05/13] Implemented offline discussion --- experimental/stats/metricregistry.go | 20 +- .../stats/metricregistry/metric_registry.go | 68 ---- experimental/stats/metricregistry_test.go | 15 + experimental/stats/stats.go | 18 +- .../instrumentregistry/instrument_registry.go | 199 ---------- .../instrument_registry_test.go | 350 ------------------ stats/metrics.go | 89 +++++ stats/stats.go | 68 ---- 8 files changed, 119 insertions(+), 708 deletions(-) delete mode 100644 experimental/stats/metricregistry/metric_registry.go delete mode 100644 internal/stats/instrumentregistry/instrument_registry.go delete mode 100644 internal/stats/instrumentregistry/instrument_registry_test.go create mode 100644 stats/metrics.go diff --git a/experimental/stats/metricregistry.go b/experimental/stats/metricregistry.go index 932169d4bc53..f9a56c0fef84 100644 --- a/experimental/stats/metricregistry.go +++ b/experimental/stats/metricregistry.go @@ -55,8 +55,8 @@ type Int64CountHandle struct { // RecordInt64Count records the int64 count value on the metrics recorder // provided. -func (h *Int64CountHandle) RecordInt64Count(recorder MetricsRecorder, labels []Label, optionalLabels []Label, incr int64) { - recorder.RecordIntCount(*h, labels, optionalLabels, incr) +func (h Int64CountHandle) RecordInt64Count(recorder MetricsRecorder, incr int64, labels ...string) { + recorder.RecordIntCount(h, incr, labels...) } // Float64CountHandle is a typed handle for a float count instrument. This handle @@ -68,8 +68,8 @@ type Float64CountHandle struct { // RecordFloat64Count records the float64 count value on the metrics recorder // provided. -func (h *Float64CountHandle) RecordFloat64Count(recorder MetricsRecorder, labels []Label, optionalLabels []Label, incr float64) { - recorder.RecordFloatCount(*h, labels, optionalLabels, incr) +func (h Float64CountHandle) RecordFloat64Count(recorder MetricsRecorder, incr float64, labels ...string) { + recorder.RecordFloatCount(h, incr, labels...) } // Int64HistoHandle is a typed handle for an int histogram instrument. This @@ -81,8 +81,8 @@ type Int64HistoHandle struct { // RecordInt64Histo records the int64 histo value on the metrics recorder // provided. -func (h *Int64HistoHandle) RecordInt64Histo(recorder MetricsRecorder, labels []Label, optionalLabels []Label, incr int64) { - recorder.RecordIntHisto(*h, labels, optionalLabels, incr) +func (h Int64HistoHandle) RecordInt64Histo(recorder MetricsRecorder, incr int64, labels ...string) { + recorder.RecordIntHisto(h, incr, labels...) } // Float64HistoHandle is a typed handle for a float histogram instrument. This @@ -94,8 +94,8 @@ type Float64HistoHandle struct { // RecordFloat64Histo records the float64 histo value on the metrics recorder // provided. -func (h *Float64HistoHandle) RecordFloat64Histo(recorder MetricsRecorder, labels []Label, optionalLabels []Label, incr float64) { - recorder.RecordFloatHisto(*h, labels, optionalLabels, incr) +func (h Float64HistoHandle) RecordFloat64Histo(recorder MetricsRecorder, incr float64, labels ...string) { + recorder.RecordFloatHisto(h, incr, labels...) } // Int64GaugeHandle is a typed handle for an int gauge instrument. This handle @@ -107,8 +107,8 @@ type Int64GaugeHandle struct { // RecordInt64Histo records the int64 histo value on the metrics recorder // provided. -func (h *Int64GaugeHandle) RecordInt64Gauge(recorder MetricsRecorder, labels []Label, optionalLabels []Label, incr int64) { - recorder.RecordIntGauge(*h, labels, optionalLabels, incr) +func (h Int64GaugeHandle) RecordInt64Gauge(recorder MetricsRecorder, incr int64, labels ...string) { + recorder.RecordIntGauge(h, incr, labels...) } // registeredInsts are the registered instrument descriptor names. diff --git a/experimental/stats/metricregistry/metric_registry.go b/experimental/stats/metricregistry/metric_registry.go deleted file mode 100644 index c099de50014e..000000000000 --- a/experimental/stats/metricregistry/metric_registry.go +++ /dev/null @@ -1,68 +0,0 @@ -/* - * - * Copyright 2024 gRPC 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 instrumentregistry contains the experimental instrument registry data -// structures exposed to users. -package instrumentregistry - -import "google.golang.org/grpc/stats" - -// Int64CountHandle is a typed handle for an int count instrument. This handle -// is passed at the recording point in order to know which instrument to record -// on. -type Int64CountHandle struct { - Index int -} - -// Float64CountHandle is a typed handle for a float count instrument. This handle -// is passed at the recording point in order to know which instrument to record -// on. -type Float64CountHandle struct { - Index int -} - -// Int64HistoHandle is a typed handle for an int histogram instrument. This -// handle is passed at the recording point in order to know which instrument to -// record on. -type Int64HistoHandle struct { - Index int -} - -// Float64HistoHandle is a typed handle for a float histogram instrument. This -// handle is passed at the recording point in order to know which instrument to -// record on. -type Float64HistoHandle struct { - Index int -} - -// Int64GaugeHandle is a typed handle for an int gauge instrument. This handle -// is passed at the recording point in order to know which instrument to record -// on. -type Int64GaugeHandle struct { - Index int -} - -// DefaultMetrics are the default metrics registered through global instruments -// registry. This is written to at initialization time only, and is read -// only after initialization. -var DefaultMetrics = stats.NewMetrics() // this I think, can join at runtime... - -// DefaultMetrics() helper in OTel that merges with this... - -// And have operations on the set including Merge... - diff --git a/experimental/stats/metricregistry_test.go b/experimental/stats/metricregistry_test.go index 0eec803b4b30..b9ce3ff1d29e 100644 --- a/experimental/stats/metricregistry_test.go +++ b/experimental/stats/metricregistry_test.go @@ -74,6 +74,9 @@ func verifyLabels(t *testing.T, labelsWant []string, optionalLabelsWant []string if len(optionalLabelsWant) != len(optionalLabelsGot) { t.Fatalf("length of optional labels expected did not match got %v, want %v", len(optionalLabelsGot), len(optionalLabelsWant)) } + + // This is essentially now a check of len(labels + optional labels) vs labels provided... + } // Test 2 for each? 5 different maps...? @@ -133,3 +136,15 @@ func (r *fakeMetricsRecorder) RecordIntGauge(handle Int64GaugeHandle, labels []L // If run out of time just push implementation...otel and metrics recorder list still come after I guess... // just push the extra file.... + + +// Tests sound good to Doug get this plumbing working... + +// switch the labels to be variadic args based on position, length check on labels + optional labels + +// optional labels are always plumbed up through otel, otel decides whether it +// wants the optional labels or not... + +// on handle and metrics recorder + + diff --git a/experimental/stats/stats.go b/experimental/stats/stats.go index 79a758675cee..cb4e2b851567 100644 --- a/experimental/stats/stats.go +++ b/experimental/stats/stats.go @@ -24,25 +24,17 @@ package stats type MetricsRecorder interface { // RecordIntCount records the measurement alongside labels on the int count // associated with the provided handle. - RecordIntCount(Int64CountHandle, []Label, []Label, int64) + RecordIntCount(Int64CountHandle, int64, ...string) // RecordFloatCount records the measurement alongside labels on the float count // associated with the provided handle. - RecordFloatCount(Float64CountHandle, []Label, []Label, float64) + RecordFloatCount(Float64CountHandle, float64, ...string) // RecordIntHisto records the measurement alongside labels on the int histo // associated with the provided handle. - RecordIntHisto(Int64HistoHandle, []Label, []Label, int64) + RecordIntHisto(Int64HistoHandle, int64, ...string) // RecordFloatHisto records the measurement alongside labels on the float // histo associated with the provided handle. - RecordFloatHisto(Float64HistoHandle, []Label, []Label, float64) + RecordFloatHisto(Float64HistoHandle, float64, ...string) // RecordIntGauge records the measurement alongside labels on the int gauge // associated with the provided handle. - RecordIntGauge(Int64GaugeHandle, []Label, []Label, int64) -} - -// Label represents a string attribute/label to attach to metrics. -type Label struct { - // Key is the key of the label. - Key string - // Value is the value of the label. - Value string + RecordIntGauge(Int64GaugeHandle, int64, ...string) } diff --git a/internal/stats/instrumentregistry/instrument_registry.go b/internal/stats/instrumentregistry/instrument_registry.go deleted file mode 100644 index 6de003b08e59..000000000000 --- a/internal/stats/instrumentregistry/instrument_registry.go +++ /dev/null @@ -1,199 +0,0 @@ -/* - * - * Copyright 2024 gRPC 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 instrumentregistry is a instrument registry that gRPC components can -// use to register instruments (metrics). -package instrumentregistry - -import ( - "log" - - "google.golang.org/grpc/experimental/stats/instrumentregistry" - "google.golang.org/grpc/stats" -) - -// InstrumentDescriptor is a data of a registered instrument (metric). -type InstrumentDescriptor struct { - // Name is the name of this metric. - Name string - // Description is the description of this metric. - Description string - // Unit is the unit of this metric. - Unit string - // Labels are the required label keys for this metric. - Labels []string - // OptionalLabels are the optional label keys for this - // metric. - OptionalLabels []string - // Default is whether this metric is on by default. - Default bool -} - -// All of these globals are written to at initialization time only, and are read -// only after initialization. - -// Int64CountInsts is information about registered int 64 count instruments in -// order of registration. -var Int64CountInsts []InstrumentDescriptor - -// Float64CountInsts is information about registered float 64 count instruments -// in order of registration. -var Float64CountInsts []InstrumentDescriptor - -// Int64HistoInsts is information about registered int 64 histo instruments in -// order of registration. -var Int64HistoInsts []InstrumentDescriptor - -// Float64HistoInsts is information about registered float 64 histo instruments -// in order of registration. -var Float64HistoInsts []InstrumentDescriptor - -// Int64GaugeInsts is information about registered int 64 gauge instruments in -// order of registration. -var Int64GaugeInsts []InstrumentDescriptor - -// registeredInsts are the registered instrument descriptor names. -var registeredInsts = make(map[string]bool) - -// ClearInstrumentRegistryForTesting clears the instrument registry for testing -// purposes only. -func ClearInstrumentRegistryForTesting() { - Int64CountInsts = nil - Float64CountInsts = nil - Int64HistoInsts = nil - Float64HistoInsts = nil - Int64GaugeInsts = nil - registeredInsts = make(map[string]bool) - instrumentregistry.DefaultMetrics = make(map[stats.Metric]bool) -} - -func registerInst(name string, def bool) { - if registeredInsts[name] { - log.Panicf("instrument %v already registered", name) - } - registeredInsts[name] = true - if def { - instrumentregistry.DefaultMetrics[stats.Metric(name)] = true - } -} - -// RegisterInt64Count registers the int count instrument description onto the -// global registry. It returns a typed handle to use when recording data. -// -// NOTE: this function must only be called during initialization time (i.e. in -// an init() function), and is not thread-safe. If multiple instruments are -// registered with the same name, this function will panic. -func RegisterInt64Count(name string, desc string, unit string, labels []string, optionalLabels []string, def bool) instrumentregistry.Int64CountHandle { - registerInst(name, def) - Int64CountInsts = append(Int64CountInsts, InstrumentDescriptor{ - Name: name, - Description: desc, - Unit: unit, - Labels: labels, - OptionalLabels: optionalLabels, - Default: def, - }) - return instrumentregistry.Int64CountHandle{ - Index: len(Int64CountInsts) - 1, - } -} - -// RegisterFloat64Count registers the float count instrument description onto the -// global registry. It returns a typed handle to use when recording data. -// -// NOTE: this function must only be called during initialization time (i.e. in -// an init() function), and is not thread-safe. If multiple instruments are -// registered with the same name, this function will panic. -func RegisterFloat64Count(name string, desc string, unit string, labels []string, optionalLabels []string, def bool) instrumentregistry.Float64CountHandle { - registerInst(name, def) - Float64CountInsts = append(Float64CountInsts, InstrumentDescriptor{ - Name: name, - Description: desc, - Unit: unit, - Labels: labels, - OptionalLabels: optionalLabels, - Default: def, - }) - return instrumentregistry.Float64CountHandle{ - Index: len(Float64CountInsts) - 1, - } -} - -// RegisterInt64Histo registers the int histogram instrument description onto the -// global registry. It returns a typed handle to use when recording data. -// -// NOTE: this function must only be called during initialization time (i.e. in -// an init() function), and is not thread-safe. If multiple instruments are -// registered with the same name, this function will panic. -func RegisterInt64Histo(name string, desc string, unit string, labels []string, optionalLabels []string, def bool) instrumentregistry.Int64HistoHandle { - registerInst(name, def) - Int64HistoInsts = append(Int64HistoInsts, InstrumentDescriptor{ - Name: name, - Description: desc, - Unit: unit, - Labels: labels, - OptionalLabels: optionalLabels, - Default: def, - }) - return instrumentregistry.Int64HistoHandle{ - Index: len(Int64HistoInsts) - 1, - } -} - -// RegisterFloat64Histo registers the float histogram instrument description -// onto the global registry. It returns a typed handle to use when recording -// data. -// -// NOTE: this function must only be called during initialization time (i.e. in -// an init() function), and is not thread-safe. If multiple instruments are -// registered with the same name, this function will panic. -func RegisterFloat64Histo(name string, desc string, unit string, labels []string, optionalLabels []string, def bool) instrumentregistry.Float64HistoHandle { - registerInst(name, def) - Float64HistoInsts = append(Float64HistoInsts, InstrumentDescriptor{ - Name: name, - Description: desc, - Unit: unit, - Labels: labels, - OptionalLabels: optionalLabels, - Default: def, - }) - return instrumentregistry.Float64HistoHandle{ - Index: len(Float64HistoInsts) - 1, - } -} - -// RegisterInt64Gauge registers the int gauge instrument description onto the -// global registry. It returns a typed handle to use when recording data. -// -// NOTE: this function must only be called during initialization time (i.e. in -// an init() function), and is not thread-safe. If multiple instruments are -// registered with the same name, this function will panic. -func RegisterInt64Gauge(name string, desc string, unit string, labels []string, optionalLabels []string, def bool) instrumentregistry.Int64GaugeHandle { - registerInst(name, def) - Int64GaugeInsts = append(Int64GaugeInsts, InstrumentDescriptor{ - Name: name, - Description: desc, - Unit: unit, - Labels: labels, - OptionalLabels: optionalLabels, - Default: def, - }) - return instrumentregistry.Int64GaugeHandle{ - Index: len(Int64GaugeInsts) - 1, - } -} diff --git a/internal/stats/instrumentregistry/instrument_registry_test.go b/internal/stats/instrumentregistry/instrument_registry_test.go deleted file mode 100644 index 362082d151bf..000000000000 --- a/internal/stats/instrumentregistry/instrument_registry_test.go +++ /dev/null @@ -1,350 +0,0 @@ -/* - * - * Copyright 2024 gRPC 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 instrumentregistry - -import ( - "testing" - - "github.com/google/go-cmp/cmp" - "google.golang.org/grpc/experimental/stats" - "google.golang.org/grpc/experimental/stats/instrumentregistry" - "google.golang.org/grpc/internal/grpctest" -) - -type s struct { - grpctest.Tester -} - -func Test(t *testing.T) { - grpctest.RunSubTests(t, s{}) -} - -// TestPanic tests that registering two instruments with the same name across -// any type of instrument triggers a panic. -func (s) TestPanic(t *testing.T) { - defer ClearInstrumentRegistryForTesting() - want := "instrument simple counter already registered" - defer func() { - if r := recover(); r != "instrument simple counter already registered" { - t.Errorf("expected panic %q, got %q", want, r) - } - }() - RegisterInt64Count("simple counter", "number of times recorded on tests", "calls", nil, nil, false) - RegisterInt64Gauge("simple counter", "number of times recorded on tests", "calls", nil, nil, false) -} - -type int64WithLabels struct { - value int64 - labels []string - optionalLabels []string -} - -type float64WithLabels struct { - value float64 - labels []string - optionalLabels []string -} - -// fakeMetricsRecorder persists data and labels based off the global instrument -// registry for use by tests. The data is passed to this recorder from a test -// using the instrument registry's handles. -// -// Do not construct directly; use newFakeMetricsRecorder() instead. -type fakeMetricsRecorder struct { - t *testing.T - - int64counts []int64WithLabels - float64counts []float64WithLabels - int64histos []int64WithLabels - float64histos []float64WithLabels - int64gauges []int64WithLabels -} - -// newFakeMetricsRecorder returns a fake metrics recorder based off the current -// state of global instrument registry. -func newFakeMetricsRecorder(t *testing.T) *fakeMetricsRecorder { - fmr := &fakeMetricsRecorder{t: t} - for _, inst := range Int64CountInsts { - fmr.int64counts = append(fmr.int64counts, int64WithLabels{ - labels: inst.Labels, - optionalLabels: inst.OptionalLabels, - }) - } - for _, inst := range Float64CountInsts { - fmr.float64counts = append(fmr.float64counts, float64WithLabels{ - labels: inst.Labels, - optionalLabels: inst.OptionalLabels, - }) - } - for _, inst := range Int64HistoInsts { - fmr.int64histos = append(fmr.int64histos, int64WithLabels{ - labels: inst.Labels, - optionalLabels: inst.OptionalLabels, - }) - } - for _, inst := range Float64HistoInsts { - fmr.float64histos = append(fmr.float64histos, float64WithLabels{ - labels: inst.Labels, - optionalLabels: inst.OptionalLabels, - }) - } - for _, inst := range Int64GaugeInsts { - fmr.int64gauges = append(fmr.int64gauges, int64WithLabels{ - labels: inst.Labels, - optionalLabels: inst.OptionalLabels, - }) - } - return fmr -} - -// TestInstrumentRegistry tests the instrument registry. It registers testing -// only instruments using the instrument registry, and creates a fake metrics -// recorder which uses these instruments. Using the handles returned from the -// instrument registry, this test records stats using the fake metrics recorder. -// Then, the test verifies the persisted metrics data in the metrics recorder is -// what is expected. Thus, this tests the interactions between the metrics -// recorder and the instruments registry. -func (s) TestInstrumentRegistry(t *testing.T) { - defer ClearInstrumentRegistryForTesting() - intCountHandle1 := RegisterInt64Count("int counter", "number of times recorded on tests", "calls", []string{"int counter label"}, []string{"int counter optional label"}, false) - RegisterInt64Count("int counter 2", "number of times recorded on tests", "calls", []string{"int counter 2 label"}, []string{"int counter 2 optional label"}, false) - - floatCountHandle1 := RegisterFloat64Count("float counter", "number of times recorded on tests", "calls", []string{"float counter label"}, []string{"float counter optional label"}, false) - intHistoHandle1 := RegisterInt64Histo("int histo", "", "calls", []string{"int histo label"}, []string{"int histo optional label"}, false) - floatHistoHandle1 := RegisterFloat64Histo("float histo", "", "calls", []string{"float histo label"}, []string{"float histo optional label"}, false) - intGaugeHandle1 := RegisterInt64Gauge("simple gauge", "the most recent int emitted by test", "int", []string{"int gauge label"}, []string{"int gauge optional label"}, false) - - fmr := newFakeMetricsRecorder(t) - fmr.RecordIntCount(intCountHandle1, []stats.Label{{Key: "int counter label", Value: "some value"}}, []stats.Label{{Key: "int counter optional label", Value: "some value"}}, 1) - - intWithLabelsWant := []int64WithLabels{ - { - value: 1, - labels: []string{"int counter label"}, - optionalLabels: []string{"int counter optional label"}, - }, - { - value: 0, - labels: []string{"int counter 2 label"}, - optionalLabels: []string{"int counter 2 optional label"}, - }, - } - if diff := cmp.Diff(fmr.int64counts, intWithLabelsWant, cmp.AllowUnexported(int64WithLabels{})); diff != "" { - t.Fatalf("fmr.int64counts (-got, +want): %v", diff) - } - - fmr.RecordFloatCount(floatCountHandle1, []stats.Label{{Key: "float counter label", Value: "some value"}}, []stats.Label{{Key: "float counter optional label", Value: "some value"}}, 1.2) - floatWithLabelsWant := []float64WithLabels{ - { - value: 1.2, - labels: []string{"float counter label"}, - optionalLabels: []string{"float counter optional label"}, - }, - } - if diff := cmp.Diff(fmr.float64counts, floatWithLabelsWant, cmp.AllowUnexported(float64WithLabels{})); diff != "" { - t.Fatalf("fmr.float64counts (-got, +want): %v", diff) - } - - fmr.RecordIntHisto(intHistoHandle1, []stats.Label{{Key: "int histo label", Value: "some value"}}, []stats.Label{{Key: "int histo optional label", Value: "some value"}}, 3) - intHistoWithLabelsWant := []int64WithLabels{ - { - value: 3, - labels: []string{"int histo label"}, - optionalLabels: []string{"int histo optional label"}, - }, - } - if diff := cmp.Diff(fmr.int64histos, intHistoWithLabelsWant, cmp.AllowUnexported(int64WithLabels{})); diff != "" { - t.Fatalf("fmr.int64histos (-got, +want): %v", diff) - } - - fmr.RecordFloatHisto(floatHistoHandle1, []stats.Label{{Key: "float histo label", Value: "some value"}}, []stats.Label{{Key: "float histo optional label", Value: "some value"}}, 4) - floatHistoWithLabelsWant := []float64WithLabels{ - { - value: 4, - labels: []string{"float histo label"}, - optionalLabels: []string{"float histo optional label"}, - }, - } - if diff := cmp.Diff(fmr.float64histos, floatHistoWithLabelsWant, cmp.AllowUnexported(float64WithLabels{})); diff != "" { - t.Fatalf("fmr.float64histos (-got, +want): %v", diff) - } - - fmr.RecordIntGauge(intGaugeHandle1, []stats.Label{{Key: "int gauge label", Value: "some value"}}, []stats.Label{{Key: "int gauge optional label", Value: "some value"}}, 7) - intGaugeWithLabelsWant := []int64WithLabels{ - { - value: 7, - labels: []string{"int gauge label"}, - optionalLabels: []string{"int gauge optional label"}, - }, - } - if diff := cmp.Diff(fmr.int64gauges, intGaugeWithLabelsWant, cmp.AllowUnexported(int64WithLabels{})); diff != "" { - t.Fatalf("fmr.int64gauges (-got, +want): %v", diff) - } -} - -// TestNumerousIntCounts tests numerous int count instruments registered onto -// the instrument registry. A component (simulated by test) should be able to -// record on the different registered int count instruments. -func (s) TestNumerousIntCounts(t *testing.T) { - defer ClearInstrumentRegistryForTesting() - intCountHandle1 := RegisterInt64Count("int counter", "number of times recorded on tests", "calls", []string{"int counter label"}, []string{"int counter optional label"}, false) - intCountHandle2 := RegisterInt64Count("int counter 2", "number of times recorded on tests", "calls", []string{"int counter 2 label"}, []string{"int counter 2 optional label"}, false) - intCountHandle3 := RegisterInt64Count("int counter 3", "number of times recorded on tests", "calls", []string{"int counter 3 label"}, []string{"int counter 3 optional label"}, false) - fmr := newFakeMetricsRecorder(t) - - fmr.RecordIntCount(intCountHandle1, []stats.Label{{Key: "int counter label", Value: "some value"}}, []stats.Label{{Key: "int counter optional label", Value: "some value"}}, 1) - intWithLabelsWant := []int64WithLabels{ - { - value: 1, - labels: []string{"int counter label"}, - optionalLabels: []string{"int counter optional label"}, - }, - { - value: 0, - labels: []string{"int counter 2 label"}, - optionalLabels: []string{"int counter 2 optional label"}, - }, - { - value: 0, - labels: []string{"int counter 3 label"}, - optionalLabels: []string{"int counter 3 optional label"}, - }, - } - if diff := cmp.Diff(fmr.int64counts, intWithLabelsWant, cmp.AllowUnexported(int64WithLabels{})); diff != "" { - t.Fatalf("fmr.int64counts (-got, +want): %v", diff) - } - - fmr.RecordIntCount(intCountHandle2, []stats.Label{{Key: "int counter 2 label", Value: "some value"}}, []stats.Label{{Key: "int counter 2 optional label", Value: "some value"}}, 1) - intWithLabelsWant = []int64WithLabels{ - { - value: 1, - labels: []string{"int counter label"}, - optionalLabels: []string{"int counter optional label"}, - }, - { - value: 1, - labels: []string{"int counter 2 label"}, - optionalLabels: []string{"int counter 2 optional label"}, - }, - { - value: 0, - labels: []string{"int counter 3 label"}, - optionalLabels: []string{"int counter 3 optional label"}, - }, - } - if diff := cmp.Diff(fmr.int64counts, intWithLabelsWant, cmp.AllowUnexported(int64WithLabels{})); diff != "" { - t.Fatalf("fmr.int64counts (-got, +want): %v", diff) - } - - fmr.RecordIntCount(intCountHandle3, []stats.Label{{Key: "int counter 3 label", Value: "some value"}}, []stats.Label{{Key: "int counter 3 optional label", Value: "some value"}}, 1) - intWithLabelsWant = []int64WithLabels{ - { - value: 1, - labels: []string{"int counter label"}, - optionalLabels: []string{"int counter optional label"}, - }, - { - value: 1, - labels: []string{"int counter 2 label"}, - optionalLabels: []string{"int counter 2 optional label"}, - }, - { - value: 1, - labels: []string{"int counter 3 label"}, - optionalLabels: []string{"int counter 3 optional label"}, - }, - } - if diff := cmp.Diff(fmr.int64counts, intWithLabelsWant, cmp.AllowUnexported(int64WithLabels{})); diff != "" { - t.Fatalf("fmr.int64counts (-got, +want): %v", diff) - } - - fmr.RecordIntCount(intCountHandle3, []stats.Label{{Key: "int counter 3 label", Value: "some value"}}, []stats.Label{{Key: "int counter 3 optional label", Value: "some value"}}, 1) - intWithLabelsWant = []int64WithLabels{ - { - value: 1, - labels: []string{"int counter label"}, - optionalLabels: []string{"int counter optional label"}, - }, - { - value: 1, - labels: []string{"int counter 2 label"}, - optionalLabels: []string{"int counter 2 optional label"}, - }, - { - value: 2, - labels: []string{"int counter 3 label"}, - optionalLabels: []string{"int counter 3 optional label"}, - }, - } - if diff := cmp.Diff(fmr.int64counts, intWithLabelsWant, cmp.AllowUnexported(int64WithLabels{})); diff != "" { - t.Fatalf("fmr.int64counts (-got, +want): %v", diff) - } -} - -// verifyLabels verifies that all of the labels keys expected are present in the -// labels received. -func verifyLabels(t *testing.T, labelsWant []string, optionalLabelsWant []string, labelsGot []stats.Label, optionalLabelsGot []stats.Label) { - for i, label := range labelsWant { - if labelsGot[i].Key != label { - t.Fatalf("label key at position %v got %v, want %v", i, labelsGot[i].Key, label) - } - } - if len(labelsWant) != len(labelsGot) { - t.Fatalf("length of labels expected did not match got %v, want %v", len(labelsGot), len(optionalLabelsWant)) - } - - for i, label := range optionalLabelsWant { - if optionalLabelsGot[i].Key != label { - t.Fatalf("optional label key at position %v got %v, want %v", i, optionalLabelsGot[i].Key, label) - } - } - if len(optionalLabelsWant) != len(optionalLabelsGot) { - t.Fatalf("length of optional labels expected did not match got %v, want %v", len(optionalLabelsGot), len(optionalLabelsWant)) - } -} - -func (r *fakeMetricsRecorder) RecordIntCount(handle instrumentregistry.Int64CountHandle, labels []stats.Label, optionalLabels []stats.Label, incr int64) { - ic := r.int64counts[handle.Index] - verifyLabels(r.t, ic.labels, ic.optionalLabels, labels, optionalLabels) - r.int64counts[handle.Index].value += incr -} - -func (r *fakeMetricsRecorder) RecordFloatCount(handle instrumentregistry.Float64CountHandle, labels []stats.Label, optionalLabels []stats.Label, incr float64) { - fc := r.float64counts[handle.Index] - verifyLabels(r.t, fc.labels, fc.optionalLabels, labels, optionalLabels) - r.float64counts[handle.Index].value += incr -} - -func (r *fakeMetricsRecorder) RecordIntHisto(handle instrumentregistry.Int64HistoHandle, labels []stats.Label, optionalLabels []stats.Label, incr int64) { - ih := r.int64histos[handle.Index] - verifyLabels(r.t, ih.labels, ih.optionalLabels, labels, optionalLabels) - r.int64histos[handle.Index].value = incr -} - -func (r *fakeMetricsRecorder) RecordFloatHisto(handle instrumentregistry.Float64HistoHandle, labels []stats.Label, optionalLabels []stats.Label, incr float64) { - fh := r.float64histos[handle.Index] - verifyLabels(r.t, fh.labels, fh.optionalLabels, labels, optionalLabels) - r.float64histos[handle.Index].value = incr -} - -func (r *fakeMetricsRecorder) RecordIntGauge(handle instrumentregistry.Int64GaugeHandle, labels []stats.Label, optionalLabels []stats.Label, incr int64) { - ig := r.int64gauges[handle.Index] - verifyLabels(r.t, ig.labels, ig.optionalLabels, labels, optionalLabels) - r.int64gauges[handle.Index].value = incr -} diff --git a/stats/metrics.go b/stats/metrics.go new file mode 100644 index 000000000000..8c0418575fb0 --- /dev/null +++ b/stats/metrics.go @@ -0,0 +1,89 @@ +/* + * + * Copyright 2024 gRPC 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 stats + +import "maps" + +// Metric is an identifier for a metric. +type Metric string + +// Metrics is a set of metrics to record. Once created, Metrics is immutable, +// however Add and Remove can make copies with specific metrics added or +// removed, respectively. +// +// Do not construct directly; use NewMetrics instead. +type Metrics struct { + // Metrics are the set of Metrics to initialize. + Metrics map[Metric]bool +} // map[stats.Metric]bool, DefaultMetrics, this nicely wraps (can call adds and just throw away old memory) and then joins on the Metrics Set are clean... + +// NewMetrics returns a Metrics containing Metrics. +func NewMetrics(metrics ...Metric) *Metrics { + newMetrics := make(map[Metric]bool) + for _, metric := range metrics { + newMetrics[metric] = true + } + return &Metrics{ + Metrics: newMetrics, + } +} + +// Add adds the metrics to the metrics set and returns a new copy with the +// additional metrics. +func (m *Metrics) Add(metrics ...Metric) *Metrics { + newMetrics := make(map[Metric]bool) + for metric := range m.Metrics { + newMetrics[metric] = true + } + + for _, metric := range metrics { + newMetrics[metric] = true + } + return &Metrics{ + Metrics: newMetrics, + } +} + +// Join joins the metrics passed in with the metrics set, and returns a new copy +// with the merged metrics. +func (m *Metrics) Join(metrics *Metrics) *Metrics { // Use this API... + newMetrics := make(map[Metric]bool) + maps.Copy(newMetrics, m.Metrics) + maps.Copy(newMetrics, metrics.Metrics) + return &Metrics{ + Metrics: newMetrics, + } +} + +// Remove removes the metrics from the metrics set and returns a new copy with +// the metrics removed. +func (m *Metrics) Remove(metrics ...Metric) *Metrics { + newMetrics := make(map[Metric]bool) + for metric := range m.Metrics { + newMetrics[metric] = true + } + + for _, metric := range metrics { + delete(newMetrics, metric) + } + return &Metrics{ + Metrics: newMetrics, + } +} + diff --git a/stats/stats.go b/stats/stats.go index 77ec5a3478e7..fdb0bd65182c 100644 --- a/stats/stats.go +++ b/stats/stats.go @@ -23,7 +23,6 @@ package stats // import "google.golang.org/grpc/stats" import ( "context" - "maps" "net" "time" @@ -348,70 +347,3 @@ func OutgoingTrace(ctx context.Context) []byte { b, _ := ctx.Value(outgoingTraceKey{}).([]byte) return b } - -// Metric is an identifier for a metric. -type Metric string - -// Metrics is a set of metrics to record. Once created, Metrics is immutable, -// however Add and Remove can make copies with specific metrics added or -// removed, respectively. -// -// Do not construct directly; use NewMetrics instead. -type Metrics struct { - // Metrics are the set of Metrics to initialize. - Metrics map[Metric]bool -} - -// NewMetrics returns a Metrics containing Metrics. -func NewMetrics(metrics ...Metric) *Metrics { - newMetrics := make(map[Metric]bool) - for _, metric := range metrics { - newMetrics[metric] = true - } - return &Metrics{ - Metrics: newMetrics, - } -} - -// Add adds the metrics to the metrics set and returns a new copy with the -// additional metrics. -func (m *Metrics) Add(metrics ...Metric) *Metrics { - newMetrics := make(map[Metric]bool) - for metric := range m.Metrics { - newMetrics[metric] = true - } - - for _, metric := range metrics { - newMetrics[metric] = true - } - return &Metrics{ - Metrics: newMetrics, - } -} - -// Join joins the metrics passed in with the metrics set, and returns a new copy -// with the merged metrics. -func (m *Metrics) Join(metrics *Metrics) *Metrics { - newMetrics := make(map[Metric]bool) - maps.Copy(newMetrics, m.Metrics) - maps.Copy(newMetrics, metrics.Metrics) - return &Metrics{ - Metrics: newMetrics, - } -} - -// Remove removes the metrics from the metrics set and returns a new copy with -// the metrics removed. -func (m *Metrics) Remove(metrics ...Metric) *Metrics { - newMetrics := make(map[Metric]bool) - for metric := range m.Metrics { - newMetrics[metric] = true - } - - for _, metric := range metrics { - delete(newMetrics, metric) - } - return &Metrics{ - Metrics: newMetrics, - } -} From 97b48c1d47cbfbe195c103f4271487f0c7180289 Mon Sep 17 00:00:00 2001 From: Zach Reyes Date: Tue, 9 Jul 2024 19:09:19 -0400 Subject: [PATCH 06/13] Implemented offline discussion --- experimental/stats/metricregistry.go | 160 +++++++----- experimental/stats/metricregistry_test.go | 298 +++++++++++++++------- experimental/stats/stats.go | 1 - stats/metrics.go | 1 - 4 files changed, 300 insertions(+), 160 deletions(-) diff --git a/experimental/stats/metricregistry.go b/experimental/stats/metricregistry.go index f9a56c0fef84..0be95ae9d9e4 100644 --- a/experimental/stats/metricregistry.go +++ b/experimental/stats/metricregistry.go @@ -24,119 +24,120 @@ import ( "google.golang.org/grpc/stats" ) -// DefaultMetrics are the default metrics registered through global instruments -// registry. This is written to at initialization time only, and is read -// only after initialization. +// DefaultMetrics are the default metrics registered through global metrics +// registry. This is written to at initialization time only, and is read only +// after initialization. var DefaultMetrics = stats.NewMetrics() // MetricDescriptor is the data for a registered metric. type MetricDescriptor struct { - // Name is the name of this metric. + // Name is the name of this metric. This name must be unique across whole + // binary (including any per call metrics). See + // https://github.com/grpc/proposal/blob/master/A79-non-per-call-metrics-architecture.md#metric-instrument-naming-conventions + // for metric naming conventions. Name stats.Metric // Description is the description of this metric. Description string - // Unit is the unit of this metric. + // Unit is the unit of this metric (e.g. entries, milliseconds). Unit string - // Labels are the required label keys for this metric. + // Labels are the required label keys for this metric. These are intended to + // metrics emitted from a stats handler. Labels []string - // OptionalLabels are the optional label keys for this - // metric. + // OptionalLabels are the optional label keys for this metric. These are + // intended to attached to metrics emitted from a stats handler if + // configured. OptionalLabels []string // Default is whether this metric is on by default. Default bool + // Type is the type of metric. This is set by the metric registry, and not + // intended to be set by a component registering a metric. + Type MetricType } -// Int64CountHandle is a typed handle for a float count instrument. This handle -// is passed at the recording point in order to know which instrument to record +// Int64CountHandle is a typed handle for a float count metric. This handle +// is passed at the recording point in order to know which metric to record // on. type Int64CountHandle struct { MetricDescriptor *MetricDescriptor } -// RecordInt64Count records the int64 count value on the metrics recorder -// provided. -func (h Int64CountHandle) RecordInt64Count(recorder MetricsRecorder, incr int64, labels ...string) { +// Record records the int64 count value on the metrics recorder provided. +func (h Int64CountHandle) Record(recorder MetricsRecorder, incr int64, labels ...string) { recorder.RecordIntCount(h, incr, labels...) } -// Float64CountHandle is a typed handle for a float count instrument. This handle -// is passed at the recording point in order to know which instrument to record -// on. +// Float64CountHandle is a typed handle for a float count metric. This handle is +// passed at the recording point in order to know which metric to record on. type Float64CountHandle struct { MetricDescriptor *MetricDescriptor } -// RecordFloat64Count records the float64 count value on the metrics recorder -// provided. -func (h Float64CountHandle) RecordFloat64Count(recorder MetricsRecorder, incr float64, labels ...string) { +// Record records the float64 count value on the metrics recorder provided. +func (h Float64CountHandle) Record(recorder MetricsRecorder, incr float64, labels ...string) { recorder.RecordFloatCount(h, incr, labels...) } -// Int64HistoHandle is a typed handle for an int histogram instrument. This -// handle is passed at the recording point in order to know which instrument to -// record on. +// Int64HistoHandle is a typed handle for an int histogram metric. This handle +// is passed at the recording point in order to know which metric to record on. type Int64HistoHandle struct { MetricDescriptor *MetricDescriptor } -// RecordInt64Histo records the int64 histo value on the metrics recorder -// provided. -func (h Int64HistoHandle) RecordInt64Histo(recorder MetricsRecorder, incr int64, labels ...string) { +// Record records the int64 histo value on the metrics recorder provided. +func (h Int64HistoHandle) Record(recorder MetricsRecorder, incr int64, labels ...string) { recorder.RecordIntHisto(h, incr, labels...) } -// Float64HistoHandle is a typed handle for a float histogram instrument. This -// handle is passed at the recording point in order to know which instrument to +// Float64HistoHandle is a typed handle for a float histogram metric. This +// handle is passed at the recording point in order to know which metric to // record on. type Float64HistoHandle struct { MetricDescriptor *MetricDescriptor } -// RecordFloat64Histo records the float64 histo value on the metrics recorder -// provided. -func (h Float64HistoHandle) RecordFloat64Histo(recorder MetricsRecorder, incr float64, labels ...string) { +// Record records the float64 histo value on the metrics recorder provided. +func (h Float64HistoHandle) Record(recorder MetricsRecorder, incr float64, labels ...string) { recorder.RecordFloatHisto(h, incr, labels...) } -// Int64GaugeHandle is a typed handle for an int gauge instrument. This handle -// is passed at the recording point in order to know which instrument to record -// on. +// Int64GaugeHandle is a typed handle for an int gauge metric. This handle is +// passed at the recording point in order to know which metric to record on. type Int64GaugeHandle struct { MetricDescriptor *MetricDescriptor } -// RecordInt64Histo records the int64 histo value on the metrics recorder -// provided. -func (h Int64GaugeHandle) RecordInt64Gauge(recorder MetricsRecorder, incr int64, labels ...string) { +// Record records the int64 histo value on the metrics recorder provided. +func (h Int64GaugeHandle) Record(recorder MetricsRecorder, incr int64, labels ...string) { recorder.RecordIntGauge(h, incr, labels...) } -// registeredInsts are the registered instrument descriptor names. -var registeredInsts = make(map[stats.Metric]bool) +// registeredMetrics are the registered metric descriptor names. +var registeredMetrics = make(map[stats.Metric]bool) -// MetricsRegistry is all the registered metrics. +// MetricsRegistry are all of the registered metrics. // // This is written to only at init time, and read only after that. var MetricsRegistry = make(map[stats.Metric]*MetricDescriptor) -func registerInst(name stats.Metric, def bool) { - if registeredInsts[name] { - log.Panicf("instrument %v already registered", name) +func registerMetric(name stats.Metric, def bool) { + if registeredMetrics[name] { + log.Panicf("metric %v already registered", name) } - registeredInsts[name] = true + registeredMetrics[name] = true if def { DefaultMetrics = DefaultMetrics.Add(name) } } -// RegisterInt64Count registers the instrument description onto the global -// registry. It returns a typed handle to use to recording data. +// RegisterInt64Count registers the metric description onto the global registry. +// It returns a typed handle to use to recording data. // // NOTE: this function must only be called during initialization time (i.e. in -// an init() function), and is not thread-safe. If multiple instruments are +// an init() function), and is not thread-safe. If multiple metrics are // registered with the same name, this function will panic. -func RegisterInt64Count(descriptor MetricDescriptor) Int64CountHandle { - registerInst(descriptor.Name, descriptor.Default) +func RegisterInt64Count(descriptor MetricDescriptor) Int64CountHandle { + registerMetric(descriptor.Name, descriptor.Default) + descriptor.Type = MetricTypeIntCount handle := Int64CountHandle{ MetricDescriptor: &descriptor, } @@ -144,14 +145,15 @@ func RegisterInt64Count(descriptor MetricDescriptor) Int64CountHandle { return handle } -// RegisterFloat64Count registers the instrument description onto the global +// RegisterFloat64Count registers the metric description onto the global // registry. It returns a typed handle to use to recording data. // // NOTE: this function must only be called during initialization time (i.e. in -// an init() function), and is not thread-safe. If multiple instruments are +// an init() function), and is not thread-safe. If multiple metrics are // registered with the same name, this function will panic. func RegisterFloat64Count(descriptor MetricDescriptor) Float64CountHandle { - registerInst(descriptor.Name, descriptor.Default) + registerMetric(descriptor.Name, descriptor.Default) + descriptor.Type = MetricTypeFloatCount handle := Float64CountHandle{ MetricDescriptor: &descriptor, } @@ -159,14 +161,15 @@ func RegisterFloat64Count(descriptor MetricDescriptor) Float64CountHandle { return handle } -// RegisterInt64Histo registers the instrument description onto the global -// registry. It returns a typed handle to use to recording data. +// RegisterInt64Histo registers the metric description onto the global registry. +// It returns a typed handle to use to recording data. // // NOTE: this function must only be called during initialization time (i.e. in -// an init() function), and is not thread-safe. If multiple instruments are +// an init() function), and is not thread-safe. If multiple metrics are // registered with the same name, this function will panic. func RegisterInt64Histo(descriptor MetricDescriptor) Int64HistoHandle { - registerInst(descriptor.Name, descriptor.Default) + registerMetric(descriptor.Name, descriptor.Default) + descriptor.Type = MetricTypeIntHisto handle := Int64HistoHandle{ MetricDescriptor: &descriptor, } @@ -174,14 +177,15 @@ func RegisterInt64Histo(descriptor MetricDescriptor) Int64HistoHandle { return handle } -// RegisterFloat64Histo registers the instrument description onto the global +// RegisterFloat64Histo registers the metric description onto the global // registry. It returns a typed handle to use to recording data. // // NOTE: this function must only be called during initialization time (i.e. in -// an init() function), and is not thread-safe. If multiple instruments are +// an init() function), and is not thread-safe. If multiple metrics are // registered with the same name, this function will panic. func RegisterFloat64Histo(descriptor MetricDescriptor) Float64HistoHandle { - registerInst(descriptor.Name, descriptor.Default) + registerMetric(descriptor.Name, descriptor.Default) + descriptor.Type = MetricTypeFloatHisto handle := Float64HistoHandle{ MetricDescriptor: &descriptor, } @@ -189,14 +193,15 @@ func RegisterFloat64Histo(descriptor MetricDescriptor) Float64HistoHandle { return handle } -// RegisterInt64Gauge registers the instrument description onto the global -// registry. It returns a typed handle to use to recording data. +// RegisterInt64Gauge registers the metric description onto the global registry. +// It returns a typed handle to use to recording data. // // NOTE: this function must only be called during initialization time (i.e. in -// an init() function), and is not thread-safe. If multiple instruments are +// an init() function), and is not thread-safe. If multiple metrics are // registered with the same name, this function will panic. func RegisterInt64Gauge(descriptor MetricDescriptor) Int64GaugeHandle { - registerInst(descriptor.Name, descriptor.Default) + registerMetric(descriptor.Name, descriptor.Default) + descriptor.Type = MetricTypeIntGauge handle := Int64GaugeHandle{ MetricDescriptor: &descriptor, } @@ -204,7 +209,32 @@ func RegisterInt64Gauge(descriptor MetricDescriptor) Int64GaugeHandle { return handle } -// Will take a list, write comments and rewrite tests/cleanup and then I think ready to send out... -// How do I even test this really? -// Internal only clear...I don't think it's worth it just for default set to do it in internal... +// MetricType is the type of metric. +type MetricType int + +const ( + MetricTypeIntCount MetricType = iota + MetricTypeFloatCount + MetricTypeIntHisto + MetricTypeFloatHisto + MetricTypeIntGauge +) +// clearMetricsRegistryForTesting clears the global data of the metrics +// registry. It returns a closure to be invoked that sets the metrics registry +// to its original state. Only called in testing functions. +func clearMetricsRegistryForTesting() func() { + oldDefaultMetrics := DefaultMetrics + oldRegisteredMetrics := registeredMetrics + oldMetricsRegistry := MetricsRegistry + + DefaultMetrics = stats.NewMetrics() + registeredMetrics = make(map[stats.Metric]bool) + MetricsRegistry = make(map[stats.Metric]*MetricDescriptor) + + return func() { + DefaultMetrics = oldDefaultMetrics + registeredMetrics = oldRegisteredMetrics + MetricsRegistry = oldMetricsRegistry + } +} diff --git a/experimental/stats/metricregistry_test.go b/experimental/stats/metricregistry_test.go index b9ce3ff1d29e..ba2c0ec7369c 100644 --- a/experimental/stats/metricregistry_test.go +++ b/experimental/stats/metricregistry_test.go @@ -19,132 +19,244 @@ package stats import ( - "google.golang.org/grpc/experimental/stats/metricregistry" "testing" -) - -func (s) TestMetricRegistry(t *testing.T) { - - // Now there's an implied metrics recorder as part of record calls... - - // Component from client conn will satisfy interface... - - // Same thing create instruments, pass a metrics recorder, instrument is expected to call that metrics recorder - - // Register one of each instrument, verify the metrics recorder works with it... -} - -type fakeMetricsRecorder struct { - t *testing.T - - // 5 different or for one for ints/floats...? + "github.com/google/go-cmp/cmp" + "google.golang.org/grpc/internal/grpctest" +) - // Test the maps built out in OTel (mention to Doug this represents it...) - intValues map[*MetricDescriptor]int64 - floatValues map[*MetricDescriptor]float64 +type s struct { + grpctest.Tester } -// MetricsRecorderList layer just looks at labels/optional labels... - -// newFakeMetricsRecorder returns a fake metrics recorder based off the current -// state of global instrument registry. -func newFakeMetricsRecorder(t *testing.T) *fakeMetricsRecorder { - // Access globals, build a map like OTel would - MetricsRegistry // map[stats.Metric]->Pointer to metrics descriptor, yeah let's test this out, make sure pointer can be used as key value... +func Test(t *testing.T) { + grpctest.RunSubTests(t, s{}) } -// verifyLabels verifies that all of the labels keys expected are present in the -// labels received. -func verifyLabels(t *testing.T, labelsWant []string, optionalLabelsWant []string, labelsGot []stats.Label, optionalLabelsGot []stats.Label) { - for i, label := range labelsWant { - if labelsGot[i].Key != label { - t.Fatalf("label key at position %v got %v, want %v", i, labelsGot[i].Key, label) +// TestPanic tests that registering two metrics with the same name across any +// type of metric triggers a panic. +func (s) TestPanic(t *testing.T) { + cleanup := clearMetricsRegistryForTesting() + defer cleanup() + want := "metric simple counter already registered" + defer func() { + if r := recover(); r != "metric simple counter already registered" { + t.Errorf("expected panic %q, got %q", want, r) } + }() + desc := MetricDescriptor{ + // Type is not expected to be set from the registerer, but meant to be + // set by the metric registry. + Name: "simple counter", + Description: "number of times recorded on tests", + Unit: "calls", } - if len(labelsWant) != len(labelsGot) { - t.Fatalf("length of labels expected did not match got %v, want %v", len(labelsGot), len(optionalLabelsWant)) + RegisterInt64Count(desc) + RegisterInt64Gauge(desc) +} + +// TestInstrumentRegistry tests the metric registry. It registers testing only +// metrics using the metric registry, and creates a fake metrics recorder which +// uses these metrics. Using the handles returned from the metric registry, this +// test records stats using the fake metrics recorder. Then, the test verifies +// the persisted metrics data in the metrics recorder is what is expected. Thus, +// this tests the interactions between the metrics recorder and the metrics +// registry. +func (s) TestMetricRegistry(t *testing.T) { + cleanup := clearMetricsRegistryForTesting() + defer cleanup() + intCountHandle1 := RegisterInt64Count(MetricDescriptor{ + Name: "simple counter", + Description: "sum of all emissions from tests", + Unit: "int", + Labels: []string{"int counter label"}, + OptionalLabels: []string{"int counter optional label"}, + Default: false, + }) + floatCountHandle1 := RegisterFloat64Count(MetricDescriptor{ + Name: "float counter", + Description: "sum of all emissions from tests", + Unit: "float", + Labels: []string{"float counter label"}, + OptionalLabels: []string{"float counter optional label"}, + Default: false, + }) + intHistoHandle1 := RegisterInt64Histo(MetricDescriptor{ + Name: "int histo", + Description: "sum of all emissions from tests", + Unit: "int", + Labels: []string{"int histo label"}, + OptionalLabels: []string{"int histo optional label"}, + Default: false, + }) + floatHistoHandle1 := RegisterFloat64Histo(MetricDescriptor{ + Name: "float histo", + Description: "sum of all emissions from tests", + Unit: "float", + Labels: []string{"float histo label"}, + OptionalLabels: []string{"float histo optional label"}, + Default: false, + }) + intGaugeHandle1 := RegisterInt64Gauge(MetricDescriptor{ + Name: "simple gauge", + Description: "the most recent int emitted by test", + Unit: "int", + Labels: []string{"int gauge label"}, + OptionalLabels: []string{"int gauge optional label"}, + Default: false, + }) + + fmr := newFakeMetricsRecorder(t) + + intCountHandle1.Record(fmr, 1, []string{"some label value", "some optional label value"}...) + // The Metric Descriptor in the handle should be able to identify the metric + // information. This is the key passed to metrics recorder to identify + // metric. + if got := fmr.intValues[intCountHandle1.MetricDescriptor]; got != 1 { + t.Fatalf("fmr.intValues[intCountHandle1.MetricDescriptor] got %v, want: %v", got, 1) } - for i, label := range optionalLabelsWant { - if optionalLabelsGot[i].Key != label { - t.Fatalf("optional label key at position %v got %v, want %v", i, optionalLabelsGot[i].Key, label) - } + floatCountHandle1.Record(fmr, 1.2, []string{"some label value", "some optional label value"}...) + if got := fmr.floatValues[floatCountHandle1.MetricDescriptor]; got != 1.2 { + t.Fatalf("fmr.floatValues[floatCountHandle1.MetricDescriptor] got %v, want: %v", got, 1.2) } - if len(optionalLabelsWant) != len(optionalLabelsGot) { - t.Fatalf("length of optional labels expected did not match got %v, want %v", len(optionalLabelsGot), len(optionalLabelsWant)) + + intHistoHandle1.Record(fmr, 3, []string{"some label value", "some optional label value"}...) + if got := fmr.intValues[intHistoHandle1.MetricDescriptor]; got != 3 { + t.Fatalf("fmr.intValues[intHistoHandle1.MetricDescriptor] got %v, want: %v", got, 3) } - // This is essentially now a check of len(labels + optional labels) vs labels provided... + floatHistoHandle1.Record(fmr, 4.3, []string{"some label value", "some optional label value"}...) + if got := fmr.floatValues[floatHistoHandle1.MetricDescriptor]; got != 4.3 { + t.Fatalf("fmr.floatValues[floatHistoHandle1.MetricDescriptor] got %v, want: %v", got, 4.3) + } + intGaugeHandle1.Record(fmr, 7, []string{"some label value", "some optional label value"}...) + if got := fmr.intValues[intGaugeHandle1.MetricDescriptor]; got != 7 { + t.Fatalf("fmr.intValues[intGaugeHandle1.MetricDescriptor] got %v, want: %v", got, 7) + } } -// Test 2 for each? 5 different maps...? - -// All the operations will get a handle with pointer above, make sure it can use to record... - -// Need a clear for testing... +// TestNumerousIntCounts tests numerous int count metrics registered onto the +// metric registry. A component (simulated by test) should be able to record on +// the different registered int count metrics. +func TestNumerousIntCounts(t *testing.T) { + cleanup := clearMetricsRegistryForTesting() + defer cleanup() + intCountHandle1 := RegisterInt64Count(MetricDescriptor{ + Name: "int counter", + Description: "sum of all emissions from tests", + Unit: "int", + Labels: []string{"int counter label"}, + OptionalLabels: []string{"int counter optional label"}, + Default: false, + }) + intCountHandle2 := RegisterInt64Count(MetricDescriptor{ + Name: "int counter 2", + Description: "sum of all emissions from tests", + Unit: "int", + Labels: []string{"int counter label"}, + OptionalLabels: []string{"int counter optional label"}, + Default: false, + }) + intCountHandle3 := RegisterInt64Count(MetricDescriptor{ + Name: "int counter 3", + Description: "sum of all emissions from tests", + Unit: "int", + Labels: []string{"int counter label"}, + OptionalLabels: []string{"int counter optional label"}, + Default: false, + }) + + fmr := newFakeMetricsRecorder(t) + + intCountHandle1.Record(fmr, 1, []string{"some label value", "some optional label value"}...) + got := []int64{fmr.intValues[intCountHandle1.MetricDescriptor], fmr.intValues[intCountHandle2.MetricDescriptor], fmr.intValues[intCountHandle3.MetricDescriptor]} + want := []int64{1, 0, 0} + if diff := cmp.Diff(got, want); diff != "" { + t.Fatalf("fmr.intValues (-got, +want): %v", diff) + } -// It still implements these methods but gets called from handle -func (r *fakeMetricsRecorder) RecordIntCount(handle Int64CountHandle, labels []Label, optionalLabels []Label, incr int64) { // Techncialy this owuld eat labels...verifyLabels too? - // Rather than reading from registry/building out data structures, labels come from handle - handle.MetricDescriptor.Labels // []string also makes sure not nil...MetricDescriptor + intCountHandle2.Record(fmr, 1, []string{"some label value", "some optional label value"}...) + got = []int64{fmr.intValues[intCountHandle1.MetricDescriptor], fmr.intValues[intCountHandle2.MetricDescriptor], fmr.intValues[intCountHandle3.MetricDescriptor]} + want = []int64{1, 1, 0} + if diff := cmp.Diff(got, want); diff != "" { + t.Fatalf("fmr.intValues (-got, +want): %v", diff) + } - handle.MetricDescriptor.OptionalLabels // []string + intCountHandle3.Record(fmr, 1, []string{"some label value", "some optional label value"}...) + got = []int64{fmr.intValues[intCountHandle1.MetricDescriptor], fmr.intValues[intCountHandle2.MetricDescriptor], fmr.intValues[intCountHandle3.MetricDescriptor]} + want = []int64{1, 1, 1} + if diff := cmp.Diff(got, want); diff != "" { + t.Fatalf("fmr.intValues (-got, +want): %v", diff) + } - verifyLabels(r.t, handle.MetricDescriptor.Labels, handle.MetricDescriptor.OptionalLabels, labels, optionalLabels) + intCountHandle3.Record(fmr, 1, []string{"some label value", "some optional label value"}...) + got = []int64{fmr.intValues[intCountHandle1.MetricDescriptor], fmr.intValues[intCountHandle2.MetricDescriptor], fmr.intValues[intCountHandle3.MetricDescriptor]} + want = []int64{1, 1, 2} + if diff := cmp.Diff(got, want); diff != "" { + t.Fatalf("fmr.intValues (-got, +want): %v", diff) + } +} - // Overall data structure of the stats handler... - // map[name]->local would create a string comp +type fakeMetricsRecorder struct { + t *testing.T - // map[*MetricDescriptor]->local would just be a pointer comp... + intValues map[*MetricDescriptor]int64 + floatValues map[*MetricDescriptor]float64 +} - // record incr against data structure built out maybe map[name]-> - // No it's a map of metricdescriptor... - // How to build this out? - r.intValues[handle.MetricDescriptor] += incr // have the handle in main use that to verify... +// newFakeMetricsRecorder returns a fake metrics recorder based off the current +// state of global metric registry. +func newFakeMetricsRecorder(t *testing.T) *fakeMetricsRecorder { + fmr := &fakeMetricsRecorder{ + t: t, + intValues: make(map[*MetricDescriptor]int64), + floatValues: make(map[*MetricDescriptor]float64), + } + for _, desc := range MetricsRegistry { + switch desc.Type { + case MetricTypeIntCount: + case MetricTypeIntHisto: + case MetricTypeIntGauge: + fmr.intValues[desc] = 0 + case MetricTypeFloatCount: + case MetricTypeFloatHisto: + fmr.floatValues[desc] = 0 + } + } + return fmr } -func (r *fakeMetricsRecorder) RecordFloatCount(handle Float64CountHandle, labels []Label, optionalLabels []Label, incr float64) { - verifyLabels(r.t, handle.MetricDescriptor.Labels, handle.MetricDescriptor.OptionalLabels, labels, optionalLabels) +// verifyLabels verifies that the labels received are of the expected length. +func verifyLabels(t *testing.T, labelsWant []string, optionalLabelsWant []string, labelsGot []string) { + if len(labelsWant)+len(optionalLabelsWant) != len(labelsGot) { + t.Fatalf("length of optional labels expected did not match got %v, want %v", len(labelsGot), len(labelsWant)+len(optionalLabelsWant)) + } +} - handle.MetricDescriptor // *MetricDescriptor - use as key to map if not found then fatalf +func (r *fakeMetricsRecorder) RecordIntCount(handle Int64CountHandle, incr int64, labels ...string) { + verifyLabels(r.t, handle.MetricDescriptor.Labels, handle.MetricDescriptor.OptionalLabels, labels) + r.intValues[handle.MetricDescriptor] += incr +} +func (r *fakeMetricsRecorder) RecordFloatCount(handle Float64CountHandle, incr float64, labels ...string) { + verifyLabels(r.t, handle.MetricDescriptor.Labels, handle.MetricDescriptor.OptionalLabels, labels) r.floatValues[handle.MetricDescriptor] += incr } -func (r *fakeMetricsRecorder) RecordIntHisto(handle Int64HistoHandle, labels []Label, optionalLabels []Label, incr int64) { - verifyLabels(r.t, handle.MetricDescriptor.Labels, handle.MetricDescriptor.OptionalLabels, labels, optionalLabels) - - handle.MetricDescriptor // *MetricDescriptor - use as key to map if not found then fatalf - - r.intValues[handle.MetricDescriptor] += incr // after 5 of these, makes sure they don't collide +func (r *fakeMetricsRecorder) RecordIntHisto(handle Int64HistoHandle, incr int64, labels ...string) { + verifyLabels(r.t, handle.MetricDescriptor.Labels, handle.MetricDescriptor.OptionalLabels, labels) + r.intValues[handle.MetricDescriptor] += incr } -func (r *fakeMetricsRecorder) RecordFloatHisto(handle Float64HistoHandle, labels []Label, optionalLabels []Label, incr float64) { - verifyLabels(r.t, handle.MetricDescriptor.Labels, handle.MetricDescriptor.OptionalLabels, labels, optionalLabels) - - handle.MetricDescriptor // *MetricDescriptor - use as key to map if not found then fatalf +func (r *fakeMetricsRecorder) RecordFloatHisto(handle Float64HistoHandle, incr float64, labels ...string) { + verifyLabels(r.t, handle.MetricDescriptor.Labels, handle.MetricDescriptor.OptionalLabels, labels) + r.floatValues[handle.MetricDescriptor] += incr } -func (r *fakeMetricsRecorder) RecordIntGauge(handle Int64GaugeHandle, labels []Label, optionalLabels []Label, incr int64) { - verifyLabels(r.t, handle.MetricDescriptor.Labels, handle.MetricDescriptor.OptionalLabels, labels, optionalLabels) - - handle.MetricDescriptor // *MetricDescriptor - use as key to map if not found then fatalf +func (r *fakeMetricsRecorder) RecordIntGauge(handle Int64GaugeHandle, incr int64, labels ...string) { + verifyLabels(r.t, handle.MetricDescriptor.Labels, handle.MetricDescriptor.OptionalLabels, labels) + r.intValues[handle.MetricDescriptor] += incr } - -// If run out of time just push implementation...otel and metrics recorder list still come after I guess... -// just push the extra file.... - - -// Tests sound good to Doug get this plumbing working... - -// switch the labels to be variadic args based on position, length check on labels + optional labels - -// optional labels are always plumbed up through otel, otel decides whether it -// wants the optional labels or not... - -// on handle and metrics recorder - - diff --git a/experimental/stats/stats.go b/experimental/stats/stats.go index cb4e2b851567..9cfd69dcf71e 100644 --- a/experimental/stats/stats.go +++ b/experimental/stats/stats.go @@ -19,7 +19,6 @@ // Package stats contains experimental metrics/stats API's. package stats - // MetricsRecorder records on metrics derived from instrument registry. type MetricsRecorder interface { // RecordIntCount records the measurement alongside labels on the int count diff --git a/stats/metrics.go b/stats/metrics.go index 8c0418575fb0..944373956ec9 100644 --- a/stats/metrics.go +++ b/stats/metrics.go @@ -86,4 +86,3 @@ func (m *Metrics) Remove(metrics ...Metric) *Metrics { Metrics: newMetrics, } } - From 94caf39da58a48d6a9942bab90ceaa3fd5a47fa3 Mon Sep 17 00:00:00 2001 From: Zach Reyes Date: Wed, 10 Jul 2024 21:24:49 -0400 Subject: [PATCH 07/13] Responded to Doug's comments --- experimental/stats/metricregistry.go | 117 ++++++++++------------ experimental/stats/metricregistry_test.go | 26 +++-- experimental/stats/stats.go | 12 +-- stats/metrics.go | 27 +++-- stats/opentelemetry/client_metrics.go | 10 +- stats/opentelemetry/server_metrics.go | 8 +- 6 files changed, 97 insertions(+), 103 deletions(-) diff --git a/experimental/stats/metricregistry.go b/experimental/stats/metricregistry.go index 0be95ae9d9e4..569538d794a4 100644 --- a/experimental/stats/metricregistry.go +++ b/experimental/stats/metricregistry.go @@ -19,38 +19,41 @@ package stats import ( - "log" + "testing" + "google.golang.org/grpc/grpclog" "google.golang.org/grpc/stats" ) +var logger = grpclog.Component("metrics-registry") + // DefaultMetrics are the default metrics registered through global metrics // registry. This is written to at initialization time only, and is read only // after initialization. -var DefaultMetrics = stats.NewMetrics() +var DefaultMetrics = stats.NewMetrics() // loop through this set,, export metrisdescriptor passed upward for pointer and labels/optional lables... // MetricDescriptor is the data for a registered metric. type MetricDescriptor struct { - // Name is the name of this metric. This name must be unique across whole - // binary (including any per call metrics). See + // The name. This name must be unique across whole binary (including any per + // call metrics). See // https://github.com/grpc/proposal/blob/master/A79-non-per-call-metrics-architecture.md#metric-instrument-naming-conventions // for metric naming conventions. Name stats.Metric - // Description is the description of this metric. + // The description of this metric. Description string - // Unit is the unit of this metric (e.g. entries, milliseconds). + // The unit (e.g. entries, seconds). Unit string - // Labels are the required label keys for this metric. These are intended to + // The required label keys for this metric. These are intended to // metrics emitted from a stats handler. Labels []string - // OptionalLabels are the optional label keys for this metric. These are - // intended to attached to metrics emitted from a stats handler if - // configured. + // The optional label keys for this metric. These are intended to attached + // to metrics emitted from a stats handler if configured. + OptionalLabels []string - // Default is whether this metric is on by default. + // Whether this metric is on by default. Default bool - // Type is the type of metric. This is set by the metric registry, and not - // intended to be set by a component registering a metric. + // The type of metric. This is set by the metric registry, and not intended + // to be set by a component registering a metric. Type MetricType } @@ -62,7 +65,7 @@ type Int64CountHandle struct { } // Record records the int64 count value on the metrics recorder provided. -func (h Int64CountHandle) Record(recorder MetricsRecorder, incr int64, labels ...string) { +func (h *Int64CountHandle) Record(recorder MetricsRecorder, incr int64, labels ...string) { recorder.RecordIntCount(h, incr, labels...) } @@ -73,7 +76,7 @@ type Float64CountHandle struct { } // Record records the float64 count value on the metrics recorder provided. -func (h Float64CountHandle) Record(recorder MetricsRecorder, incr float64, labels ...string) { +func (h *Float64CountHandle) Record(recorder MetricsRecorder, incr float64, labels ...string) { recorder.RecordFloatCount(h, incr, labels...) } @@ -84,7 +87,7 @@ type Int64HistoHandle struct { } // Record records the int64 histo value on the metrics recorder provided. -func (h Int64HistoHandle) Record(recorder MetricsRecorder, incr int64, labels ...string) { +func (h *Int64HistoHandle) Record(recorder MetricsRecorder, incr int64, labels ...string) { recorder.RecordIntHisto(h, incr, labels...) } @@ -96,7 +99,7 @@ type Float64HistoHandle struct { } // Record records the float64 histo value on the metrics recorder provided. -func (h Float64HistoHandle) Record(recorder MetricsRecorder, incr float64, labels ...string) { +func (h *Float64HistoHandle) Record(recorder MetricsRecorder, incr float64, labels ...string) { recorder.RecordFloatHisto(h, incr, labels...) } @@ -107,21 +110,28 @@ type Int64GaugeHandle struct { } // Record records the int64 histo value on the metrics recorder provided. -func (h Int64GaugeHandle) Record(recorder MetricsRecorder, incr int64, labels ...string) { +func (h *Int64GaugeHandle) Record(recorder MetricsRecorder, incr int64, labels ...string) { recorder.RecordIntGauge(h, incr, labels...) } // registeredMetrics are the registered metric descriptor names. var registeredMetrics = make(map[stats.Metric]bool) -// MetricsRegistry are all of the registered metrics. +// metricsRegistry contains all of the registered metrics. // // This is written to only at init time, and read only after that. -var MetricsRegistry = make(map[stats.Metric]*MetricDescriptor) +var metricsRegistry = make(map[stats.Metric]*MetricDescriptor) // so OTel just loops through set provided by user into this thing...and calls get + +// GetMetric returns the MetricDescriptor from the global registry. +// +// Returns nil if MetricDescriptor not present. +func GetMetric(metric stats.Metric) *MetricDescriptor { + return metricsRegistry[metric] // will this be nil if not present? yes from playground becomes zero value... +} func registerMetric(name stats.Metric, def bool) { if registeredMetrics[name] { - log.Panicf("metric %v already registered", name) + logger.Fatalf("metric %v already registered", name) } registeredMetrics[name] = true if def { @@ -135,14 +145,11 @@ func registerMetric(name stats.Metric, def bool) { // NOTE: this function must only be called during initialization time (i.e. in // an init() function), and is not thread-safe. If multiple metrics are // registered with the same name, this function will panic. -func RegisterInt64Count(descriptor MetricDescriptor) Int64CountHandle { +func RegisterInt64Count(descriptor MetricDescriptor) *Int64CountHandle { registerMetric(descriptor.Name, descriptor.Default) descriptor.Type = MetricTypeIntCount - handle := Int64CountHandle{ - MetricDescriptor: &descriptor, - } - MetricsRegistry[descriptor.Name] = &descriptor - return handle + metricsRegistry[descriptor.Name] = &descriptor + return &Int64CountHandle{MetricDescriptor: &descriptor} } // RegisterFloat64Count registers the metric description onto the global @@ -151,14 +158,11 @@ func RegisterInt64Count(descriptor MetricDescriptor) Int64CountHandle { // NOTE: this function must only be called during initialization time (i.e. in // an init() function), and is not thread-safe. If multiple metrics are // registered with the same name, this function will panic. -func RegisterFloat64Count(descriptor MetricDescriptor) Float64CountHandle { +func RegisterFloat64Count(descriptor MetricDescriptor) *Float64CountHandle { registerMetric(descriptor.Name, descriptor.Default) descriptor.Type = MetricTypeFloatCount - handle := Float64CountHandle{ - MetricDescriptor: &descriptor, - } - MetricsRegistry[descriptor.Name] = &descriptor - return handle + metricsRegistry[descriptor.Name] = &descriptor + return &Float64CountHandle{MetricDescriptor: &descriptor} } // RegisterInt64Histo registers the metric description onto the global registry. @@ -167,14 +171,11 @@ func RegisterFloat64Count(descriptor MetricDescriptor) Float64CountHandle { // NOTE: this function must only be called during initialization time (i.e. in // an init() function), and is not thread-safe. If multiple metrics are // registered with the same name, this function will panic. -func RegisterInt64Histo(descriptor MetricDescriptor) Int64HistoHandle { +func RegisterInt64Histo(descriptor MetricDescriptor) *Int64HistoHandle { registerMetric(descriptor.Name, descriptor.Default) descriptor.Type = MetricTypeIntHisto - handle := Int64HistoHandle{ - MetricDescriptor: &descriptor, - } - MetricsRegistry[descriptor.Name] = &descriptor - return handle + metricsRegistry[descriptor.Name] = &descriptor + return &Int64HistoHandle{MetricDescriptor: &descriptor} } // RegisterFloat64Histo registers the metric description onto the global @@ -183,14 +184,11 @@ func RegisterInt64Histo(descriptor MetricDescriptor) Int64HistoHandle { // NOTE: this function must only be called during initialization time (i.e. in // an init() function), and is not thread-safe. If multiple metrics are // registered with the same name, this function will panic. -func RegisterFloat64Histo(descriptor MetricDescriptor) Float64HistoHandle { +func RegisterFloat64Histo(descriptor MetricDescriptor) *Float64HistoHandle { registerMetric(descriptor.Name, descriptor.Default) descriptor.Type = MetricTypeFloatHisto - handle := Float64HistoHandle{ - MetricDescriptor: &descriptor, - } - MetricsRegistry[descriptor.Name] = &descriptor - return handle + metricsRegistry[descriptor.Name] = &descriptor + return &Float64HistoHandle{MetricDescriptor: &descriptor} } // RegisterInt64Gauge registers the metric description onto the global registry. @@ -199,14 +197,11 @@ func RegisterFloat64Histo(descriptor MetricDescriptor) Float64HistoHandle { // NOTE: this function must only be called during initialization time (i.e. in // an init() function), and is not thread-safe. If multiple metrics are // registered with the same name, this function will panic. -func RegisterInt64Gauge(descriptor MetricDescriptor) Int64GaugeHandle { +func RegisterInt64Gauge(descriptor MetricDescriptor) *Int64GaugeHandle { registerMetric(descriptor.Name, descriptor.Default) descriptor.Type = MetricTypeIntGauge - handle := Int64GaugeHandle{ - MetricDescriptor: &descriptor, - } - MetricsRegistry[descriptor.Name] = &descriptor - return handle + metricsRegistry[descriptor.Name] = &descriptor + return &Int64GaugeHandle{MetricDescriptor: &descriptor} } // MetricType is the type of metric. @@ -220,21 +215,17 @@ const ( MetricTypeIntGauge ) -// clearMetricsRegistryForTesting clears the global data of the metrics -// registry. It returns a closure to be invoked that sets the metrics registry -// to its original state. Only called in testing functions. -func clearMetricsRegistryForTesting() func() { +// snapshotMetricsRegistryForTesting snapshots the global data of the metrics +// registry. Registers a cleanup function on the provided testing.T that sets +// the metrics registry to its original state. Only called in testing functions. +func snapshotMetricsRegistryForTesting(t *testing.T) { oldDefaultMetrics := DefaultMetrics oldRegisteredMetrics := registeredMetrics - oldMetricsRegistry := MetricsRegistry - - DefaultMetrics = stats.NewMetrics() - registeredMetrics = make(map[stats.Metric]bool) - MetricsRegistry = make(map[stats.Metric]*MetricDescriptor) + oldMetricsRegistry := metricsRegistry - return func() { + t.Cleanup(func() { DefaultMetrics = oldDefaultMetrics registeredMetrics = oldRegisteredMetrics - MetricsRegistry = oldMetricsRegistry - } + metricsRegistry = oldMetricsRegistry + }) } diff --git a/experimental/stats/metricregistry_test.go b/experimental/stats/metricregistry_test.go index ba2c0ec7369c..9f95d80f6a20 100644 --- a/experimental/stats/metricregistry_test.go +++ b/experimental/stats/metricregistry_test.go @@ -19,6 +19,7 @@ package stats import ( + "strings" "testing" "github.com/google/go-cmp/cmp" @@ -36,12 +37,11 @@ func Test(t *testing.T) { // TestPanic tests that registering two metrics with the same name across any // type of metric triggers a panic. func (s) TestPanic(t *testing.T) { - cleanup := clearMetricsRegistryForTesting() - defer cleanup() + snapshotMetricsRegistryForTesting(t) want := "metric simple counter already registered" defer func() { - if r := recover(); r != "metric simple counter already registered" { - t.Errorf("expected panic %q, got %q", want, r) + if r := recover(); !strings.Contains(r.(string), want) { + t.Errorf("expected panic contains %q, got %q", want, r) } }() desc := MetricDescriptor{ @@ -63,8 +63,7 @@ func (s) TestPanic(t *testing.T) { // this tests the interactions between the metrics recorder and the metrics // registry. func (s) TestMetricRegistry(t *testing.T) { - cleanup := clearMetricsRegistryForTesting() - defer cleanup() + snapshotMetricsRegistryForTesting(t) intCountHandle1 := RegisterInt64Count(MetricDescriptor{ Name: "simple counter", Description: "sum of all emissions from tests", @@ -141,8 +140,7 @@ func (s) TestMetricRegistry(t *testing.T) { // metric registry. A component (simulated by test) should be able to record on // the different registered int count metrics. func TestNumerousIntCounts(t *testing.T) { - cleanup := clearMetricsRegistryForTesting() - defer cleanup() + snapshotMetricsRegistryForTesting(t) intCountHandle1 := RegisterInt64Count(MetricDescriptor{ Name: "int counter", Description: "sum of all emissions from tests", @@ -215,7 +213,7 @@ func newFakeMetricsRecorder(t *testing.T) *fakeMetricsRecorder { floatValues: make(map[*MetricDescriptor]float64), } - for _, desc := range MetricsRegistry { + for _, desc := range metricsRegistry { switch desc.Type { case MetricTypeIntCount: case MetricTypeIntHisto: @@ -236,27 +234,27 @@ func verifyLabels(t *testing.T, labelsWant []string, optionalLabelsWant []string } } -func (r *fakeMetricsRecorder) RecordIntCount(handle Int64CountHandle, incr int64, labels ...string) { +func (r *fakeMetricsRecorder) RecordIntCount(handle *Int64CountHandle, incr int64, labels ...string) { verifyLabels(r.t, handle.MetricDescriptor.Labels, handle.MetricDescriptor.OptionalLabels, labels) r.intValues[handle.MetricDescriptor] += incr } -func (r *fakeMetricsRecorder) RecordFloatCount(handle Float64CountHandle, incr float64, labels ...string) { +func (r *fakeMetricsRecorder) RecordFloatCount(handle *Float64CountHandle, incr float64, labels ...string) { verifyLabels(r.t, handle.MetricDescriptor.Labels, handle.MetricDescriptor.OptionalLabels, labels) r.floatValues[handle.MetricDescriptor] += incr } -func (r *fakeMetricsRecorder) RecordIntHisto(handle Int64HistoHandle, incr int64, labels ...string) { +func (r *fakeMetricsRecorder) RecordIntHisto(handle *Int64HistoHandle, incr int64, labels ...string) { verifyLabels(r.t, handle.MetricDescriptor.Labels, handle.MetricDescriptor.OptionalLabels, labels) r.intValues[handle.MetricDescriptor] += incr } -func (r *fakeMetricsRecorder) RecordFloatHisto(handle Float64HistoHandle, incr float64, labels ...string) { +func (r *fakeMetricsRecorder) RecordFloatHisto(handle *Float64HistoHandle, incr float64, labels ...string) { verifyLabels(r.t, handle.MetricDescriptor.Labels, handle.MetricDescriptor.OptionalLabels, labels) r.floatValues[handle.MetricDescriptor] += incr } -func (r *fakeMetricsRecorder) RecordIntGauge(handle Int64GaugeHandle, incr int64, labels ...string) { +func (r *fakeMetricsRecorder) RecordIntGauge(handle *Int64GaugeHandle, incr int64, labels ...string) { verifyLabels(r.t, handle.MetricDescriptor.Labels, handle.MetricDescriptor.OptionalLabels, labels) r.intValues[handle.MetricDescriptor] += incr } diff --git a/experimental/stats/stats.go b/experimental/stats/stats.go index 9cfd69dcf71e..74323a50a51b 100644 --- a/experimental/stats/stats.go +++ b/experimental/stats/stats.go @@ -19,21 +19,21 @@ // Package stats contains experimental metrics/stats API's. package stats -// MetricsRecorder records on metrics derived from instrument registry. +// MetricsRecorder records on metrics derived from metric registry. type MetricsRecorder interface { // RecordIntCount records the measurement alongside labels on the int count // associated with the provided handle. - RecordIntCount(Int64CountHandle, int64, ...string) + RecordIntCount(handle *Int64CountHandle, incr int64, labels ...string) // RecordFloatCount records the measurement alongside labels on the float count // associated with the provided handle. - RecordFloatCount(Float64CountHandle, float64, ...string) + RecordFloatCount(handle *Float64CountHandle, incr float64, labels ...string) // RecordIntHisto records the measurement alongside labels on the int histo // associated with the provided handle. - RecordIntHisto(Int64HistoHandle, int64, ...string) + RecordIntHisto(handle *Int64HistoHandle, incr int64, labels ...string) // RecordFloatHisto records the measurement alongside labels on the float // histo associated with the provided handle. - RecordFloatHisto(Float64HistoHandle, float64, ...string) + RecordFloatHisto(handle *Float64HistoHandle, incr float64, labels ...string) // RecordIntGauge records the measurement alongside labels on the int gauge // associated with the provided handle. - RecordIntGauge(Int64GaugeHandle, int64, ...string) + RecordIntGauge(handle *Int64GaugeHandle, incr int64, labels ...string) } diff --git a/stats/metrics.go b/stats/metrics.go index 944373956ec9..ec82df746e92 100644 --- a/stats/metrics.go +++ b/stats/metrics.go @@ -29,9 +29,9 @@ type Metric string // // Do not construct directly; use NewMetrics instead. type Metrics struct { - // Metrics are the set of Metrics to initialize. - Metrics map[Metric]bool -} // map[stats.Metric]bool, DefaultMetrics, this nicely wraps (can call adds and just throw away old memory) and then joins on the Metrics Set are clean... + // metrics are the set of metrics to initialize. + metrics map[Metric]bool +} // NewMetrics returns a Metrics containing Metrics. func NewMetrics(metrics ...Metric) *Metrics { @@ -40,15 +40,20 @@ func NewMetrics(metrics ...Metric) *Metrics { newMetrics[metric] = true } return &Metrics{ - Metrics: newMetrics, + metrics: newMetrics, } } +// Metrics returns the metrics set. +func (m *Metrics) Metrics() map[Metric]bool { + return m.metrics +} + // Add adds the metrics to the metrics set and returns a new copy with the // additional metrics. func (m *Metrics) Add(metrics ...Metric) *Metrics { newMetrics := make(map[Metric]bool) - for metric := range m.Metrics { + for metric := range m.metrics { newMetrics[metric] = true } @@ -56,7 +61,7 @@ func (m *Metrics) Add(metrics ...Metric) *Metrics { newMetrics[metric] = true } return &Metrics{ - Metrics: newMetrics, + metrics: newMetrics, } } @@ -64,10 +69,10 @@ func (m *Metrics) Add(metrics ...Metric) *Metrics { // with the merged metrics. func (m *Metrics) Join(metrics *Metrics) *Metrics { // Use this API... newMetrics := make(map[Metric]bool) - maps.Copy(newMetrics, m.Metrics) - maps.Copy(newMetrics, metrics.Metrics) + maps.Copy(newMetrics, m.metrics) + maps.Copy(newMetrics, metrics.metrics) return &Metrics{ - Metrics: newMetrics, + metrics: newMetrics, } } @@ -75,7 +80,7 @@ func (m *Metrics) Join(metrics *Metrics) *Metrics { // Use this API... // the metrics removed. func (m *Metrics) Remove(metrics ...Metric) *Metrics { newMetrics := make(map[Metric]bool) - for metric := range m.Metrics { + for metric := range m.metrics { newMetrics[metric] = true } @@ -83,6 +88,6 @@ func (m *Metrics) Remove(metrics ...Metric) *Metrics { delete(newMetrics, metric) } return &Metrics{ - Metrics: newMetrics, + metrics: newMetrics, } } diff --git a/stats/opentelemetry/client_metrics.go b/stats/opentelemetry/client_metrics.go index 60f786ea6877..7f8d494653a1 100644 --- a/stats/opentelemetry/client_metrics.go +++ b/stats/opentelemetry/client_metrics.go @@ -54,11 +54,11 @@ func (h *clientStatsHandler) initializeMetrics() { metrics = DefaultMetrics } - h.clientMetrics.attemptStarted = createInt64Counter(metrics.Metrics, "grpc.client.attempt.started", meter, otelmetric.WithUnit("attempt"), otelmetric.WithDescription("Number of client call attempts started.")) - h.clientMetrics.attemptDuration = createFloat64Histogram(metrics.Metrics, "grpc.client.attempt.duration", meter, otelmetric.WithUnit("s"), otelmetric.WithDescription("End-to-end time taken to complete a client call attempt."), otelmetric.WithExplicitBucketBoundaries(DefaultLatencyBounds...)) - h.clientMetrics.attemptSentTotalCompressedMessageSize = createInt64Histogram(metrics.Metrics, "grpc.client.attempt.sent_total_compressed_message_size", meter, otelmetric.WithUnit("By"), otelmetric.WithDescription("Compressed message bytes sent per client call attempt."), otelmetric.WithExplicitBucketBoundaries(DefaultSizeBounds...)) - h.clientMetrics.attemptRcvdTotalCompressedMessageSize = createInt64Histogram(metrics.Metrics, "grpc.client.attempt.rcvd_total_compressed_message_size", meter, otelmetric.WithUnit("By"), otelmetric.WithDescription("Compressed message bytes received per call attempt."), otelmetric.WithExplicitBucketBoundaries(DefaultSizeBounds...)) - h.clientMetrics.callDuration = createFloat64Histogram(metrics.Metrics, "grpc.client.call.duration", meter, otelmetric.WithUnit("s"), otelmetric.WithDescription("Time taken by gRPC to complete an RPC from application's perspective."), otelmetric.WithExplicitBucketBoundaries(DefaultLatencyBounds...)) + h.clientMetrics.attemptStarted = createInt64Counter(metrics.Metrics(), "grpc.client.attempt.started", meter, otelmetric.WithUnit("attempt"), otelmetric.WithDescription("Number of client call attempts started.")) + h.clientMetrics.attemptDuration = createFloat64Histogram(metrics.Metrics(), "grpc.client.attempt.duration", meter, otelmetric.WithUnit("s"), otelmetric.WithDescription("End-to-end time taken to complete a client call attempt."), otelmetric.WithExplicitBucketBoundaries(DefaultLatencyBounds...)) + h.clientMetrics.attemptSentTotalCompressedMessageSize = createInt64Histogram(metrics.Metrics(), "grpc.client.attempt.sent_total_compressed_message_size", meter, otelmetric.WithUnit("By"), otelmetric.WithDescription("Compressed message bytes sent per client call attempt."), otelmetric.WithExplicitBucketBoundaries(DefaultSizeBounds...)) + h.clientMetrics.attemptRcvdTotalCompressedMessageSize = createInt64Histogram(metrics.Metrics(), "grpc.client.attempt.rcvd_total_compressed_message_size", meter, otelmetric.WithUnit("By"), otelmetric.WithDescription("Compressed message bytes received per call attempt."), otelmetric.WithExplicitBucketBoundaries(DefaultSizeBounds...)) + h.clientMetrics.callDuration = createFloat64Histogram(metrics.Metrics(), "grpc.client.call.duration", meter, otelmetric.WithUnit("s"), otelmetric.WithDescription("Time taken by gRPC to complete an RPC from application's perspective."), otelmetric.WithExplicitBucketBoundaries(DefaultLatencyBounds...)) } func (h *clientStatsHandler) unaryInterceptor(ctx context.Context, method string, req, reply any, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { diff --git a/stats/opentelemetry/server_metrics.go b/stats/opentelemetry/server_metrics.go index d1f25d229985..70f0d38cb54b 100644 --- a/stats/opentelemetry/server_metrics.go +++ b/stats/opentelemetry/server_metrics.go @@ -53,10 +53,10 @@ func (h *serverStatsHandler) initializeMetrics() { metrics = DefaultMetrics } - h.serverMetrics.callStarted = createInt64Counter(metrics.Metrics, "grpc.server.call.started", meter, otelmetric.WithUnit("call"), otelmetric.WithDescription("Number of server calls started.")) - h.serverMetrics.callSentTotalCompressedMessageSize = createInt64Histogram(metrics.Metrics, "grpc.server.call.sent_total_compressed_message_size", meter, otelmetric.WithUnit("By"), otelmetric.WithDescription("Compressed message bytes sent per server call."), otelmetric.WithExplicitBucketBoundaries(DefaultSizeBounds...)) - h.serverMetrics.callRcvdTotalCompressedMessageSize = createInt64Histogram(metrics.Metrics, "grpc.server.call.rcvd_total_compressed_message_size", meter, otelmetric.WithUnit("By"), otelmetric.WithDescription("Compressed message bytes received per server call."), otelmetric.WithExplicitBucketBoundaries(DefaultSizeBounds...)) - h.serverMetrics.callDuration = createFloat64Histogram(metrics.Metrics, "grpc.server.call.duration", meter, otelmetric.WithUnit("s"), otelmetric.WithDescription("End-to-end time taken to complete a call from server transport's perspective."), otelmetric.WithExplicitBucketBoundaries(DefaultLatencyBounds...)) + h.serverMetrics.callStarted = createInt64Counter(metrics.Metrics(), "grpc.server.call.started", meter, otelmetric.WithUnit("call"), otelmetric.WithDescription("Number of server calls started.")) + h.serverMetrics.callSentTotalCompressedMessageSize = createInt64Histogram(metrics.Metrics(), "grpc.server.call.sent_total_compressed_message_size", meter, otelmetric.WithUnit("By"), otelmetric.WithDescription("Compressed message bytes sent per server call."), otelmetric.WithExplicitBucketBoundaries(DefaultSizeBounds...)) + h.serverMetrics.callRcvdTotalCompressedMessageSize = createInt64Histogram(metrics.Metrics(), "grpc.server.call.rcvd_total_compressed_message_size", meter, otelmetric.WithUnit("By"), otelmetric.WithDescription("Compressed message bytes received per server call."), otelmetric.WithExplicitBucketBoundaries(DefaultSizeBounds...)) + h.serverMetrics.callDuration = createFloat64Histogram(metrics.Metrics(), "grpc.server.call.duration", meter, otelmetric.WithUnit("s"), otelmetric.WithDescription("End-to-end time taken to complete a call from server transport's perspective."), otelmetric.WithExplicitBucketBoundaries(DefaultLatencyBounds...)) } // attachLabelsTransportStream intercepts SetHeader and SendHeader calls of the From 94bc27bba73adb44e072ea9212fe2158a718b104 Mon Sep 17 00:00:00 2001 From: Zach Reyes Date: Thu, 11 Jul 2024 17:26:30 -0400 Subject: [PATCH 08/13] Responded to some of Doug's comments --- experimental/stats/metricregistry.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/experimental/stats/metricregistry.go b/experimental/stats/metricregistry.go index 569538d794a4..c591853051bf 100644 --- a/experimental/stats/metricregistry.go +++ b/experimental/stats/metricregistry.go @@ -30,25 +30,24 @@ var logger = grpclog.Component("metrics-registry") // DefaultMetrics are the default metrics registered through global metrics // registry. This is written to at initialization time only, and is read only // after initialization. -var DefaultMetrics = stats.NewMetrics() // loop through this set,, export metrisdescriptor passed upward for pointer and labels/optional lables... +var DefaultMetrics = stats.NewMetrics() // MetricDescriptor is the data for a registered metric. type MetricDescriptor struct { - // The name. This name must be unique across whole binary (including any per - // call metrics). See + // The name of this metric. This name must be unique across the whole binary + // (including any per call metrics). See // https://github.com/grpc/proposal/blob/master/A79-non-per-call-metrics-architecture.md#metric-instrument-naming-conventions // for metric naming conventions. Name stats.Metric // The description of this metric. Description string - // The unit (e.g. entries, seconds). + // The unit (e.g. entries, seconds) of this metric. Unit string // The required label keys for this metric. These are intended to // metrics emitted from a stats handler. Labels []string // The optional label keys for this metric. These are intended to attached // to metrics emitted from a stats handler if configured. - OptionalLabels []string // Whether this metric is on by default. Default bool @@ -120,13 +119,13 @@ var registeredMetrics = make(map[stats.Metric]bool) // metricsRegistry contains all of the registered metrics. // // This is written to only at init time, and read only after that. -var metricsRegistry = make(map[stats.Metric]*MetricDescriptor) // so OTel just loops through set provided by user into this thing...and calls get +var metricsRegistry = make(map[stats.Metric]*MetricDescriptor) -// GetMetric returns the MetricDescriptor from the global registry. +// DescriptorForMetric returns the MetricDescriptor from the global registry. // // Returns nil if MetricDescriptor not present. -func GetMetric(metric stats.Metric) *MetricDescriptor { - return metricsRegistry[metric] // will this be nil if not present? yes from playground becomes zero value... +func DescriptorForMetric(metric stats.Metric) *MetricDescriptor { + return metricsRegistry[metric] } func registerMetric(name stats.Metric, def bool) { From e413a802e97e4a92c34135d63e16b8175889856d Mon Sep 17 00:00:00 2001 From: Zach Reyes Date: Thu, 11 Jul 2024 17:36:27 -0400 Subject: [PATCH 09/13] Move metrics and metrics recorder to experimental/stats/metrics.go --- experimental/stats/metricregistry.go | 13 ++++---- {stats => experimental/stats}/metrics.go | 20 ++++++++++++ experimental/stats/stats.go | 39 ------------------------ stats/opentelemetry/client_metrics.go | 11 ++++--- stats/opentelemetry/example_test.go | 6 ++-- stats/opentelemetry/opentelemetry.go | 12 ++++---- stats/opentelemetry/server_metrics.go | 9 +++--- 7 files changed, 46 insertions(+), 64 deletions(-) rename {stats => experimental/stats}/metrics.go (68%) delete mode 100644 experimental/stats/stats.go diff --git a/experimental/stats/metricregistry.go b/experimental/stats/metricregistry.go index c591853051bf..5a60d3151405 100644 --- a/experimental/stats/metricregistry.go +++ b/experimental/stats/metricregistry.go @@ -22,7 +22,6 @@ import ( "testing" "google.golang.org/grpc/grpclog" - "google.golang.org/grpc/stats" ) var logger = grpclog.Component("metrics-registry") @@ -30,7 +29,7 @@ var logger = grpclog.Component("metrics-registry") // DefaultMetrics are the default metrics registered through global metrics // registry. This is written to at initialization time only, and is read only // after initialization. -var DefaultMetrics = stats.NewMetrics() +var DefaultMetrics = NewMetrics() // MetricDescriptor is the data for a registered metric. type MetricDescriptor struct { @@ -38,7 +37,7 @@ type MetricDescriptor struct { // (including any per call metrics). See // https://github.com/grpc/proposal/blob/master/A79-non-per-call-metrics-architecture.md#metric-instrument-naming-conventions // for metric naming conventions. - Name stats.Metric + Name Metric // The description of this metric. Description string // The unit (e.g. entries, seconds) of this metric. @@ -114,21 +113,21 @@ func (h *Int64GaugeHandle) Record(recorder MetricsRecorder, incr int64, labels . } // registeredMetrics are the registered metric descriptor names. -var registeredMetrics = make(map[stats.Metric]bool) +var registeredMetrics = make(map[Metric]bool) // metricsRegistry contains all of the registered metrics. // // This is written to only at init time, and read only after that. -var metricsRegistry = make(map[stats.Metric]*MetricDescriptor) +var metricsRegistry = make(map[Metric]*MetricDescriptor) // DescriptorForMetric returns the MetricDescriptor from the global registry. // // Returns nil if MetricDescriptor not present. -func DescriptorForMetric(metric stats.Metric) *MetricDescriptor { +func DescriptorForMetric(metric Metric) *MetricDescriptor { return metricsRegistry[metric] } -func registerMetric(name stats.Metric, def bool) { +func registerMetric(name Metric, def bool) { if registeredMetrics[name] { logger.Fatalf("metric %v already registered", name) } diff --git a/stats/metrics.go b/experimental/stats/metrics.go similarity index 68% rename from stats/metrics.go rename to experimental/stats/metrics.go index ec82df746e92..f4e83fa1d39a 100644 --- a/stats/metrics.go +++ b/experimental/stats/metrics.go @@ -16,10 +16,30 @@ * */ +// Package stats contains experimental metrics/stats API's. package stats import "maps" +// MetricsRecorder records on metrics derived from metric registry. +type MetricsRecorder interface { + // RecordIntCount records the measurement alongside labels on the int count + // associated with the provided handle. + RecordIntCount(handle *Int64CountHandle, incr int64, labels ...string) + // RecordFloatCount records the measurement alongside labels on the float count + // associated with the provided handle. + RecordFloatCount(handle *Float64CountHandle, incr float64, labels ...string) + // RecordIntHisto records the measurement alongside labels on the int histo + // associated with the provided handle. + RecordIntHisto(handle *Int64HistoHandle, incr int64, labels ...string) + // RecordFloatHisto records the measurement alongside labels on the float + // histo associated with the provided handle. + RecordFloatHisto(handle *Float64HistoHandle, incr float64, labels ...string) + // RecordIntGauge records the measurement alongside labels on the int gauge + // associated with the provided handle. + RecordIntGauge(handle *Int64GaugeHandle, incr int64, labels ...string) +} + // Metric is an identifier for a metric. type Metric string diff --git a/experimental/stats/stats.go b/experimental/stats/stats.go deleted file mode 100644 index 74323a50a51b..000000000000 --- a/experimental/stats/stats.go +++ /dev/null @@ -1,39 +0,0 @@ -/* - * - * Copyright 2024 gRPC 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 stats contains experimental metrics/stats API's. -package stats - -// MetricsRecorder records on metrics derived from metric registry. -type MetricsRecorder interface { - // RecordIntCount records the measurement alongside labels on the int count - // associated with the provided handle. - RecordIntCount(handle *Int64CountHandle, incr int64, labels ...string) - // RecordFloatCount records the measurement alongside labels on the float count - // associated with the provided handle. - RecordFloatCount(handle *Float64CountHandle, incr float64, labels ...string) - // RecordIntHisto records the measurement alongside labels on the int histo - // associated with the provided handle. - RecordIntHisto(handle *Int64HistoHandle, incr int64, labels ...string) - // RecordFloatHisto records the measurement alongside labels on the float - // histo associated with the provided handle. - RecordFloatHisto(handle *Float64HistoHandle, incr float64, labels ...string) - // RecordIntGauge records the measurement alongside labels on the int gauge - // associated with the provided handle. - RecordIntGauge(handle *Int64GaugeHandle, incr int64, labels ...string) -} diff --git a/stats/opentelemetry/client_metrics.go b/stats/opentelemetry/client_metrics.go index 7f8d494653a1..f9bcf466532f 100644 --- a/stats/opentelemetry/client_metrics.go +++ b/stats/opentelemetry/client_metrics.go @@ -22,6 +22,7 @@ import ( "time" "google.golang.org/grpc" + estats "google.golang.org/grpc/experimental/stats" istats "google.golang.org/grpc/internal/stats" "google.golang.org/grpc/metadata" "google.golang.org/grpc/stats" @@ -238,17 +239,17 @@ func (h *clientStatsHandler) processRPCEnd(ctx context.Context, ai *attemptInfo, const ( // ClientAttemptStarted is the number of client call attempts started. - ClientAttemptStarted stats.Metric = "grpc.client.attempt.started" + ClientAttemptStarted estats.Metric = "grpc.client.attempt.started" // ClientAttemptDuration is the end-to-end time taken to complete a client // call attempt. - ClientAttemptDuration stats.Metric = "grpc.client.attempt.duration" + ClientAttemptDuration estats.Metric = "grpc.client.attempt.duration" // ClientAttemptSentCompressedTotalMessageSize is the compressed message // bytes sent per client call attempt. - ClientAttemptSentCompressedTotalMessageSize stats.Metric = "grpc.client.attempt.sent_total_compressed_message_size" + ClientAttemptSentCompressedTotalMessageSize estats.Metric = "grpc.client.attempt.sent_total_compressed_message_size" // ClientAttemptRcvdCompressedTotalMessageSize is the compressed message // bytes received per call attempt. - ClientAttemptRcvdCompressedTotalMessageSize stats.Metric = "grpc.client.attempt.rcvd_total_compressed_message_size" + ClientAttemptRcvdCompressedTotalMessageSize estats.Metric = "grpc.client.attempt.rcvd_total_compressed_message_size" // ClientCallDuration is the time taken by gRPC to complete an RPC from // application's perspective. - ClientCallDuration stats.Metric = "grpc.client.call.duration" + ClientCallDuration estats.Metric = "grpc.client.call.duration" ) diff --git a/stats/opentelemetry/example_test.go b/stats/opentelemetry/example_test.go index 09ca9af3bfa3..156918e2496c 100644 --- a/stats/opentelemetry/example_test.go +++ b/stats/opentelemetry/example_test.go @@ -19,7 +19,7 @@ package opentelemetry_test import ( "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" - "google.golang.org/grpc/stats" + stats2 "google.golang.org/grpc/experimental/stats" "google.golang.org/grpc/stats/opentelemetry" "go.opentelemetry.io/otel/sdk/metric" @@ -103,7 +103,7 @@ func ExampleMetrics_disableAll() { // To disable all metrics, initialize Options as follows: opts := opentelemetry.Options{ MetricsOptions: opentelemetry.MetricsOptions{ - Metrics: stats.NewMetrics(), // Distinct to nil, which creates default metrics. This empty set creates no metrics. + Metrics: stats2.NewMetrics(), // Distinct to nil, which creates default metrics. This empty set creates no metrics. }, } do := opentelemetry.DialOption(opts) @@ -118,7 +118,7 @@ func ExampleMetrics_enableSome() { // To only create specific metrics, initialize Options as follows: opts := opentelemetry.Options{ MetricsOptions: opentelemetry.MetricsOptions{ - Metrics: stats.NewMetrics(opentelemetry.ClientAttemptDuration, opentelemetry.ClientAttemptRcvdCompressedTotalMessageSize), // only create these metrics + Metrics: stats2.NewMetrics(opentelemetry.ClientAttemptDuration, opentelemetry.ClientAttemptRcvdCompressedTotalMessageSize), // only create these metrics }, } do := opentelemetry.DialOption(opts) diff --git a/stats/opentelemetry/opentelemetry.go b/stats/opentelemetry/opentelemetry.go index db48bfdf6a93..aa5354d7a264 100644 --- a/stats/opentelemetry/opentelemetry.go +++ b/stats/opentelemetry/opentelemetry.go @@ -25,9 +25,9 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/codes" + estats "google.golang.org/grpc/experimental/stats" "google.golang.org/grpc/grpclog" "google.golang.org/grpc/internal" - "google.golang.org/grpc/stats" otelinternal "google.golang.org/grpc/stats/opentelemetry/internal" "go.opentelemetry.io/otel/metric" @@ -65,7 +65,7 @@ type MetricsOptions struct { // for corresponding metric supported by the client and server // instrumentation components if applicable. If not set, the default metrics // will be recorded. - Metrics *stats.Metrics + Metrics *estats.Metrics // MethodAttributeFilter is to record the method name of RPCs handled by // grpc.UnknownServiceHandler, but take care to limit the values allowed, as @@ -207,7 +207,7 @@ type serverMetrics struct { callDuration metric.Float64Histogram } -func createInt64Counter(setOfMetrics map[stats.Metric]bool, metricName stats.Metric, meter metric.Meter, options ...metric.Int64CounterOption) metric.Int64Counter { +func createInt64Counter(setOfMetrics map[estats.Metric]bool, metricName estats.Metric, meter metric.Meter, options ...metric.Int64CounterOption) metric.Int64Counter { if _, ok := setOfMetrics[metricName]; !ok { return noop.Int64Counter{} } @@ -219,7 +219,7 @@ func createInt64Counter(setOfMetrics map[stats.Metric]bool, metricName stats.Met return ret } -func createInt64Histogram(setOfMetrics map[stats.Metric]bool, metricName stats.Metric, meter metric.Meter, options ...metric.Int64HistogramOption) metric.Int64Histogram { +func createInt64Histogram(setOfMetrics map[estats.Metric]bool, metricName estats.Metric, meter metric.Meter, options ...metric.Int64HistogramOption) metric.Int64Histogram { if _, ok := setOfMetrics[metricName]; !ok { return noop.Int64Histogram{} } @@ -231,7 +231,7 @@ func createInt64Histogram(setOfMetrics map[stats.Metric]bool, metricName stats.M return ret } -func createFloat64Histogram(setOfMetrics map[stats.Metric]bool, metricName stats.Metric, meter metric.Meter, options ...metric.Float64HistogramOption) metric.Float64Histogram { +func createFloat64Histogram(setOfMetrics map[estats.Metric]bool, metricName estats.Metric, meter metric.Meter, options ...metric.Float64HistogramOption) metric.Float64Histogram { if _, ok := setOfMetrics[metricName]; !ok { return noop.Float64Histogram{} } @@ -254,5 +254,5 @@ var ( // DefaultSizeBounds are the default bounds for metrics which record size. DefaultSizeBounds = []float64{0, 1024, 2048, 4096, 16384, 65536, 262144, 1048576, 4194304, 16777216, 67108864, 268435456, 1073741824, 4294967296} // DefaultMetrics are the default metrics provided by this module. - DefaultMetrics = stats.NewMetrics(ClientAttemptStarted, ClientAttemptDuration, ClientAttemptSentCompressedTotalMessageSize, ClientAttemptRcvdCompressedTotalMessageSize, ClientCallDuration, ServerCallStarted, ServerCallSentCompressedTotalMessageSize, ServerCallRcvdCompressedTotalMessageSize, ServerCallDuration) + DefaultMetrics = estats.NewMetrics(ClientAttemptStarted, ClientAttemptDuration, ClientAttemptSentCompressedTotalMessageSize, ClientAttemptRcvdCompressedTotalMessageSize, ClientCallDuration, ServerCallStarted, ServerCallSentCompressedTotalMessageSize, ServerCallRcvdCompressedTotalMessageSize, ServerCallDuration) ) diff --git a/stats/opentelemetry/server_metrics.go b/stats/opentelemetry/server_metrics.go index 70f0d38cb54b..6f9ad7a80929 100644 --- a/stats/opentelemetry/server_metrics.go +++ b/stats/opentelemetry/server_metrics.go @@ -22,6 +22,7 @@ import ( "time" "google.golang.org/grpc" + estats "google.golang.org/grpc/experimental/stats" "google.golang.org/grpc/internal" "google.golang.org/grpc/metadata" "google.golang.org/grpc/stats" @@ -254,14 +255,14 @@ func (h *serverStatsHandler) processRPCEnd(ctx context.Context, ai *attemptInfo, const ( // ServerCallStarted is the number of server calls started. - ServerCallStarted stats.Metric = "grpc.server.call.started" + ServerCallStarted estats.Metric = "grpc.server.call.started" // ServerCallSentCompressedTotalMessageSize is the compressed message bytes // sent per server call. - ServerCallSentCompressedTotalMessageSize stats.Metric = "grpc.server.call.sent_total_compressed_message_size" + ServerCallSentCompressedTotalMessageSize estats.Metric = "grpc.server.call.sent_total_compressed_message_size" // ServerCallRcvdCompressedTotalMessageSize is the compressed message bytes // received per server call. - ServerCallRcvdCompressedTotalMessageSize stats.Metric = "grpc.server.call.rcvd_total_compressed_message_size" + ServerCallRcvdCompressedTotalMessageSize estats.Metric = "grpc.server.call.rcvd_total_compressed_message_size" // ServerCallDuration is the end-to-end time taken to complete a call from // server transport's perspective. - ServerCallDuration stats.Metric = "grpc.server.call.duration" + ServerCallDuration estats.Metric = "grpc.server.call.duration" ) From e47ecccb6f0305b01325f4d0f11fc27d3b44ffd7 Mon Sep 17 00:00:00 2001 From: Zach Reyes Date: Thu, 11 Jul 2024 18:26:03 -0400 Subject: [PATCH 10/13] Fix tests and switch handles to MetricDescriptor aliases --- experimental/stats/metricregistry.go | 53 ++++++++++++----------- experimental/stats/metricregistry_test.go | 38 ++++++++-------- 2 files changed, 46 insertions(+), 45 deletions(-) diff --git a/experimental/stats/metricregistry.go b/experimental/stats/metricregistry.go index 5a60d3151405..4a3c497dc678 100644 --- a/experimental/stats/metricregistry.go +++ b/experimental/stats/metricregistry.go @@ -19,6 +19,7 @@ package stats import ( + "maps" "testing" "google.golang.org/grpc/grpclog" @@ -55,12 +56,10 @@ type MetricDescriptor struct { Type MetricType } -// Int64CountHandle is a typed handle for a float count metric. This handle +// Int64CountHandle is a typed handle for a int count metric. This handle // is passed at the recording point in order to know which metric to record // on. -type Int64CountHandle struct { - MetricDescriptor *MetricDescriptor -} +type Int64CountHandle MetricDescriptor // Record records the int64 count value on the metrics recorder provided. func (h *Int64CountHandle) Record(recorder MetricsRecorder, incr int64, labels ...string) { @@ -69,9 +68,7 @@ func (h *Int64CountHandle) Record(recorder MetricsRecorder, incr int64, labels . // Float64CountHandle is a typed handle for a float count metric. This handle is // passed at the recording point in order to know which metric to record on. -type Float64CountHandle struct { - MetricDescriptor *MetricDescriptor -} +type Float64CountHandle MetricDescriptor // Record records the float64 count value on the metrics recorder provided. func (h *Float64CountHandle) Record(recorder MetricsRecorder, incr float64, labels ...string) { @@ -80,9 +77,7 @@ func (h *Float64CountHandle) Record(recorder MetricsRecorder, incr float64, labe // Int64HistoHandle is a typed handle for an int histogram metric. This handle // is passed at the recording point in order to know which metric to record on. -type Int64HistoHandle struct { - MetricDescriptor *MetricDescriptor -} +type Int64HistoHandle MetricDescriptor // Record records the int64 histo value on the metrics recorder provided. func (h *Int64HistoHandle) Record(recorder MetricsRecorder, incr int64, labels ...string) { @@ -92,9 +87,7 @@ func (h *Int64HistoHandle) Record(recorder MetricsRecorder, incr int64, labels . // Float64HistoHandle is a typed handle for a float histogram metric. This // handle is passed at the recording point in order to know which metric to // record on. -type Float64HistoHandle struct { - MetricDescriptor *MetricDescriptor -} +type Float64HistoHandle MetricDescriptor // Record records the float64 histo value on the metrics recorder provided. func (h *Float64HistoHandle) Record(recorder MetricsRecorder, incr float64, labels ...string) { @@ -103,9 +96,7 @@ func (h *Float64HistoHandle) Record(recorder MetricsRecorder, incr float64, labe // Int64GaugeHandle is a typed handle for an int gauge metric. This handle is // passed at the recording point in order to know which metric to record on. -type Int64GaugeHandle struct { - MetricDescriptor *MetricDescriptor -} +type Int64GaugeHandle MetricDescriptor // Record records the int64 histo value on the metrics recorder provided. func (h *Int64GaugeHandle) Record(recorder MetricsRecorder, incr int64, labels ...string) { @@ -146,8 +137,9 @@ func registerMetric(name Metric, def bool) { func RegisterInt64Count(descriptor MetricDescriptor) *Int64CountHandle { registerMetric(descriptor.Name, descriptor.Default) descriptor.Type = MetricTypeIntCount - metricsRegistry[descriptor.Name] = &descriptor - return &Int64CountHandle{MetricDescriptor: &descriptor} + descPtr := &descriptor + metricsRegistry[descriptor.Name] = descPtr + return (*Int64CountHandle)(descPtr) } // RegisterFloat64Count registers the metric description onto the global @@ -159,8 +151,9 @@ func RegisterInt64Count(descriptor MetricDescriptor) *Int64CountHandle { func RegisterFloat64Count(descriptor MetricDescriptor) *Float64CountHandle { registerMetric(descriptor.Name, descriptor.Default) descriptor.Type = MetricTypeFloatCount - metricsRegistry[descriptor.Name] = &descriptor - return &Float64CountHandle{MetricDescriptor: &descriptor} + descPtr := &descriptor + metricsRegistry[descriptor.Name] = descPtr + return (*Float64CountHandle)(descPtr) } // RegisterInt64Histo registers the metric description onto the global registry. @@ -172,8 +165,9 @@ func RegisterFloat64Count(descriptor MetricDescriptor) *Float64CountHandle { func RegisterInt64Histo(descriptor MetricDescriptor) *Int64HistoHandle { registerMetric(descriptor.Name, descriptor.Default) descriptor.Type = MetricTypeIntHisto - metricsRegistry[descriptor.Name] = &descriptor - return &Int64HistoHandle{MetricDescriptor: &descriptor} + descPtr := &descriptor + metricsRegistry[descriptor.Name] = descPtr + return (*Int64HistoHandle)(descPtr) } // RegisterFloat64Histo registers the metric description onto the global @@ -185,8 +179,9 @@ func RegisterInt64Histo(descriptor MetricDescriptor) *Int64HistoHandle { func RegisterFloat64Histo(descriptor MetricDescriptor) *Float64HistoHandle { registerMetric(descriptor.Name, descriptor.Default) descriptor.Type = MetricTypeFloatHisto - metricsRegistry[descriptor.Name] = &descriptor - return &Float64HistoHandle{MetricDescriptor: &descriptor} + descPtr := &descriptor + metricsRegistry[descriptor.Name] = descPtr + return (*Float64HistoHandle)(descPtr) } // RegisterInt64Gauge registers the metric description onto the global registry. @@ -198,8 +193,9 @@ func RegisterFloat64Histo(descriptor MetricDescriptor) *Float64HistoHandle { func RegisterInt64Gauge(descriptor MetricDescriptor) *Int64GaugeHandle { registerMetric(descriptor.Name, descriptor.Default) descriptor.Type = MetricTypeIntGauge - metricsRegistry[descriptor.Name] = &descriptor - return &Int64GaugeHandle{MetricDescriptor: &descriptor} + descPtr := &descriptor + metricsRegistry[descriptor.Name] = descPtr + return (*Int64GaugeHandle)(descPtr) } // MetricType is the type of metric. @@ -221,6 +217,11 @@ func snapshotMetricsRegistryForTesting(t *testing.T) { oldRegisteredMetrics := registeredMetrics oldMetricsRegistry := metricsRegistry + registeredMetrics = make(map[Metric]bool) + metricsRegistry = make(map[Metric]*MetricDescriptor) + maps.Copy(registeredMetrics, registeredMetrics) + maps.Copy(metricsRegistry, metricsRegistry) + t.Cleanup(func() { DefaultMetrics = oldDefaultMetrics registeredMetrics = oldRegisteredMetrics diff --git a/experimental/stats/metricregistry_test.go b/experimental/stats/metricregistry_test.go index 9f95d80f6a20..a77b9bcb3a1d 100644 --- a/experimental/stats/metricregistry_test.go +++ b/experimental/stats/metricregistry_test.go @@ -111,27 +111,27 @@ func (s) TestMetricRegistry(t *testing.T) { // The Metric Descriptor in the handle should be able to identify the metric // information. This is the key passed to metrics recorder to identify // metric. - if got := fmr.intValues[intCountHandle1.MetricDescriptor]; got != 1 { + if got := fmr.intValues[(*MetricDescriptor)(intCountHandle1)]; got != 1 { t.Fatalf("fmr.intValues[intCountHandle1.MetricDescriptor] got %v, want: %v", got, 1) } floatCountHandle1.Record(fmr, 1.2, []string{"some label value", "some optional label value"}...) - if got := fmr.floatValues[floatCountHandle1.MetricDescriptor]; got != 1.2 { + if got := fmr.floatValues[(*MetricDescriptor)(floatCountHandle1)]; got != 1.2 { t.Fatalf("fmr.floatValues[floatCountHandle1.MetricDescriptor] got %v, want: %v", got, 1.2) } intHistoHandle1.Record(fmr, 3, []string{"some label value", "some optional label value"}...) - if got := fmr.intValues[intHistoHandle1.MetricDescriptor]; got != 3 { + if got := fmr.intValues[(*MetricDescriptor)(intHistoHandle1)]; got != 3 { t.Fatalf("fmr.intValues[intHistoHandle1.MetricDescriptor] got %v, want: %v", got, 3) } floatHistoHandle1.Record(fmr, 4.3, []string{"some label value", "some optional label value"}...) - if got := fmr.floatValues[floatHistoHandle1.MetricDescriptor]; got != 4.3 { + if got := fmr.floatValues[(*MetricDescriptor)(floatHistoHandle1)]; got != 4.3 { t.Fatalf("fmr.floatValues[floatHistoHandle1.MetricDescriptor] got %v, want: %v", got, 4.3) } intGaugeHandle1.Record(fmr, 7, []string{"some label value", "some optional label value"}...) - if got := fmr.intValues[intGaugeHandle1.MetricDescriptor]; got != 7 { + if got := fmr.intValues[(*MetricDescriptor)(intGaugeHandle1)]; got != 7 { t.Fatalf("fmr.intValues[intGaugeHandle1.MetricDescriptor] got %v, want: %v", got, 7) } } @@ -169,28 +169,28 @@ func TestNumerousIntCounts(t *testing.T) { fmr := newFakeMetricsRecorder(t) intCountHandle1.Record(fmr, 1, []string{"some label value", "some optional label value"}...) - got := []int64{fmr.intValues[intCountHandle1.MetricDescriptor], fmr.intValues[intCountHandle2.MetricDescriptor], fmr.intValues[intCountHandle3.MetricDescriptor]} + got := []int64{fmr.intValues[(*MetricDescriptor)(intCountHandle1)], fmr.intValues[(*MetricDescriptor)(intCountHandle2)], fmr.intValues[(*MetricDescriptor)(intCountHandle3)]} want := []int64{1, 0, 0} if diff := cmp.Diff(got, want); diff != "" { t.Fatalf("fmr.intValues (-got, +want): %v", diff) } intCountHandle2.Record(fmr, 1, []string{"some label value", "some optional label value"}...) - got = []int64{fmr.intValues[intCountHandle1.MetricDescriptor], fmr.intValues[intCountHandle2.MetricDescriptor], fmr.intValues[intCountHandle3.MetricDescriptor]} + got = []int64{fmr.intValues[(*MetricDescriptor)(intCountHandle1)], fmr.intValues[(*MetricDescriptor)(intCountHandle2)], fmr.intValues[(*MetricDescriptor)(intCountHandle3)]} want = []int64{1, 1, 0} if diff := cmp.Diff(got, want); diff != "" { t.Fatalf("fmr.intValues (-got, +want): %v", diff) } intCountHandle3.Record(fmr, 1, []string{"some label value", "some optional label value"}...) - got = []int64{fmr.intValues[intCountHandle1.MetricDescriptor], fmr.intValues[intCountHandle2.MetricDescriptor], fmr.intValues[intCountHandle3.MetricDescriptor]} + got = []int64{fmr.intValues[(*MetricDescriptor)(intCountHandle1)], fmr.intValues[(*MetricDescriptor)(intCountHandle2)], fmr.intValues[(*MetricDescriptor)(intCountHandle3)]} want = []int64{1, 1, 1} if diff := cmp.Diff(got, want); diff != "" { t.Fatalf("fmr.intValues (-got, +want): %v", diff) } intCountHandle3.Record(fmr, 1, []string{"some label value", "some optional label value"}...) - got = []int64{fmr.intValues[intCountHandle1.MetricDescriptor], fmr.intValues[intCountHandle2.MetricDescriptor], fmr.intValues[intCountHandle3.MetricDescriptor]} + got = []int64{fmr.intValues[(*MetricDescriptor)(intCountHandle1)], fmr.intValues[(*MetricDescriptor)(intCountHandle2)], fmr.intValues[(*MetricDescriptor)(intCountHandle3)]} want = []int64{1, 1, 2} if diff := cmp.Diff(got, want); diff != "" { t.Fatalf("fmr.intValues (-got, +want): %v", diff) @@ -235,26 +235,26 @@ func verifyLabels(t *testing.T, labelsWant []string, optionalLabelsWant []string } func (r *fakeMetricsRecorder) RecordIntCount(handle *Int64CountHandle, incr int64, labels ...string) { - verifyLabels(r.t, handle.MetricDescriptor.Labels, handle.MetricDescriptor.OptionalLabels, labels) - r.intValues[handle.MetricDescriptor] += incr + verifyLabels(r.t, (*MetricDescriptor)(handle).Labels, (*MetricDescriptor)(handle).OptionalLabels, labels) + r.intValues[(*MetricDescriptor)(handle)] += incr } func (r *fakeMetricsRecorder) RecordFloatCount(handle *Float64CountHandle, incr float64, labels ...string) { - verifyLabels(r.t, handle.MetricDescriptor.Labels, handle.MetricDescriptor.OptionalLabels, labels) - r.floatValues[handle.MetricDescriptor] += incr + verifyLabels(r.t, (*MetricDescriptor)(handle).Labels, (*MetricDescriptor)(handle).OptionalLabels, labels) + r.floatValues[(*MetricDescriptor)(handle)] += incr } func (r *fakeMetricsRecorder) RecordIntHisto(handle *Int64HistoHandle, incr int64, labels ...string) { - verifyLabels(r.t, handle.MetricDescriptor.Labels, handle.MetricDescriptor.OptionalLabels, labels) - r.intValues[handle.MetricDescriptor] += incr + verifyLabels(r.t, (*MetricDescriptor)(handle).Labels, (*MetricDescriptor)(handle).OptionalLabels, labels) + r.intValues[(*MetricDescriptor)(handle)] += incr } func (r *fakeMetricsRecorder) RecordFloatHisto(handle *Float64HistoHandle, incr float64, labels ...string) { - verifyLabels(r.t, handle.MetricDescriptor.Labels, handle.MetricDescriptor.OptionalLabels, labels) - r.floatValues[handle.MetricDescriptor] += incr + verifyLabels(r.t, (*MetricDescriptor)(handle).Labels, (*MetricDescriptor)(handle).OptionalLabels, labels) + r.floatValues[(*MetricDescriptor)(handle)] += incr } func (r *fakeMetricsRecorder) RecordIntGauge(handle *Int64GaugeHandle, incr int64, labels ...string) { - verifyLabels(r.t, handle.MetricDescriptor.Labels, handle.MetricDescriptor.OptionalLabels, labels) - r.intValues[handle.MetricDescriptor] += incr + verifyLabels(r.t, (*MetricDescriptor)(handle).Labels, (*MetricDescriptor)(handle).OptionalLabels, labels) + r.intValues[(*MetricDescriptor)(handle)] += incr } From 83d5a1d4b7dfced83c9b331cf07b4100c32e46b3 Mon Sep 17 00:00:00 2001 From: Zach Reyes Date: Fri, 12 Jul 2024 14:01:50 -0400 Subject: [PATCH 11/13] Responded to most of Doug's comments --- experimental/stats/metricregistry.go | 32 +++++++++++----------- experimental/stats/metricregistry_test.go | 10 +++---- experimental/stats/metrics.go | 33 ++++++++++++----------- 3 files changed, 38 insertions(+), 37 deletions(-) diff --git a/experimental/stats/metricregistry.go b/experimental/stats/metricregistry.go index 4a3c497dc678..392256942679 100644 --- a/experimental/stats/metricregistry.go +++ b/experimental/stats/metricregistry.go @@ -56,6 +56,17 @@ type MetricDescriptor struct { Type MetricType } +// MetricType is the type of metric. +type MetricType int + +const ( + MetricTypeIntCount MetricType = iota + MetricTypeFloatCount + MetricTypeIntHisto + MetricTypeFloatHisto + MetricTypeIntGauge +) + // Int64CountHandle is a typed handle for a int count metric. This handle // is passed at the recording point in order to know which metric to record // on. @@ -63,7 +74,7 @@ type Int64CountHandle MetricDescriptor // Record records the int64 count value on the metrics recorder provided. func (h *Int64CountHandle) Record(recorder MetricsRecorder, incr int64, labels ...string) { - recorder.RecordIntCount(h, incr, labels...) + recorder.RecordInt64Count(h, incr, labels...) } // Float64CountHandle is a typed handle for a float count metric. This handle is @@ -72,7 +83,7 @@ type Float64CountHandle MetricDescriptor // Record records the float64 count value on the metrics recorder provided. func (h *Float64CountHandle) Record(recorder MetricsRecorder, incr float64, labels ...string) { - recorder.RecordFloatCount(h, incr, labels...) + recorder.RecordFloat64Count(h, incr, labels...) } // Int64HistoHandle is a typed handle for an int histogram metric. This handle @@ -81,7 +92,7 @@ type Int64HistoHandle MetricDescriptor // Record records the int64 histo value on the metrics recorder provided. func (h *Int64HistoHandle) Record(recorder MetricsRecorder, incr int64, labels ...string) { - recorder.RecordIntHisto(h, incr, labels...) + recorder.RecordInt64Histo(h, incr, labels...) } // Float64HistoHandle is a typed handle for a float histogram metric. This @@ -91,7 +102,7 @@ type Float64HistoHandle MetricDescriptor // Record records the float64 histo value on the metrics recorder provided. func (h *Float64HistoHandle) Record(recorder MetricsRecorder, incr float64, labels ...string) { - recorder.RecordFloatHisto(h, incr, labels...) + recorder.RecordFloat64Histo(h, incr, labels...) } // Int64GaugeHandle is a typed handle for an int gauge metric. This handle is @@ -100,7 +111,7 @@ type Int64GaugeHandle MetricDescriptor // Record records the int64 histo value on the metrics recorder provided. func (h *Int64GaugeHandle) Record(recorder MetricsRecorder, incr int64, labels ...string) { - recorder.RecordIntGauge(h, incr, labels...) + recorder.RecordInt64Gauge(h, incr, labels...) } // registeredMetrics are the registered metric descriptor names. @@ -198,17 +209,6 @@ func RegisterInt64Gauge(descriptor MetricDescriptor) *Int64GaugeHandle { return (*Int64GaugeHandle)(descPtr) } -// MetricType is the type of metric. -type MetricType int - -const ( - MetricTypeIntCount MetricType = iota - MetricTypeFloatCount - MetricTypeIntHisto - MetricTypeFloatHisto - MetricTypeIntGauge -) - // snapshotMetricsRegistryForTesting snapshots the global data of the metrics // registry. Registers a cleanup function on the provided testing.T that sets // the metrics registry to its original state. Only called in testing functions. diff --git a/experimental/stats/metricregistry_test.go b/experimental/stats/metricregistry_test.go index a77b9bcb3a1d..e7ba273245e8 100644 --- a/experimental/stats/metricregistry_test.go +++ b/experimental/stats/metricregistry_test.go @@ -234,27 +234,27 @@ func verifyLabels(t *testing.T, labelsWant []string, optionalLabelsWant []string } } -func (r *fakeMetricsRecorder) RecordIntCount(handle *Int64CountHandle, incr int64, labels ...string) { +func (r *fakeMetricsRecorder) RecordInt64Count(handle *Int64CountHandle, incr int64, labels ...string) { verifyLabels(r.t, (*MetricDescriptor)(handle).Labels, (*MetricDescriptor)(handle).OptionalLabels, labels) r.intValues[(*MetricDescriptor)(handle)] += incr } -func (r *fakeMetricsRecorder) RecordFloatCount(handle *Float64CountHandle, incr float64, labels ...string) { +func (r *fakeMetricsRecorder) RecordFloat64Count(handle *Float64CountHandle, incr float64, labels ...string) { verifyLabels(r.t, (*MetricDescriptor)(handle).Labels, (*MetricDescriptor)(handle).OptionalLabels, labels) r.floatValues[(*MetricDescriptor)(handle)] += incr } -func (r *fakeMetricsRecorder) RecordIntHisto(handle *Int64HistoHandle, incr int64, labels ...string) { +func (r *fakeMetricsRecorder) RecordInt64Histo(handle *Int64HistoHandle, incr int64, labels ...string) { verifyLabels(r.t, (*MetricDescriptor)(handle).Labels, (*MetricDescriptor)(handle).OptionalLabels, labels) r.intValues[(*MetricDescriptor)(handle)] += incr } -func (r *fakeMetricsRecorder) RecordFloatHisto(handle *Float64HistoHandle, incr float64, labels ...string) { +func (r *fakeMetricsRecorder) RecordFloat64Histo(handle *Float64HistoHandle, incr float64, labels ...string) { verifyLabels(r.t, (*MetricDescriptor)(handle).Labels, (*MetricDescriptor)(handle).OptionalLabels, labels) r.floatValues[(*MetricDescriptor)(handle)] += incr } -func (r *fakeMetricsRecorder) RecordIntGauge(handle *Int64GaugeHandle, incr int64, labels ...string) { +func (r *fakeMetricsRecorder) RecordInt64Gauge(handle *Int64GaugeHandle, incr int64, labels ...string) { verifyLabels(r.t, (*MetricDescriptor)(handle).Labels, (*MetricDescriptor)(handle).OptionalLabels, labels) r.intValues[(*MetricDescriptor)(handle)] += incr } diff --git a/experimental/stats/metrics.go b/experimental/stats/metrics.go index f4e83fa1d39a..3221f7a633a3 100644 --- a/experimental/stats/metrics.go +++ b/experimental/stats/metrics.go @@ -23,21 +23,21 @@ import "maps" // MetricsRecorder records on metrics derived from metric registry. type MetricsRecorder interface { - // RecordIntCount records the measurement alongside labels on the int count - // associated with the provided handle. - RecordIntCount(handle *Int64CountHandle, incr int64, labels ...string) - // RecordFloatCount records the measurement alongside labels on the float count - // associated with the provided handle. - RecordFloatCount(handle *Float64CountHandle, incr float64, labels ...string) - // RecordIntHisto records the measurement alongside labels on the int histo - // associated with the provided handle. - RecordIntHisto(handle *Int64HistoHandle, incr int64, labels ...string) - // RecordFloatHisto records the measurement alongside labels on the float + // RecordInt64Count records the measurement alongside labels on the int + // count associated with the provided handle. + RecordInt64Count(handle *Int64CountHandle, incr int64, labels ...string) + // RecordFloat64Count records the measurement alongside labels on the float + // count associated with the provided handle. + RecordFloat64Count(handle *Float64CountHandle, incr float64, labels ...string) + // RecordInt64Histo records the measurement alongside labels on the int // histo associated with the provided handle. - RecordFloatHisto(handle *Float64HistoHandle, incr float64, labels ...string) - // RecordIntGauge records the measurement alongside labels on the int gauge - // associated with the provided handle. - RecordIntGauge(handle *Int64GaugeHandle, incr int64, labels ...string) + RecordInt64Histo(handle *Int64HistoHandle, incr int64, labels ...string) + // RecordFloat64Histo records the measurement alongside labels on the float + // histo associated with the provided handle. + RecordFloat64Histo(handle *Float64HistoHandle, incr float64, labels ...string) + // RecordInt64Gauge records the measurement alongside labels on the int + // gauge associated with the provided handle. + RecordInt64Gauge(handle *Int64GaugeHandle, incr int64, labels ...string) } // Metric is an identifier for a metric. @@ -64,7 +64,8 @@ func NewMetrics(metrics ...Metric) *Metrics { } } -// Metrics returns the metrics set. +// Metrics returns the metrics set. The returned map is read-only and must not +// be modified. func (m *Metrics) Metrics() map[Metric]bool { return m.metrics } @@ -87,7 +88,7 @@ func (m *Metrics) Add(metrics ...Metric) *Metrics { // Join joins the metrics passed in with the metrics set, and returns a new copy // with the merged metrics. -func (m *Metrics) Join(metrics *Metrics) *Metrics { // Use this API... +func (m *Metrics) Join(metrics *Metrics) *Metrics { newMetrics := make(map[Metric]bool) maps.Copy(newMetrics, m.metrics) maps.Copy(newMetrics, metrics.metrics) From 6f8104022535b6387cb290faa7cfef519110651c Mon Sep 17 00:00:00 2001 From: Zach Reyes Date: Fri, 12 Jul 2024 14:05:02 -0400 Subject: [PATCH 12/13] Added nil check for panic --- experimental/stats/metricregistry_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/stats/metricregistry_test.go b/experimental/stats/metricregistry_test.go index e7ba273245e8..66ebf932907d 100644 --- a/experimental/stats/metricregistry_test.go +++ b/experimental/stats/metricregistry_test.go @@ -40,7 +40,7 @@ func (s) TestPanic(t *testing.T) { snapshotMetricsRegistryForTesting(t) want := "metric simple counter already registered" defer func() { - if r := recover(); !strings.Contains(r.(string), want) { + if r := recover(); r == nil || !strings.Contains(r.(string), want) { t.Errorf("expected panic contains %q, got %q", want, r) } }() From 1ec4e54a90c7dbfc0dbb26daccce48d24631e101 Mon Sep 17 00:00:00 2001 From: Zach Reyes Date: Fri, 12 Jul 2024 14:30:26 -0400 Subject: [PATCH 13/13] switch to fmt.Sprint --- experimental/stats/metricregistry_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/experimental/stats/metricregistry_test.go b/experimental/stats/metricregistry_test.go index 66ebf932907d..bd243648c262 100644 --- a/experimental/stats/metricregistry_test.go +++ b/experimental/stats/metricregistry_test.go @@ -19,6 +19,7 @@ package stats import ( + "fmt" "strings" "testing" @@ -40,7 +41,7 @@ func (s) TestPanic(t *testing.T) { snapshotMetricsRegistryForTesting(t) want := "metric simple counter already registered" defer func() { - if r := recover(); r == nil || !strings.Contains(r.(string), want) { + if r := recover(); !strings.Contains(fmt.Sprint(r), want) { t.Errorf("expected panic contains %q, got %q", want, r) } }()