Handle sql.ErrNoRows properly (#23)

* Handle sql.ErrNoRows properly

* bump version

internal/constants.go

@@ -7,5 +7,5 @@ const (
 
 	// InstrumentationVersion is the version of the otelpgx library. This will
 	// be used as an attribute on each span.
-	InstrumentationVersion = "v0.5.1"
+	InstrumentationVersion = "v0.5.2"
 )

tracer.go

@@ -2,20 +2,29 @@ package otelpgx
 
 import (
 	"context"
+	"database/sql"
+	"errors"
 	"fmt"
 	"strings"
 
 	"github.com/jackc/pgx/v5"
 	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/codes"
-	semconv "go.opentelemetry.io/otel/semconv/v1.12.0"
+	semconv "go.opentelemetry.io/otel/semconv/v1.17.0"
 	"go.opentelemetry.io/otel/trace"
 
 	"github.com/exaring/otelpgx/internal"
 )
 
-var RowsAffectedKey = attribute.Key("pgx.rows_affected")
+const (
+	// RowsAffectedKey represents the number of rows affected.
+	RowsAffectedKey = attribute.Key("pgx.rows_affected")
+	// QueryParametersKey represents the query parameters.
+	QueryParametersKey = attribute.Key("pgx.query.parameters")
+	// BatchSizeKey represents the batch size.
+	BatchSizeKey = attribute.Key("pgx.batch.size")
+)
 
 // Tracer is a wrapper around the pgx tracer interfaces which instrument
 // queries.
@@ -65,13 +74,13 @@ func NewTracer(opts ...Option) *Tracer {
 }
 
 func recordError(span trace.Span, err error) {
-	if err != nil {
+	if err != nil && !errors.Is(err, sql.ErrNoRows) {
 		span.RecordError(err)
 		span.SetStatus(codes.Error, err.Error())
 	}
 }
 
-const sqlOperationUnknkown = "UNKNOWN"
+const sqlOperationUnknown = "UNKNOWN"
 
 // sqlOperationName attempts to get the first 'word' from a given SQL query, which usually
 // is the operation name (e.g. 'SELECT').
@@ -88,7 +97,7 @@ func (t *Tracer) sqlOperationName(stmt string) string {
 		// Fall back to a fixed value to prevent creating lots of tracing operations
 		// differing only by the amount of whitespace in them (in case we'd fall back
 		// to the full query or a cut-off version).
-		return sqlOperationUnknkown
+		return sqlOperationUnknown
 	}
 	return strings.ToUpper(parts[0])
 }
@@ -98,9 +107,11 @@ func (t *Tracer) sqlOperationName(stmt string) string {
 func connectionAttributesFromConfig(config *pgx.ConnConfig) []trace.SpanStartOption {
 	if config != nil {
 		return []trace.SpanStartOption{
-			trace.WithAttributes(attribute.String(string(semconv.NetPeerNameKey), config.Host)),
-			trace.WithAttributes(attribute.Int(string(semconv.NetPeerPortKey), int(config.Port))),
-			trace.WithAttributes(attribute.String(string(semconv.DBUserKey), config.User)),
+			trace.WithAttributes(
+				semconv.NetPeerName(config.Host),
+				semconv.NetPeerPort(int(config.Port)),
+				semconv.DBUser(config.User),
+			),
 		}
 	}
 	return nil
@@ -123,7 +134,7 @@ func (t *Tracer) TraceQueryStart(ctx context.Context, conn *pgx.Conn, data pgx.T
 	}
 
 	if t.logSQLStatement {
-		opts = append(opts, trace.WithAttributes(semconv.DBStatementKey.String(data.SQL)))
+		opts = append(opts, trace.WithAttributes(semconv.DBStatement(data.SQL)))
 		if t.includeParams {
 			opts = append(opts, trace.WithAttributes(makeParamsAttribute(data.Args)))
 		}
@@ -144,7 +155,9 @@ func (t *Tracer) TraceQueryEnd(ctx context.Context, _ *pgx.Conn, data pgx.TraceQ
 	span := trace.SpanFromContext(ctx)
 	recordError(span, data.Err)
 
-	span.SetAttributes(RowsAffectedKey.Int(int(data.CommandTag.RowsAffected())))
+	if data.Err != nil {
+		span.SetAttributes(RowsAffectedKey.Int64(data.CommandTag.RowsAffected()))
+	}
 
 	span.End()
 }
@@ -160,7 +173,7 @@ func (t *Tracer) TraceCopyFromStart(ctx context.Context, conn *pgx.Conn, data pg
 	opts := []trace.SpanStartOption{
 		trace.WithSpanKind(trace.SpanKindClient),
 		trace.WithAttributes(t.attrs...),
-		trace.WithAttributes(attribute.String("db.table", data.TableName.Sanitize())),
+		trace.WithAttributes(semconv.DBSQLTable(data.TableName.Sanitize())),
 	}
 
 	if conn != nil {
@@ -177,7 +190,9 @@ func (t *Tracer) TraceCopyFromEnd(ctx context.Context, _ *pgx.Conn, data pgx.Tra
 	span := trace.SpanFromContext(ctx)
 	recordError(span, data.Err)
 
-	span.SetAttributes(RowsAffectedKey.Int(int(data.CommandTag.RowsAffected())))
+	if data.Err != nil {
+		span.SetAttributes(RowsAffectedKey.Int64(data.CommandTag.RowsAffected()))
+	}
 
 	span.End()
 }
@@ -198,7 +213,7 @@ func (t *Tracer) TraceBatchStart(ctx context.Context, conn *pgx.Conn, data pgx.T
 	opts := []trace.SpanStartOption{
 		trace.WithSpanKind(trace.SpanKindClient),
 		trace.WithAttributes(t.attrs...),
-		trace.WithAttributes(attribute.Int("pgx.batch.size", size)),
+		trace.WithAttributes(BatchSizeKey.Int(size)),
 	}
 
 	if conn != nil {
@@ -222,7 +237,7 @@ func (t *Tracer) TraceBatchQuery(ctx context.Context, conn *pgx.Conn, data pgx.T
 	}
 
 	if t.logSQLStatement {
-		opts = append(opts, trace.WithAttributes(semconv.DBStatementKey.String(data.SQL)))
+		opts = append(opts, trace.WithAttributes(semconv.DBStatement(data.SQL)))
 		if t.includeParams {
 			opts = append(opts, trace.WithAttributes(makeParamsAttribute(data.Args)))
 		}
@@ -296,7 +311,7 @@ func (t *Tracer) TracePrepareStart(ctx context.Context, conn *pgx.Conn, data pgx
 	}
 
 	if t.logSQLStatement {
-		opts = append(opts, trace.WithAttributes(semconv.DBStatementKey.String(data.SQL)))
+		opts = append(opts, trace.WithAttributes(semconv.DBStatement(data.SQL)))
 	}
 
 	spanName := "prepare " + data.SQL
@@ -322,7 +337,5 @@ func makeParamsAttribute(args []any) attribute.KeyValue {
 	for i := range args {
 		ss[i] = fmt.Sprintf("%+v", args[i])
 	}
-	// Since there doesn't appear to be a standard key for this in semconv, prefix it to avoid
-	// clashing with future standard attributes.
-	return attribute.Key("pgx.query.parameters").StringSlice(ss)
+	return QueryParametersKey.StringSlice(ss)
 }

tracer_test.go

@@ -34,13 +34,13 @@ func TestTracer_sqlOperationName(t *testing.T) {
 			name:    "Whitespace-only query",
 			query:   " \n\t",
 			tracer:  NewTracer(),
-			expName: sqlOperationUnknkown,
+			expName: sqlOperationUnknown,
 		},
 		{
 			name:    "Empty query",
 			query:   "",
 			tracer:  NewTracer(),
-			expName: sqlOperationUnknkown,
+			expName: sqlOperationUnknown,
 		},
 		{
 			name:    "Functional span name (-- comment style)",
@@ -64,19 +64,19 @@ func TestTracer_sqlOperationName(t *testing.T) {
 			name:    "Functional span name (no annotation)",
 			query:   "--\nSELECT * FROM user",
 			tracer:  NewTracer(WithSpanNameFunc(defaultSpanNameFunc)),
-			expName: sqlOperationUnknkown,
+			expName: sqlOperationUnknown,
 		},
 		{
 			name:    "Custom SQL name query (normal comment)",
 			query:   "-- foo \nSELECT * FROM users",
 			tracer:  NewTracer(WithSpanNameFunc(defaultSpanNameFunc)),
-			expName: sqlOperationUnknkown,
+			expName: sqlOperationUnknown,
 		},
 		{
 			name:    "Custom SQL name query (invalid formatting)",
 			query:   "foo \nSELECT * FROM users",
 			tracer:  NewTracer(WithSpanNameFunc(defaultSpanNameFunc)),
-			expName: sqlOperationUnknkown,
+			expName: sqlOperationUnknown,
 		},
 	}
 	for _, tt := range tests {
@@ -115,21 +115,21 @@ var defaultSpanNameFunc SpanNameFunc = func(query string) string {
 			continue
 		}
 		if !strings.HasPrefix(rest, " name: ") {
-			return sqlOperationUnknkown
+			return sqlOperationUnknown
 		}
 
 		part := strings.Split(strings.TrimSpace(line), " ")
 		if prefix == "/*" {
 			part = part[:len(part)-1] // removes the trailing "*/" element
 		}
 		if len(part) == 2 {
-			return sqlOperationUnknkown
+			return sqlOperationUnknown
 		}
 
 		queryName := part[2]
 		queryType := strings.TrimSpace(part[3])
 
 		return queryName + " " + queryType
 	}
-	return sqlOperationUnknkown
+	return sqlOperationUnknown
 }