Skip to content

feat(traces): migrate to embed.TraceConsumer + ProducerModule with realistic per-span emission timing (PIPE-1024)#234

Merged
Dylan-M merged 2 commits into
mainfrom
dylanmyers/pipe-1024-traces-migration
Jun 3, 2026
Merged

feat(traces): migrate to embed.TraceConsumer + ProducerModule with realistic per-span emission timing (PIPE-1024)#234
Dylan-M merged 2 commits into
mainfrom
dylanmyers/pipe-1024-traces-migration

Conversation

@Dylan-M
Copy link
Copy Markdown
Contributor

@Dylan-M Dylan-M commented Jun 2, 2026

Proposed Change

traces.New now takes a Config struct (Logger, Workers, Rate, Hostname, Consumer, Seed); Start becomes Start(ctx context.Context) error (embed.ProducerModule shape); Name() returns "traces".

Realistic per-span emission timing. Rather than batching all 2–5 spans of a trace into a single ConsumeTraces call, each span is scheduled individually at its own EndTime via time.AfterFunc, so the consumer observes spans arriving at staggered wall-clock moments the way a real distributed system delivers them. This is a load-bearing design choice for downstream distributed-blitz simulation where traces span multiple blitz instances. Pending emissions are tracked in a sync.WaitGroup so Stop attempts to drain them within a bound of ctx or 20s (whichever fires first). On bound elapsed, pending time.AfterFunc callbacks continue to fire on their original schedule — their emissions complete asynchronously after Stop returns, and the Generator stays reachable until they resolve. Not a leak, but documented explicitly in the Stop docstring so callers aren't surprised; callers who need strict no-post-Stop emissions can wrap the supplied consumer with a guard.

Trace shape. Each trace = 1 HTTP server root + 1–4 children randomly drawn from {DB query, cache lookup, internal processing, downstream HTTP call, input validation} — yielding 2–5 spans per trace. Variety + child count seed-controlled. The fixed root + db + optional proc shape is gone.

New Seed int64 field on TracesGeneratorConfig (and on traces.Config), mirroring FIX's precedent. Seed >= 0 deterministically seeds worker N with Seed+N; Seed < 0 falls back to time.Now().UnixNano()+N per worker. YAML omitted ⇒ randomize: dispatch layer translates YAML zero-value (or explicit seed: 0) into the negative-randomize path so YAML users get stochastic data by default. TraceID / SpanID always come from crypto/rand regardless of Seed — global uniqueness is preserved across blitz instances. Per-machine identity determinism (hostname) is governed separately by PIPE-1036's environment.seed_config.

Hostname. New optional Hostname field on traces.Config + YAML — when empty, a deterministic Linux-style hostname is generated from Seed via datagen.GenerateHostname, matching the hostmetrics convention so records from both signals attribute consistently to the same simulated machine. Every emitted span's Metadata.Resource is populated via resource.Default("traces") with host.name overridden by this hostname.

Count tracker semantics. SetCountTracker: one Acquire equals one trace, not one span. Documented on the method.

The legacy generator.TraceGenerator interface and the corresponding internal/service/service.go type-switch case are deleted in this PR — traces was the sole implementer.

Tests

  • Rewritten traces_test.go uses require.Eventually (no time.Sleep), new mockTraceConsumer captures spans + arrival times, new nil-consumer error case ("TraceConsumer cannot be nil").
  • New TestSpansEmittedAtEndTime proves arrival time ≥ EndTime per-span — holds under both abort and drain semantics.
  • New TestSeedDeterminism for deterministic Name/Kind across runs.
  • Rewritten TestTraceStructure validates the new root + 1–4 children shape, with every child parented to root.
  • Removed stubTraceGen from service_test.go (dead path).

Part of PIPE-1024. Stacked on #233.

@Dylan-M Dylan-M requested review from a team as code owners June 2, 2026 15:46
Copy link
Copy Markdown

@eKuG eKuG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a Nit

Comment on lines +209 to +214
case <-drainCtx.Done():
g.logger.Warn("Traces generator stop drain bounded",
zap.Duration("bound", stopDrainTimeout),
zap.Error(drainCtx.Err()),
)
return drainCtx.Err()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stop() drain timeout fires but background goroutine still blocks on g.wg.Wait() until pending
time.AfterFunc callbacks complete. Generator object held in memory until last pending span emits. PR description explicitly intended "in-flight emissions abandoned", actual behavior: emissions complete eventually, just after Stop
returns. Not a leak; document the lingering-goroutine behavior or cancel pending AfterFunc on stop.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — the doc was misleading. Took the doc-only path you suggested: rewrote the Stop docstring to honestly describe the actual semantic. Emissions that don't fire within the drain bound continue asynchronously after Stop returns; the Generator stays reachable until they resolve. Not a leak (every callback eventually runs and the wg drains), but it's worth being explicit that "Stop returned a timeout error" doesn't mean "no further emissions will reach the consumer". Also noted in the docstring that callers needing strict no-post-Stop emissions can wrap the supplied consumer with a guard.

@Dylan-M Dylan-M force-pushed the dylanmyers/pipe-1024-traces-migration branch from 73fa41c to f3df873 Compare June 3, 2026 12:29
@Dylan-M Dylan-M force-pushed the dylanmyers/pipe-1024-trace-consumer-adapter branch from f6ebb9d to 7dc72c2 Compare June 3, 2026 12:29
@Dylan-M Dylan-M requested a review from eKuG June 3, 2026 12:30
@Dylan-M Dylan-M force-pushed the dylanmyers/pipe-1024-traces-migration branch from f3df873 to 45aabb7 Compare June 3, 2026 13:42
@Dylan-M Dylan-M force-pushed the dylanmyers/pipe-1024-trace-consumer-adapter branch from 7dc72c2 to 069816e Compare June 3, 2026 13:42
@Dylan-M Dylan-M force-pushed the dylanmyers/pipe-1024-traces-migration branch from 45aabb7 to c130826 Compare June 3, 2026 14:01
@Dylan-M Dylan-M force-pushed the dylanmyers/pipe-1024-trace-consumer-adapter branch from 069816e to 2178b70 Compare June 3, 2026 14:01
Base automatically changed from dylanmyers/pipe-1024-trace-consumer-adapter to main June 3, 2026 14:20
@Dylan-M Dylan-M added this pull request to the merge queue Jun 3, 2026
Merged via the queue into main with commit 6bee584 Jun 3, 2026
12 of 23 checks passed
@Dylan-M Dylan-M deleted the dylanmyers/pipe-1024-traces-migration branch June 3, 2026 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants