Skip to content

Commit

Permalink
contrib/database/sql: add in ddh, dddb propagation (DataDog#2550)
Browse files Browse the repository at this point in the history
contrib/database/sql: add in ddh, dddb propagation
  • Loading branch information
tabgok committed Feb 23, 2024
1 parent 1dfe613 commit 5d3b29b
Show file tree
Hide file tree
Showing 6 changed files with 100 additions and 15 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,10 @@ might be running versions different from the vendored one, creating hard to debu
To run integration tests locally, you should set the `INTEGRATION` environment variable. The dependencies of the integration tests are best run via Docker. To get an
idea about the versions and the set-up take a look at our [docker-compose config](./docker-compose.yaml).

The best way to run the entire test suite is using the [test.sh](./test.sh) script. You'll need Docker and docker-compose installed. Run `./test.sh --all` to run all of the integration tests through the docker-compose environment. Run `./test.sh --help` for more options.
The best way to run the entire test suite is using the [test.sh](./test.sh) script. You'll need Docker and docker-compose installed. If this is your first time running the tests, you should run `./test.sh -t` to install any missing test tools/dependencies. Run `./test.sh --all` to run all of the integration tests through the docker-compose environment. Run `./test.sh --help` for more options.

If you're only interested in the tests for a specific integration it can be useful to spin up just the required containers via docker-compose.
For example if you're running tests that need the `mysql` database container to be up:
```shell
docker compose -f docker-compose.yaml -p dd-trace-go up -d mysql
```
```
2 changes: 1 addition & 1 deletion contrib/database/sql/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ func (tc *TracedConn) injectComments(ctx context.Context, query string, mode tra
if span, ok := tracer.SpanFromContext(ctx); ok {
spanCtx = span.Context()
}
carrier := tracer.SQLCommentCarrier{Query: query, Mode: mode, DBServiceName: tc.cfg.serviceName}
carrier := tracer.SQLCommentCarrier{Query: query, Mode: mode, DBServiceName: tc.cfg.serviceName, PeerDBHostname: tc.meta[ext.TargetHost], PeerDBName: tc.meta[ext.DBName]}
if err := carrier.Inject(spanCtx); err != nil {
// this should never happen
log.Warn("contrib/database/sql: failed to inject query comments: %v", err)
Expand Down
20 changes: 19 additions & 1 deletion contrib/database/sql/internal/dsn.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ package internal // import "gopkg.in/DataDog/dd-trace-go.v1/contrib/database/sql

import (
"net"
"net/url"
"strings"

"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext"
"gopkg.in/DataDog/dd-trace-go.v1/internal/log"
)

// ParseDSN parses various supported DSN types into a map of key/value pairs which can be used as valid tags.
Expand All @@ -19,20 +21,36 @@ func ParseDSN(driverName, dsn string) (meta map[string]string, err error) {
case "mysql":
meta, err = parseMySQLDSN(dsn)
if err != nil {
log.Debug("Error parsing DSN for mysql: %v", err)
return
}
case "postgres", "pgx":
meta, err = parsePostgresDSN(dsn)
if err != nil {
log.Debug("Error parsing DSN for postgres: %v", err)
return
}
case "sqlserver":
meta, err = parseSQLServerDSN(dsn)
if err != nil {
log.Debug("Error parsing DSN for sqlserver: %v", err)
return
}
default:
// not supported
// Try to parse the DSN and see if the scheme contains a known driver name.
u, e := url.Parse(dsn)
if e != nil {
// dsn is not a valid URL, so just ignore
log.Debug("Error parsing driver name from DSN: %v", e)
return
}
if driverName != u.Scheme {
// In some cases the driver is registered under a non-official name.
// For example, "Test" may be the registered name with a DSN of "postgres://postgres:[email protected]:5432/fakepreparedb"
// for the purposes of testing/mocking.
// In these cases, we try to parse the DSN based upon the DSN itself, instead of the registered driver name
return ParseDSN(u.Scheme, dsn)
}
}
return reduceKeys(meta), nil
}
Expand Down
17 changes: 12 additions & 5 deletions contrib/database/sql/propagation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ func TestDBMPropagation(t *testing.T) {
opts []RegisterOption
callDB func(ctx context.Context, db *sql.DB) error
prepared []string
dsn string
executed []*regexp.Regexp
}{
{
Expand Down Expand Up @@ -73,7 +74,8 @@ func TestDBMPropagation(t *testing.T) {
_, err := db.PrepareContext(ctx, "SELECT 1 from DUAL")
return err
},
prepared: []string{"/*dddbs='test.db',dde='test-env',ddps='test-service',ddpv='1.0.0'*/ SELECT 1 from DUAL"},
dsn: "postgres://postgres:[email protected]:5432/fakepreparedb?sslmode=disable",
prepared: []string{"/*dddbs='test.db',dde='test-env',ddps='test-service',ddpv='1.0.0',ddh='127.0.0.1',dddb='fakepreparedb'*/ SELECT 1 from DUAL"},
},
{
name: "query",
Expand Down Expand Up @@ -109,7 +111,8 @@ func TestDBMPropagation(t *testing.T) {
_, err := db.QueryContext(ctx, "SELECT 1 from DUAL")
return err
},
executed: []*regexp.Regexp{regexp.MustCompile("/\\*dddbs='test.db',dde='test-env',ddps='test-service',ddpv='1.0.0',traceparent='00-00000000000000000000000000000001-[\\da-f]{16}-01'\\*/ SELECT 1 from DUAL")},
dsn: "postgres://postgres:[email protected]:5432/fakequerydb?sslmode=disable",
executed: []*regexp.Regexp{regexp.MustCompile("/\\*dddbs='test.db',dde='test-env',ddps='test-service',ddpv='1.0.0',traceparent='00-00000000000000000000000000000001-[\\da-f]{16}-01',ddh='127.0.0.1',dddb='fakequerydb'\\*/ SELECT 1 from DUAL")},
},
{
name: "exec",
Expand Down Expand Up @@ -145,7 +148,8 @@ func TestDBMPropagation(t *testing.T) {
_, err := db.ExecContext(ctx, "SELECT 1 from DUAL")
return err
},
executed: []*regexp.Regexp{regexp.MustCompile("/\\*dddbs='test.db',dde='test-env',ddps='test-service',ddpv='1.0.0',traceparent='00-00000000000000000000000000000001-[\\da-f]{16}-01'\\*/ SELECT 1 from DUAL")},
dsn: "postgres://postgres:[email protected]:5432/fakeexecdb?sslmode=disable",
executed: []*regexp.Regexp{regexp.MustCompile("/\\*dddbs='test.db',dde='test-env',ddps='test-service',ddpv='1.0.0',traceparent='00-00000000000000000000000000000001-[\\da-f]{16}-01',ddh='127.0.0.1',dddb='fakeexecdb'\\*/ SELECT 1 from DUAL")},
},
}

Expand All @@ -163,9 +167,12 @@ func TestDBMPropagation(t *testing.T) {
Register("test", d, tc.opts...)
defer unregister("test")

db, err := Open("test", "dn")
dsn := "dn"
if tc.dsn != "" {
dsn = tc.dsn
}
db, err := Open("test", dsn)
require.NoError(t, err)

s, ctx := tracer.StartSpanFromContext(context.Background(), "test.call", tracer.WithSpanID(1))
err = tc.callDB(ctx, db)
s.Finish()
Expand Down
22 changes: 17 additions & 5 deletions ddtrace/tracer/sqlcomment.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ const (
sqlCommentDBService = "dddbs"
sqlCommentParentVersion = "ddpv"
sqlCommentEnv = "dde"
// These keys are for the database we are connecting to, instead of the service we are running in.
// "Peer" is the OpenTelemetry nomenclature for "thing I am talking to"
sqlCommentPeerHostname = "ddh"
sqlCommentPeerDBName = "dddb"
)

// Current trace context version (see https://www.w3.org/TR/trace-context/#version)
Expand All @@ -66,10 +70,12 @@ const w3cContextVersion = "00"
// of a sqlcommenter formatted comment prepended to the original query text.
// See https://google.github.io/sqlcommenter/spec/ for more details.
type SQLCommentCarrier struct {
Query string
Mode DBMPropagationMode
DBServiceName string
SpanID uint64
Query string
Mode DBMPropagationMode
DBServiceName string
SpanID uint64
PeerDBHostname string
PeerDBName string
}

// Inject injects a span context in the carrier's Query field as a comment.
Expand Down Expand Up @@ -105,6 +111,12 @@ func (c *SQLCommentCarrier) Inject(spanCtx ddtrace.SpanContext) error {
if v, ok := ctx.meta(ext.Version); ok && v != "" {
tags[sqlCommentParentVersion] = v
}
if c.PeerDBName != "" {
tags[sqlCommentPeerDBName] = c.PeerDBName
}
if c.PeerDBHostname != "" {
tags[sqlCommentPeerHostname] = c.PeerDBHostname
}
}
if globalconfig.ServiceName() != "" {
tags[sqlCommentParentService] = globalconfig.ServiceName()
Expand Down Expand Up @@ -155,7 +167,7 @@ func commentQuery(query string, tags map[string]string) string {
var b strings.Builder
// the sqlcommenter specification dictates that tags should be sorted. Since we know all injected keys,
// we skip a sorting operation by specifying the order of keys statically
orderedKeys := []string{sqlCommentDBService, sqlCommentEnv, sqlCommentParentService, sqlCommentParentVersion, sqlCommentTraceParent}
orderedKeys := []string{sqlCommentDBService, sqlCommentEnv, sqlCommentParentService, sqlCommentParentVersion, sqlCommentTraceParent, sqlCommentPeerHostname, sqlCommentPeerDBName}
first := true
for _, k := range orderedKeys {
if v, ok := tags[k]; ok {
Expand Down
50 changes: 49 additions & 1 deletion ddtrace/tracer/sqlcomment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ func TestSQLCommentCarrier(t *testing.T) {
mode DBMPropagationMode
injectSpan bool
samplingPriority int
peerDBHostname string
peerDBName string
expectedQuery string
expectedSpanIDGen bool
expectedExtractErr error
Expand All @@ -35,6 +37,8 @@ func TestSQLCommentCarrier(t *testing.T) {
query: "SELECT * from FOO",
mode: DBMPropagationModeFull,
injectSpan: true,
peerDBHostname: "",
peerDBName: "",
expectedQuery: "/*dddbs='whiskey-db',dde='test-env',ddps='whiskey-service%20%21%23%24%25%26%27%28%29%2A%2B%2C%2F%3A%3B%3D%3F%40%5B%5D',ddpv='1.0.0',traceparent='00-0000000000000000000000000000000a-<span_id>-00'*/ SELECT * from FOO",
expectedSpanIDGen: true,
expectedExtractErr: nil,
Expand All @@ -44,6 +48,8 @@ func TestSQLCommentCarrier(t *testing.T) {
query: "SELECT * from FOO",
mode: DBMPropagationModeService,
injectSpan: true,
peerDBHostname: "",
peerDBName: "",
expectedQuery: "/*dddbs='whiskey-db',dde='test-env',ddps='whiskey-service%20%21%23%24%25%26%27%28%29%2A%2B%2C%2F%3A%3B%3D%3F%40%5B%5D',ddpv='1.0.0'*/ SELECT * from FOO",
expectedSpanIDGen: false,
expectedExtractErr: ErrSpanContextNotFound,
Expand All @@ -52,6 +58,8 @@ func TestSQLCommentCarrier(t *testing.T) {
name: "no-trace",
query: "SELECT * from FOO",
mode: DBMPropagationModeFull,
peerDBHostname: "",
peerDBName: "",
expectedQuery: "/*dddbs='whiskey-db',ddps='whiskey-service%20%21%23%24%25%26%27%28%29%2A%2B%2C%2F%3A%3B%3D%3F%40%5B%5D',traceparent='00-0000000000000000<span_id>-<span_id>-00'*/ SELECT * from FOO",
expectedSpanIDGen: true,
expectedExtractErr: nil,
Expand All @@ -61,6 +69,8 @@ func TestSQLCommentCarrier(t *testing.T) {
query: "",
mode: DBMPropagationModeFull,
injectSpan: true,
peerDBHostname: "",
peerDBName: "",
expectedQuery: "/*dddbs='whiskey-db',dde='test-env',ddps='whiskey-service%20%21%23%24%25%26%27%28%29%2A%2B%2C%2F%3A%3B%3D%3F%40%5B%5D',ddpv='1.0.0',traceparent='00-0000000000000000000000000000000a-<span_id>-00'*/",
expectedSpanIDGen: true,
expectedExtractErr: nil,
Expand Down Expand Up @@ -91,10 +101,48 @@ func TestSQLCommentCarrier(t *testing.T) {
mode: DBMPropagationModeFull,
injectSpan: true,
samplingPriority: 1,
peerDBHostname: "",
peerDBName: "",
expectedQuery: "/*dddbs='whiskey-db',dde='test-env',ddps='whiskey-service%20%21%23%24%25%26%27%28%29%2A%2B%2C%2F%3A%3B%3D%3F%40%5B%5D',ddpv='1.0.0',traceparent='00-0000000000000000000000000000000a-<span_id>-01'*/ /* c */ SELECT * from FOO /**/",
expectedSpanIDGen: true,
expectedExtractErr: nil,
},
{
name: "peer_entity_tags_dddb",
query: "/* c */ SELECT * from FOO /**/",
mode: DBMPropagationModeFull,
injectSpan: true,
samplingPriority: 1,
peerDBName: "fake-database",
peerDBHostname: "",
expectedQuery: "/*dddbs='whiskey-db',dde='test-env',ddps='whiskey-service%20%21%23%24%25%26%27%28%29%2A%2B%2C%2F%3A%3B%3D%3F%40%5B%5D',ddpv='1.0.0',traceparent='00-0000000000000000000000000000000a-<span_id>-01',dddb='fake-database'*/ /* c */ SELECT * from FOO /**/",
expectedSpanIDGen: true,
expectedExtractErr: nil,
},
{
name: "peer_entity_tags_ddh",
query: "/* c */ SELECT * from FOO /**/",
mode: DBMPropagationModeFull,
injectSpan: true,
samplingPriority: 1,
peerDBHostname: "fake-hostname",
peerDBName: "",
expectedQuery: "/*dddbs='whiskey-db',dde='test-env',ddps='whiskey-service%20%21%23%24%25%26%27%28%29%2A%2B%2C%2F%3A%3B%3D%3F%40%5B%5D',ddpv='1.0.0',traceparent='00-0000000000000000000000000000000a-<span_id>-01',ddh='fake-hostname'*/ /* c */ SELECT * from FOO /**/",
expectedSpanIDGen: true,
expectedExtractErr: nil,
},
{
name: "peer_entity_tags_dddb_and_ddh",
query: "/* c */ SELECT * from FOO /**/",
mode: DBMPropagationModeFull,
injectSpan: true,
samplingPriority: 1,
peerDBHostname: "fake-hostname",
peerDBName: "fake-database",
expectedQuery: "/*dddbs='whiskey-db',dde='test-env',ddps='whiskey-service%20%21%23%24%25%26%27%28%29%2A%2B%2C%2F%3A%3B%3D%3F%40%5B%5D',ddpv='1.0.0',traceparent='00-0000000000000000000000000000000a-<span_id>-01',ddh='fake-hostname',dddb='fake-database'*/ /* c */ SELECT * from FOO /**/",
expectedSpanIDGen: true,
expectedExtractErr: nil,
},
}

for _, tc := range testCases {
Expand All @@ -114,7 +162,7 @@ func TestSQLCommentCarrier(t *testing.T) {
spanCtx = root.Context()
}

carrier := SQLCommentCarrier{Query: tc.query, Mode: tc.mode, DBServiceName: "whiskey-db"}
carrier := SQLCommentCarrier{Query: tc.query, Mode: tc.mode, DBServiceName: "whiskey-db", PeerDBHostname: tc.peerDBHostname, PeerDBName: tc.peerDBName}
err := carrier.Inject(spanCtx)
require.NoError(t, err)
expected := strings.ReplaceAll(tc.expectedQuery, "<span_id>", fmt.Sprintf("%016s", strconv.FormatUint(carrier.SpanID, 16)))
Expand Down

0 comments on commit 5d3b29b

Please sign in to comment.