From 5d3b29bbf27a7d843cd8b5ff39f7c40f1db33f60 Mon Sep 17 00:00:00 2001 From: Teague Bick Date: Fri, 23 Feb 2024 08:45:55 -0500 Subject: [PATCH] contrib/database/sql: add in ddh, dddb propagation (#2550) contrib/database/sql: add in ddh, dddb propagation --- README.md | 4 +- contrib/database/sql/conn.go | 2 +- contrib/database/sql/internal/dsn.go | 20 +++++++++- contrib/database/sql/propagation_test.go | 17 +++++--- ddtrace/tracer/sqlcomment.go | 22 ++++++++--- ddtrace/tracer/sqlcomment_test.go | 50 +++++++++++++++++++++++- 6 files changed, 100 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index af7b00ea84..6e6f275865 100644 --- a/README.md +++ b/README.md @@ -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 -``` +``` \ No newline at end of file diff --git a/contrib/database/sql/conn.go b/contrib/database/sql/conn.go index 7e88044e84..58b644c367 100644 --- a/contrib/database/sql/conn.go +++ b/contrib/database/sql/conn.go @@ -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) diff --git a/contrib/database/sql/internal/dsn.go b/contrib/database/sql/internal/dsn.go index 6a7f11dffc..9d9eee9d40 100644 --- a/contrib/database/sql/internal/dsn.go +++ b/contrib/database/sql/internal/dsn.go @@ -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. @@ -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:postgres@127.0.0.1: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 } diff --git a/contrib/database/sql/propagation_test.go b/contrib/database/sql/propagation_test.go index afed0707b9..7ec0e659de 100644 --- a/contrib/database/sql/propagation_test.go +++ b/contrib/database/sql/propagation_test.go @@ -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 }{ { @@ -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:postgres@127.0.0.1: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", @@ -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:postgres@127.0.0.1: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", @@ -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:postgres@127.0.0.1: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")}, }, } @@ -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() diff --git a/ddtrace/tracer/sqlcomment.go b/ddtrace/tracer/sqlcomment.go index ce275a5ec0..72ad4229b7 100644 --- a/ddtrace/tracer/sqlcomment.go +++ b/ddtrace/tracer/sqlcomment.go @@ -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) @@ -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. @@ -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() @@ -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 { diff --git a/ddtrace/tracer/sqlcomment_test.go b/ddtrace/tracer/sqlcomment_test.go index 37c3661a0c..9acb645d89 100644 --- a/ddtrace/tracer/sqlcomment_test.go +++ b/ddtrace/tracer/sqlcomment_test.go @@ -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 @@ -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--00'*/ SELECT * from FOO", expectedSpanIDGen: true, expectedExtractErr: nil, @@ -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, @@ -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--00'*/ SELECT * from FOO", expectedSpanIDGen: true, expectedExtractErr: nil, @@ -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--00'*/", expectedSpanIDGen: true, expectedExtractErr: nil, @@ -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--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--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--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--01',ddh='fake-hostname',dddb='fake-database'*/ /* c */ SELECT * from FOO /**/", + expectedSpanIDGen: true, + expectedExtractErr: nil, + }, } for _, tc := range testCases { @@ -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, "", fmt.Sprintf("%016s", strconv.FormatUint(carrier.SpanID, 16)))