Skip to content

Commit

Permalink
Remove dbSystem parameter from all exported functions (#80)
Browse files Browse the repository at this point in the history
* Remove `dbSystem` parameter from all exported functions

* Update CHANGELOG
  • Loading branch information
XSAM authored Apr 5, 2022
1 parent a91b677 commit 80d1572
Show file tree
Hide file tree
Showing 10 changed files with 49 additions and 70 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 2 additions & 10 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -51,8 +51,6 @@ type config struct {

SpanOptions SpanOptions

DBSystem string

// Attributes will be set to each span.
Attributes []attribute.KeyValue

Expand Down Expand Up @@ -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()),
Expand Down
7 changes: 3 additions & 4 deletions config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion conn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down
6 changes: 4 additions & 2 deletions driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down
8 changes: 6 additions & 2 deletions example/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
12 changes: 5 additions & 7 deletions example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ import (
"database/sql"
"database/sql/driver"

semconv "go.opentelemetry.io/otel/semconv/v1.7.0"

"github.com/XSAM/otelsql"
)

Expand All @@ -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)
}
Expand All @@ -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 {
Expand All @@ -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)
}
Expand Down
42 changes: 11 additions & 31 deletions sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -67,30 +62,20 @@ 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
}
}
return "", errors.New("unable to register driver, all slots have been taken")
}

// 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 {
Expand All @@ -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)
Expand All @@ -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)
Expand Down
19 changes: 7 additions & 12 deletions sql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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")),
)

Expand All @@ -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)
}
Expand All @@ -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() {
Expand All @@ -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)
})
Expand All @@ -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())
Expand All @@ -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)
}
Expand All @@ -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)
}

Expand All @@ -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)
}
2 changes: 1 addition & 1 deletion stmt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 80d1572

Please sign in to comment.