Cleanup of self-review

contrib/database/sql/conn.go

@@ -54,17 +54,14 @@ func (tc *tracedConn) BeginTx(ctx context.Context, opts driver.TxOptions) (tx dr
 	if err != nil {
 		return nil, err
 	}
-	if err != nil {
-		return nil, err
-	}
 	return &tracedTx{tx, tc.traceParams, ctx}, nil
 }
 
 func (tc *tracedConn) PrepareContext(ctx context.Context, query string) (stmt driver.Stmt, err error) {
 	start := time.Now()
 	if connPrepareCtx, ok := tc.Conn.(driver.ConnPrepareContext); ok {
 		sqlCommentCarrier := tracer.SQLCommentCarrier{DiscardDynamicTags: true}
-		span := tc.tryStartTrace(ctx, queryTypePrepare, query, start, &sqlCommentCarrier, err)
+		span := tc.tryStartTrace(ctx, queryTypePrepare, query, start, &sqlCommentCarrier)
 		if span != nil {
 			defer func() {
 				span.Finish(tracer.WithError(err))
@@ -78,7 +75,7 @@ func (tc *tracedConn) PrepareContext(ctx context.Context, query string) (stmt dr
 		return &tracedStmt{Stmt: stmt, traceParams: tc.traceParams, ctx: ctx, query: query}, nil
 	}
 	sqlCommentCarrier := tracer.SQLCommentCarrier{DiscardDynamicTags: true}
-	span := tc.tryStartTrace(ctx, queryTypePrepare, query, start, &sqlCommentCarrier, err)
+	span := tc.tryStartTrace(ctx, queryTypePrepare, query, start, &sqlCommentCarrier)
 	if span != nil {
 		defer func() {
 			span.Finish(tracer.WithError(err))
@@ -96,7 +93,7 @@ func (tc *tracedConn) ExecContext(ctx context.Context, query string, args []driv
 	start := time.Now()
 	if execContext, ok := tc.Conn.(driver.ExecerContext); ok {
 		sqlCommentCarrier := tracer.SQLCommentCarrier{}
-		span := tc.tryStartTrace(ctx, queryTypeExec, query, start, &sqlCommentCarrier, err)
+		span := tc.tryStartTrace(ctx, queryTypeExec, query, start, &sqlCommentCarrier)
 		if span != nil {
 			defer func() {
 				span.Finish(tracer.WithError(err))
@@ -116,7 +113,7 @@ func (tc *tracedConn) ExecContext(ctx context.Context, query string, args []driv
 		default:
 		}
 		sqlCommentCarrier := tracer.SQLCommentCarrier{}
-		span := tc.tryStartTrace(ctx, queryTypeExec, query, start, &sqlCommentCarrier, err)
+		span := tc.tryStartTrace(ctx, queryTypeExec, query, start, &sqlCommentCarrier)
 		if span != nil {
 			defer func() {
 				span.Finish(tracer.WithError(err))
@@ -146,7 +143,7 @@ func (tc *tracedConn) QueryContext(ctx context.Context, query string, args []dri
 	start := time.Now()
 	if queryerContext, ok := tc.Conn.(driver.QueryerContext); ok {
 		sqlCommentCarrier := tracer.SQLCommentCarrier{}
-		span := tc.tryStartTrace(ctx, queryTypeQuery, query, start, &sqlCommentCarrier, err)
+		span := tc.tryStartTrace(ctx, queryTypeQuery, query, start, &sqlCommentCarrier)
 		if span != nil {
 			defer func() {
 				span.Finish(tracer.WithError(err))
@@ -167,7 +164,7 @@ func (tc *tracedConn) QueryContext(ctx context.Context, query string, args []dri
 		default:
 		}
 		sqlCommentCarrier := tracer.SQLCommentCarrier{}
-		span := tc.tryStartTrace(ctx, queryTypeQuery, query, start, &sqlCommentCarrier, err)
+		span := tc.tryStartTrace(ctx, queryTypeQuery, query, start, &sqlCommentCarrier)
 		if span != nil {
 			defer func() {
 				span.Finish(tracer.WithError(err))
@@ -215,14 +212,7 @@ func WithSpanTags(ctx context.Context, tags map[string]string) context.Context {
 }
 
 // tryStartTrace will create a span using the given arguments, but will act as a no-op when err is driver.ErrSkip.
-func (tp *traceParams) tryStartTrace(ctx context.Context, qtype queryType, query string, startTime time.Time, sqlCommentCarrier *tracer.SQLCommentCarrier, err error) (span tracer.Span) {
-	if err == driver.ErrSkip {
-		// Not a user error: driver is telling sql package that an
-		// optional interface method is not implemented. There is
-		// nothing to trace here.
-		// See: https://github.com/DataDog/dd-trace-go/issues/270
-		return nil
-	}
+func (tp *traceParams) tryStartTrace(ctx context.Context, qtype queryType, query string, startTime time.Time, sqlCommentCarrier *tracer.SQLCommentCarrier) (span tracer.Span) {
 	if _, exists := tracer.SpanFromContext(ctx); tp.cfg.childSpansOnly && !exists {
 		return nil
 	}
@@ -253,7 +243,7 @@ func (tp *traceParams) tryStartTrace(ctx context.Context, qtype queryType, query
 
 	if sqlCommentCarrier != nil && tp.cfg.commentInjectionMode != commentInjectionDisabled {
 		injectionOpts := injectionOptionsForMode(tp.cfg.commentInjectionMode, sqlCommentCarrier.DiscardDynamicTags)
-		err = tracer.InjectWithOptions(span.Context(), sqlCommentCarrier, injectionOpts...)
+		err := tracer.InjectWithOptions(span.Context(), sqlCommentCarrier, injectionOpts...)
 		if err != nil {
 			// this should never happen
 			fmt.Fprintf(os.Stderr, "contrib/database/sql: failed to inject query comments: %v\n", err)

contrib/database/sql/option.go

@@ -9,7 +9,6 @@ import (
 	"math"
 
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"
-
 	"gopkg.in/DataDog/dd-trace-go.v1/internal"
 )
 

contrib/database/sql/sql.go

@@ -129,7 +129,7 @@ type tracedConnector struct {
 	cfg        *config
 }
 
-func (t *tracedConnector) Connect(ctx context.Context) (c driver.Conn, err error) {
+func (t *tracedConnector) Connect(ctx context.Context) (driver.Conn, error) {
 	tp := &traceParams{
 		driverName: t.driverName,
 		cfg:        t.cfg,

contrib/database/sql/stmt.go

@@ -27,7 +27,7 @@ type tracedStmt struct {
 // Close sends a span before closing a statement
 func (s *tracedStmt) Close() (err error) {
 	start := time.Now()
-	span := s.tryStartTrace(s.ctx, queryTypeClose, "", start, nil, err)
+	span := s.tryStartTrace(s.ctx, queryTypeClose, "", start, nil)
 	if span != nil {
 		defer func() {
 			span.Finish(tracer.WithError(err))

contrib/internal/sqltest/sqltest.go

@@ -122,15 +122,15 @@ func testQuery(cfg *Config) func(*testing.T) {
 		cfg.mockTracer.Reset()
 		assert := assert.New(t)
 		rows, err := cfg.DB.Query(query)
-		rows.Close()
+		defer rows.Close()
 		assert.Nil(err)
 
 		spans := cfg.mockTracer.FinishedSpans()
 		var querySpan mocktracer.Span
 		if cfg.DriverName == "sqlserver" {
 			//The mssql driver doesn't support non-prepared queries so there are 3 spans
-			//connect, prepare, query and close
-			assert.Len(spans, 4)
+			//connect, prepare and query
+			assert.Len(spans, 3)
 			span := spans[1]
 			cfg.ExpectTags["sql.query_type"] = "Prepare"
 			assert.Equal(cfg.ExpectName, span.OperationName())

ddtrace/mocktracer/mockspan.go

@@ -118,9 +118,6 @@ func (s *mockspan) SetTag(key string, value interface{}) {
 	if s.tags == nil {
 		s.tags = make(map[string]interface{}, 1)
 	}
-	if key == "sql.query_type" {
-		fmt.Printf("New span for type %s\n", value)
-	}
 	if key == ext.SamplingPriority {
 		switch p := value.(type) {
 		case int:
@@ -191,7 +188,6 @@ func (s *mockspan) SetBaggageItem(key, val string) {
 
 // Finish finishes the current span with the given options.
 func (s *mockspan) Finish(opts ...ddtrace.FinishOption) {
-	fmt.Printf("Finishing span with type %v\n", s.Tag("sql.query_type"))
 	var cfg ddtrace.FinishConfig
 	for _, fn := range opts {
 		fn(&cfg)

ddtrace/mocktracer/mocktracer.go

@@ -13,7 +13,6 @@
 package mocktracer
 
 import (
-	"fmt"
 	"strconv"
 	"strings"
 	"sync"
@@ -104,9 +103,6 @@ func (t *mocktracer) OpenSpans() []Span {
 func (t *mocktracer) FinishedSpans() []Span {
 	t.RLock()
 	defer t.RUnlock()
-	for _, s := range t.finishedSpans {
-		fmt.Printf("returning finished span of type %v\n", s.Tag("sql.query_type"))
-	}
 	return t.finishedSpans
 }
 
@@ -133,7 +129,6 @@ func (t *mocktracer) addFinishedSpan(s Span) {
 	if t.finishedSpans == nil {
 		t.finishedSpans = make([]Span, 0, 1)
 	}
-	fmt.Printf("adding finished span %#v\n", s)
 	t.finishedSpans = append(t.finishedSpans, s)
 }
 

ddtrace/tracer/textmap.go

@@ -13,10 +13,9 @@ import (
 	"strconv"
 	"strings"
 
-	"gopkg.in/DataDog/dd-trace-go.v1/internal/globalconfig"
-
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace"
 	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext"
+	"gopkg.in/DataDog/dd-trace-go.v1/internal/globalconfig"
 	"gopkg.in/DataDog/dd-trace-go.v1/internal/log"
 	"gopkg.in/DataDog/dd-trace-go.v1/internal/samplernames"
 )