From 80d157218e28e717a1759af6817925b20621ab8a Mon Sep 17 00:00:00 2001 From: Sam Xie Date: Tue, 5 Apr 2022 20:56:29 +0800 Subject: [PATCH] Remove `dbSystem` parameter from all exported functions (#80) * Remove `dbSystem` parameter from all exported functions * Update CHANGELOG --- CHANGELOG.md | 9 +++++++++ config.go | 12 ++---------- config_test.go | 7 +++---- conn_test.go | 2 +- driver_test.go | 6 ++++-- example/main.go | 8 ++++++-- example_test.go | 12 +++++------- sql.go | 42 +++++++++++------------------------------- sql_test.go | 19 +++++++------------ stmt_test.go | 2 +- 10 files changed, 49 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ec6562..fc739f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,15 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### ⚠️ Notice ⚠️ + +This update is a breaking change of `Open`, `OpenDB`, `Register`, `WrapDriver` and `RegisterDBStatsMetrics` methods. +Code instrumented with these methods will need to be modified. + +### Removed + +- Remove `dbSystem` parameter from all exported functions. (#80) + ## [0.13.0] - 2022-04-04 ### Added diff --git a/config.go b/config.go index 6eec956..6b7a23a 100644 --- a/config.go +++ b/config.go @@ -21,7 +21,7 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" metricglobal "go.opentelemetry.io/otel/metric/global" - semconv "go.opentelemetry.io/otel/semconv/v1.4.0" + semconv "go.opentelemetry.io/otel/semconv/v1.7.0" "go.opentelemetry.io/otel/trace" ) @@ -51,8 +51,6 @@ type config struct { SpanOptions SpanOptions - DBSystem string - // Attributes will be set to each span. Attributes []attribute.KeyValue @@ -98,22 +96,16 @@ func (f *defaultSpanNameFormatter) Format(ctx context.Context, method Method, qu } // newConfig returns a config with all Options set. -func newConfig(dbSystem string, options ...Option) config { +func newConfig(options ...Option) config { cfg := config{ TracerProvider: otel.GetTracerProvider(), MeterProvider: metricglobal.MeterProvider(), - DBSystem: dbSystem, SpanNameFormatter: &defaultSpanNameFormatter{}, } for _, opt := range options { opt.Apply(&cfg) } - if cfg.DBSystem != "" { - cfg.Attributes = append(cfg.Attributes, - semconv.DBSystemKey.String(cfg.DBSystem), - ) - } cfg.Tracer = cfg.TracerProvider.Tracer( instrumentationName, trace.WithInstrumentationVersion(Version()), diff --git a/config_test.go b/config_test.go index 049ba84..c9b4851 100644 --- a/config_test.go +++ b/config_test.go @@ -22,12 +22,12 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/metric/global" - semconv "go.opentelemetry.io/otel/semconv/v1.4.0" + semconv "go.opentelemetry.io/otel/semconv/v1.7.0" "go.opentelemetry.io/otel/trace" ) func TestNewConfig(t *testing.T) { - cfg := newConfig("db", WithSpanOptions(SpanOptions{Ping: true})) + cfg := newConfig(WithSpanOptions(SpanOptions{Ping: true}), WithAttributes(semconv.DBSystemMySQL)) assert.Equal(t, config{ TracerProvider: otel.GetTracerProvider(), Tracer: otel.GetTracerProvider().Tracer( @@ -42,9 +42,8 @@ func TestNewConfig(t *testing.T) { // No need to check values of instruments in this part. Instruments: cfg.Instruments, SpanOptions: SpanOptions{Ping: true}, - DBSystem: "db", Attributes: []attribute.KeyValue{ - semconv.DBSystemKey.String(cfg.DBSystem), + semconv.DBSystemMySQL, }, SpanNameFormatter: &defaultSpanNameFormatter{}, }, cfg) diff --git a/conn_test.go b/conn_test.go index 0082763..ddbe125 100644 --- a/conn_test.go +++ b/conn_test.go @@ -23,7 +23,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opentelemetry.io/otel/attribute" - semconv "go.opentelemetry.io/otel/semconv/v1.4.0" + semconv "go.opentelemetry.io/otel/semconv/v1.7.0" "go.opentelemetry.io/otel/trace" ) diff --git a/driver_test.go b/driver_test.go index f9f92f7..9159c49 100644 --- a/driver_test.go +++ b/driver_test.go @@ -21,6 +21,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/attribute" + semconv "go.opentelemetry.io/otel/semconv/v1.7.0" ) type mockDriver struct { @@ -63,12 +65,12 @@ var ( ) func TestNewDriver(t *testing.T) { - d := newDriver(newMockDriver(false), config{DBSystem: "test"}) + d := newDriver(newMockDriver(false), config{Attributes: []attribute.KeyValue{semconv.DBSystemMySQL}}) otelDriver, ok := d.(*otDriver) require.True(t, ok) assert.Equal(t, newMockDriver(false), otelDriver.driver) - assert.Equal(t, config{DBSystem: "test"}, otelDriver.cfg) + assert.Equal(t, config{Attributes: []attribute.KeyValue{semconv.DBSystemMySQL}}, otelDriver.cfg) } func TestOtDriver_Open(t *testing.T) { diff --git a/example/main.go b/example/main.go index 560115d..826ce96 100644 --- a/example/main.go +++ b/example/main.go @@ -91,13 +91,17 @@ func main() { initMeter() // Connect to database - db, err := otelsql.Open("mysql", mysqlDSN, semconv.DBSystemMySQL.Value.AsString()) + db, err := otelsql.Open("mysql", mysqlDSN, otelsql.WithAttributes( + semconv.DBSystemMySQL, + )) if err != nil { panic(err) } defer db.Close() - err = otelsql.RegisterDBStatsMetrics(db, semconv.DBSystemMySQL.Value.AsString()) + err = otelsql.RegisterDBStatsMetrics(db, otelsql.WithAttributes( + semconv.DBSystemMySQL, + )) if err != nil { panic(err) } diff --git a/example_test.go b/example_test.go index 72f3282..b7e7321 100644 --- a/example_test.go +++ b/example_test.go @@ -18,8 +18,6 @@ import ( "database/sql" "database/sql/driver" - semconv "go.opentelemetry.io/otel/semconv/v1.7.0" - "github.com/XSAM/otelsql" ) @@ -35,7 +33,7 @@ var ( func ExampleOpen() { // Connect to database - db, err := otelsql.Open("mysql", mysqlDSN, semconv.DBSystemMySQL.Value.AsString()) + db, err := otelsql.Open("mysql", mysqlDSN) if err != nil { panic(err) } @@ -45,12 +43,12 @@ func ExampleOpen() { func ExampleOpenDB() { // Connect to database - db := otelsql.OpenDB(connector, semconv.DBSystemMySQL.Value.AsString()) + db := otelsql.OpenDB(connector) defer db.Close() } func ExampleWrapDriver() { - otDriver := otelsql.WrapDriver(dri, semconv.DBSystemMySQL.Value.AsString()) + otDriver := otelsql.WrapDriver(dri) connector, err := otDriver.(driver.DriverContext).OpenConnector(mysqlDSN) if err != nil { @@ -65,13 +63,13 @@ func ExampleWrapDriver() { func ExampleRegister() { // Register an OTel driver - driverName, err := otelsql.Register("mysql", semconv.DBSystemMySQL.Value.AsString()) + driverName, err := otelsql.Register("mysql") if err != nil { panic(err) } // Connect to database - db, err := otelsql.Open(driverName, mysqlDSN, semconv.DBSystemMySQL.Value.AsString()) + db, err := otelsql.Open(driverName, mysqlDSN) if err != nil { panic(err) } diff --git a/sql.go b/sql.go index d06b547..ccb4f35 100644 --- a/sql.go +++ b/sql.go @@ -29,16 +29,11 @@ var registerLock sync.Mutex var maxDriverSlot = 1000 -// Register initializes and registers our OTel wrapped database driver +// Register initializes and registers OTel wrapped database driver // identified by its driverName, using provided Option. // It is possible to register multiple wrappers for the same database driver if // needing different Option for different connections. -// Parameter dbSystem is an identifier for the database management system (DBMS) -// product being used. -// -// For more information, see semantic conventions for database -// https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/database.md -func Register(driverName string, dbSystem string, options ...Option) (string, error) { +func Register(driverName string, options ...Option) (string, error) { // Retrieve the driver implementation we need to wrap with instrumentation db, err := sql.Open(driverName, "") if err != nil { @@ -67,7 +62,7 @@ func Register(driverName string, dbSystem string, options ...Option) (string, er } } if !found { - sql.Register(regName, newDriver(dri, newConfig(dbSystem, options...))) + sql.Register(regName, newDriver(dri, newConfig(options...))) return regName, nil } } @@ -75,22 +70,12 @@ func Register(driverName string, dbSystem string, options ...Option) (string, er } // WrapDriver takes a SQL driver and wraps it with OTel instrumentation. -// Parameter dbSystem is an identifier for the database management system (DBMS) -// product being used. -// -// For more information, see semantic conventions for database -// https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/database.md -func WrapDriver(dri driver.Driver, dbSystem string, options ...Option) driver.Driver { - return newDriver(dri, newConfig(dbSystem, options...)) +func WrapDriver(dri driver.Driver, options ...Option) driver.Driver { + return newDriver(dri, newConfig(options...)) } // Open is a wrapper over sql.Open with OTel instrumentation. -// Parameter dbSystem is an identifier for the database management system (DBMS) -// product being used. -// -// For more information, see semantic conventions for database -// https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/database.md -func Open(driverName, dataSourceName string, dbSystem string, options ...Option) (*sql.DB, error) { +func Open(driverName, dataSourceName string, options ...Option) (*sql.DB, error) { // Retrieve the driver implementation we need to wrap with instrumentation db, err := sql.Open(driverName, "") if err != nil { @@ -101,7 +86,7 @@ func Open(driverName, dataSourceName string, dbSystem string, options ...Option) return nil, err } - otDriver := newOtDriver(d, newConfig(dbSystem, options...)) + otDriver := newOtDriver(d, newConfig(options...)) if _, ok := d.(driver.DriverContext); ok { connector, err := otDriver.OpenConnector(dataSourceName) @@ -115,21 +100,16 @@ func Open(driverName, dataSourceName string, dbSystem string, options ...Option) } // OpenDB is a wrapper over sql.OpenDB with OTel instrumentation. -// Parameter dbSystem is an identifier for the database management system (DBMS) -// product being used. -// -// For more information, see semantic conventions for database -// https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/database.md -func OpenDB(c driver.Connector, dbSystem string, options ...Option) *sql.DB { - d := newOtDriver(c.Driver(), newConfig(dbSystem, options...)) +func OpenDB(c driver.Connector, options ...Option) *sql.DB { + d := newOtDriver(c.Driver(), newConfig(options...)) connector := newConnector(c, d) return sql.OpenDB(connector) } // RegisterDBStatsMetrics register sql.DBStats metrics with OTel instrumentation. -func RegisterDBStatsMetrics(db *sql.DB, dbSystem string, opts ...Option) error { - cfg := newConfig(dbSystem, opts...) +func RegisterDBStatsMetrics(db *sql.DB, opts ...Option) error { + cfg := newConfig(opts...) meter := cfg.Meter instruments, err := newDBStatsInstruments(meter) diff --git a/sql_test.go b/sql_test.go index cc71839..c375cbb 100644 --- a/sql_test.go +++ b/sql_test.go @@ -25,7 +25,6 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric/nonrecording" - semconv "go.opentelemetry.io/otel/semconv/v1.4.0" ) var driverName string @@ -41,7 +40,7 @@ func init() { maxDriverSlot = 1 var err error - driverName, err = Register(testDriverName, "test-db", + driverName, err = Register(testDriverName, WithAttributes(attribute.String("foo", "bar")), ) if err != nil { @@ -60,17 +59,16 @@ func TestRegister(t *testing.T) { require.True(t, ok) assert.Equal(t, &mockDriver{openConnectorCount: 2}, otelDriver.driver) assert.ElementsMatch(t, []attribute.KeyValue{ - semconv.DBSystemKey.String("test-db"), attribute.String("foo", "bar"), }, otelDriver.cfg.Attributes) // Exceed max slot count - _, err = Register(testDriverName, "test-db") + _, err = Register(testDriverName) assert.Error(t, err) } func TestWrapDriver(t *testing.T) { - driver := WrapDriver(newMockDriver(false), "test-db", + driver := WrapDriver(newMockDriver(false), WithAttributes(attribute.String("foo", "bar")), ) @@ -79,7 +77,6 @@ func TestWrapDriver(t *testing.T) { require.True(t, ok) assert.IsType(t, &mockDriver{}, otelDriver.driver) assert.ElementsMatch(t, []attribute.KeyValue{ - semconv.DBSystemKey.String("test-db"), attribute.String("foo", "bar"), }, otelDriver.cfg.Attributes) } @@ -101,7 +98,7 @@ func TestOpen(t *testing.T) { for _, tc := range testCases { t.Run(tc.driverName, func(t *testing.T) { - db, err := Open(tc.driverName, "", "test-db", + db, err := Open(tc.driverName, "", WithAttributes(attribute.String("foo", "bar")), ) t.Cleanup(func() { @@ -118,7 +115,6 @@ func TestOpen(t *testing.T) { require.True(t, ok) assert.IsType(t, tc.expectedDriverType, otelDriver.driver) assert.ElementsMatch(t, []attribute.KeyValue{ - semconv.DBSystemKey.String("test-db"), attribute.String("foo", "bar"), }, otelDriver.cfg.Attributes) }) @@ -129,7 +125,7 @@ func TestOpenDB(t *testing.T) { connector, err := newMockDriver(false).OpenConnector("") require.NoError(t, err) - db := OpenDB(connector, "test-db", WithAttributes(attribute.String("foo", "bar"))) + db := OpenDB(connector, WithAttributes(attribute.String("foo", "bar"))) require.NotNil(t, db) _, err = db.Conn(context.Background()) @@ -139,7 +135,6 @@ func TestOpenDB(t *testing.T) { require.True(t, ok) assert.IsType(t, &mockDriver{}, otelDriver.driver) assert.ElementsMatch(t, []attribute.KeyValue{ - semconv.DBSystemKey.String("test-db"), attribute.String("foo", "bar"), }, otelDriver.cfg.Attributes) } @@ -148,7 +143,7 @@ func TestRegisterDBStatsMetrics(t *testing.T) { db, err := sql.Open(driverName, "") require.NoError(t, err) - err = RegisterDBStatsMetrics(db, "test-db") + err = RegisterDBStatsMetrics(db) assert.NoError(t, err) } @@ -159,7 +154,7 @@ func TestRecordDBStatsMetricsNoPanic(t *testing.T) { instruments, err := newDBStatsInstruments(nonrecording.NewNoopMeterProvider().Meter("test")) require.NoError(t, err) - cfg := newConfig("db") + cfg := newConfig() recordDBStatsMetrics(context.Background(), db.Stats(), instruments, cfg) } diff --git a/stmt_test.go b/stmt_test.go index fb0aea7..c75b581 100644 --- a/stmt_test.go +++ b/stmt_test.go @@ -23,7 +23,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opentelemetry.io/otel/attribute" - semconv "go.opentelemetry.io/otel/semconv/v1.4.0" + semconv "go.opentelemetry.io/otel/semconv/v1.7.0" ) type mockStmt struct {