Skip to content

Commit f59d5ed

Browse files
wzy9607wzy9607
authored
revert: remove optional non-standard counter metrics (#41)
1 parent a4d4c6c commit f59d5ed

File tree

4 files changed

+9
-185
lines changed

4 files changed

+9
-185
lines changed

‎config.go

-12
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@ type config struct {
2929
mp metric.MeterProvider
3030
meter metric.Meter
3131

32-
counterMetricsEnabled bool
33-
3432
poolName string
3533
}
3634

@@ -59,8 +57,6 @@ func newConfig(opts ...Option) *config {
5957
mp: otel.GetMeterProvider(),
6058
meter: nil,
6159

62-
counterMetricsEnabled: false,
63-
6460
poolName: "",
6561
}
6662

@@ -131,14 +127,6 @@ func DisableMetrics() Option {
131127
})
132128
}
133129

134-
// WithCounterMetrics tells the hook to record non-standard counter metrics
135-
// "db.client.connection.create_count", "db.client.operation.count".
136-
func WithCounterMetrics() Option {
137-
return option(func(conf *config) {
138-
conf.counterMetricsEnabled = true
139-
})
140-
}
141-
142130
// Attributes returns the common attributes.
143131
func (conf *config) Attributes() []attribute.KeyValue {
144132
return slices.Clone(conf.attrs)

‎hook.go

+9-25
Original file line numberDiff line numberDiff line change
@@ -146,10 +146,6 @@ func (ch *clientHook) DialHook(hook redis.DialHook) redis.DialHook {
146146

147147
ch.instruments.createTime.Record(ctx, dur.Seconds(),
148148
metric.WithAttributeSet(ch.baseAttrSet), metric.WithAttributeSet(attrs))
149-
if ch.instruments.createCnt != nil {
150-
ch.instruments.createCnt.Add(ctx, 1,
151-
metric.WithAttributeSet(ch.baseAttrSet), metric.WithAttributeSet(attrs))
152-
}
153149
}
154150
return conn, err
155151
}
@@ -158,7 +154,8 @@ func (ch *clientHook) DialHook(hook redis.DialHook) redis.DialHook {
158154
func (ch *clientHook) ProcessHook(hook redis.ProcessHook) redis.ProcessHook {
159155
return func(ctx context.Context, cmd redis.Cmder) error {
160156
oprName := cmd.FullName()
161-
attrs := make([]attribute.KeyValue, 0, 8) //nolint:mnd // ignore
157+
attrs := make([]attribute.KeyValue, 0, 8) //nolint:mnd // ignore
158+
metricAttrs := make([]attribute.KeyValue, 0, 3) //nolint:mnd // ignore
162159
attrs = append(attrs, funcFileLine("github.com/redis/go-redis")...)
163160
attrs = append(attrs,
164161
semconv.DBOperationName(oprName),
@@ -179,22 +176,16 @@ func (ch *clientHook) ProcessHook(hook redis.ProcessHook) redis.ProcessHook {
179176
err := hook(ctx, cmd)
180177

181178
dur := time.Since(start)
182-
metricAttrs := make([]attribute.KeyValue, 0, 3) //nolint:mnd // ignore
183179
if err != nil {
184180
metricAttrs = ch.recordError(span, metricAttrs, err)
185181
}
186182

187183
if ch.conf.metricsEnabled {
188184
metricAttrs = append(metricAttrs, semconv.DBOperationName(oprName))
189-
metricAttrsSet := attribute.NewSet(metricAttrs...)
190185
ch.instruments.oprDuration.Record(ctx, dur.Seconds(), metric.WithAttributeSet(ch.operationAttrSet),
191-
metric.WithAttributeSet(metricAttrsSet))
186+
metric.WithAttributeSet(attribute.NewSet(metricAttrs...)))
192187
ch.instruments.useTime.Record(ctx, dur.Seconds(), metric.WithAttributeSet(ch.poolAttrSet),
193188
metric.WithAttributes(statusAttr(err)))
194-
if ch.instruments.oprCnt != nil {
195-
ch.instruments.oprCnt.Add(ctx, 1, metric.WithAttributeSet(ch.operationAttrSet),
196-
metric.WithAttributeSet(metricAttrsSet))
197-
}
198189
}
199190
return err
200191
}
@@ -205,7 +196,8 @@ func (ch *clientHook) ProcessPipelineHook(hook redis.ProcessPipelineHook) redis.
205196
summary, cmdsString := rediscmd.CmdsString(cmds)
206197
oprName := "pipeline " + summary
207198

208-
attrs := make([]attribute.KeyValue, 0, 8) //nolint:mnd // ignore
199+
attrs := make([]attribute.KeyValue, 0, 8) //nolint:mnd // ignore
200+
metricAttrs := make([]attribute.KeyValue, 0, 4) //nolint:mnd // ignore
209201
attrs = append(attrs, funcFileLine("github.com/redis/go-redis")...)
210202
attrs = append(attrs,
211203
semconv.DBOperationName(oprName),
@@ -226,26 +218,18 @@ func (ch *clientHook) ProcessPipelineHook(hook redis.ProcessPipelineHook) redis.
226218
err := hook(ctx, cmds)
227219

228220
dur := time.Since(start)
229-
metricAttrs := make([]attribute.KeyValue, 0, 2)
230221
if err != nil {
231222
metricAttrs = ch.recordError(span, metricAttrs, err)
232223
}
233224

234225
if ch.conf.metricsEnabled {
235-
metricAttrsSet := attribute.NewSet(metricAttrs...)
226+
metricAttrs = append(metricAttrs,
227+
semconv.DBOperationName(oprName),
228+
semconv.DBOperationBatchSize(len(cmds)))
236229
ch.instruments.oprDuration.Record(ctx, dur.Seconds(), metric.WithAttributeSet(ch.operationAttrSet),
237-
metric.WithAttributeSet(metricAttrsSet),
238-
metric.WithAttributeSet(attribute.NewSet(semconv.DBOperationName(oprName),
239-
semconv.DBOperationBatchSize(len(cmds)))))
230+
metric.WithAttributeSet(attribute.NewSet(metricAttrs...)))
240231
ch.instruments.useTime.Record(ctx, dur.Seconds(), metric.WithAttributeSet(ch.poolAttrSet),
241232
metric.WithAttributes(statusAttr(err)))
242-
if ch.instruments.oprCnt != nil {
243-
for _, cmd := range cmds {
244-
ch.instruments.oprCnt.Add(ctx, 1, metric.WithAttributeSet(ch.operationAttrSet),
245-
metric.WithAttributeSet(metricAttrsSet),
246-
metric.WithAttributeSet(attribute.NewSet(semconv.DBOperationName("pipeline:"+cmd.FullName()))))
247-
}
248-
}
249233
}
250234
return err
251235
}

‎hook_test.go

-126
Original file line numberDiff line numberDiff line change
@@ -80,20 +80,6 @@ func assertOprDuration(t *testing.T, metrics metricdata.Metrics, wantAttrs []att
8080
metricdatatest.IgnoreValue())
8181
}
8282

83-
func assertOprCnt(t *testing.T, metrics metricdata.Metrics, wantAttrs []attribute.KeyValue) {
84-
t.Helper()
85-
metricdatatest.AssertEqual(t, metricdata.Metrics{
86-
Name: "db.client.operation.count",
87-
Description: "Number of database client operations.",
88-
Unit: "{operation}",
89-
Data: metricdata.Sum[int64]{
90-
DataPoints: []metricdata.DataPoint[int64]{
91-
{Attributes: attribute.NewSet(wantAttrs...), Value: 1},
92-
}, Temporality: metricdata.CumulativeTemporality, IsMonotonic: true,
93-
},
94-
}, metrics, metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreExemplars())
95-
}
96-
9783
func assertCreateTime(t *testing.T, metrics metricdata.Metrics, wantAttrs []attribute.KeyValue) {
9884
t.Helper()
9985
metricdatatest.AssertEqual(t, metricdata.Metrics{
@@ -109,20 +95,6 @@ func assertCreateTime(t *testing.T, metrics metricdata.Metrics, wantAttrs []attr
10995
metricdatatest.IgnoreValue())
11096
}
11197

112-
func assertCreateCnt(t *testing.T, metrics metricdata.Metrics, wantAttrs []attribute.KeyValue) {
113-
t.Helper()
114-
metricdatatest.AssertEqual(t, metricdata.Metrics{
115-
Name: "db.client.connection.create_count",
116-
Description: "Number of database client connections created.",
117-
Unit: "{connection}",
118-
Data: metricdata.Sum[int64]{
119-
DataPoints: []metricdata.DataPoint[int64]{
120-
{Attributes: attribute.NewSet(wantAttrs...), Value: 1},
121-
}, Temporality: metricdata.CumulativeTemporality, IsMonotonic: true,
122-
},
123-
}, metrics, metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreExemplars())
124-
}
125-
12698
func assertUseTime(t *testing.T, metrics metricdata.Metrics, wantAttrs []attribute.KeyValue) {
12799
t.Helper()
128100
metricdatatest.AssertEqual(t, metricdata.Metrics{
@@ -230,49 +202,6 @@ func Test_clientHook_DialHook(t *testing.T) {
230202
attribute.String("status", "error"),
231203
})
232204
},
233-
}, {
234-
name: "enable WithCounterMetrics option",
235-
fields: fields{
236-
rdsOpt: &redis.Options{
237-
DB: 3,
238-
},
239-
opts: []Option{WithCounterMetrics()},
240-
},
241-
args: args{
242-
hook: func(ctx context.Context, network, addr string) (net.Conn, error) {
243-
return fakeConn{remoteAddr: net.IPAddr{IP: net.ParseIP("10.1.1.1")}}, nil
244-
},
245-
network: "tcp",
246-
addr: "FailoverClient",
247-
},
248-
checkSpan: func(t *testing.T, span sdktrace.ReadOnlySpan) {
249-
t.Helper()
250-
assert.Equal(t, "redis.dial", span.Name())
251-
assert.Equal(t, sdktrace.Status{Code: codes.Unset}, span.Status())
252-
t.Logf("attrs: %v", span.Attributes())
253-
254-
wantAttrs := []attribute.KeyValue{
255-
semconv.DBSystemNameRedis,
256-
semconv.DBClientConnectionPoolName("10.1.1.1/3"),
257-
}
258-
assert.Subset(t, span.Attributes(), wantAttrs)
259-
},
260-
checkMetrics: func(t *testing.T, sm metricdata.ScopeMetrics) {
261-
t.Helper()
262-
require.Len(t, sm.Metrics, 2)
263-
264-
assertCreateTime(t, sm.Metrics[0], []attribute.KeyValue{
265-
semconv.DBSystemNameRedis,
266-
semconv.DBClientConnectionPoolName("10.1.1.1/3"),
267-
attribute.String("status", "ok"),
268-
})
269-
270-
assertCreateCnt(t, sm.Metrics[1], []attribute.KeyValue{
271-
semconv.DBSystemNameRedis,
272-
semconv.DBClientConnectionPoolName("10.1.1.1/3"),
273-
attribute.String("status", "ok"),
274-
})
275-
},
276205
},
277206
}
278207
for _, tt := range tests {
@@ -517,61 +446,6 @@ func Test_clientHook_ProcessHook(t *testing.T) { //nolint:maintidx //table drive
517446
attribute.String("status", "ok"),
518447
})
519448
},
520-
}, {
521-
name: "enable WithCounterMetrics option",
522-
fields: fields{
523-
rdsOpt: &redis.Options{Addr: "10.1.1.1:6379", DB: 3},
524-
opts: []Option{WithCounterMetrics()},
525-
},
526-
args: args{
527-
hook: func(ctx context.Context, cmd redis.Cmder) error { return nil },
528-
cmd: redis.NewCmd(context.Background(), "set", "key", "value"),
529-
},
530-
checkSpan: func(t *testing.T, span sdktrace.ReadOnlySpan) {
531-
t.Helper()
532-
t.Logf("attrs: %v", span.Attributes())
533-
534-
wantAttrs := []attribute.KeyValue{
535-
semconv.DBSystemNameRedis,
536-
semconv.DBNamespace("3"),
537-
semconv.DBOperationName("set"),
538-
semconv.ServerAddress("10.1.1.1"),
539-
semconv.ServerPort(6379),
540-
}
541-
assert.Subset(t, span.Attributes(), wantAttrs)
542-
543-
wantNotExistAttrs := []attribute.Key{semconv.DBResponseStatusCodeKey, semconv.DBQueryTextKey}
544-
attrs := attrMap(span.Attributes())
545-
for _, key := range wantNotExistAttrs {
546-
assert.NotContains(t, attrs, key)
547-
}
548-
},
549-
checkMetrics: func(t *testing.T, sm metricdata.ScopeMetrics) {
550-
t.Helper()
551-
require.Len(t, sm.Metrics, 3)
552-
553-
assertOprDuration(t, sm.Metrics[0], []attribute.KeyValue{
554-
semconv.DBSystemNameRedis,
555-
semconv.DBNamespace("3"),
556-
semconv.DBOperationName("set"),
557-
semconv.ServerAddress("10.1.1.1"),
558-
semconv.ServerPort(6379),
559-
})
560-
561-
assertUseTime(t, sm.Metrics[1], []attribute.KeyValue{
562-
semconv.DBSystemNameRedis,
563-
semconv.DBClientConnectionPoolName("10.1.1.1:6379/3"),
564-
attribute.String("status", "ok"),
565-
})
566-
567-
assertOprCnt(t, sm.Metrics[2], []attribute.KeyValue{
568-
semconv.DBSystemNameRedis,
569-
semconv.DBNamespace("3"),
570-
semconv.DBOperationName("set"),
571-
semconv.ServerAddress("10.1.1.1"),
572-
semconv.ServerPort(6379),
573-
})
574-
},
575449
},
576450
}
577451
for _, tt := range tests {

‎instruments.go

-22
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,8 @@ type poolStatsInstruments struct {
2121

2222
type hookInstruments struct {
2323
oprDuration metric.Float64Histogram
24-
oprCnt metric.Int64Counter
2524

2625
createTime metric.Float64Histogram
27-
createCnt metric.Int64Counter
2826
useTime metric.Float64Histogram
2927
}
3028

@@ -147,28 +145,8 @@ func newHookInstruments(conf *config) (*hookInstruments, error) {
147145

148146
instruments := &hookInstruments{
149147
oprDuration: oprDuration,
150-
oprCnt: nil,
151148
createTime: createTime,
152-
createCnt: nil,
153149
useTime: useTime,
154150
}
155-
if conf.counterMetricsEnabled {
156-
instruments.oprCnt, err = conf.meter.Int64Counter(
157-
"db.client.operation.count",
158-
metric.WithDescription("Number of database client operations."),
159-
metric.WithUnit("{operation}"),
160-
)
161-
if err != nil {
162-
return nil, fmt.Errorf("failed to create db.client.operation.count instrument: %w", err)
163-
}
164-
instruments.createCnt, err = conf.meter.Int64Counter(
165-
"db.client.connection.create_count",
166-
metric.WithDescription("Number of database client connections created."),
167-
metric.WithUnit("{connection}"),
168-
)
169-
if err != nil {
170-
return nil, fmt.Errorf("failed to create db.client.connection.create_count instrument: %w", err)
171-
}
172-
}
173151
return instruments, nil
174152
}

0 commit comments

Comments
 (0)