chore: more stable outbox metrics tests - #445
Closed
sjvans wants to merge 19 commits into
Closed
Conversation
Adds HTTP semantic convention attributes to remote service spans by wrapping `fetchClient.executeHttpRequest`. Alternative considered: Using `@opentelemetry/instrumentation-undici` response hooks to propagate attributes from HTTP child spans. This approach was rejected as it requires an additional dependency and creates duplicate spans. --------- Co-authored-by: D050513 <sebastian.van.syckel@sap.com> Co-authored-by: sjvans <30337871+sjvans@users.noreply.github.com>
- [x] traces - [x] metrics - [x] logs - [x] #442 --------- Co-authored-by: I548646 <paul.erlenwein@sap.com> Co-authored-by: Paul <paul.erlenwein@gmail.com>
) # Improve `VCAP_SERVICES` Service Binding Resolution with Greedy Matchers ### Chore 🔧 Updated service instance matching for SAP Cloud Logging and Dynatrace to use greedy (array-based) VCAP matchers, extending the default matching without breaking existing behavior. Error messages and documentation have been updated to guide users toward the correct configuration. ### Changes * `package.json`: Updated `vcap` matching for `telemetry-to-dynatrace` and `telemetry-to-cloud-logging` from a single object to an array of matchers. Dynatrace now matches by `label: "dynatrace"` or `tag: "dynatrace"`; Cloud Logging matches by `label: "cloud-logging"` or `tag: "Cloud Logging"`. * `lib/utils.js`: Improved inline comments to clearly document the legacy compatibility behavior for `getCredsForDTAsUPS()` and `getCredsForCLSAsUPS()`. Updated the warning message for Cloud Logging user-provided services to recommend only the `"Cloud Logging"` tag (dropped `"cloud-logging"` from the recommendation). * `lib/logging/index.js`, `lib/metrics/index.js`, `lib/tracing/index.js`: Improved error messages when credentials are not found — now explicitly instruct users to ensure the bound service instance uses the correct tag (`"Cloud Logging"` or `"dynatrace"`). * `README.md`: Clarified that user-provided service instances for SAP Cloud Logging must use the tag `"Cloud Logging"` (removed reference to the legacy `cloud-logging` tag). Added a tip with the `cf update-user-provided-service` command and a link to CAP's service binding documentation. - [ ] 🔄 Regenerate and Update Summary <details> <summary>PR Bot Information</summary> **Version:** `1.26.5` - Correlation ID: `81bbd7c7-2290-4812-89e9-d178ac318c25` - Event Trigger: `issue_comment.edited` </details> --------- Co-authored-by: sjvans <30337871+sjvans@users.noreply.github.com>
# Fix: Workaround for `cds.cli.command` Bug with `build` Command ### Bug Fix 🐛 Added a workaround to prevent the telemetry plugin from loading when the `cds build` command is invoked. Due to a bug in `cds.cli.command`, the `build` command incorrectly resolves to an empty string (`''`), causing the plugin initialization check to pass unintentionally. ### Changes * `cds-plugin.js`: Added an early return guard that checks if `build` is present in `process.argv`. This ensures the plugin exits before proceeding with initialization when the `cds build` command is used, mirroring the existing workaround for `cds add`. - [ ] 🔄 Regenerate and Update Summary <details> <summary>PR Bot Information</summary> **Version:** `1.26.5` - Correlation ID: `4743ca7e-193a-4675-a529-8872b09ede9e` - Summary Prompt: [Default Prompt](https://github.tools.sap/intelligent-insights/i2-pull-request/blob/main/src/services/llm/prompts/summary_instructions_prompt.md) - LLM: `anthropic--claude-4.6-sonnet` - Output Template: [Default Template](https://github.tools.sap/intelligent-insights/i2-pull-request/blob/main/src/services/llm/prompts/summary_default_output_template.md) - Event Trigger: `pull_request.ready_for_review` - File Content Strategy: Full file content </details> --------- Co-authored-by: hyperspace-insights[bot] <209611008+hyperspace-insights[bot]@users.noreply.github.com>
# Chore: Upgrade to `cds^10` — Migrate Test Config to `.cdsrc.json` and CDS Profiles ### Refactor ♻️ Migrates test configuration from inline `process.env` overrides to a dedicated `.cdsrc.json` file and CDS CLI profiles, aligning with the `cds^10` approach for environment/profile-based configuration. Also includes test timing optimizations and minor bug fixes. ### Changes * `.github/workflows/ci.yml`: Commented out the `cds@8` downgrade step; updated comment to reference `cds@9`. * `.github/workflows/release.yml`: Added a `REVISIT` comment to remove the `better-sqlite3` install workaround with `cds^10`. * `.gitignore`: Removed `test/bookshop/.cdsrc.json` from the ignore list so it can be tracked in version control. * `test/bookshop/.cdsrc.json`: New file defining CDS profiles (`logging`, `metrics`, `metrics-outbox`, `metrics-outbox-disabled`, `tracing-attributes`, `persistent-outbox`, `without-outbox`) consolidating telemetry, messaging, and metrics settings previously scattered across test files. * `test/bookshop/package.json`: Removed `_outbox` and profile-specific `[metrics-outbox]`/`[metrics-outbox-disabled]` blocks (now in `.cdsrc.json`); added `_scheduling: true`. * `test/logging.test.js`: Removed inline `process.env` overrides for telemetry/log format; switched to `cds.test(..., '--profile', 'logging')` with a minimal remaining env override for `cls_custom_fields`. * `test/metrics.test.js`: Removed `exportIntervalMillis` env override; switched to `cds.test(..., '--profile', 'metrics')`. * `test/metrics-outbox.test.js` / `test/metrics-outbox-multitenant.test.js`: Reduced metric collection wait times from `300ms` to `150ms`; fixed retry loop bug where both loops checked `E1` instead of `E1`/`E2` respectively. * `test/tracing-attributes.test.js`: Removed inline exporter env var; switched to `cds.test(..., '--profile', 'tracing-attributes')`. * `test/tracing-messaging-persistent-outbox.test.js`: Renamed `CASE` to `persistent-outbox`; simplified env setup; marked `describe` block as `skip` pending vitest migration. * `test/tracing-messaging-without-outbox.test.js`: Renamed `CASE` to `without-outbox`; simplified env setup using `outboxed: false`. * `test/tracing-messaging-with-in-memory-outbox.test.js`: Removed this test file entirely. * `test/tracing-messaging.js`: Switched from `cds.test().in(...)` to `cds.test(..., '--profile', CASE)`. * `test/tracing.test.js`: Switched from `cds.test().in(...)` to `cds.test(...)`. - [ ] 🔄 Regenerate and Update Summary <details> <summary>PR Bot Information</summary> **Version:** `1.26.11` - Summary Prompt: [Default Prompt](https://github.tools.sap/intelligent-insights/i2-pull-request/blob/main/src/services/llm/prompts/summary_instructions_prompt.md) - Correlation ID: `14c5ae4d-4c97-4eff-a004-330c9fdcf2fe` - Event Trigger: `pull_request.edited` - File Content Strategy: Full file content - Output Template: [Default Template](https://github.tools.sap/intelligent-insights/i2-pull-request/blob/main/src/services/llm/prompts/summary_default_output_template.md) - LLM: `anthropic--claude-4.6-sonnet` </details>
![hyperspace-pr-bot[bot]](https://avatars.githubusercontent.com/u/2531208?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
sjvans
commented
Jul 1, 2026
sjvans
commented
Jul 1, 2026
Co-authored-by: sjvans <30337871+sjvans@users.noreply.github.com>
Contributor
Author
|
Folding this into #478 (replace console spying with in-memory exporters). Your |
This was referenced Aug 11, 2026
Contributor
Author
sjvans
added a commit
that referenced
this pull request
Aug 11, 2026
… 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.
Contributor
Author
|
Folded into #479 (merged) — its |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Improve Stability of Outbox Metrics Tests with State-Based Polling
Test
♻️ Refactor: Replaced all fixed-duration
wait(150)/wait(600)sleeps in outbox metrics tests with a deterministic, state-basedexpectEventually()helper that polls viaforceFlush(), eliminating race conditions in slower CI environments.The
expectEventually(assertion)function forces the OpenTelemetry metric pipeline to flush (metrics.getMeterProvider().forceFlush?.()) and then re-evaluates the assertion, retrying at short intervals until it passes or a timeout is reached. This ensures assertions are evaluated only after metrics are actually available, rather than relying on arbitrary sleep durations.Changes
test/metrics-outbox.test.js: AddedexpectEventually()helper andATTEMPTS_TO_FAIL = 3named constant; replaced allawait wait(...)+ bare assertion blocks withawait expectEventually(() => { ... })calls. Comments clarify the rationale for the one remaining wall-clock wait (storage time gauges requiring ≥1 second elapsed).test/metrics-outbox-multitenant.test.js: Same treatment for multi-tenant scenarios — introducedexpectEventually(), replaced polling loops and fixed sleeps, removed unuseddidProcessstate tracking, and updated the retry count threshold to useATTEMPTS_TO_FAIL.test/metrics-outbox-disabled.test.js: Replacedawait wait(150)withawait metrics.getMeterProvider().forceFlush?.()to deterministically drain the metric pipeline before asserting no metrics were emitted. Also updated the comment describing theconsole.dirmock.PR Bot Information
Version:
1.26.117f9204b3-fd3f-42c5-a682-592d8b722f47pull_request.editedanthropic--claude-4.6-sonnet