-
Notifications
You must be signed in to change notification settings - Fork 13
chore: cds^10 #443
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: cds^10 #443
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -135,6 +135,5 @@ dist | |
| # test | ||
| test/msg-box | ||
| test/bookshop/gen | ||
| test/bookshop/.cdsrc.json | ||
|
|
||
| .claude/ | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -30,11 +30,11 @@ | |||||
| "@opentelemetry/semantic-conventions": "^1.41" | ||||||
| }, | ||||||
| "peerDependencies": { | ||||||
| "@sap/cds": "^8 || ^9" | ||||||
| "@sap/cds": "^9" | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: The
Suggested change
Double-check suggestion before committing. Edit this comment for amendments. Please provide feedback on the review comment by checking the appropriate box:
|
||||||
| }, | ||||||
| "devDependencies": { | ||||||
| "@cap-js/cds-test": "^0", | ||||||
| "@cap-js/sqlite": "^1 || ^2", | ||||||
| "@cap-js/sqlite": "^2", | ||||||
| "@cap-js/telemetry": "file:.", | ||||||
| "@grpc/grpc-js": "^1.9.14", | ||||||
| "@opentelemetry/exporter-metrics-otlp-grpc": "^0.219", | ||||||
|
|
@@ -43,7 +43,7 @@ | |||||
| "@opentelemetry/exporter-trace-otlp-proto": "^0.219", | ||||||
| "@opentelemetry/instrumentation-host-metrics": "^0.2.0", | ||||||
| "@opentelemetry/instrumentation-runtime-node": "^0.32.0", | ||||||
| "@sap/cds-mtxs": "^2 || ^3", | ||||||
| "@sap/cds-mtxs": "^3", | ||||||
| "axios": "^1.6.7", | ||||||
| "chai": "^4.4.1", | ||||||
| "chai-as-promised": "^7.1.1", | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| { | ||
| "[logging]": { | ||
| "requires": { | ||
| "telemetry": { | ||
| "tracing": { | ||
| "sampler": { | ||
| "ignoreIncomingPaths": ["/odata/v4/admin/Genres"] | ||
| } | ||
| }, | ||
| "logging": { | ||
| "exporter": { | ||
| "module": "@opentelemetry/sdk-logs", | ||
| "class": "ConsoleLogRecordExporter" | ||
| }, | ||
| "processor": { | ||
| "module": "./lib/MySimpleLogRecordProcessor.js", | ||
| "class": "MySimpleLogRecordProcessor" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "log": { | ||
| "format": "json", | ||
| "cls_custom_fields": ["foo"] | ||
| } | ||
| }, | ||
| "[metrics]": { | ||
| "requires": { | ||
| "telemetry": { | ||
| "metrics": { | ||
| "config": { | ||
| "exportIntervalMillis": 100 | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "[metrics-outbox]": { | ||
| "requires": { | ||
| "queue": true, | ||
| "telemetry": { | ||
| "metrics": { | ||
| "config": { | ||
| "exportIntervalMillis": 100 | ||
| }, | ||
| "_db_pool": false, | ||
| "_queue": true, | ||
| "exporter": { | ||
| "module": "@opentelemetry/sdk-metrics", | ||
| "class": "ConsoleMetricExporter" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "[metrics-outbox-disabled]": { | ||
| "requires": { | ||
| "queue": true, | ||
| "telemetry": { | ||
| "metrics": { | ||
| "config": { | ||
| "exportIntervalMillis": 250 | ||
| }, | ||
| "_db_pool": false, | ||
| "_queue": false, | ||
| "exporter": { | ||
| "module": "@opentelemetry/sdk-metrics", | ||
| "class": "ConsoleMetricExporter" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "[tracing-attributes]": { | ||
| "requires": { | ||
| "telemetry": { | ||
| "tracing": { | ||
| "exporter": { | ||
| "module": "@opentelemetry/sdk-trace-node" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "[persistent-outbox]": { | ||
| "requires": { | ||
| "messaging": { | ||
| "kind": "file-based-messaging", | ||
| "file": "../persistent-outbox" | ||
| } | ||
| } | ||
| }, | ||
| "[without-outbox]": { | ||
| "requires": { | ||
| "messaging": { | ||
| "kind": "file-based-messaging", | ||
| "file": "../without-outbox", | ||
| "outboxed": false | ||
| } | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,22 +1,20 @@ | ||
| const CASE = 'with_persistent-outbox' | ||
| const CASE = 'persistent-outbox' | ||
|
|
||
| const env = { | ||
| // REVISIT: even with profile "persistent-outbox", messaging kind and file from package.json wins | ||
| process.env.cds_requires_messaging = JSON.stringify({ | ||
| kind: 'file-based-messaging', | ||
| outbox: true, | ||
| file: `../${CASE}` | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: The Please provide feedback on the review comment by checking the appropriate box:
|
||
| } | ||
| process.env.cds_requires_messaging = JSON.stringify(env) | ||
| process.env.cds_requires_outbox = JSON.stringify({ kind: 'persistent-outbox' }) | ||
|
|
||
| process.env.cds_requires_telemetry_metrics = null | ||
| }) | ||
|
|
||
| // REVISIT: check json exports | ||
| const CHECK = (log, expect) => { | ||
| // 3: outbox -> consumers get new root context | ||
| // REVISIT: for some reason, span "cds.spawn run task" has no parent when running in jest | ||
| expect(log.output.match(/\[telemetry\] - elapsed times:/g).length).to.equal(4) //> actually 3 | ||
| expect(log.output.match(/cds.spawn - schedule task/g).length).to.equal(1) | ||
| } | ||
|
|
||
| describe(`tracing messaging - ${CASE}`, () => { | ||
| // REVISIT: re-enable with switch to vitest | ||
| describe.skip(`tracing messaging - ${CASE}`, () => { | ||
| require('./tracing-messaging')(CASE, CHECK) | ||
| }) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: The commented-out step originally forced
@sap/cds@8(now corrected to@sap/cds@9in the comment), but the active matrix only testscds-version: [9]whilecds-version: [10]is also commented out. Thebetter-sqlite3rebuild step on line 40 still fires forcds-version -eq 9, so this is fine for now — but the commented line references a workaround that is no longer needed ifcds@9is already installed via line 37. The comment text says@sap/cds@9but this step was originally meant to downgrade when cds-dk installed a newer version; the comment is now misleading about its purpose.Please provide feedback on the review comment by checking the appropriate box: