contrib/database/sql: trace when new connection is established

[stroem]

This will create a new span when database establishes a new connection

Issue: #760

[gbbr]

@CAFxX perhaps you can share some thoughts

[CAFxX]

LGTM, would only suggest to add a test to ensure that the span is emitted even if the context is cancelled.

[knusbaum]

@stroem would you be able to add a test for cancelled context?

[knusbaum]

Let's add a test for cancelled context.

[adw1n]

@CAFxX @knusbaum the test that you're asking for would look more or less like this:

Suggested change

	func testConnectCancelledCtx(cfg *Config) func(*testing.T) {
	return func(t *testing.T) {
	cfg.mockTracer.Reset()
	assert := assert.New(t)
	ctx, cancel := context.WithCancel(context.Background())
	cancel()
	rs := reflect.ValueOf(cfg.DB).Elem()
	rf := rs.FieldByName("connector")
	rf = reflect.NewAt(rf.Type(), unsafe.Pointer(rf.UnsafeAddr())).Elem()
	_, err := rf.Interface().(driver.Connector).Connect(ctx)
	assert.ErrorIs(err, context.Canceled)
	spans := cfg.mockTracer.FinishedSpans()
	_ = spans
	// assert.Len(spans, 2)
	// ...
	}
	}

You can't use cfg.DB.PingContext(ctx) because sql.DB.conn checks the context at the start (https://github.com/golang/go/blob/7eb31d999cf2769deb0e7bdcafc30e18f52ceb48/src/database/sql/sql.go#L1205) and tracedConnector::Connect isn't even called.

This test won't pass because

dd-trace-go/contrib/database/sql/sql.go

Line 141 in 9892197

func (t dsnConnector) Connect(_ context.Context) (driver.Conn, error) {

ignores the context.

@CAFxX @knusbaum Taking the complexity of the test in mind and the fact that fixing dsnConnector.Connect isn't the goal of this PR I'd suggest to allow to merge this PR.

To run the integration test deploy postgres like docker run -it --rm --name dd-tests-postgres -e POSTGRES_PASSWORD=postgres -e POSTGRES_USER=postgres -p 127.0.0.1:5432:5432/tcp postgres and set env variable INTEGRATION=true.

[CAFxX]

What I had in mind was more along the lines of:

• write hangingConnector, a driver.Connector that, when e.g. PingContext is called, hangs until the context is cancelled
• construct a tracedConnector wrapping hangingConnector
• call PingContext on the tracedConnector in a goroutine
• wait until we know we are blocked in hangingConnector
• cancel the context
• wait until the PingContext call returns
• check that the span was emitted

[knusbaum]

We want this and the PR is close. It would be great if someone could finish this, else I'll get to it when I can.

[moonsub-kim]

I wish the PR is merged! I'm waiting for 10 months (ㅠ_ㅠ)

[CAFxX]

FWIW, LGTM

[gbbr]

Let's close in favour of #1154 which is actively being worked on.