internal/exectracetest: move execution trace testing to separate modu…

…le (#2709)

.github/workflows/unit-integration-tests.yml

@@ -311,14 +311,16 @@ jobs:
             mkdir -p $TEST_RESULTS
             PACKAGE_NAMES=$(go list ./... | grep -v /contrib/)
             gotestsum --junitfile ${TEST_RESULTS}/gotestsum-report.xml -- $PACKAGE_NAMES -v -race -coverprofile=coverage.txt -covermode=atomic
+            cd ./internal/exectracetest
+            gotestsum --junitfile ${TEST_RESULTS}/gotestsum-report-exectrace.xml -- -v -race -coverprofile=coverage.txt -covermode=atomic
 
       - name: Upload the results to Datadog CI App
         if: always()
         continue-on-error: true
         uses: ./.github/actions/dd-ci-upload
         with:
           dd-api-key: ${{ secrets.DD_CI_API_KEY }}
-          files: ${{ env.TEST_RESULTS }}/gotestsum-report.xml
+          files: ${{ env.TEST_RESULTS }}/gotestsum-report.xml ${{ env.TEST_RESULTS }}/gotestsum-report-exectrace.xml
           tags: go:${{ inputs.go-version }}},arch:${{ runner.arch }},os:${{ runner.os }},distribution:${{ runner.distribution }}
       - name: Upload Coverage
         if: always()

internal/exectracetest/exectrace_go120.go

@@ -0,0 +1,8 @@
+// Unless explicitly stated otherwise all files in this repository are licensed
+// under the Apache License Version 2.0.
+// This product includes software developed at Datadog (https://www.datadoghq.com/).
+// Copyright 2024 Datadog, Inc.
+
+package exectracetest
+
+// Placeholder -- the latestgolang.org/x/exp does not support go1.20

internal/exectracetest/exectrace_test.go

@@ -0,0 +1,141 @@
+// Unless explicitly stated otherwise all files in this repository are licensed
+// under the Apache License Version 2.0.
+// This product includes software developed at Datadog (https://www.datadoghq.com/).
+// Copyright 2024 Datadog, Inc.
+
+//go:build go1.21
+
+// exectracetest tests execution tracer-related functionality.
+// The official execution trace parser lives in golang.org/x/exp,
+// which we generally should avoid upgrading as it is prone
+// to breaking changes which could affect our customers.
+// So, this package lives in a separate module in order to
+// freely upgrade golang.org/x/exp/trace as the trace format changes.
+package exectracetest
+
+import (
+	"bytes"
+	"context"
+	"io"
+	"regexp"
+	"runtime/pprof"
+	"runtime/trace"
+	"testing"
+	"time"
+
+	"github.com/google/pprof/profile"
+	exptrace "golang.org/x/exp/trace"
+	"gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer"
+)
+
+type discardLogger struct{}
+
+func (discardLogger) Log(msg string) {}
+
+// collectTestData runs f under the CPU profiler and execution tracer
+func collectTestData(t *testing.T, f func()) (*profile.Profile, []exptrace.Event) {
+	cpuProf := new(bytes.Buffer)
+	execTrace := new(bytes.Buffer)
+	if pprof.StartCPUProfile(cpuProf) != nil {
+		t.Skip("CPU profiler already running")
+	}
+	defer pprof.StopCPUProfile() // okay to double-stop
+	if trace.Start(execTrace) != nil {
+		t.Skip("execution tracer already running")
+	}
+	defer trace.Stop() // okay to double-stop
+
+	f()
+
+	trace.Stop()
+	pprof.StopCPUProfile()
+
+	pprof, err := profile.Parse(cpuProf)
+	if err != nil {
+		t.Fatalf("parsing profile: %s", err)
+	}
+	reader, err := exptrace.NewReader(execTrace)
+	if err != nil {
+		t.Fatalf("reading execution trace: %s", err)
+	}
+	var traceEvents []exptrace.Event
+	for {
+		ev, err := reader.ReadEvent()
+		if err == io.EOF {
+			break
+		} else if err != nil {
+			t.Fatalf("reading event: %s", err)
+		}
+		traceEvents = append(traceEvents, ev)
+	}
+	return pprof, traceEvents
+}
+
+func waste(d time.Duration) {
+	now := time.Now()
+	for time.Since(now) < d {
+	}
+}
+
+func TestSpanDoubleFinish(t *testing.T) {
+	generate := func(d time.Duration) {
+		tracer.Start(tracer.WithLogger(discardLogger{}))
+		defer tracer.Stop()
+		foo, ctx := tracer.StartSpanFromContext(context.Background(), "foo")
+		bar, _ := tracer.StartSpanFromContext(ctx, "bar")
+		bar.Finish()
+		foo.Finish()
+		bar.Finish()
+		// If we don't handle double finish right, we will see CPU profile samples
+		// for waste tagged with the trace/span IDs from foo
+		waste(d)
+	}
+
+	var (
+		pprof     *profile.Profile
+		execTrace []exptrace.Event
+		retries   int
+	)
+	const maxRetries = 5
+	for duration := 30 * time.Millisecond; retries < maxRetries; retries++ {
+		pprof, execTrace = collectTestData(t, func() {
+			generate(duration)
+		})
+		focus, _, _, _ := pprof.FilterSamplesByName(regexp.MustCompile("waste"), nil, nil, nil)
+		if !focus || len(execTrace) == 0 {
+			// Retry with longer run to reduce flake likelihood
+			duration *= 2
+			continue
+		}
+		break
+	}
+	if retries == maxRetries {
+		t.Fatalf("could not collect sufficient data after %d retries", maxRetries)
+	}
+
+	// Check CPU profile: we should have un-set the labels
+	// for the goroutine by the time waste5 runs
+	for _, sample := range pprof.Sample {
+		if labels := sample.Label; len(labels) > 0 {
+			t.Errorf("unexpected labels for sample: %+v", labels)
+		}
+	}
+
+	// Check execution trace: we should not emit a second task end event
+	// even though we finish the "bar" span twice
+	taskEvents := make(map[exptrace.TaskID][]exptrace.Event)
+	for _, ev := range execTrace {
+		switch ev.Kind() {
+		case exptrace.EventTaskBegin, exptrace.EventTaskEnd:
+			id := ev.Task().ID
+			taskEvents[id] = append(taskEvents[id], ev)
+		}
+	}
+	for id, events := range taskEvents {
+		if len(events) > 2 {
+			t.Errorf("extraneous events for task %d: %v", id, events)
+		}
+	}
+}
+
+// TODO: move database/sql tests here? likely requires copying over contrib/sql/internal.MockDriver

