test: enable HTTP instrumentation in tests; assert incoming SERVER spans + traceparent propagation (#475) - #491
test: enable HTTP instrumentation in tests; assert incoming SERVER spans + traceparent propagation (#475)#491sjvans wants to merge 1 commit into
Conversation
…ans + traceparent propagation (#475)
SummaryThe following content is AI-generated and provides a summary of the pull request: Enable HTTP Instrumentation in Tests and Assert Incoming SERVER SpansTest✨ Re-enables HTTP instrumentation in the test app and adds assertions for incoming HTTP SERVER spans and W3C ChangesThe core change is removing A shared
Jira Issues (ONLY add this section if JIRA issues are available)N/A Images and LinksRelated GitHub issues:
PR Bot InformationVersion:
|
![hyperspace-pr-bot[bot]](https://avatars.githubusercontent.com/u/2531208?s=60&v=4){kind=small}
There was a problem hiding this comment.
The four review comments failed to post because the targeted lines were not part of the diff (they are unchanged context lines, not newly added lines). Since the review comment tool requires the original_code to match an added line from the diff, I cannot post those inline comments.
To summarize the overall quality of this PR: the changes are well-structured and clearly documented, with the asExternalClient / suppressTracing pattern consistently applied to the most critical assertions. However, several HTTP requests in tracing.test.js that were not changed by this PR — GET is traced, NonRecordingSpans are handled correctly, cds.spawn is traced, and emit is traced — still issue requests without asExternalClient. With outgoing HTTP instrumentation now enabled, the in-process test client will generate CLIENT spans for those calls too, which may introduce extra roots and cause the root-count assertions in cds.spawn is traced (lengthOf(2)) and emit is traced (lengthOf(1)) to become flaky or fail, and may also cause the /admin/Authors span-absence assertion in NonRecordingSpans to fail. Those four tests should be updated in a follow-up to wrap their HTTP calls in asExternalClient for consistency and correctness.
PR Bot Information
Version: 1.29.30
- LLM:
anthropic--claude-4.6-sonnet - Correlation ID:
ed8b0340-9ad8-11f1-8077-1963f7225a67 - File Content Strategy: Full file content
- Event Trigger:
pull_request.opened
1 participant
What
Re-enables HTTP instrumentation in the test app (disabled by #474 to stay behavior-neutral under jest) and asserts the incoming-request tracing behavior it produces.
test/bookshop/package.json): removeddisableIncomingRequestInstrumentation/disableOutgoingRequestInstrumentationso both default to on; keptignoreIncomingRequestHook. Incoming HTTP requests now produce a SERVER span (the root of each request trace) and outgoing requests produce CLIENT spans.GET with traceparent is traced(wasxtest): asserts the incoming SERVER span adopts the W3C context from thetraceparentheader — trace id0af7651916cd43dd8448eb211c80319c, parent span idb7ad6b7169203331.instrumentation hooks(was emptyxtest): asserts both suppression mechanisms — the sampler'signoreIncomingPaths(/odata/v4/admin/Authors) andMyIgnoreIncomingRequestHook(/Books(252)) — produce NO incoming SERVER span, while a non-ignored path (/Books) does.$batch is traced: updated for reparenting — the single$batchPOST now yields ONE incoming SERVER root with both batch sub-operations (CREATE Genres draft, READ Genres) nested beneath it (was 2 roots).without-outbox,inboxed,persistent-outbox): the producer trace is now rooted at the incoming SERVER span (AdminService - txnests under it), so the root assertion was updated from nameAdminService - txtoSpanKind.SERVERwhile keeping the same nested-span assertions.Why
The test's HTTP client runs in-process; with outgoing instrumentation now on, it would itself create a CLIENT span (an artificial extra root that also overwrites a manually-set
traceparent). A sharedasExternalClienthelper runs client-side requests undersuppressTracingto model a real external, un-instrumented caller — the outgoing CLIENT artifact is skipped and the genuine incoming SERVER span is the trace root.No
lib/change (config + tests only). No CHANGELOG.Verification (sqlite)
test/tracing.test.js— 11 passed / 2 skipped (the 2 formerly-xtestnow real + passing;$batchfixed; Eliminate all test skips and CI test-subset exclusions (sqlite + HANA) #477 §3 stubs remain skipped)test/tracing-remote-cloudsdk.test.js+test/tracing-remote-native.test.js— pass (outgoing CLIENT instrumentation didn't break them)HANA note
inboxedandpersistent-outboxmessaging tests are HANA-only (skipped on sqlite) but share the same reparented producer-root assertion, so they were updated too — HANA CI must be verified on this PR.closes #475