From 3b962e6655a4f93fbe51e9f7f175507ce449e694 Mon Sep 17 00:00:00 2001 From: Mikhail Mazurskiy Date: Sat, 16 Dec 2023 15:44:08 +1100 Subject: [PATCH 1/4] Remove mutable globals --- rueidisotel/metrics.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/rueidisotel/metrics.go b/rueidisotel/metrics.go index 6fbe00da..89851b6c 100644 --- a/rueidisotel/metrics.go +++ b/rueidisotel/metrics.go @@ -14,15 +14,9 @@ import ( ) var ( - DefaultHistogramBuckets = []float64{ + defaultHistogramBuckets = []float64{ .005, .01, .025, .05, .1, .25, .5, 1, 2.5, 5, 10, } - DefaultDialFn = func(dst string, dialer *net.Dialer, cfg *tls.Config) (conn net.Conn, err error) { - if cfg != nil { - return tls.DialWithDialer(dialer, "tcp", dst, cfg) - } - return dialer.Dial("tcp", dst) - } ) type HistogramOption struct { @@ -47,7 +41,7 @@ func NewClient(clientOption rueidis.ClientOption, opts ...Option) (rueidis.Clien oclient := newClient(opts...) if clientOption.DialFn == nil { - clientOption.DialFn = DefaultDialFn + clientOption.DialFn = defaultDialFn } attempt, err := oclient.meter.Int64Counter("rueidis_dial_attempt") @@ -96,7 +90,7 @@ func newClient(opts ...Option) *otelclient { opt(cli) } if cli.histogramOption.Buckets == nil { - cli.histogramOption.Buckets = DefaultHistogramBuckets + cli.histogramOption.Buckets = defaultHistogramBuckets } if cli.meterProvider == nil { cli.meterProvider = otel.GetMeterProvider() // Default to global MeterProvider @@ -157,3 +151,10 @@ func (t *connTracker) Close() error { return t.Conn.Close() } + +func defaultDialFn(dst string, dialer *net.Dialer, cfg *tls.Config) (conn net.Conn, err error) { + if cfg != nil { + return tls.DialWithDialer(dialer, "tcp", dst, cfg) + } + return dialer.Dial("tcp", dst) +} From cfc8c742c1c969c8af35ef56be4cef48ed96ed0c Mon Sep 17 00:00:00 2001 From: Mikhail Mazurskiy Date: Sun, 17 Dec 2023 21:16:31 +1100 Subject: [PATCH 2/4] Handle errors explicitly --- rueidisotel/metrics.go | 20 +++++++++++++++----- rueidisotel/trace.go | 5 ++++- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/rueidisotel/metrics.go b/rueidisotel/metrics.go index 89851b6c..d771c4cc 100644 --- a/rueidisotel/metrics.go +++ b/rueidisotel/metrics.go @@ -38,7 +38,10 @@ func WithHistogramOption(histogramOption HistogramOption) Option { // - rueidis_dial_conns: number of active connections // - rueidis_dial_latency: dial latency in seconds func NewClient(clientOption rueidis.ClientOption, opts ...Option) (rueidis.Client, error) { - oclient := newClient(opts...) + oclient, err := newClient(opts...) + if err != nil { + return nil, err + } if clientOption.DialFn == nil { clientOption.DialFn = defaultDialFn @@ -84,7 +87,7 @@ func NewClient(clientOption rueidis.ClientOption, opts ...Option) (rueidis.Clien return oclient, nil } -func newClient(opts ...Option) *otelclient { +func newClient(opts ...Option) (*otelclient, error) { cli := &otelclient{} for _, opt := range opts { opt(cli) @@ -103,9 +106,16 @@ func newClient(opts ...Option) *otelclient { cli.meter = cli.meterProvider.Meter(name) cli.tracer = cli.tracerProvider.Tracer(name) // Now create the counters using the meter - cli.cscMiss, _ = cli.meter.Int64Counter("rueidis_do_cache_miss") - cli.cscHits, _ = cli.meter.Int64Counter("rueidis_do_cache_hits") - return cli + var err error + cli.cscMiss, err = cli.meter.Int64Counter("rueidis_do_cache_miss") + if err != nil { + return nil, err + } + cli.cscHits, err = cli.meter.Int64Counter("rueidis_do_cache_hits") + if err != nil { + return nil, err + } + return cli, nil } func trackDialing( diff --git a/rueidisotel/trace.go b/rueidisotel/trace.go index d62a997b..14947e65 100644 --- a/rueidisotel/trace.go +++ b/rueidisotel/trace.go @@ -23,7 +23,10 @@ var _ rueidis.Client = (*otelclient)(nil) // WithClient creates a new rueidis.Client with OpenTelemetry tracing enabled. func WithClient(client rueidis.Client, opts ...Option) rueidis.Client { - cli := newClient(opts...) + cli, err := newClient(opts...) + if err != nil { + panic(err) + } cli.client = client return cli } From a4d059bf2fad7589ed11c43fb40b6d4ca1760895 Mon Sep 17 00:00:00 2001 From: Mikhail Mazurskiy Date: Sun, 17 Dec 2023 21:47:49 +1100 Subject: [PATCH 3/4] Use floating point division for higher precision --- rueidisotel/metrics.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rueidisotel/metrics.go b/rueidisotel/metrics.go index d771c4cc..9b9e8531 100644 --- a/rueidisotel/metrics.go +++ b/rueidisotel/metrics.go @@ -136,7 +136,8 @@ func trackDialing( return nil, err } - dialLatency.Record(ctx, time.Since(start).Seconds()) + // Use floating point division for higher precision (instead of Seconds method). + dialLatency.Record(ctx, float64(time.Since(start))/float64(time.Second)) success.Add(ctx, 1) conns.Add(ctx, 1) From 5479ac3230a0dd688918cdd4f6e7b2ef9928b94e Mon Sep 17 00:00:00 2001 From: Mikhail Mazurskiy Date: Sun, 17 Dec 2023 23:20:53 +1100 Subject: [PATCH 4/4] Mark WithClient as deprecated --- rueidisotel/trace.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rueidisotel/trace.go b/rueidisotel/trace.go index 14947e65..7a4ab2c2 100644 --- a/rueidisotel/trace.go +++ b/rueidisotel/trace.go @@ -22,6 +22,8 @@ var ( var _ rueidis.Client = (*otelclient)(nil) // WithClient creates a new rueidis.Client with OpenTelemetry tracing enabled. +// +// Deprecated: use NewClient() instead. func WithClient(client rueidis.Client, opts ...Option) rueidis.Client { cli, err := newClient(opts...) if err != nil {