Skip to content

Commit e410855

Browse files
zasweqzasweq
committed
Responded to Doug's comments
1 parent 6c69fc7 commit e410855

File tree

3 files changed

+14
-44
lines changed

3 files changed

+14
-44
lines changed

‎internal/stats/metrics_recorder_list.go‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,13 @@
1717
package stats
1818

1919
import (
20+
"fmt"
21+
2022
estats "google.golang.org/grpc/experimental/stats"
21-
"google.golang.org/grpc/grpclog"
2223
"google.golang.org/grpc/stats"
2324
)
2425

25-
var logger = grpclog.Component("metrics-recorder-list")
26-
27-
// MetricsRecorderList forwards Record calls to all of it's metricsRecorders.
26+
// MetricsRecorderList forwards Record calls to all of its metricsRecorders.
2827
//
2928
// It eats any record calls where the label values provided do not match the
3029
// number of label keys.
@@ -49,46 +48,46 @@ func NewMetricsRecorderList(shs []stats.Handler) *MetricsRecorderList {
4948
}
5049
}
5150

52-
func verifyLabels(labels []string, optionalLabels []string, labelsRecv ...string) {
53-
if got, want := len(labelsRecv), len(labels)+len(optionalLabels); got != want {
54-
logger.Fatalf("Length of labels passed to Record incorrect, got: %v, want: %v.", got, want)
51+
func verifyLabels(desc *estats.MetricDescriptor, labelsRecv ...string) {
52+
if len(labelsRecv) != len(desc.Labels)+len(desc.OptionalLabels) {
53+
panic(fmt.Sprintf("Length of labels passed to Record incorrect for metric %q.", desc.Name))
5554
}
5655
}
5756

5857
func (l *MetricsRecorderList) RecordInt64Count(handle *estats.Int64CountHandle, incr int64, labels ...string) {
59-
verifyLabels(handle.Labels, handle.OptionalLabels, labels...)
58+
verifyLabels((*estats.MetricDescriptor)(handle), labels...)
6059

6160
for _, metricRecorder := range l.metricsRecorders {
6261
metricRecorder.RecordInt64Count(handle, incr, labels...)
6362
}
6463
}
6564

6665
func (l *MetricsRecorderList) RecordFloat64Count(handle *estats.Float64CountHandle, incr float64, labels ...string) {
67-
verifyLabels(handle.Labels, handle.OptionalLabels, labels...)
66+
verifyLabels((*estats.MetricDescriptor)(handle), labels...)
6867

6968
for _, metricRecorder := range l.metricsRecorders {
7069
metricRecorder.RecordFloat64Count(handle, incr, labels...)
7170
}
7271
}
7372

7473
func (l *MetricsRecorderList) RecordInt64Histo(handle *estats.Int64HistoHandle, incr int64, labels ...string) {
75-
verifyLabels(handle.Labels, handle.OptionalLabels, labels...)
74+
verifyLabels((*estats.MetricDescriptor)(handle), labels...)
7675

7776
for _, metricRecorder := range l.metricsRecorders {
7877
metricRecorder.RecordInt64Histo(handle, incr, labels...)
7978
}
8079
}
8180

8281
func (l *MetricsRecorderList) RecordFloat64Histo(handle *estats.Float64HistoHandle, incr float64, labels ...string) {
83-
verifyLabels(handle.Labels, handle.OptionalLabels, labels...)
82+
verifyLabels((*estats.MetricDescriptor)(handle), labels...)
8483

8584
for _, metricRecorder := range l.metricsRecorders {
8685
metricRecorder.RecordFloat64Histo(handle, incr, labels...)
8786
}
8887
}
8988

9089
func (l *MetricsRecorderList) RecordInt64Gauge(handle *estats.Int64GaugeHandle, incr int64, labels ...string) {
91-
verifyLabels(handle.Labels, handle.OptionalLabels, labels...)
90+
verifyLabels((*estats.MetricDescriptor)(handle), labels...)
9291

9392
for _, metricRecorder := range l.metricsRecorders {
9493
metricRecorder.RecordInt64Gauge(handle, incr, labels...)

‎internal/stats/metrics_recorder_list_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ func (s) TestMetricRecorderListPanic(t *testing.T) {
223223
})
224224
mrl := istats.NewMetricsRecorderList(nil)
225225

226-
want := "Length of labels passed to Record incorrect, got: 1, want: 2."
226+
want := `Length of labels passed to Record incorrect for metric "simple counter".`
227227
defer func() {
228228
if r := recover(); !strings.Contains(fmt.Sprint(r), want) {
229229
t.Errorf("expected panic contains %q, got %q", want, r)

‎internal/testutils/stats/test_metrics_recorder.go‎

Lines changed: 2 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,6 @@ import (
3636
type TestMetricsRecorder struct {
3737
t *testing.T
3838

39-
// TODO: scale this persisted storage up with label key values.
40-
intValues map[*estats.MetricDescriptor]int64
41-
floatValues map[*estats.MetricDescriptor]float64
42-
4339
intCountCh *testutils.Channel
4440
floatCountCh *testutils.Channel
4541
intHistoCh *testutils.Channel
@@ -48,31 +44,15 @@ type TestMetricsRecorder struct {
4844
}
4945

5046
func NewTestMetricsRecorder(t *testing.T, metrics []string) *TestMetricsRecorder {
51-
tmr := &TestMetricsRecorder{
52-
t: t,
53-
intValues: make(map[*estats.MetricDescriptor]int64),
54-
floatValues: make(map[*estats.MetricDescriptor]float64),
47+
return &TestMetricsRecorder{
48+
t: t,
5549

5650
intCountCh: testutils.NewChannelWithSize(10),
5751
floatCountCh: testutils.NewChannelWithSize(10),
5852
intHistoCh: testutils.NewChannelWithSize(10),
5953
floatHistoCh: testutils.NewChannelWithSize(10),
6054
intGaugeCh: testutils.NewChannelWithSize(10),
6155
}
62-
63-
for _, metric := range metrics {
64-
desc := estats.DescriptorForMetric(estats.Metric(metric))
65-
switch desc.Type {
66-
case estats.MetricTypeIntCount:
67-
case estats.MetricTypeIntHisto:
68-
case estats.MetricTypeIntGauge:
69-
tmr.intValues[desc] = 0
70-
case estats.MetricTypeFloatCount:
71-
case estats.MetricTypeFloatHisto:
72-
tmr.floatValues[desc] = 0
73-
}
74-
}
75-
return tmr
7656
}
7757

7858
type MetricsData struct {
@@ -105,8 +85,6 @@ func (r *TestMetricsRecorder) RecordInt64Count(handle *estats.Int64CountHandle,
10585
LabelKeys: append(handle.Labels, handle.OptionalLabels...),
10686
LabelVals: labels,
10787
})
108-
109-
r.intValues[(*estats.MetricDescriptor)(handle)] += incr
11088
}
11189

11290
func (r *TestMetricsRecorder) WaitForFloat64Count(ctx context.Context, metricsDataWant MetricsData) {
@@ -127,8 +105,6 @@ func (r *TestMetricsRecorder) RecordFloat64Count(handle *estats.Float64CountHand
127105
LabelKeys: append(handle.Labels, handle.OptionalLabels...),
128106
LabelVals: labels,
129107
})
130-
131-
r.floatValues[(*estats.MetricDescriptor)(handle)] += incr
132108
}
133109

134110
func (r *TestMetricsRecorder) WaitForInt64Histo(ctx context.Context, metricsDataWant MetricsData) {
@@ -149,8 +125,6 @@ func (r *TestMetricsRecorder) RecordInt64Histo(handle *estats.Int64HistoHandle,
149125
LabelKeys: append(handle.Labels, handle.OptionalLabels...),
150126
LabelVals: labels,
151127
})
152-
153-
r.intValues[(*estats.MetricDescriptor)(handle)] += incr
154128
}
155129

156130
func (r *TestMetricsRecorder) WaitForFloat64Histo(ctx context.Context, metricsDataWant MetricsData) {
@@ -171,7 +145,6 @@ func (r *TestMetricsRecorder) RecordFloat64Histo(handle *estats.Float64HistoHand
171145
LabelKeys: append(handle.Labels, handle.OptionalLabels...),
172146
LabelVals: labels,
173147
})
174-
r.floatValues[(*estats.MetricDescriptor)(handle)] += incr
175148
}
176149

177150
func (r *TestMetricsRecorder) WaitForInt64Gauge(ctx context.Context, metricsDataWant MetricsData) {
@@ -192,8 +165,6 @@ func (r *TestMetricsRecorder) RecordInt64Gauge(handle *estats.Int64GaugeHandle,
192165
LabelKeys: append(handle.Labels, handle.OptionalLabels...),
193166
LabelVals: labels,
194167
})
195-
196-
r.intValues[(*estats.MetricDescriptor)(handle)] = incr
197168
}
198169

199170
// To implement a stats.Handler, which allows it to be set as a dial option:

0 commit comments

Comments
 (0)