diff --git a/contrib/database/sql/conn.go b/contrib/database/sql/conn.go index dd4258ca4e..6e0a7b84b3 100644 --- a/contrib/database/sql/conn.go +++ b/contrib/database/sql/conn.go @@ -208,6 +208,10 @@ func WithSpanTags(ctx context.Context, tags map[string]string) context.Context { // with a span id injected into sql comments. If a span ID is returned, the caller should make sure to use it when creating // the span following the traced database call. func (tp *traceParams) withSQLCommentsInjected(ctx context.Context, query string, discardDynamicTags bool) (cquery string, injectedSpanID uint64) { + // The sql span only gets created after the call to the database because we need to be able to skip spans + // when a driver returns driver.ErrSkip. In order to work with those constraints, the parent span is used + // during SQL comment injection and a new span ID is generated for the sql span and used later when/if the span + // gets created. var spanContext ddtrace.SpanContext if span, ok := tracer.SpanFromContext(ctx); ok { spanContext = span.Context() diff --git a/ddtrace/ext/tags.go b/ddtrace/ext/tags.go index 000db9b337..b343ee5834 100644 --- a/ddtrace/ext/tags.go +++ b/ddtrace/ext/tags.go @@ -46,12 +46,7 @@ const ( // Version is a tag that specifies the current application version. Version = "version" - - // ParentVersion is a tag that specifies the parent's application version - // (i.e. for database service spans, this would be the version of the running application rather than - // the database version). - ParentVersion = "parent.version" - + // ResourceName defines the Resource name for the Span. ResourceName = "resource.name" diff --git a/ddtrace/tracer/sqlcomment.go b/ddtrace/tracer/sqlcomment.go index adaf754284..f815a0a73c 100644 --- a/ddtrace/tracer/sqlcomment.go +++ b/ddtrace/tracer/sqlcomment.go @@ -98,15 +98,18 @@ func (p *SQLCommentPropagator) injectWithCommentCarrier(spanCtx ddtrace.SpanCont if p.mode == CommentInjectionDisabled { return nil } + spanID := random.Uint64() + carrier.AddSpanID(spanID) + if p.mode == StaticTagsSQLCommentInjection || p.mode == FullSQLCommentInjection { ctx, ok := spanCtx.(*spanContext) - var env, pversion string + var env, version string if ok { if e, ok := ctx.meta(ext.Environment); ok { env = e } - if version, ok := ctx.meta(ext.ParentVersion); ok { - pversion = version + if v, ok := ctx.meta(ext.Version); ok { + version = v } } if globalconfig.ServiceName() != "" { @@ -115,14 +118,14 @@ func (p *SQLCommentPropagator) injectWithCommentCarrier(spanCtx ddtrace.SpanCont if env != "" { carrier.Set(ServiceEnvironmentSQLCommentKey, env) } - if pversion != "" { - carrier.Set(ServiceVersionSQLCommentKey, pversion) + if version != "" { + carrier.Set(ServiceVersionSQLCommentKey, version) } } if p.mode == FullSQLCommentInjection { samplingPriority := 0 - var traceID, spanID uint64 + var traceID uint64 ctx, ok := spanCtx.(*spanContext) if ok { if sp, ok := ctx.samplingPriority(); ok { @@ -131,13 +134,6 @@ func (p *SQLCommentPropagator) injectWithCommentCarrier(spanCtx ddtrace.SpanCont if ctx.TraceID() > 0 { traceID = ctx.TraceID() } - if ctx.SpanID() > 0 { - spanID = ctx.SpanID() - } - } - if spanID == 0 { - spanID = random.Uint64() - carrier.AddSpanID(spanID) } if traceID == 0 { traceID = spanID diff --git a/ddtrace/tracer/sqlcomment_test.go b/ddtrace/tracer/sqlcomment_test.go index cb4d511b8e..cabdd0ead5 100644 --- a/ddtrace/tracer/sqlcomment_test.go +++ b/ddtrace/tracer/sqlcomment_test.go @@ -19,7 +19,7 @@ import ( func TestSQLCommentPropagator(t *testing.T) { prepareSpanContextWithSpanID := func(tracer *tracer) ddtrace.SpanContext { - root := tracer.StartSpan("db.call", WithSpanID(10), ServiceName("whiskey-db")).(*span) + root := tracer.StartSpan("service.calling.db", WithSpanID(10)).(*span) root.SetTag(ext.SamplingPriority, 2) return root.Context() } @@ -39,8 +39,8 @@ func TestSQLCommentPropagator(t *testing.T) { mode: FullSQLCommentInjection, carrierOpts: nil, prepareSpanContext: prepareSpanContextWithSpanID, - expectedQuery: "/*dde='test-env',ddsid='10',ddsn='whiskey-service',ddsp='2',ddsv='1.0.0',ddtid='10'*/ SELECT * from FOO", - expectedSpanIDGen: false, + expectedQuery: "/*dde='test-env',ddsid='',ddsn='whiskey-service',ddsp='2',ddsv='1.0.0',ddtid='10'*/ SELECT * from FOO", + expectedSpanIDGen: true, }, { name: "no existing trace", @@ -59,8 +59,8 @@ func TestSQLCommentPropagator(t *testing.T) { mode: FullSQLCommentInjection, carrierOpts: nil, prepareSpanContext: prepareSpanContextWithSpanID, - expectedQuery: "/*dde='test-env',ddsid='10',ddsn='whiskey-service',ddsp='2',ddsv='1.0.0',ddtid='10'*/", - expectedSpanIDGen: false, + expectedQuery: "/*dde='test-env',ddsid='',ddsn='whiskey-service',ddsp='2',ddsv='1.0.0',ddtid='10'*/", + expectedSpanIDGen: true, }, { name: "query with existing comment", @@ -68,8 +68,8 @@ func TestSQLCommentPropagator(t *testing.T) { mode: FullSQLCommentInjection, carrierOpts: nil, prepareSpanContext: prepareSpanContextWithSpanID, - expectedQuery: "/*dde='test-env',ddsid='10',ddsn='whiskey-service',ddsp='2',ddsv='1.0.0',ddtid='10'*/ SELECT * from FOO -- test query", - expectedSpanIDGen: false, + expectedQuery: "/*dde='test-env',ddsid='',ddsn='whiskey-service',ddsp='2',ddsv='1.0.0',ddtid='10'*/ SELECT * from FOO -- test query", + expectedSpanIDGen: true, }, { name: "discard dynamic tags", @@ -102,14 +102,10 @@ func TestSQLCommentPropagator(t *testing.T) { require.NoError(t, err) commented, spanID := carrier.CommentQuery(tc.query) - if tc.expectedSpanIDGen { - assert.Greater(t, spanID, uint64(0)) - expected := strings.ReplaceAll(tc.expectedQuery, "", strconv.FormatUint(spanID, 10)) - assert.Equal(t, expected, commented) - } else { - assert.Equal(t, uint64(0), spanID) - assert.Equal(t, tc.expectedQuery, commented) - } + + assert.Greater(t, spanID, uint64(0)) + expected := strings.ReplaceAll(tc.expectedQuery, "", strconv.FormatUint(spanID, 10)) + assert.Equal(t, expected, commented) }) } } diff --git a/ddtrace/tracer/tracer.go b/ddtrace/tracer/tracer.go index c6ee1b8ee7..ee32b00efb 100644 --- a/ddtrace/tracer/tracer.go +++ b/ddtrace/tracer/tracer.go @@ -426,11 +426,6 @@ func (t *tracer) StartSpan(operationName string, options ...ddtrace.StartSpanOpt if t.config.universalVersion || (!t.config.universalVersion && span.Service == t.config.serviceName) { span.setMeta(ext.Version, t.config.version) } - // For SQL spans which have a different database-inferred service name, set the service version as - // the parent version to avoid confusion with the database service's version - if span.Service != t.config.serviceName { - span.setMeta(ext.ParentVersion, t.config.version) - } } if t.config.env != "" { span.setMeta(ext.Environment, t.config.env)