test: add ConsoleMetricExporter unit test + assert metrics via in-memory reader - #480
Conversation
SummaryThe following content is AI-generated and provides a summary of the pull request: Add
|
![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 well-structured and clearly mirrors the established console-span-exporter.test.js pattern. The only structural issue found is that const { expect } = require('@cap-js/cds-test') is placed after the helper functions that use it (line 50), whereas the ConsoleSpanExporter test keeps it at the top alongside the other imports — the late import is harmless at runtime today but is inconsistent and could cause a ReferenceError if exportAndCapture were ever called at module-evaluation time. This was flagged with a suggested fix above.
PR Bot Information
Version: 1.29.18
- LLM:
anthropic--claude-4.6-sonnet - Event Trigger:
pull_request.opened - File Content Strategy: Full file content
- Correlation ID:
0632a390-956a-11f1-8168-6c863a2dcb37
| const ConsoleMetricExporter = require('../lib/exporter/ConsoleMetricExporter') | ||
|
|
||
| afterAll(() => { | ||
| telemetryLog.info = originalInfo | ||
| }) | ||
|
|
||
| beforeEach(() => { | ||
| infoCalls.length = 0 | ||
| }) | ||
|
|
||
| // --- helpers --------------------------------------------------------------- | ||
|
|
||
| // Builds a minimal ScopeMetrics-shaped object. | ||
| function scopeMetrics(name, metrics) { | ||
| return { scope: { name }, metrics } | ||
| } | ||
|
|
||
| // Builds a minimal MetricData-shaped object. `dataPoints` are `{ attributes, value }`. | ||
| function metric(name, dataPoints, description = name) { | ||
| return { descriptor: { name, description }, dataPoints } | ||
| } | ||
|
|
||
| // Drives the exporter and returns the lines logged. Asserts the result callback got SUCCESS. | ||
| function exportAndCapture(scopes) { | ||
| const exporter = new ConsoleMetricExporter() | ||
| let result | ||
| exporter.export({ scopeMetrics: scopes }, r => (result = r)) | ||
| expect(result).to.deep.equal({ code: 0 /* ExportResultCode.SUCCESS */ }) | ||
| return infoCalls.map(args => args[0]) | ||
| } | ||
|
|
||
| // --- assertions ------------------------------------------------------------ | ||
|
|
||
| const { expect } = require('@cap-js/cds-test') |
There was a problem hiding this comment.
Bug: expect is required after it is first used, causing a ReferenceError at runtime.
exportAndCapture (called from the test bodies) uses expect at line 44, but expect is not imported until line 50 — after all the helper definitions. Because exportAndCapture is only called at runtime this works in practice today, but the exportAndCapture definition itself references expect in a closure that executes before the import on line 50 if test helpers are ever invoked at module-evaluation time. More importantly, it is inconsistent with console-span-exporter.test.js, which places the expect import immediately after hooking LOG.info (before the helpers). Move the import to the top of the file, alongside the other require calls.
| const ConsoleMetricExporter = require('../lib/exporter/ConsoleMetricExporter') | |
| afterAll(() => { | |
| telemetryLog.info = originalInfo | |
| }) | |
| beforeEach(() => { | |
| infoCalls.length = 0 | |
| }) | |
| // --- helpers --------------------------------------------------------------- | |
| // Builds a minimal ScopeMetrics-shaped object. | |
| function scopeMetrics(name, metrics) { | |
| return { scope: { name }, metrics } | |
| } | |
| // Builds a minimal MetricData-shaped object. `dataPoints` are `{ attributes, value }`. | |
| function metric(name, dataPoints, description = name) { | |
| return { descriptor: { name, description }, dataPoints } | |
| } | |
| // Drives the exporter and returns the lines logged. Asserts the result callback got SUCCESS. | |
| function exportAndCapture(scopes) { | |
| const exporter = new ConsoleMetricExporter() | |
| let result | |
| exporter.export({ scopeMetrics: scopes }, r => (result = r)) | |
| expect(result).to.deep.equal({ code: 0 /* ExportResultCode.SUCCESS */ }) | |
| return infoCalls.map(args => args[0]) | |
| } | |
| // --- assertions ------------------------------------------------------------ | |
| const { expect } = require('@cap-js/cds-test') | |
| const { expect } = require('@cap-js/cds-test') | |
| const ConsoleMetricExporter = require('../lib/exporter/ConsoleMetricExporter') | |
| afterAll(() => { | |
| telemetryLog.info = originalInfo | |
| }) | |
| beforeEach(() => { | |
| infoCalls.length = 0 | |
| }) | |
| // --- helpers --------------------------------------------------------------- | |
| // Builds a minimal ScopeMetrics-shaped object. | |
| function scopeMetrics(name, metrics) { | |
| return { scope: { name }, metrics } | |
| } | |
| // Builds a minimal MetricData-shaped object. `dataPoints` are `{ attributes, value }`. | |
| function metric(name, dataPoints, description = name) { | |
| return { descriptor: { name, description }, dataPoints } | |
| } | |
| // Drives the exporter and returns the lines logged. Asserts the result callback got SUCCESS. | |
| function exportAndCapture(scopes) { | |
| const exporter = new ConsoleMetricExporter() | |
| let result | |
| exporter.export({ scopeMetrics: scopes }, r => (result = r)) | |
| expect(result).to.deep.equal({ code: 0 /* ExportResultCode.SUCCESS */ }) | |
| return infoCalls.map(args => args[0]) | |
| } |
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
71b7877 to
d948065
Compare
… 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.
1 participant
What
Completes the metrics-test story so it mirrors the tracing side, which already has both a
console-span-exporter.test.jsunit test and in-memory-exporter–based integration tests.test/console-metric-exporter.test.js— a pure unit test oflib/exporter/ConsoleMetricExporter.js(modeled onconsole-span-exporter.test.js): hookscds.log('telemetry').info, feeds craftedResourceMetricsfixtures, and asserts the formatted output. Covers all branches: thedb.pooltable, thequeuetable, "other" metrics (single-datapoint unwrapped vs multi-datapoint array), tenant-labeled variants, host-metrics aggregation (with/withoutHOST_METRICS_LOG_SYSTEM), and the shutdown→FAILED path.test/metrics.test.js— converted from scrapingcds.test.log()output to asserting on the in-memoryMyInMemoryMetricReaderdatapoints, via the sameexpectEventually()force-flush polling helper used by the outbox suites. Now asserts on what is actually collected (process metrics present, system/network absent by default;nodejs.eventloop.timehas multiple datapoints vsutilization's one) rather than on log strings — the formatting of those is covered by the new unit test.[metrics]profile in.cdsrc.json(matching what test: capture outbox+console metrics via in-memory reader & unit-test ConsoleMetricExporter #479 did for the outbox profiles).Why
Follow-up to #465 (span test infra) and #479 (outbox metrics reader): eliminate the last console/log-string spying in the metrics suite and give
ConsoleMetricExporterdirect unit coverage.Notes
test/in-memory-metric-reader) —MyInMemoryMetricReaderis that PR's deliverable. Base is set to test: capture outbox+console metrics via in-memory reader & unit-test ConsoleMetricExporter #479's branch; merge test: capture outbox+console metrics via in-memory reader & unit-test ConsoleMetricExporter #479 first (this will then retarget todevelopautomatically).lib/change), so no CHANGELOG entry — consistent with feat: trace queue worker transactions + structured span test infrastructure #465/chore: migrate test runner from jest to vitest #474/chore: adopt oxfmt for code formatting #476/test: capture outbox+console metrics via in-memory reader & unit-test ConsoleMetricExporter #479.