feat: inspectable tracer by darccio · Pull Request #4512 · DataDog/dd-trace-go

darccio · 2026-03-09T13:10:27Z

What does this PR do?

Adds an inspectable tracer that replaces the other four different ways to inspect spans and mocking the tracer.

Migrates a few example tests that we using any of the different existing approaches in the codebase.

Important

testracer.Start starts a test tracer but it doesn't register it as global tracer.
testtracer.Bootstrap starts a test tracer, a test agent, and registers the former as global tracer.
We are not able to run our tests in parallel, although some of the migrated ones could be parallelized already because they don't depend on the public ddtrace/tracer API.

Motivation

Replace existing implementations, although now we keep the original APIs for them.
Reduce code to maintain.
Exercize the tracer in tests, don't mock it.
Reduces dependencies although they are not completely removed as they still hang as indirect.

Reviewer's Checklist

Changed code has unit tests for its functionality at or near 100% coverage.
New code is free of linting errors. You can check this by running make lint locally.
New code doesn't break existing tests. You can check this by running make test locally.
Add an appropriate team label so this PR gets put in the right place for the release notes.
All generated files are up to date. You can check this by running make generate locally.

Unsure? Have a question? Request a review!

This allows to swap the flush handling on the tracer with other implementations that require different flush semantics, like synchronous ones.

…integration tests to use {test,agent}test

…ests as example

…th synctest

datadog-datadog-prod-us1 · 2026-03-09T15:03:55Z

✅ Tests

🎉 All green!

❄️ No new flaky tests detected
🧪 All tests passed

🎯 Code Coverage (details)
• Patch Coverage: 35.83%
• Overall Coverage: 60.01% (+4.05%)

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: b2e7bd2 | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback!}

pr-commenter · 2026-03-09T15:33:16Z

Benchmarks

Benchmark execution time: 2026-03-25 16:10:32

Comparing candidate commit b2e7bd2 in PR branch dario.castane/rd/inspectable-tracer-v2 with baseline commit d52d8ea in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 215 metrics, 9 unstable metrics.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

🟩 = significantly better candidate vs. baseline
🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

…es by replacing FreePort+ListenAndServe with a pre-bound FreeListener+Serve and adding a TestCasePreBootstrap interface so os/lfi.go can configure AppSec env vars before the tracer starts

darccio · 2026-03-09T22:16:40Z

@codex review

…Tags to attributes, Meta and Metrics to agenttest span tags

…n to expected traces

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 078b37add2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-09T22:24:47Z

ddtrace/tracer/tracertest.go

+	o := append([]StartOption{
+		WithAgentAddr(agent.Addr()),
+	}, opts...)
+	tracer, err := newTracer(o...)


Invoke full startup path for inspectable tracer

startInspectableTracer creates the tracer with newTracer(...) but never executes the extra initialization done in tracer.Start (notably AppSec startup and other post-construction runtime hooks). This means options passed through tracertest.Bootstrap, such as tracer.WithAppSecEnabled(true) in the orchestrion harness, are not actually activated, so AppSec-dependent integration scenarios can silently lose coverage or fail inconsistently. The inspectable bootstrap path should run the same startup hooks as Start before returning.

Useful? React with 👍 / 👎.

This requires a significant refactor. Doing it in a future PR.

internal/orchestrion/_integration/internal/harness/harness.go

darccio · 2026-03-10T07:56:13Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6f6465bb5f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

ddtrace/tracer/tracertest.go

chatgpt-codex-connector · 2026-03-10T08:10:15Z

ddtrace/tracer/tracertest.go

+		}
+	drained:
+		tracer.traceWriter.flush()
+		tracer.traceWriter.(*agentTraceWriter).wg.Wait()


Avoid assuming agentTraceWriter in inspectable flush path

This hard cast panics when the tracer is configured with a non-agent writer (for example log-to-stdout or CI Visibility modes), even though Start accepts arbitrary tracer options/environment. A Flush() call from such tests will crash instead of flushing; guard the cast or handle non-agent writers explicitly.

Useful? React with 👍 / 👎.

To do in another PR.

…transport option

codecov · 2026-03-10T09:35:08Z

Codecov Report

❌ Patch coverage is 32.97003% with 246 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.60%. Comparing base (d52d8ea) to head (b2e7bd2).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
ddtrace/x/llmobstest/collector.go	0.00%	82 Missing ⚠️
ddtrace/x/agenttest/agent.go	0.00%	72 Missing ⚠️
ddtrace/x/agenttest/span.go	0.00%	43 Missing ⚠️
ddtrace/tracer/tracertest.go	68.14%	28 Missing and 8 partials ⚠️
ddtrace/tracer/option.go	45.45%	5 Missing and 1 partial ⚠️
internal/llmobs/llmobs.go	88.23%	3 Missing and 1 partial ⚠️
internal/llmobs/transport/transport.go	0.00%	3 Missing ⚠️

