Conversation
Prepare `develop` for the next minor release. - bump version `2.0.1` → `2.1.0` - add `## Version 2.1.0 - tbd` changelog skeleton (Added / Changed / Fixed) Feature PRs merging into `develop` add their bullet under this section; the date is set when `develop` → `main` is promoted.
Add `develop` to the CI workflow's `push` and `pull_request` branch triggers so PRs into `develop` (the new integration branch) get lint + the test matrix. `main` triggers unchanged. HANA remains `workflow_dispatch`-only.
Contributor
SummaryThe following content is AI-generated and provides a summary of the pull request: Release:
|
![hyperspace-pr-bot[bot]](https://avatars.githubusercontent.com/u/2531208?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
This is a clean, minimal release-preparation PR. The only thing to address before merging to main is trimming the empty changelog sub-sections (### Added, ### Changed, ### Fixed) that have no entries — leaving them in would result in bare headings in the published changelog.
PR Bot Information
Version: 1.29.18
- LLM:
anthropic--claude-4.6-sonnet - Correlation ID:
fd848e30-9239-11f1-802e-ae2522cdec59 - Event Trigger:
pull_request.opened - File Content Strategy: Full file content
Comment on lines
+9
to
+13
| ### Added | ||
|
|
||
| ### Changed | ||
|
|
||
| ### Fixed |
Contributor
There was a problem hiding this comment.
Best Practices: Empty changelog sub-sections will be published as-is if merged without content.
The ### Added, ### Changed, and ### Fixed headers under ## Version 2.1.0 are currently empty placeholders. Before merging into main, any sub-section that has no entries should be removed so the published changelog doesn't contain bare headings with nothing beneath them. Consider removing the unused sub-sections as part of the final merge preparation.
Suggested change
| ### Added | |
| ### Changed | |
| ### Fixed | |
| ### Changed |
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
## Problem
Cloud SDK v4 exposes `executeHttpRequest` /
`executeHttpRequestWithOrigin` as **getter-only properties**. The plugin
patched them with plain assignment, which silently fails on a getter —
so the Cloud SDK outbound path (CAP's default when
`@sap-cloud-sdk/http-client` is installed) produced **no CLIENT span**
and no `sap.btp.destination`.
## Fix
Patch via `Object.defineProperty(cloudSDK, name, { value, writable:
true, configurable: true })` — `writable`/`configurable` keep the
exports re-patchable. Verified the wrapper now fires end-to-end.
## Tests
- `test/tracing-remote-cloudsdk.test.js` — Cloud SDK path: asserts a
`@cap-js/telemetry` CLIENT span with `sap.btp.destination`, and no
undici span.
- `test/tracing-remote-native.test.js` — native-fetch path: asserts the
span comes from `@opentelemetry/instrumentation-undici` (not `-http`)
with `http.*`/`url.*`/`server.*` attributes.
Both drive a real local HTTP call; gated on `cds.version >= 9`.
Changelog updated.
---------
Co-authored-by: sjvans <sjvans@users.noreply.github.com>
Add `target-branch: 'develop'` to both dependabot update entries (npm + github-actions) so dependency-bump PRs open against `develop` instead of the default branch (`main`). They then promote to `main` via the reviewed `develop → main` PR, like every other change.
…ucture (#465) ## 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> - tx` spans under the `cds.spawn - run task` root, instead of each top-level CAP call becoming an orphan root. Guarded so `$batch` sub-requests (active `EventContext`) 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`) and `groupedByTrace()` / `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/cds` uses a `setTimeout` bypass, not `cds.spawn`) — verified on HANA in CI. Follow-up #467 removes the skips once the cds queue-spawn fix ships. Changelog updated. Targets `develop`.
Supersedes the #437 stash. Migrates the test runner jest → vitest. ## Why it's a clean win Vitest with `pool: 'forks'` + per-file isolation tears each test file's child process down when the file finishes — so the OTLP exporter's lingering handles die with the child, and **the suite exits cleanly with no `--forceExit`**. This is the same open-handle class that hangs jest (see #472/#466). Verified: exit 0, ~8s, no hang across repeated runs. ## Config (`vitest.config.mjs`) - `globals: true` — `describe/test/beforeEach/...` stay available, test bodies unchanged. - `pool: 'forks'`, `isolate: true`, `teardownTimeout: 1000` — the clean-exit mechanism (documented inline). - Ports the old `jest.config.js` HANA logic faithfully: default `testTimeout: 42000`; under `CI && HANA_DRIVER` → `include` restricted to `tracing-attributes` + `passport`, timeout ×10, `cds_requires_telemetry_tracing` set when `HANA_PROM`. Verified the subset selection. ## Changes - `package.json`: `test` → `vitest run --silent`; jest removed, vitest added. `jest.config.js` deleted. - 8 `jest.spyOn`/`jest.fn` → `vi.spyOn`/`vi.fn`. - 5 `beforeAll(done => …)` hooks → promise-returning (vitest treats a hook arg as a fixture). - `eslint.config.mjs`: test-files override declaring `vi` global. Lint clean. - Lockfile regenerated against public npm (0 internal-registry URLs). ##⚠️ Behavioral note — HTTP instrumentation disabled in the test app Under jest, OTel's `require-in-the-middle` http patching was **silently broken** by jest's module sandbox, so incoming HTTP SERVER spans never existed in tests (the existing `xtest` skips document this). Under vitest (real `require`) the instrumentation works and reparents trace trees, breaking several assertions. To keep this migration **behavior-neutral**, `test/bookshop/package.json` now sets `disableIncomingRequestInstrumentation` + `disableOutgoingRequestInstrumentation` on the http instrumentation — reproducing jest's effective environment. Consequence: the HTTP-instrumentation path stays untested (same blind spot as jest, now explicit config rather than an accident). **Follow-up issue filed to enable it and assert on the real incoming spans.** The tracing-attributes client-span assertions still pass because those spans come via undici / cloud-sdk, not instrumentation-http. ## Coordination Parallel PR #473 (prettier) touches `package.json` (additive) + lockfile. Lockfile will conflict — whichever merges second rebases. #473 excludes `jest.config.js` from formatting (this PR deletes it). ## Verified `npm run test` 53 pass / 14 skip, exit 0, ~8s, clean exit ×3 · `npm run lint` clean · HANA subset selection confirmed.
Supersedes the #438 oxfmt spike — adopts **oxfmt** (`0.63.0`, pinned) properly. ## Config (`.oxfmtrc.jsonc`) Based on the #438 spike, verified against the `lib/*.js` house style: `singleQuote`, `semi: false`, `printWidth: 120`, `tabWidth: 2`, `trailingComma: none`, `arrowParens: avoid`. `ignorePatterns` excludes `*.md`, `node_modules`, `package-lock.json`, `CHANGELOG.md`, and `jest.config.js` (see coordination). ## Scripts - `format` → `npx oxfmt` (write is oxfmt's default) - `format:check` → `npx oxfmt --check` No git hook / husky / lint-staged — CI `format:check` + scripts only (the intrusive hook from the #438 spike is intentionally not carried over). ## Commits (reviewable split) 1. `chore: add oxfmt formatter tooling` — config + scripts + devDep + lockfile + CI step 2. `chore: apply oxfmt formatting` — repo-wide reformat (11 files, line-wrapping only; verified non-semantic via `git diff -w`) ## eslint coexistence `@sap/cds/eslint.config.mjs` is `recommended` + `no-unused-vars`/`no-console` only (no stylistic rules) → no conflict. `npm run lint` stays green. ## CI One line added to the `lint` job in `ci.yml`: `npm run format:check`. ## Verified `npm run format:check` ✅ (56 files) · `npm run lint` (--max-warnings=0) ✅ · `npm run test` → 53 pass / 14 skip, exit 0 · lockfile resolved from public npm (0 internal-registry URLs). ## Coordination Parallel PR #474 (jest→vitest) deletes `jest.config.js` — this PR excludes it from formatting (0-line diff confirmed) so no collision. `package.json` change here is additive (scripts + devDep); lockfile will conflict with #474 — whichever merges second rebases.
… 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.
…-logs, #482) (#483) ## What Recreates dependabot #472 against `develop`, bumping the OTLP dev-dependency group to 0.221: - `@opentelemetry/exporter-metrics-otlp-grpc`: `^0.219` → `^0.221` - `@opentelemetry/exporter-metrics-otlp-proto`: `^0.219` → `^0.221` - `@opentelemetry/exporter-trace-otlp-grpc`: `^0.219` → `^0.221` - `@opentelemetry/exporter-trace-otlp-proto`: `^0.219` → `^0.221` - `@opentelemetry/instrumentation-host-metrics`: `^0.2.0` → `^0.4.0` - `@opentelemetry/instrumentation-runtime-node`: `^0.32.0` → `^0.34.0` ## The pin Adds a top-level `overrides` block pinning `@opentelemetry/sdk-logs` to `0.219.0`: ```json "overrides": { "@opentelemetry/sdk-logs": "0.219.0" } ``` `@opentelemetry/sdk-logs` 0.221 introduces an unbounded memory leak that OOM-crashes `test/logging.test.js` (heap climbs to ~3.8GB, crash after ~110s). Root-caused and tracked in #482. The pin keeps sdk-logs at 0.219 while everything else moves to 0.221, so `logging.test.js` behaves like `develop` again (passes in ~5s). Remove the pin once #482 is fixed. ## Verification - `test/logging.test.js`: passes in ~4.9s, exits promptly (no OOM/hang) - Full suite: 63 passed / 14 skipped, exits cleanly in ~7.4s - Lint (`eslint . --max-warnings=0`) + format (`oxfmt --check`): clean - Lock resolves sdk-logs to `0.219.0` while `exporter-trace-otlp-proto` is `0.221.0` — pin is surgical Supersedes #472. No CHANGELOG entry (dev-deps only). Refs #482.
…, §5) (#481) ## What Started as removing dead test exclusions for #477 (§2 cds<9 guards, §5 HANA CI test-subset). Removing the HANA subset surfaced HANA-only failures — including **two real production bugs** that the old 2-file subset had been masking — so this PR also fixes those. ### Production fixes (lib/) - **fix(tracing):** raw SQL leaked into HANA `INSERT` `prepare` span names. The name-normalization regex used `.` which doesn't match newlines, so HANA's multi-line `INSERT … WITH SRC AS (…)` SQL survived in the span name. Now uses `[\s\S]` so it's stripped to operation + table (matching SELECT); the SQL stays in `db.query.text`. - **fix(metrics):** `*_storage_time_in_seconds` gauges were skewed by the machine's UTC offset on HANA — HANA's `min()`/`max()` aggregates return timezone-naive timestamps that `new Date()` parsed as local time. Normalize to UTC before parsing. Both carry CHANGELOG `### Fixed` entries. ### Test-suite changes (§2/§5 + HANA robustness) - Remove all 8 dead `cds < 9` guards (§2) and the HANA CI 2-file test-subset (§5) so the full suite runs on HANA. - Convert queue/outbox span assertions to force-flush + poll (spans export after fixed waits on slower HANA); filter the outbox-scan trace primer out of the logging assertion; fix a lifecycle bug where a retry handler fired with an undefined counter. - **HANA CI config** (`vitest.config.mjs`, HANA-only): run files serially (all files share one HDI container vs sqlite's per-file in-memory DB), raise `hookTimeout`, HANA-only outbox settle in `afterAll`, and `retry: 2` for residual shared-remote-container timing variance. sqlite unchanged (retry:0, full parallelism). ### Skips (deviation from "no skips except passport" — conscious) - Multitenancy tests (`tracing-mt`, `metrics-outbox-multitenant`) **skip on HANA** with an explanatory comment: they need a bound BTP Service Manager for MTX tenant subscription, which the single pre-provisioned HDI container in CI doesn't provide. They still run fully on sqlite. This means multitenancy has no HANA coverage — acknowledged; can be revisited if the CI HANA setup gains MTX. ## Verification - sqlite: 63 passed / 14 skipped / 0 failed. - HANA (serial + retry:2): 64 passed / 6 skipped / 0 failed, stable across repeated runs (the 6 skips = 2 multitenancy + 4 pre-existing xtest/TODO in tracing.test.js). Refs #477 (§1 queue-worker sqlite skips remain, gated on the cds queue-spawn fix; §3 stubs pending).
# Chore: Clean Up Release Workflow ♻️ **Refactor**: Removed an outdated workaround comment from the release workflow. ### Changes * `.github/workflows/release.yml`: Removed the `# REVISIT: remove "npm explore better-sqlite3 -- npm run install" with cds^10` comment that was no longer relevant, cleaning up the release workflow configuration. - [ ] 🔄 Regenerate and Update Summary <details> <summary>PR Bot Information</summary> **Version:** `1.29.26` - Output Template: [Default Template](https://github.tools.sap/Code-Change-Intelligence/pr-bot/blob/main/src/services/llm/prompts/summary_default_output_template.md) - Summary Prompt: [Default Prompt](https://github.tools.sap/Code-Change-Intelligence/pr-bot/blob/main/src/services/llm/prompts/summary_instructions_prompt.md) - File Content Strategy: Full file content - Correlation ID: `8ac5e9d0-97bf-11f1-9e0b-c00ff05b9b1b` - LLM: `anthropic--claude-4.6-sonnet` - Event Trigger: `pull_request.opened` </details>
…unpin sdk-logs (#482) (#489) ## Problem Bumping the OTLP dev-deps to the `0.221` line pulled `@opentelemetry/sdk-logs` 0.219 → 0.221, which made `test/logging.test.js` OOM-crash (heap climbed to ~3.8 GB, ~110 s of GC thrashing before dying). #483 shipped an interim workaround pinning sdk-logs to `0.219.0` via a top-level `overrides` block. This PR removes that pin and fixes the actual defect. Closes #482. ## Root cause Two things combine: 1. **Constructor signature change (the real trigger).** In sdk-logs 0.221 the `SimpleLogRecordProcessor` and `BatchLogRecordProcessor` constructors changed from positional `(exporter)` to an options object `({ exporter })`. `lib/logging/index.js` still passed the exporter positionally (both the built-in path and the custom-processor path in `_getCustomProcessor`; the test's `MySimpleLogRecordProcessor` extends the SDK base and forwards its args). So `options.exporter` was `undefined`, and every emit called `core.internal._export(undefined, …)`, which throws. 2. **Diag re-entrancy loop.** The thrown export error hits `.catch(globalErrorHandler)` → `diag.error(...)`. Because `lib/index.js` wires `diag.setLogger(cds.log('telemetry'), …)`, that diagnostic goes back through `cds.log('telemetry')` → the overridden `cds.log.format` → `logger.emit()` → export throws again → … an unbounded loop. Each hop is a **microtask** (`processTicksAndRejections`), so the recursion is asynchronous. ## Fix - Construct the log processors with the `{ exporter }` options object 0.221 expects (built-in + custom-processor paths). This stops the export from throwing, which removes the source of the diag error loop — the OOM is gone. - Add a re-entrancy guard (`let emitting`) around `logger.emit()` in the `cds.log.format` interception: while inside our own `emit()` we still run the original format work (log output unchanged) but skip re-emitting. This is defense-in-depth against any **synchronous** re-entry through the export/diag path; `try/finally` guarantees the flag resets even if `emit()` throws. ## Unpin - Removed the `@opentelemetry/sdk-logs` `overrides` pin; regenerated the lockfile. sdk-logs now floats to `0.221.0`, matching the other OTLP deps. This removes the #483 workaround pin. ## Verification - `vitest run test/logging.test.js` — passes in ~1.3 s (was ~3.8 GB / ~110 s OOM crash on 0.221). The `logging.test.js` OOM was the repro; ran it repeatedly, stable. - Full sqlite suite: 63 passed / 14 skipped, no regressions. - eslint `--max-warnings=0` clean; `oxfmt --check` clean. - Lockfile: no internal-registry URLs; `npm ci` reproduces cleanly.
…er; drop tracing-attributes profile (#478) (#490) ## What Group 1 of #478. Converts the three tracing tests that still spied on `console.dir` to read structured `ReadableSpan` objects directly from the in-memory span exporter: - `test/tracing-remote-cloudsdk.test.js` - `test/tracing-remote-native.test.js` - `test/tracing-span-names.test.js` Each now uses `--profile tracing-in-memory` (which wires `MyInMemorySpanExporter` via `.cdsrc.json`) and reads over `captured` (the module-level array of `ReadableSpan`s) instead of the `console.dir` spy. `beforeEach(reset)` clears the buffer per test. All existing assertions are preserved verbatim — `instrumentationScope.name` filters (`@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). `captured` holds full ReadableSpans with `instrumentationScope`, so the filters just point at `captured`. No flush/poll was needed: the spans for these single request/DB ops appear synchronously in `captured` after the awaited call. In `tracing-span-names.test.js`, `data.reset()` is itself traced, so the buffer is cleared *after* reset (mirroring `tracing-attributes.test.js`). ## Why No more `console.dir` spying in the tracing tests. With all three files migrated, the `[tracing-attributes]` profile in `test/bookshop/.cdsrc.json` has no remaining consumers (verified: only these 3 used it; `tracing-attributes.test.js` despite its name already uses `tracing-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).
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
1 participant
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.
Release:
v2.1.0Standing release PR — accumulates everything merged into
developfor the next minor. Do not squash-merge; review and merge with a team member to satisfy themainreview requirement. Description kept current as PRs land ondevelop.Changes so far
package.json/CHANGELOG.md: version →2.1.0;## Version 2.1.0changelog section.develop(ci: run on pull requests to develop #469); dependabot targetsdevelop(ci: target develop for dependabot updates #470).<service> - txspans under thecds.spawn - run taskroot (feat: trace queue worker transactions + structured span test infrastructure #465), plus structured in-memory span test infrastructure (MyInMemorySpanExporter) replacing console-log-regex assertions.@sap-cloud-sdk/http-clientexports patched viaObject.defineProperty; added to devDeps so the path runs in CI; cloud-sdk + native-fetch tracing tests (fix: trace Cloud SDK outbound requests (getter-only exports) #451).--forceExitvia forked-pool teardown (chore: migrate test runner from jest to vitest #474).oxfmtcode formatter +format/format:checkscripts + CI check (chore: adopt oxfmt for code formatting #476).MyInMemoryMetricReader(DELTA temporality) instead ofconsole.dirspying; added aConsoleMetricExporterunit test; state-basedexpectEventuallypolling replaces fixed sleeps (test: capture outbox+console metrics via in-memory reader & unit-test ConsoleMetricExporter #479).0.221, with@opentelemetry/sdk-logspinned to0.219viaoverrides(0.221 leaks memory via the log-format re-entrancy — see Memory leak: sdk-logs 0.221 re-enters cds.log.format interception → logging.test.js OOM #482) (chore(deps-dev): bump the OTLP dev-dependency group to 0.221 (pin sdk-logs, #482) #483).preparespan names, and*_storage_time_in_secondsgauges skewed by the machine's UTC offset (test: run full suite on HANA + fix two HANA span/metric bugs (#477 §2, §5) #481).@opentelemetry/sdk-logs0.221's export path — log processors are constructed with the arg shape the installed sdk-logs version expects (version-adaptive), plus a re-entrancy guard; the temporary sdk-logs 0.219 pin (chore(deps-dev): bump the OTLP dev-dependency group to 0.221 (pin sdk-logs, #482) #483) is removed (fix(logging): guard cds.log.format interception against re-entrancy; unpin sdk-logs (#482) #489, closes Memory leak: sdk-logs 0.221 re-enters cds.log.format interception → logging.test.js OOM #482).console.dirspan-scraping in tracing tests — remote/span-name tests now read the in-memory span exporter; dropped the unusedtracing-attributesprofile (test: convert remote/span-name tracing tests to in-memory span exporter; drop tracing-attributes profile (#478) #490, Replace console spying in tests with in-memory exporters #478 group 1).Follow-ups tracked (not blocking this release)
compiled of: