Engine observability and seeded-run determinism#168
Merged
Conversation
SpanInfo gains ParentService and ParentOperation, threaded through both the walk and plan/emit paths, so observers can attribute spans to the calling operation (per-edge statistics). A new PlanEventObserver interface receives timeouts, retries, queue rejections, and circuit breaker trips as timestamped events; previously these plan-phase decisions were only visible as aggregate counters in Stats.
Resolved topology and override attributes change from map[string]AttributeGenerator to Attributes, a key-sorted pair slice built once at topology construction. Map iteration order previously randomised RNG consumption during attribute generation: with a normal distribution generator (variable draw count) alongside other consuming generators, seeded runs could diverge structurally, and attribute values were never reproducible. Ordered iteration makes seeded runs fully deterministic — pinned by a test that fails under map iteration. The log observer's hand-rolled sorted-key workaround is removed; the type now guarantees what it enforced locally. Scenario override merges use a linear merge of sorted sets instead of map copies.
A non-zero seed makes engine, metric, and log observer RNGs deterministic, each on a fixed PCG stream of the same seed so that enabling one signal does not perturb the others. A seeded topology plus motel version fully describes a run's decisions, making runs reproducible after the fact. Determinism is best-effort: it is not guaranteed across motel versions, and wall-clock timing in --realtime mode remains nondeterministic.
There was a problem hiding this comment.
Pull request overview
This PR extracts engine-level improvements to support better observability and deterministic seeded workflows, independent of any visualization work.
Changes:
- Adds parent attribution to
SpanInfo(both walk-path and realtime plan/emit path) and introducesPlanEventObserverfor plan-phase decisions (timeouts, retries, queue rejections, circuit breaker trips). - Replaces attribute generator maps with a deterministic, key-sorted
Attributesslice to make seeded runs reproducible and remove per-call-site key sorting. - Adds
motel run --seedwith per-signal RNG stream separation so enabling metrics/logs doesn’t perturb engine decisions.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/synth/topology.go | Resolves operation/event/metric/log attributes into deterministic Attributes. |
| pkg/synth/topology_test.go | Updates topology tests for Attributes.Get(...) access. |
| pkg/synth/state_test.go | Updates walkTrace calls for new parent parameter. |
| pkg/synth/scenario.go | Uses Attributes in overrides and deterministic merge semantics. |
| pkg/synth/scenario_test.go | Updates scenario tests for Attributes construction/lookup. |
| pkg/synth/property_test.go | Updates property tests for new walkTrace signature and Attributes. |
| pkg/synth/plan.go | Uses Attributes.Merge(...), adds plan-event notifications, deterministic attr iteration. |
| pkg/synth/plan_test.go | Updates plan tests for new walkTrace signature. |
| pkg/synth/observer.go | Extends SpanInfo with parent fields; adds PlanEventObserver plumbing. |
| pkg/synth/observer_test.go | Adds coverage for parent attribution + plan-event observation. |
| pkg/synth/metrics.go | Iterates metric attribute generators deterministically via Attributes. |
| pkg/synth/metrics_test.go | Updates metric tests to construct Attributes. |
| pkg/synth/logs.go | Removes local sorted-key workaround; relies on Attributes for determinism. |
| pkg/synth/logs_test.go | Updates log tests to construct Attributes. |
| pkg/synth/engine.go | Threads parent attribution through walk path; adds plan-event notifications; deterministic attr iteration. |
| pkg/synth/engine_test.go | Updates tests for new walkTrace signature; adds determinism pin test. |
| pkg/synth/emit.go | Derives parent attribution from SpanPlan.ParentIndex during realtime emission. |
| pkg/synth/check.go | Updates walkTrace calls for new parent parameter. |
| pkg/synth/benchmark_test.go | Updates benchmark calls for new walkTrace signature. |
| pkg/synth/attributes.go | Introduces Attributes (sorted slice), merge, and lookup APIs. |
| pkg/synth/attributes_test.go | Adds ordering/merge/lookup tests for Attributes. |
| cmd/motel/main.go | Adds --seed and per-stream RNG wiring for engine/metrics/logs. |
| cmd/motel/main_test.go | Adds unit test for deterministic per-stream RNG behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
610
to
613
| // emitRejectionSpan creates a short error span for a rejected request. | ||
| func (e *Engine) emitRejectionSpan(ctx context.Context, op *Operation, startTime time.Time, reason string, scenarioNames []string, stats *Stats, spanCount *int, isAsync bool) (time.Time, bool) { | ||
| func (e *Engine) emitRejectionSpan(ctx context.Context, op, parent *Operation, startTime time.Time, reason string, scenarioNames []string, stats *Stats, spanCount *int, isAsync bool) (time.Time, bool) { | ||
| *spanCount++ | ||
| tracer := e.Tracers(op.Service.Name) |
| stats.CircuitBreakerTrips++ | ||
| notifyPlanEvent(e.Observers, PlanEvent{Kind: PlanEventCircuitBreakerTrip, Service: op.Service.Name, Operation: op.Name, Timestamp: startTime}) | ||
| } | ||
| return e.planRejectionSpan(op, parentIndex, startTime, reason, scenarioNames, plans, spanCount) |
Comment on lines
97
to
+100
| for k, v := range op.Service.Attributes { | ||
| spanAttrs = append(spanAttrs, attribute.String(k, v)) | ||
| } | ||
| for k, gen := range opAttrs { | ||
| spanAttrs = append(spanAttrs, typedAttribute(k, gen.Generate(e.Rng))) | ||
| for _, a := range opAttrs { |
…nning A rejected operation incremented the per-trace span counter twice (once in walkTrace/planTrace, again in the rejection helper) for a single emitted span, hitting the span limit early. The rejection helpers no longer re-count. planTrace also never marked async callees as CONSUMER spans, so realtime mode emitted CLIENT kinds where fast mode emitted CONSUMER. The async flag is now threaded through planning, mirroring walkTrace. Both pre-existing; surfaced during review of this change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extracts the engine-level improvements from the visualisation spike (PR 165) into a standalone, mergeable change. No visualisation code is included; these three commits are useful to any observer or seeded workflow independent of how the graph work proceeds.
Changes
Span parentage and plan-phase events for observers.
SpanInfogainsParentService/ParentOperation, threaded through both the walk path and the realtime plan/emit path (derived from the existingSpanPlan.ParentIndex), so observers can attribute spans to their calling operation. A newPlanEventObserverinterface surfaces timeouts, retries, queue rejections, and circuit breaker trips as timestamped events — simulation ground truth that was previously only visible as aggregate counters inStats.Deterministic attribute iteration. Resolved topology and override attributes change from
map[string]AttributeGeneratortoAttributes, a key-sorted pair slice built once at topology construction. Map iteration order previously randomised RNG consumption during attribute generation: with a normal-distribution generator (variable draw count via ziggurat sampling) alongside other consuming generators, seeded runs could diverge structurally, and generated attribute values were never reproducible. This is a correctness fix for any seeded use, includingmotel check. The log observer's hand-rolled sorted-key workaround is removed — the type now guarantees what that code enforced locally. Pinned by a determinism test verified to fail under map iteration.motel run --seed. A non-zero seed makes engine, metric, and log observer RNGs deterministic, each on a fixed PCG stream of the same seed so enabling one signal does not perturb the others. A seeded topology plus motel version reproduces a run's decision sequence after the fact. Determinism is best-effort: not guaranteed across motel versions, and wall-clock pacing in--realtimemode remains nondeterministic.Verification
make testandmake lintpass. New tests cover parent attribution on both engine paths, plan-event observation,Attributesordering/merge/lookup semantics, per-stream RNG separation, and full structural-plus-value determinism of seeded runs.https://claude.ai/code/session_01P53wZUbnoSxTDMftXxrFLi
Generated by Claude Code