Additional details and impacted files

Files with missing lines	Coverage Δ
ddtrace/tracer/tracer.go	`88.05% <100.00%> (ø)`
internal/llmobs/config/config.go	`0.00% <ø> (ø)`
internal/llmobs/transport/transport.go	`0.00% <0.00%> (ø)`
internal/llmobs/llmobs.go	`77.51% <88.23%> (ø)`
ddtrace/tracer/option.go	`83.28% <45.45%> (ø)`
ddtrace/tracer/tracertest.go	`68.14% <68.14%> (ø)`
ddtrace/x/agenttest/span.go	`0.00% <0.00%> (ø)`
ddtrace/x/agenttest/agent.go	`0.00% <0.00%> (ø)`
ddtrace/x/llmobstest/collector.go	`0.00% <0.00%> (ø)`

... and 431 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…rking

darccio · 2026-03-11T12:59:10Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 63cc74f440

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-11T13:13:17Z

ddtrace/tracer/tracertest.go

+		}
+	drained:
+		tracer.traceWriter.flush()
+		tracer.traceWriter.(*agentTraceWriter).wg.Wait()


Guard flush wait against non-agent trace writers

startInspectableTracer accepts arbitrary tracer.StartOptions, but the custom flush handler unconditionally casts tracer.traceWriter to *agentTraceWriter; if callers pass options that select a different writer (for example log/stdout or CI Visibility modes), Flush() will panic at runtime instead of flushing. This makes the new ddtrace/x/tracertest helper fragile in valid option combinations and can crash integration tests unexpectedly.

Useful? React with 👍 / 👎.

Already mentioned previously. I'll fix it when needed.

chatgpt-codex-connector · 2026-03-11T13:13:17Z

internal/llmobs/llmobs.go

