Add OpenTelemetry tracing for individual Go tests#321
Conversation
Parse go test -json output during the test phase to create spans for each test. Test spans are children of the package span, showing test parallelism and duration in build traces. Co-authored-by: Ona <no-reply@ona.com>
CompositeReporter now delegates GetGoTestTracer to any wrapped reporter that implements TestTracingReporter, enabling test tracing when using multiple reporters. Co-authored-by: Ona <no-reply@ona.com>
Test tracing is opt-in since it can generate many spans for large test suites. Use --enable-test-tracing to create OpenTelemetry spans for individual Go tests. Co-authored-by: Ona <no-reply@ona.com>
WVerlaek
force-pushed
the
wv/go-test-tracing
branch
from
e8e3a3e to
ea21843
Compare
In non-verbose mode, only show package-level output and failed test output. Buffer test output and flush on failure to preserve error details while reducing noise for passing tests. Co-authored-by: Ona <no-reply@ona.com>
Simplify the design by detecting go test commands in the run() function rather than using a separate TestExecutor. This removes: - TestExecutor field from packageBuild - Special case in executeBuildPhase - GetGoTestTracer from Reporter interface - CompositeReporter delegation for test tracing The tracing is now an implementation detail of command execution. Co-authored-by: Ona <no-reply@ona.com>
kylos101
left a comment
kylos101
left a comment
There was a problem hiding this comment.
LGTM, I added some questions and nits, but nothing blocking. I didn't get a chance to test, but see you included success and failures, which is great. Also, i see in the results a test ran for 200ms (so much longer than others).
| key := spanKey(event.Package, event.Test) | ||
| if output, ok := testOutput[key]; ok { | ||
| for _, line := range output { | ||
| _, _ = outputWriter.Write([]byte(line)) |
There was a problem hiding this comment.
Nit, not blocking:
We don't handle when outputWriter is nil for the fail scenario, but do above.
There was a problem hiding this comment.
Good catch, removed the nil checks to clean up the logic, it's never nil
| type testSpanData struct { | ||
| span trace.Span | ||
| startTime time.Time | ||
| pkg string |
There was a problem hiding this comment.
Question, not blocking:
How is the pkg field being used? I see it is being set, but not read anywhere.
|
|
||
| // goTestEvent represents a single event from `go test -json` output. | ||
| // See https://pkg.go.dev/cmd/test2json for the format specification. | ||
| type goTestEvent struct { |
There was a problem hiding this comment.
Question, not blocking:
I see FailedBuild was omitted, intentional?
| _, _ = outputWriter.Write([]byte(event.Output)) | ||
| } else if event.Test == "" { | ||
| // Non-verbose: always show package-level output | ||
| _, _ = outputWriter.Write([]byte(event.Output)) |
There was a problem hiding this comment.
Question, not blocking:
Did you intend to have this be event.Package, instead of Output?
There was a problem hiding this comment.
Output is correct here, it's for package-level events (e.g. starting/ending tests for a package), to match the output of go test (https://arc.net/l/quote/pdvwsptx)
| func (t *GoTestTracer) handleEvent(event *goTestEvent) { | ||
| switch event.Action { | ||
| case "run": | ||
| t.handleRun(event) | ||
| case "pause": | ||
| t.handlePause(event) | ||
| case "cont": | ||
| t.handleCont(event) | ||
| case "pass", "fail", "skip": | ||
| t.handleEnd(event) | ||
| case "output": | ||
| // Output is already written to outputWriter | ||
| case "start": | ||
| // Package test started - we could create a package-level span here | ||
| t.handlePackageStart(event) | ||
| } | ||
| } |
There was a problem hiding this comment.
Observation, not blocking:
We are missing bench, which I guess is generally only done locally
There was a problem hiding this comment.
yeah bench shouldn't be showing here, this only handles go test commands
outputWriter is never nil in this context. Co-authored-by: Ona <no-reply@ona.com>
pkg and startTime were set but never read. Co-authored-by: Ona <no-reply@ona.com>
Handle build failures separately from test failures by recording the failed package in the span attributes. Co-authored-by: Ona <no-reply@ona.com>
2 participants
Description
Add per-test OpenTelemetry spans during Go package builds. When tracing is enabled, leeway parses
go test -jsonoutput and creates child spans for each test under the package span. This provides visibility into individual test duration and parallelism in build traces.Can be enabled by passing the
--enable-test-tracingflag, disabled by defaulthttps://ui.honeycomb.io/gitpod/environments/ci/datasets/leeway/result/FhLg2E7Cfp4/trace/8cAj6RnnZpf?fields%5B%5D=s_name&fields%5B%5D=s_serviceName&fields%5B%5D=c_leeway.package.name&fields%5B%5D=c_leeway.package.last_phase&span=14df73420d330fac
https://ui.honeycomb.io/gitpod/environments/ci/datasets/leeway/result/chTYCtKiNU8/trace/3gqNBXCwcwZ?fields%5B%5D=s_name&fields%5B%5D=s_serviceName&fields%5B%5D=c_leeway.package.name&fields%5B%5D=c_leeway.package.last_phase&span=2bc30322f9351793
Related Issue(s)
Fixes CORE-6451
How to test