internal/exectracetest/go.mod

@@ -0,0 +1,40 @@
+module gopkg.in/DataDog/dd-trace-go.v1/internal/exectracetest
+
+go 1.20
+
+require (
+	github.com/google/pprof v0.0.0-20230817174616-7a8ec2ada47b
+	golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842
+	gopkg.in/DataDog/dd-trace-go.v1 v1.64.0
+)
+
+require (
+	github.com/DataDog/appsec-internal-go v1.5.0 // indirect
+	github.com/DataDog/datadog-agent/pkg/obfuscate v0.48.0 // indirect
+	github.com/DataDog/datadog-agent/pkg/remoteconfig/state v0.48.1 // indirect
+	github.com/DataDog/datadog-go/v5 v5.3.0 // indirect
+	github.com/DataDog/go-libddwaf/v3 v3.2.0 // indirect
+	github.com/DataDog/go-tuf v1.0.2-0.5.2 // indirect
+	github.com/DataDog/sketches-go v1.4.5 // indirect
+	github.com/Microsoft/go-winio v0.6.1 // indirect
+	github.com/cespare/xxhash/v2 v2.2.0 // indirect
+	github.com/dustin/go-humanize v1.0.1 // indirect
+	github.com/ebitengine/purego v0.6.0-alpha.5 // indirect
+	github.com/google/uuid v1.5.0 // indirect
+	github.com/outcaste-io/ristretto v0.2.3 // indirect
+	github.com/philhofer/fwd v1.1.2 // indirect
+	github.com/pkg/errors v0.9.1 // indirect
+	github.com/secure-systems-lab/go-securesystemslib v0.7.0 // indirect
+	github.com/tinylib/msgp v1.1.8 // indirect
+	go.uber.org/atomic v1.11.0 // indirect
+	golang.org/x/mod v0.17.0 // indirect
+	golang.org/x/sync v0.7.0 // indirect
+	golang.org/x/sys v0.20.0 // indirect
+	golang.org/x/time v0.3.0 // indirect
+	golang.org/x/tools v0.21.0 // indirect
+	golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
+	google.golang.org/protobuf v1.32.0 // indirect
+)
+
+// use local version of dd-trace-go
+replace gopkg.in/DataDog/dd-trace-go.v1 => ../..