+// FlushSync flushes all currently buffered data and blocks until the HTTP send completes.
+func (l *LLMObs) FlushSync() {
+	done := make(chan struct{})
+	l.flushSyncCh <- done


Avoid blocking FlushSync when LLMObs is not running

FlushSync always sends on the unbuffered flushSyncCh without checking lifecycle state, so calling it after Stop() (or before Run() starts receiving) blocks forever because no goroutine is left to read that channel. Since this commit wires llmobs.FlushSync() into tracer test flushing, that deadlock path can hang test runs whenever LLMObs is stopped out of band.

Useful? React with 👍 / 👎.

Good to know. I'll fix it in another PR.

…urning slices or iterators

kakkoyun

I did an initial pass. I will need more time on this. This PR does a lot. But I can see it's neatly separated into commits. So I'll do a commit-by-commit review.

contrib/mark3labs/mcp-go/tracing_test.go

kakkoyun · 2026-03-12T09:38:56Z

ddtrace/tracer/tracer.go

+	// flushHandler is called by the worker when an explicit flush is triggered
+	// (i.e. when Flush() is called). It defaults to defaultFlushHandler.
+	// Tests can override this to provide stronger flush semantics.
+	flushHandler func(done chan<- struct{})


I will have to revisit https://github.com/DataDog/dd-trace-go/pull/3995/changes
or close after this PR.

kakkoyun · 2026-03-12T09:40:05Z

ddtrace/tracer/tracer.go

+	// flushHandler is called by the worker when an explicit flush is triggered
+	// (i.e. when Flush() is called). It defaults to defaultFlushHandler.
+	// Tests can override this to provide stronger flush semantics.
+	flushHandler func(done chan<- struct{})


nit: Maybe we should have an interface type Flasher and FlushFunc for future and possibly more complicated flushing mechanisms.

Agreed. Should we do it in a followup PR?

darccio · 2026-03-12T10:09:06Z

@kakkoyun I agree that the PR does a lot, but the core is in ddtrace/x.

The rest are adapted tests and a fix for Orchestrion integration tests to avoid flakiness that the new free-wait approach unfortunately introduced.

Co-authored-by: Kemal Akkoyun <kakkoyun@users.noreply.github.com>

This reverts commit 9098ddd.

…ectable-tracer-v2

darccio added the team:apm-go label Mar 9, 2026

darccio added 8 commits March 9, 2026 15:47

feat(ddtrace/tracer): customizable tracer flush handling

a3b9390

This allows to swap the flush handling on the tracer with other implementations that require different flush semantics, like synchronous ones.

feat(ddtrace/x/agenttest): mock agent to inspect spans

19dce55

feat(ddtrace/x/tracertest): inspectable tracer using agenttest

da33948

feat(contrib/*): migrate tests to {tracer,agent}test as examples

388c687

feat(ddtrace/tracer): migrate tests to {tracer,agent}test as examples

85fe854

feat(internal/orchestrion/_integration): migrate all the Orchestrion …

c1c20c0

…integration tests to use {test,agent}test

feat(ddtrace/x/llmobstest): support LLMObs testing and migrate some t…

385fc93

…ests as example

chore(ddtrace/tracer): reapply inspectable tracer to updated tests wi…

3c74aaf

…th synctest

darccio force-pushed the dario.castane/rd/inspectable-tracer-v2 branch from 8212d72 to 3c74aaf Compare March 9, 2026 15:03

chore: make format

d9dc20f

darccio added 4 commits March 9, 2026 16:35

docs(ddtrace/x/*test): add comments to new public APIs

eb860a1

fix(internal/orchestrion): fix race conditions and AppSec test failur…

ff87841

…es by replacing FreePort+ListenAndServe with a pre-bound FreeListener+Serve and adding a TestCasePreBootstrap interface so os/lfi.go can configure AppSec env vars before the tracer starts

fix(ddtrace/tracer): add checklocks directives

25a471f

chore(ddtrace/x/*test): add copyright headers

078b37a

darccio added 4 commits March 9, 2026 23:17

fix(ddtrace/tracer): add appsec initialization to tracertest

29682f7

fix(ddtrace/x/agenttest): add logging features to agent and span

a5a9e9d

fix(internal/orchestrion/_integration/internal/harness): map harness …

505f1c8

…Tags to attributes, Meta and Metrics to agenttest span tags

fix(internal/orchestrion/_integration/fiber.v2): add intermediate spa…

6f6465b

…n to expected traces

chatgpt-codex-connector bot reviewed Mar 9, 2026

View reviewed changes

chatgpt-codex-connector bot reviewed Mar 10, 2026

View reviewed changes

fix(ddtrace/tracer): avoid all networking on tests by a custom agent …

990acd6

…transport option

darccio added 3 commits March 10, 2026 11:35

fix(ddtrace/x/llmobstest): add in-process Roundtripper to avoid netwo…

1655187

…rking

feat: add Condition() to SpanMatch API for custom checks

813f336

chore: clean dead code

91d526e

docs(ddtrace/tracer): add more explanation on config.agentTransport

63cc74f

darccio marked this pull request as ready for review March 11, 2026 12:59

darccio requested review from a team as code owners March 11, 2026 12:59

chatgpt-codex-connector bot reviewed Mar 11, 2026

View reviewed changes

docs(ddtrace/x/agenttest): enhance design decisions on no methods ret…

63e6c59

…urning slices or iterators

darccio added the AI Assisted AI/LLM assistance used in this PR (partially or fully) label Mar 11, 2026

kakkoyun reviewed Mar 12, 2026

View reviewed changes

darccio and others added 2 commits March 12, 2026 06:13

feedback: apply suggestion from @kakkoyun

9098ddd

Co-authored-by: Kemal Akkoyun <kakkoyun@users.noreply.github.com>

Revert "feedback: apply suggestion from @kakkoyun"

083ccb7

This reverts commit 9098ddd.

github-actions bot added the apm:ecosystem contrib/* related feature requests or bugs label Mar 12, 2026

darccio requested a review from kakkoyun March 13, 2026 14:20

darccio mentioned this pull request Mar 17, 2026

feat(ddtrace/tracer): promote span fields out of meta map into a typed SpanAttributes struct #4538

Open

7 tasks

darccio added 3 commits March 25, 2026 10:28

Merge remote-tracking branch 'origin/main' into dario.castane/rd/insp…

94042b4

…ectable-tracer-v2

fix(llmobs/experiment): upgrade tests

8f0fd40

fix(internal): make fix-modules

b2e7bd2

Conversation

darccio commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Motivation

Reviewer's Checklist

Uh oh!

datadog-datadog-prod-us1 bot commented Mar 9, 2026 • edited by datadog-datadog-prod-us1-2 bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pr-commenter bot commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks

Explanation

More details about the CI and significant changes

Uh oh!

darccio commented Mar 9, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

darccio commented Mar 10, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector bot Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

darccio commented Mar 11, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kakkoyun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

darccio commented Mar 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

darccio commented Mar 9, 2026 •

edited

Loading

datadog-datadog-prod-us1 bot commented Mar 9, 2026 •

edited by datadog-datadog-prod-us1-2 bot

Loading

pr-commenter bot commented Mar 9, 2026 •

edited

Loading

codecov bot commented Mar 10, 2026 •

edited

Loading