Skip to content

Commit fdd3a74

Browse files
committed
Handle errors explicitly
First, this change updates database stats recording to return early if there is an error when creating instruments. Before, the resulting instrument was nil and RegisterCallback returned an error instead. Second, it adds oteltest.NewMetricProviderWithError to help with testing this behavior. It also documents panic behavior for Wrap function and removes non-idiomatic handleErr functions.
1 parent 198e703 commit fdd3a74

7 files changed

Lines changed: 170 additions & 61 deletions

File tree

driver.go

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ func RegisterWithSource(driverName string, source string, options ...DriverOptio
8787
return "", errors.New("unable to register driver, all slots have been taken")
8888
}
8989

90-
// Wrap takes a SQL driver and wraps it with OpenTelemetry instrumentation.
90+
// Wrap takes an SQL driver and wraps it with OpenTelemetry instrumentation.
91+
// It panics if there is an error when creating instruments.
9192
func Wrap(d driver.Driver, opts ...DriverOption) driver.Driver {
9293
o := driverOptions{
9394
meterProvider: otel.GetMeterProvider(),
@@ -102,14 +103,18 @@ func Wrap(d driver.Driver, opts ...DriverOption) driver.Driver {
102103
option.applyDriverOptions(&o)
103104
}
104105

105-
return wrapDriver(d, o)
106+
cc, err := newConnConfig(o)
107+
if err != nil {
108+
panic(err)
109+
}
110+
111+
return wrapDriver(d, cc)
106112
}
107113

108-
func wrapDriver(d driver.Driver, o driverOptions) driver.Driver {
114+
func wrapDriver(d driver.Driver, cc connConfig) driver.Driver {
109115
drv := otDriver{
110116
parent: d,
111-
connConfig: newConnConfig(o),
112-
close: func() error { return nil },
117+
connConfig: cc,
113118
}
114119

115120
if _, ok := d.(driver.DriverContext); ok {
@@ -122,7 +127,7 @@ func wrapDriver(d driver.Driver, o driverOptions) driver.Driver {
122127
return struct{ driver.Driver }{drv}
123128
}
124129

125-
func newConnConfig(opts driverOptions) connConfig {
130+
func newConnConfig(opts driverOptions) (connConfig, error) {
126131
meter := opts.meterProvider.Meter(instrumentationName)
127132
tracer := newMethodTracer(
128133
opts.tracerProvider.Tracer(instrumentationName,
@@ -139,13 +144,17 @@ func newConnConfig(opts driverOptions) connConfig {
139144
metric.WithUnit(unitMilliseconds),
140145
metric.WithDescription(`The distribution of latencies of various calls in milliseconds`),
141146
)
142-
mustNoError(err)
147+
if err != nil {
148+
return connConfig{}, err
149+
}
143150

144151
callsCounter, err := meter.Int64Counter(dbSQLClientCalls,
145152
metric.WithUnit(unitDimensionless),
146153
metric.WithDescription(`The number of various calls of methods`),
147154
)
148-
mustNoError(err)
155+
if err != nil {
156+
return connConfig{}, err
157+
}
149158

150159
latencyRecorder := newMethodRecorder(latencyMsHistogram.Record, callsCounter.Add, opts.defaultAttributes...)
151160

@@ -161,7 +170,7 @@ func newConnConfig(opts driverOptions) connConfig {
161170
queryFuncMiddlewares: makeQueryerContextMiddlewares(latencyRecorder, tracerOrNil(tracer, opts.trace.AllowRoot), newQueryConfig(opts, metricMethodStmtQuery, traceMethodStmtQuery)),
162171
queryContextFuncMiddlewares: makeQueryerContextMiddlewares(latencyRecorder, tracer, newQueryConfig(opts, metricMethodStmtQuery, traceMethodStmtQuery)),
163172
}),
164-
}
173+
}, nil
165174
}
166175

167176
var _ driver.Driver = (*otDriver)(nil)
@@ -184,7 +193,11 @@ func (d otDriver) Open(name string) (driver.Conn, error) {
184193
}
185194

186195
func (d otDriver) Close() error {
187-
return d.close()
196+
if d.close != nil {
197+
return d.close()
198+
}
199+
200+
return nil
188201
}
189202

190203
func (d otDriver) OpenConnector(name string) (driver.Connector, error) {

driver_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,22 @@ func TestWrap_DriverContext_CloseError(t *testing.T) {
259259
assert.Equal(t, expectedError, err)
260260
}
261261

262+
func TestWrap_Panic(t *testing.T) {
263+
t.Parallel()
264+
265+
parent := driverOpenFunc(func(string) (driver.Conn, error) {
266+
return nil, errors.New("open error")
267+
})
268+
269+
meterProviderOption := otelsql.WithMeterProvider(
270+
oteltest.NewMeterProviderWithError(assert.AnError),
271+
)
272+
273+
assert.PanicsWithValue(t, assert.AnError, func() {
274+
_ = otelsql.Wrap(parent, meterProviderOption)
275+
})
276+
}
277+
262278
func Test_Open_Error(t *testing.T) {
263279
t.Parallel()
264280

errors.go

Lines changed: 0 additions & 15 deletions
This file was deleted.

errors_internal_test.go

Lines changed: 0 additions & 28 deletions
This file was deleted.

internal/test/oteltest/errors.go

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
package oteltest
2+
3+
import (
4+
"go.opentelemetry.io/otel/metric"
5+
"go.opentelemetry.io/otel/metric/embedded"
6+
)
7+
8+
type errorMeterProvider struct {
9+
embedded.MeterProvider
10+
11+
Error error
12+
}
13+
14+
// NewMeterProviderWithError returns a new [metric.MeterProvider] that always
15+
// returns the given error.
16+
func NewMeterProviderWithError(e error) metric.MeterProvider {
17+
return &errorMeterProvider{
18+
Error: e,
19+
}
20+
}
21+
22+
func (e *errorMeterProvider) Meter(string, ...metric.MeterOption) metric.Meter {
23+
return &errorMeter{
24+
Error: e.Error,
25+
}
26+
}
27+
28+
type errorMeter struct {
29+
embedded.Meter
30+
31+
Error error
32+
}
33+
34+
func (e *errorMeter) Int64Counter(string, ...metric.Int64CounterOption) (metric.Int64Counter, error) {
35+
return nil, e.Error
36+
}
37+
38+
func (e *errorMeter) Int64UpDownCounter(string, ...metric.Int64UpDownCounterOption) (metric.Int64UpDownCounter, error) {
39+
return nil, e.Error
40+
}
41+
42+
func (e *errorMeter) Int64Histogram(string, ...metric.Int64HistogramOption) (metric.Int64Histogram, error) {
43+
return nil, e.Error
44+
}
45+
46+
func (e *errorMeter) Int64Gauge(string, ...metric.Int64GaugeOption) (metric.Int64Gauge, error) {
47+
return nil, e.Error
48+
}
49+
50+
func (e *errorMeter) Int64ObservableCounter(string, ...metric.Int64ObservableCounterOption) (metric.Int64ObservableCounter, error) {
51+
return nil, e.Error
52+
}
53+
54+
func (e *errorMeter) Int64ObservableUpDownCounter(string, ...metric.Int64ObservableUpDownCounterOption) (metric.Int64ObservableUpDownCounter, error) {
55+
return nil, e.Error
56+
}
57+
58+
func (e *errorMeter) Int64ObservableGauge(string, ...metric.Int64ObservableGaugeOption) (metric.Int64ObservableGauge, error) {
59+
return nil, e.Error
60+
}
61+
62+
func (e *errorMeter) Float64Counter(string, ...metric.Float64CounterOption) (metric.Float64Counter, error) {
63+
return nil, e.Error
64+
}
65+
66+
func (e *errorMeter) Float64UpDownCounter(string, ...metric.Float64UpDownCounterOption) (metric.Float64UpDownCounter, error) {
67+
return nil, e.Error
68+
}
69+
70+
func (e *errorMeter) Float64Histogram(string, ...metric.Float64HistogramOption) (metric.Float64Histogram, error) {
71+
return nil, e.Error
72+
}
73+
74+
func (e *errorMeter) Float64Gauge(string, ...metric.Float64GaugeOption) (metric.Float64Gauge, error) {
75+
return nil, e.Error
76+
}
77+
78+
func (e *errorMeter) Float64ObservableCounter(string, ...metric.Float64ObservableCounterOption) (metric.Float64ObservableCounter, error) {
79+
return nil, e.Error
80+
}
81+
82+
func (e *errorMeter) Float64ObservableUpDownCounter(string, ...metric.Float64ObservableUpDownCounterOption) (metric.Float64ObservableUpDownCounter, error) {
83+
return nil, e.Error
84+
}
85+
86+
func (e *errorMeter) Float64ObservableGauge(string, ...metric.Float64ObservableGaugeOption) (metric.Float64ObservableGauge, error) {
87+
return nil, e.Error
88+
}
89+
90+
func (e *errorMeter) RegisterCallback(metric.Callback, ...metric.Observable) (metric.Registration, error) {
91+
return nil, e.Error
92+
}

stats.go

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,56 +75,72 @@ func recordStats(
7575
metric.WithUnit(unitDimensionless),
7676
metric.WithDescription("Count of open connections in the pool"),
7777
)
78-
handleErr(err)
78+
if err != nil {
79+
return err
80+
}
7981

8082
idleConnections, err = meter.Int64ObservableGauge(
8183
dbSQLConnectionsIdle,
8284
metric.WithUnit(unitDimensionless),
8385
metric.WithDescription("Count of idle connections in the pool"),
8486
)
85-
handleErr(err)
87+
if err != nil {
88+
return err
89+
}
8690

8791
activeConnections, err = meter.Int64ObservableGauge(
8892
dbSQLConnectionsActive,
8993
metric.WithUnit(unitDimensionless),
9094
metric.WithDescription("Count of active connections in the pool"),
9195
)
92-
handleErr(err)
96+
if err != nil {
97+
return err
98+
}
9399

94100
waitCount, err = meter.Int64ObservableCounter(
95101
dbSQLConnectionsWaitCount,
96102
metric.WithUnit(unitDimensionless),
97103
metric.WithDescription("The total number of connections waited for"),
98104
)
99-
handleErr(err)
105+
if err != nil {
106+
return err
107+
}
100108

101109
waitDuration, err = meter.Float64ObservableCounter(
102110
dbSQLConnectionsWaitDuration,
103111
metric.WithUnit(unitMilliseconds),
104112
metric.WithDescription("The total time blocked waiting for a new connection"),
105113
)
106-
handleErr(err)
114+
if err != nil {
115+
return err
116+
}
107117

108118
idleClosed, err = meter.Int64ObservableCounter(
109119
dbSQLConnectionsIdleClosed,
110120
metric.WithUnit(unitDimensionless),
111121
metric.WithDescription("The total number of connections closed due to SetMaxIdleConns"),
112122
)
113-
handleErr(err)
123+
if err != nil {
124+
return err
125+
}
114126

115127
idleTimeClosed, err = meter.Int64ObservableCounter(
116128
dbSQLConnectionsIdleTimeClosed,
117129
metric.WithUnit(unitDimensionless),
118130
metric.WithDescription("The total number of connections closed due to SetConnMaxIdleTime"),
119131
)
120-
handleErr(err)
132+
if err != nil {
133+
return err
134+
}
121135

122136
lifetimeClosed, err = meter.Int64ObservableCounter(
123137
dbSQLConnectionsLifetimeClosed,
124138
metric.WithUnit(unitDimensionless),
125139
metric.WithDescription("The total number of connections closed due to SetConnMaxLifetime"),
126140
)
127-
handleErr(err)
141+
if err != nil {
142+
return err
143+
}
128144

129145
_, err = meter.RegisterCallback(func(_ context.Context, obs metric.Observer) error {
130146
lock.Lock()

stats_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"testing"
55
"time"
66

7+
"github.com/stretchr/testify/assert"
78
"github.com/stretchr/testify/require"
89
semconv "go.opentelemetry.io/otel/semconv/v1.20.0"
910

@@ -12,6 +13,20 @@ import (
1213
"go.nhat.io/otelsql/internal/test/sqlmock"
1314
)
1415

16+
func TestRecordStatsError(t *testing.T) {
17+
t.Parallel()
18+
19+
oteltest.New().Run(t, func(sc oteltest.SuiteContext) {
20+
db, err := newDB(sc.DatabaseDSN())
21+
require.NoError(t, err)
22+
23+
err = otelsql.RecordStats(db, otelsql.WithMeterProvider(
24+
oteltest.NewMeterProviderWithError(assert.AnError),
25+
))
26+
require.ErrorIs(t, err, assert.AnError)
27+
})
28+
}
29+
1530
func TestRecordStats(t *testing.T) {
1631
t.Parallel()
1732

0 commit comments

Comments
 (0)