feat: trace queue worker transactions + structured span test infrastructure - #465
Conversation
SummaryThe following content is AI-generated and provides a summary of the pull request: Queue Worker Tracing + Structured In-Memory Span Test InfrastructureNew Features✨ Two major improvements to
Changes
Jira Issues (ONLY add this section if JIRA issues are available)
PR Bot InformationVersion:
|
There was a problem hiding this comment.
The PR is a solid improvement — replacing regex-based console log assertions with structured ReadableSpan object assertions is a meaningful step forward in test reliability, and the cds.Service.prototype.tx wrap is a well-reasoned fix for the orphaned queue-worker span problem. The main issues flagged are: a byParent map that keys root spans under undefined making it awkward to use, implicit string-vs-number version comparisons in the skip guards, a redundant double-require inside a sort comparator, and a risk of double-wrapping if the cds.js module is loaded more than once. The multitenancy test assertions could also be tightened to avoid false positives if background spans leak through the reset boundary.
PR Bot Information
Version: 1.29.9
- Experiment Variant:
control - LLM:
anthropic--claude-4.6-sonnet - File Content Strategy: Full file content
- Correlation ID:
6a9f1b40-8cd9-11f1-8971-da8293717686 - Event Trigger:
pull_request.opened
5883c81 to
7070494
Compare
b281c8f to
9b88203
Compare
89d1e87 to
29710d5
Compare
29710d5 to
3983e27
Compare
… log-regex tests with structured in-memory spans
Queue worker tracing:
- Wraps cds.Service.prototype.tx() so that the queue worker's two-transaction
structure (SELECT+UPDATE lock tx, then handle+DELETE dispatch tx) appears as
child spans under their trace root instead of each top-level CAP call inside
them becoming an orphan root.
- Guard: skips when this.context instanceof cds.EventContext so $batch sub-requests
are not affected, and skips the bare {} context from processInboundMsg so
file-based messaging consumer delivery gets a messaging - tx root span.
- SQLite note: CDS uses a raw setTimeout bypass for sqlite queue workers (to avoid
deadlocks) so cds.spawn is not called, and spawn-root tracing is not available on
sqlite. Affected tests are skipped on sqlite and verified on HANA in CI.
Test infrastructure (from PR #450):
- MyInMemorySpanExporter: structured in-memory span store replacing fragile
cds.test.log() regex matching. groupedByTrace() / rootSpans() helpers let
tests assert on span trees directly.
- tracing-in-memory profile in .cdsrc.json; tracing-attributes profile preserved.
- tracing.test.js, tracing-attributes.test.js, tracing-mt.test.js,
tracing-messaging*.js rewritten to use structured span assertions.
- New test files: tracing-scheduled, tracing-outboxed-batch,
tracing-messaging-inboxed, tracing-messaging-outboxed-and-inboxed,
tracing-messaging-persistent-outbox, console-span-exporter (unit test).
- tracing-messaging.js: configurable waitMs (default 2500ms, 4000ms for
inbox/persistent-outbox scenarios); afterAll wait bumped to 2s.
- admin-service: test_outboxed_send / test_outboxed_send_batch / test_scheduled
actions added for the new test scenarios.
The queue-worker tracing tests (scheduled, outboxed-batch, and the inboxed/ outboxed-and-inboxed/persistent-outbox messaging cases) assert on the 'cds.spawn - run task' root span and its child tx spans. That root only appears when @sap/cds routes the sqlite queue worker through cds.spawn. Published cds uses a raw setTimeout bypass on sqlite (to avoid a single-writer deadlock), so those spans never appear there. Skip these suites on sqlite until the cds fix lands (cap/cds branch test/queue-spawn-sqlite-extended-tenant, which removes the bypass by fixing the actual deadlock root cause). A follow-up PR removes these skips once the required cds version is released. HANA CI already exercises the full path.
…quire, tighten mt assertion
- Number(cds.version.split('.')[0]) < 9 instead of relying on string coercion
(tracing-outboxed-batch, tracing-scheduled)
- hoist single hrTimeToNanoseconds require, drop the double require in the sort
comparator (tracing-outboxed-batch)
- multitenancy: assert exactly one AdminService READ span (filter + length 1)
instead of find, guarding against residue leaking past reset (tracing-mt)
Skipped bot findings: byParent[undefined] (field is unused by any test);
tx double-wrap guard (same pattern as existing emit/handle wraps, plugin
loads once per process).
3983e27 to
74d924a
Compare
Messaging is outboxed by default, so `outboxed: true, inboxed: true` exercises the exact same code path and asserts the identical span shape as the `inboxed` test. Remove the duplicate test + its unused .cdsrc profile.
… ConsoleMetricExporter (#479) ## What Consolidates all outbox/metrics test-quality work into one PR (formerly split as #479 + the stacked #480). - **In-memory metric reader** — `test/bookshop/lib/MyInMemoryMetricReader.js`, the metrics counterpart to `MyInMemorySpanExporter` (#465). Mirrors production **DELTA** temporality: SUM counters are accumulated across flushes into per-series running totals; GAUGE datapoints keep the latest absolute value. Wired via the `metrics-outbox`, `metrics-outbox-disabled`, and `metrics` profiles in `.cdsrc.json`. - **Outbox suites off console spying** — the three `metrics-outbox*.test.js` suites drop the `console.dir` spy and fixed `wait()` sleeps in favor of the reader + an `expectEventually()` force-flush polling helper (fails fast if the meter provider isn't wired). Folds in #445's polling approach. - **ConsoleMetricExporter unit test** — new `test/console-metric-exporter.test.js`, a pure unit test of the exporter's formatting (db.pool table, queue table, other single-vs-array, tenant variants, host-metrics aggregation, shutdown→FAILED), mirroring `console-span-exporter.test.js`. - **`metrics.test.js`** converted from scraping `cds.test.log()` output to asserting on the in-memory reader's datapoints. Metrics testing now mirrors the tracing side exactly: a to-console unit test **plus** in-memory-exporter–based integration tests. ## Why Follow-up to #465 (span test infra): eliminate console/log spying in the metrics suite and give `ConsoleMetricExporter` direct unit coverage. ## Review addressed - Bot review triaged: explicit `COUNTER_METRIC_NAMES` dispatch for `isCounter`; real wall-clock debounce in the multitenant test; isolation NOTE on the module-level singletons. - Dropped the unused debug-log silencer in the multitenant suite (never asserted). Kept the single-tenant `debugLog` mock — it backs a real `unknown service` assertion. Test-only change (no `lib/` change), so no CHANGELOG entry — consistent with #465/#474/#476. closes #478 Supersedes #445 and #480 (both folded in here) — I'll close them once this merges.
Added
Wraps
cds.Service.prototype.tx()so the queue worker's two-transaction structure (tx1: SELECT+UPDATE lock, tx2: handle+DELETE dispatch) appears as coherent<service> - txspans under thecds.spawn - run taskroot, instead of each top-level CAP call becoming an orphan root. Guarded so$batchsub-requests (activeEventContext) are unaffected; file-based messaging consumer delivery (bare{}context) still gets a root span.Test infrastructure
Replaces fragile
cds.test.log()regex assertions with a structured in-memory span exporter (MyInMemorySpanExporter) andgroupedByTrace()/rootSpans()helpers. Rewrites the existing tracing suites and adds coverage for scheduled tasks, outboxed batch fan-out, and inbox/outbox messaging combinations.SQLite note
Queue-worker suites skip on sqlite (published
@sap/cdsuses asetTimeoutbypass, notcds.spawn) — verified on HANA in CI. Follow-up #467 removes the skips once the cds queue-spawn fix ships.Changelog updated. Targets
develop.