Skip to content

feat: emit INTERNAL span kind for same-service sync calls#231

Merged
andrewh merged 2 commits into
mainfrom
claude/issue-213-pr-sfrtu8
Jul 2, 2026
Merged

feat: emit INTERNAL span kind for same-service sync calls#231
andrewh merged 2 commits into
mainfrom
claude/issue-213-pr-sfrtu8

Conversation

@andrewh

@andrewh andrewh commented Jul 2, 2026

Copy link
Copy Markdown
Owner

Closes #213.

Per the OTel SpanKind spec, INTERNAL represents an operation that does not cross a remote/process boundary. motel previously emitted CLIENT for every non-root sync callee, including calls to another operation on the same service.

Changes

  • spanKindFor now takes the parent operation and returns SpanKindInternal when a sync callee lives on the same service as its caller. Precedence is unchanged otherwise: roots always win as SERVER, then producer → PRODUCER, then async → CONSUMER, then same-service → INTERNAL, else CLIENT.
  • walkTrace already carried the parent operation; planTrace/planRejectionSpan (realtime plan mode) gained a parent parameter so both paths agree, including rejection spans.
  • Confirmed the topology DSL already expresses same-service operation→operation edges: cycle detection is per-operation, so checkout.submit → checkout.validate validates fine (verified end-to-end via motel run --stdout: SERVER root, INTERNAL same-service child, CLIENT cross-service grandchild).

Tests

  • TestSameServiceSyncCallSpanKindIsInternal: same-service sync callee is INTERNAL, its cross-service downstream call is still CLIENT, root stays SERVER.
  • TestSameServiceAsyncCallSpanKindIsConsumer: async precedence pinned — a same-service fire-and-forget callee stays CONSUMER.
  • TestPlanTraceSameServiceSyncCallKind: plan-mode parity for the same topology.
  • Existing cross-service kind tests (TestSyncCallSpanKindIsClient, producer/consumer tests) unchanged and passing.

Docs

  • cmd/motel/README.md: kind-selection rules documented in the calls section, with a note on modelling in-process sub-operations.
  • CHANGELOG.md: entry under Unreleased → Changed (behavior change for existing configs with same-service sync calls).
  • docs/explanation/property-testing.md: clarified that the "non-root spans are CLIENT" property holds because the generator only produces cross-service calls.

🤖 Generated with Claude Code

https://claude.ai/code/session_019nKG5nUHLBi6KNegsvqR1V

Generated by Claude Code

