test: convert remote/span-name tracing tests to in-memory span exporter; drop tracing-attributes profile (#478) - #490
Conversation
…er; drop tracing-attributes profile (#478)
SummaryThe following content is AI-generated and provides a summary of the pull request: test: Convert Remote/Span-Name Tracing Tests to In-Memory Span Exporter; Drop
|
![hyperspace-pr-bot[bot]](https://avatars.githubusercontent.com/u/2531208?s=60&v=4){kind=small}
There was a problem hiding this comment.
The PR is a clean, well-motivated refactor. The only minor inconsistency is in tracing-span-names.test.js, which manually resets the span buffer via captured.length = 0 rather than calling the shared reset() helper that the other two migrated files use — comments have been left inline for that. Everything else looks correct: the profile removal is properly justified (no remaining consumers), the beforeEach/beforeAll ordering is sound, and the assertion logic is preserved faithfully.
PR Bot Information
Version: 1.29.30
- File Content Strategy: Full file content
- Event Trigger:
pull_request.opened - LLM:
anthropic--claude-4.6-sonnet - Correlation ID:
2448efe0-9a35-11f1-8ec3-aaf7cc3158a5
| // The tracing-in-memory profile (see test/bookshop/.cdsrc.json) configures | ||
| // MyInMemorySpanExporter as the trace exporter. We read the captured ReadableSpan | ||
| // objects directly out of its shared buffer — no console spy. | ||
| const { captured } = require('./bookshop/lib/MyInMemorySpanExporter') |
There was a problem hiding this comment.
Best Practices: reset is not imported, so the buffer is cleared manually with captured.length = 0 instead of calling the shared reset() helper.
The other two migrated files (tracing-remote-cloudsdk.test.js, tracing-remote-native.test.js) both import and call reset. Using captured.length = 0 directly duplicates the reset logic and will silently diverge if reset() in MyInMemorySpanExporter.js ever does anything beyond clearing the array. Consider importing reset here for consistency.
| const { captured } = require('./bookshop/lib/MyInMemorySpanExporter') | |
| const { captured, reset } = require('./bookshop/lib/MyInMemorySpanExporter') |
Double-check suggestion before committing. Edit this comment for amendments.
Please provide feedback on the review comment by checking the appropriate box:
- 🌟 Awesome comment, a human might have missed that.
- ✅ Helpful comment
- 🤷 Neutral
- ❌ This comment is not helpful
| // data.reset is itself heavily traced (it runs DELETEs + INSERTs for the seed data) — | ||
| // run it first, THEN clear the buffer so the test only sees its own spans. | ||
| await data.reset() | ||
| captured.length = 0 |
There was a problem hiding this comment.
Best Practices: Buffer is cleared manually instead of using the imported reset() helper.
After importing reset (see sibling comment), replace captured.length = 0 with reset() to keep the teardown consistent with the other migrated test files and resilient to future changes in MyInMemorySpanExporter.
| captured.length = 0 | |
| reset() |
Double-check suggestion before committing. Edit this comment for amendments.
Please provide feedback on the review comment by checking the appropriate box:
- 🌟 Awesome comment, a human might have missed that.
- ✅ Helpful comment
- 🤷 Neutral
- ❌ This comment is not helpful
1 participant
What
Group 1 of #478. Converts the three tracing tests that still spied on
console.dirto read structuredReadableSpanobjects directly from the in-memory span exporter:test/tracing-remote-cloudsdk.test.jstest/tracing-remote-native.test.jstest/tracing-span-names.test.jsEach now uses
--profile tracing-in-memory(which wiresMyInMemorySpanExportervia.cdsrc.json) and reads overcaptured(the module-level array ofReadableSpans) instead of theconsole.dirspy.beforeEach(reset)clears the buffer per test. All existing assertions are preserved verbatim —instrumentationScope.namefilters (@cap-js/telemetry,@opentelemetry/instrumentation-undici), span names, and attributes (code.function.name,db.query.text,sap.btp.destination, the no-raw-SQL / no-URL-in-span-name checks).capturedholds full ReadableSpans withinstrumentationScope, so the filters just point atcaptured.No flush/poll was needed: the spans for these single request/DB ops appear synchronously in
capturedafter the awaited call. Intracing-span-names.test.js,data.reset()is itself traced, so the buffer is cleared after reset (mirroringtracing-attributes.test.js).Why
No more
console.dirspying in the tracing tests. With all three files migrated, the[tracing-attributes]profile intest/bookshop/.cdsrc.jsonhas no remaining consumers (verified: only these 3 used it;tracing-attributes.test.jsdespite its name already usestracing-in-memory), so it is removed. This also resolves the deferred "rename tracing-attributes → tracing-console" note — the profile simply goes away.Test-only change. No lib change, no CHANGELOG.
Refs #478 (group 1 only; group 2 done via #479, group 3 stays).