diff --git a/options.go b/options.go index f987d4d..2568195 100644 --- a/options.go +++ b/options.go @@ -62,12 +62,30 @@ func WithMeterAttributes(attrs ...attribute.KeyValue) Option { }) } -// WithTrimSQLInSpanName will use the SQL statement's first word as the span -// name. By default, the whole SQL statement is used as a span name, where -// applicable. +// Deprecated: This is now the default behavior; use [WithFullSQLInSpanName] to +// opt back into the previous behavior of using the whole SQL statement. +// +// WithTrimSQLInSpanName uses the SQL statement's first word (the operation +// name, e.g. "SELECT") as the span name. func WithTrimSQLInSpanName() Option { return optionFunc(func(cfg *tracerConfig) { - cfg.trimQuerySpanName = true + cfg.fullQuerySpanName = false + }) +} + +// WithFullSQLInSpanName uses the whole SQL statement as the span name. +// +// This is generally discouraged: the OpenTelemetry database span conventions +// recommend a low-cardinality span name, and redaction/masking rules are +// typically applied to the db.query.text attribute rather than the span name, +// so any sensitive data embedded in the statement can leak through the name. +// By default, the low-cardinality operation name (e.g. "SELECT") is used +// instead. +// +// See https://opentelemetry.io/docs/specs/semconv/db/database-spans/. +func WithFullSQLInSpanName() Option { + return optionFunc(func(cfg *tracerConfig) { + cfg.fullQuerySpanName = true }) } @@ -92,7 +110,9 @@ type SpanNameCtxFunc func(ctx context.Context, stmt string) string // a SQL statement. The function will be called with the SQL statement as a // parameter. // -// By default, the whole SQL statement is used as a span name, where applicable. +// By default, the low-cardinality operation name (e.g. "SELECT") is extracted +// from the SQL statement and used as the span name. This function also +// determines the value of the db.operation.name attribute. func WithSpanNameFunc(fn SpanNameFunc) Option { return WithSpanNameCtxFunc(func(_ context.Context, stmt string) string { return fn(stmt) @@ -103,21 +123,35 @@ func WithSpanNameFunc(fn SpanNameFunc) Option { // for a SQL statement. The function will be called with the context.Context and // SQL statement as a parameter. // -// By default, the whole SQL statement is used as a span name, where applicable. +// By default, the low-cardinality operation name (e.g. "SELECT") is extracted +// from the SQL statement and used as the span name. This function also +// determines the value of the db.operation.name attribute. func WithSpanNameCtxFunc(fn SpanNameCtxFunc) Option { return optionFunc(func(cfg *tracerConfig) { cfg.spanNameCtxFunc = fn }) } -// WithDisableQuerySpanNamePrefix will disable the default prefix for the span -// name. By default, the span name is prefixed with "batch query" or "query". +// Deprecated: Span names are no longer prefixed by default, so this is a no-op. +// Use [WithQuerySpanNamePrefix] to opt back into the previous prefixing +// behavior. +// +// WithDisableQuerySpanNamePrefix disables the prefix for the span name. func WithDisableQuerySpanNamePrefix() Option { return optionFunc(func(cfg *tracerConfig) { cfg.prefixQuerySpanName = false }) } +// WithQuerySpanNamePrefix prefixes the span name with the operation kind, i.e. +// "query ", "prepare " or "batch query ". By default no prefix is added so that +// span names follow the OpenTelemetry database span conventions. +func WithQuerySpanNamePrefix() Option { + return optionFunc(func(cfg *tracerConfig) { + cfg.prefixQuerySpanName = true + }) +} + // WithDisableConnectionDetailsInAttributes will disable logging the connection details. // in the span's attributes. func WithDisableConnectionDetailsInAttributes() Option { diff --git a/tracer.go b/tracer.go index 9b2e9bd..46946cc 100644 --- a/tracer.go +++ b/tracer.go @@ -75,7 +75,7 @@ type Tracer struct { operationDuration dbconv.ClientOperationDuration operationErrors metric.Int64Counter - trimQuerySpanName bool + fullQuerySpanName bool spanNameCtxFunc SpanNameCtxFunc prefixQuerySpanName bool logSQLStatement bool @@ -91,7 +91,7 @@ type tracerConfig struct { tracerAttrs []attribute.KeyValue meterAttrs []attribute.KeyValue - trimQuerySpanName bool + fullQuerySpanName bool spanNameCtxFunc SpanNameCtxFunc prefixQuerySpanName bool logSQLStatement bool @@ -111,9 +111,9 @@ func NewTracer(opts ...Option) *Tracer { meterAttrs: []attribute.KeyValue{ semconv.DBSystemNamePostgreSQL, }, - trimQuerySpanName: false, + fullQuerySpanName: false, spanNameCtxFunc: defaultSpanNameCtxFunc, - prefixQuerySpanName: true, + prefixQuerySpanName: false, logSQLStatement: true, logConnectionDetails: true, includeParams: false, @@ -141,7 +141,7 @@ func NewTracer(opts ...Option) *Tracer { }, tracerAttrs: cfg.tracerAttrs, meterAttrs: cfg.meterAttrs, - trimQuerySpanName: cfg.trimQuerySpanName, + fullQuerySpanName: cfg.fullQuerySpanName, spanNameCtxFunc: cfg.spanNameCtxFunc, prefixQuerySpanName: cfg.prefixQuerySpanName, logSQLStatement: cfg.logSQLStatement, @@ -262,10 +262,12 @@ func (t *Tracer) TraceQueryStart(ctx context.Context, conn *pgx.Conn, data pgx.T attrs = append(attrs, connectionAttributesFromConfig(conn.Config())...) } + operationName := t.spanNameCtxFunc(ctx, data.SQL) + if t.logSQLStatement { attrs = append(attrs, semconv.DBQueryText(data.SQL), - semconv.DBOperationName(t.spanNameCtxFunc(ctx, data.SQL)), + semconv.DBOperationName(operationName), ) if t.includeParams { @@ -278,18 +280,31 @@ func (t *Tracer) TraceQueryStart(ctx context.Context, conn *pgx.Conn, data pgx.T trace.WithAttributes(attrs...), ) - spanName := data.SQL - if t.trimQuerySpanName { - spanName = t.spanNameCtxFunc(ctx, data.SQL) + spanName := t.spanName(data.SQL, operationName, "query ") + + ctx, _ = t.tracer.Start(ctx, spanName, opts...) + + return ctx +} + +// spanName builds the span name following the OpenTelemetry database span +// conventions: the low-cardinality operation name (operationName, e.g. +// "SELECT") by default, or the full SQL statement when WithFullSQLInSpanName is +// set, optionally prefixed when WithQuerySpanNamePrefix is set. +// +// See https://opentelemetry.io/docs/specs/semconv/db/database-spans/ for the +// span name guidance this follows. +func (t *Tracer) spanName(sql, operationName, prefix string) string { + name := operationName + if t.fullQuerySpanName { + name = sql } if t.prefixQuerySpanName { - spanName = "query " + spanName + name = prefix + name } - ctx, _ = t.tracer.Start(ctx, spanName, opts...) - - return ctx + return name } // TraceQueryEnd is called at the end of Query, QueryRow, and Exec calls. @@ -429,10 +444,12 @@ func (t *Tracer) TraceBatchQuery(ctx context.Context, conn *pgx.Conn, data pgx.T attrs = append(attrs, connectionAttributesFromConfig(conn.Config())...) } + operationName := t.spanNameCtxFunc(ctx, data.SQL) + if t.logSQLStatement { attrs = append(attrs, semconv.DBQueryText(data.SQL), - semconv.DBOperationName(t.spanNameCtxFunc(ctx, data.SQL)), + semconv.DBOperationName(operationName), ) if t.includeParams { @@ -445,18 +462,7 @@ func (t *Tracer) TraceBatchQuery(ctx context.Context, conn *pgx.Conn, data pgx.T trace.WithAttributes(attrs...), ) - var spanName string - if t.trimQuerySpanName { - spanName = t.spanNameCtxFunc(ctx, data.SQL) - if t.prefixQuerySpanName { - spanName = "query " + spanName - } - } else { - spanName = data.SQL - if t.prefixQuerySpanName { - spanName = "batch query " + spanName - } - } + spanName := t.spanName(data.SQL, operationName, "batch query ") _, span := t.tracer.Start(ctx, spanName, opts...) recordSpanError(span, data.Err) @@ -556,7 +562,8 @@ func (t *Tracer) TracePrepareStart(ctx context.Context, conn *pgx.Conn, data pgx attrs = append(attrs, connectionAttributesFromConfig(conn.Config())...) } - attrs = append(attrs, semconv.DBOperationName(t.spanNameCtxFunc(ctx, data.SQL))) + operationName := t.spanNameCtxFunc(ctx, data.SQL) + attrs = append(attrs, semconv.DBOperationName(operationName)) if t.logSQLStatement { attrs = append(attrs, semconv.DBQueryText(data.SQL)) @@ -567,13 +574,7 @@ func (t *Tracer) TracePrepareStart(ctx context.Context, conn *pgx.Conn, data pgx trace.WithAttributes(attrs...), ) - spanName := data.SQL - if t.trimQuerySpanName { - spanName = t.spanNameCtxFunc(ctx, data.SQL) - } - if t.prefixQuerySpanName { - spanName = "prepare " + spanName - } + spanName := t.spanName(data.SQL, operationName, "prepare ") ctx, _ = t.tracer.Start(ctx, spanName, opts...) diff --git a/tracer_test.go b/tracer_test.go index 89f97c9..246efc0 100644 --- a/tracer_test.go +++ b/tracer_test.go @@ -388,3 +388,106 @@ func TestTracer_spanAttributes(t *testing.T) { }) } } + +func TestTracer_spanName(t *testing.T) { + conn := newMockConn(t, "fakehost", 5432, "fakeuser", "fakedb") + + const sql = "SELECT * FROM users" + + query := func(ctx context.Context, tracer *Tracer, conn *pgx.Conn) { + ctx = tracer.TraceQueryStart(ctx, conn, pgx.TraceQueryStartData{SQL: sql}) + tracer.TraceQueryEnd(ctx, conn, pgx.TraceQueryEndData{}) + } + prepare := func(ctx context.Context, tracer *Tracer, conn *pgx.Conn) { + ctx = tracer.TracePrepareStart(ctx, conn, pgx.TracePrepareStartData{Name: "stmt1", SQL: sql}) + tracer.TracePrepareEnd(ctx, conn, pgx.TracePrepareEndData{}) + } + batch := func(ctx context.Context, tracer *Tracer, conn *pgx.Conn) { + tracer.TraceBatchQuery(ctx, conn, pgx.TraceBatchQueryData{SQL: sql}) + } + + tests := []struct { + name string + opts []Option + drive func(ctx context.Context, tracer *Tracer, conn *pgx.Conn) + wantName string + }{ + { + name: "query defaults to operation name", + drive: query, + wantName: "SELECT", + }, + { + name: "prepare defaults to operation name", + drive: prepare, + wantName: "SELECT", + }, + { + name: "batch query defaults to operation name", + drive: batch, + wantName: "SELECT", + }, + { + name: "query with full SQL", + opts: []Option{WithFullSQLInSpanName()}, + drive: query, + wantName: sql, + }, + { + name: "query with prefix", + opts: []Option{WithQuerySpanNamePrefix()}, + drive: query, + wantName: "query SELECT", + }, + { + name: "prepare with prefix", + opts: []Option{WithQuerySpanNamePrefix()}, + drive: prepare, + wantName: "prepare SELECT", + }, + { + name: "batch query with prefix", + opts: []Option{WithQuerySpanNamePrefix()}, + drive: batch, + wantName: "batch query SELECT", + }, + { + // Full SQL plus the prefix reproduces the pre-compliance default + // behavior exactly; this is the documented migration path. + name: "full SQL with prefix matches legacy default", + opts: []Option{WithFullSQLInSpanName(), WithQuerySpanNamePrefix()}, + drive: query, + wantName: "query " + sql, + }, + { + // A custom span name function now drives the span name by default, + // not just the db.operation.name attribute. + name: "custom span name func drives the name", + opts: []Option{WithSpanNameCtxFunc(func(context.Context, string) string { + return "custom" + })}, + drive: query, + wantName: "custom", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + exporter := tracetest.NewInMemoryExporter() + tp := sdktrace.NewTracerProvider(sdktrace.WithSyncer(exporter)) + t.Cleanup(func() { require.NoError(t, tp.Shutdown(context.Background())) }) + + opts := append([]Option{WithTracerProvider(tp)}, tt.opts...) + tracer := NewTracer(opts...) + + ctx, parentSpan := tp.Tracer("test").Start(context.Background(), "parent") + tt.drive(ctx, tracer, conn) + parentSpan.End() + + spans := exporter.GetSpans() + require.Greater(t, len(spans), 0, "no spans recorded") + + assert.Equal(t, tt.wantName, spans[0].Name) + }) + } +}