@claude
feat: emit INTERNAL span kind for same-service sync calls (#213)
58acfd2 
Per the OTel SpanKind spec, INTERNAL represents an operation that does
not cross a remote boundary. Sync calls targeting an operation on the
caller's own service now emit INTERNAL spans instead of CLIENT; cross-
service sync calls keep CLIENT, and async/producer callees keep
CONSUMER/PRODUCER regardless of service. Roots still always win as
SERVER.

spanKindFor now takes the parent operation, threaded through walkTrace
(which already had it) and planTrace/planRejectionSpan (new parameter)
so realtime plan mode matches.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_019nKG5nUHLBi6KNegsvqR1V
@andrewh Claude

andrewh commented Jul 2, 2026

Copy link
Copy Markdown
Owner Author

Adversarial review results

Ran an 8-angle adversarial review of this PR (line-by-line diff scan, removed-behavior audit, cross-file tracing, reuse/simplification/efficiency/altitude checks, conventions). Findings, most severe first:

Confirmed — will fix in follow-up commits

1. Replay mode diverges from generation for same-service children — pkg/synth/replay.go:346
buildReplayPlans derives span kind purely from tree position (parentIndex < 0 ? SERVER : CLIENT). With this PR, motel run emits INTERNAL for a same-service sync child, but replaying a recording of the identical topology stamps that child CLIENT. Three independent review angles converged on this. Fix: apply the same-service → INTERNAL rule when deriving replayed kinds (the parent's service is already available in the plan being built).

2. String comparison where pointer comparison is guaranteed equivalent — pkg/synth/engine.go (spanKindFor)
BuildTopology constructs every operation on a service with the same *Service pointer, so parent.Service == op.Service is exactly equivalent to comparing names and cheaper in the per-span hot path. Minor, but free.

3. Duplicated test topology — pkg/synth/engine_test.go / pkg/synth/plan_test.go
The 3-service gateway.handle → gateway.validate → backend.process config literal is copy-pasted between TestSameServiceSyncCallSpanKindIsInternal and TestPlanTraceSameServiceSyncCallKind. Fix: shared helper (same package).

Considered and refuted

  • Root-precedence concern (same-service call targeting a root would emit SERVER, not INTERNAL): impossible in base topology — findRoots only marks operations that are never a call target. Reachable only via scenario add_calls, where "roots always win" is pre-existing, documented precedence unchanged by this PR.
  • "The same-service async test is redundant": it isn't — existing async tests are all cross-service, so TestSameServiceAsyncCallSpanKindIsConsumer is the only coverage of the async-beats-INTERNAL precedence.
  • Downstream consumers of kind: metrics.go, logs.go, emit.go, observer.go, traceimport, and semconv never branch on span kind — INTERNAL flows through unharmed. TestProperty_Engine_NonRootSpanIsClient remains valid because its generator only wires cross-service calls.

Design notes (no change planned)

  • planTrace now threads both parent *Operation and parentIndex; the parent's service is technically derivable from (*plans)[parentIndex].Service. Kept as-is for signature parity with walkTrace, which already threaded parent for observer attribution — but flagging the redundancy for the record.
  • Same-service-ness is a static property of a call edge, so the kind decision could be precomputed on Call at build time (as Async/Producer already are). Deferring: it's a larger refactor touching scenario add_calls resolution, and the runtime compare is a single pointer equality after fix 2.

Fixes for 1–3 incoming.

🤖 Generated with Claude Code

Generated by Claude Code

@claude
review fixes: align replay span kinds, pointer compare, dedupe test c…
e8c5e36 
…onfig

Address adversarial review findings on #231:

- Replay mode now derives INTERNAL for a recorded child span on its
  parent's service, matching spanKindFor's rules for generated traces
  (previously every non-root replayed span was stamped CLIENT).
- spanKindFor compares *Service pointers instead of names; BuildTopology
  guarantees all operations on a service share one Service instance.
- The same-service test topology is a shared helper instead of being
  duplicated between engine_test.go and plan_test.go.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_019nKG5nUHLBi6KNegsvqR1V
@andrewh
andrewh merged commit 5be6e85 into main Jul 2, 2026
2 checks passed
@andrewh
andrewh deleted the claude/issue-213-pr-sfrtu8 branch July 2, 2026 16:50
@andrewh andrewh mentioned this pull request Jul 8, 2026
Merged
andrewh added a commit that referenced this pull request Jul 8, 2026
@andrewh @claude
docs(changelog): reconcile release history and prepare 0.11.0 (#237)
9541b03 
Back-fill the 0.9.1, 0.9.2, and 0.10.0 sections that were released via
GoReleaser but never recorded in CHANGELOG.md, and move already-shipped
work out of [Unreleased]: replay mode (#209, 0.10.0), PRODUCER span kind
(#214, 0.9.2), and reader-based replay entry points (#216, 0.9.2).

Promote the genuinely-unreleased work to [0.11.0]: the INTERNAL span-kind
change (#213/#231), the public pkg/synth trace-generation API (#199/#234),
and the sampling trace-integrity invariants (#74/#235/#236), plus the OTLP
exporter dependency bumps (#228, #229).

Note that 0.9.2 was tagged after 0.10.0 from a later commit, so the next
minor release is 0.11.0, and fix the compare links.


Claude-Session: https://claude.ai/code/session_01G1HxLtj1Uu2Y3K6Ea5P9zR

Co-authored-by: Claude <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Emit INTERNAL span kind for in-process (same-service) calls

2 participants

@andrewh @claude