search

cap-js / telemetry

CDS plugin providing observability features, incl. automatic OpenTelemetry instrumentation.
https://cap.cloud.sap/docs
Apache License 2.0
8 stars 6 forks source link

Handle corruption for cds log levels other than "info" #127

Closed corporateuser closed 6 months ago

corporateuser commented 6 months ago

Hello, currently when we're setting log level to anything other than "info" it corrupts handle method for every service existing: https://github.com/cap-js/telemetry/blob/337e92a9a95d6c2290ea661bc040b96087122f37/lib/tracing/cds.js#L76

What happens here, you're wrapping handle method for Service class in https://github.com/cap-js/telemetry/blob/main/lib/tracing/cds.js#L39, but then later cds itself wrap handle method for ApplicationService https://github.tools.sap/cap/cds/blob/main/libx/_runtime/fiori/lean-draft.js#L208 to handle lean draft, so by the moment you're trying to "unwrap" there is no __original property in handle anymore. So any service call would fail with:

[cds] - TypeError: this.handle is not a function
    at CatalogService.dispatch (/Users/dmitry/sap/telemetry/test/bookshop/node_modules/@sap/cds/lib/srv/srv-dispatch.js:45:15)
    at _readCollection (/Users/dmitry/sap/telemetry/test/bookshop/node_modules/@sap/cds/libx/_runtime/cds-services/adapter/odata-v4/handlers/read.js:250:28)
    at _readAndTransform (/Users/dmitry/sap/telemetry/test/bookshop/node_modules/@sap/cds/libx/_runtime/cds-services/adapter/odata-v4/handlers/read.js:456:12)
    at /Users/dmitry/sap/telemetry/test/bookshop/node_modules/@sap/cds/libx/_runtime/cds-services/adapter/odata-v4/handlers/read.js:544:22
    at /Users/dmitry/sap/telemetry/test/bookshop/node_modules/@sap/cds/libx/_runtime/cds-services/adapter/odata-v4/okra/odata-server/core/Dispatcher.js:135:7
    at new Promise (<anonymous>)
    at Dispatcher._handle (/Users/dmitry/sap/telemetry/test/bookshop/node_modules/@sap/cds/libx/_runtime/cds-services/adapter/odata-v4/okra/odata-server/core/Dispatcher.js:134:12)
    at Dispatcher.dispatch (/Users/dmitry/sap/telemetry/test/bookshop/node_modules/@sap/cds/libx/_runtime/cds-services/adapter/odata-v4/okra/odata-server/core/Dispatcher.js:91:21)
    at DispatcherCommand.execute (/Users/dmitry/sap/telemetry/test/bookshop/node_modules/@sap/cds/libx/_runtime/cds-services/adapter/odata-v4/okra/odata-server/invocation/DispatcherCommand.js:63:10)
    at CommandExecutor._execute (/Users/dmitry/sap/telemetry/test/bookshop/node_modules/@sap/cds/libx/_runtime/cds-services/adapter/odata-v4/okra/odata-server/invocation/CommandExecutor.js:71:17) {
  id: '1931710',
  level: 'ERROR',
  timestamp: 1710410306617
}

To test it just add:

    "log": {
      "levels": {
        "cds": "error"
      }
    },

to your https://github.com/cap-js/telemetry/blob/main/test/bookshop/package.json, run the server and try to access any service.

Not sure, that tracing should depend on "cds" log level at all, and also unwrapping should be done with keeping in mind, there could be other wrappers around.

sjvans commented 6 months ago

hi @corporateuser

thanks for reporting. you're absolutely right, it doesn't make much sense. basically leftover poc coding 😬.

best, sebastian