cap-js / telemetry

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

Option to disable database tracing while PostgreSQL is not supported #182

Closed xLexip closed 3 months ago

xLexip commented 5 months ago

Hey there,

it would be nice to have an option to disable database tracing in the config while PostgreSQL databases are not supported (#21). That way it would be possible to take advantage of the rest of the logging. Currently, using it with a postgre db crash the backend / db stuff.

vkozyura commented 4 months ago

@xLexip: Hi, on my side there is no backend crash. Anything special I should take into account?

[telemetry] - elapsed times:
    0.00 →  13.76 =  13.76 ms  GET /odata/v4/cat/ABC
    3.78 →  11.27 =   7.49 ms    cat - READ cat.ABC
    4.59 →   4.75 =   0.16 ms      @cap-js/postgres - prepare SELECT set_config(key::text,$1->>key,false) FROM json…
    4.84 →   6.85 =   2.00 ms      @cap-js/postgres - stmt.run SELECT set_config(key::text,$1->>key,false) FROM jso…
    5.27 →   5.41 =   0.13 ms      @cap-js/postgres - prepare SELECT collname FROM pg_collation WHERE collname = 'e…
    5.50 →   8.08 =   2.58 ms      @cap-js/postgres - stmt.all SELECT collname FROM pg_collation WHERE collname = '…
    9.20 →  11.24 =   2.03 ms      db - READ cat.ABC
    9.85 →   9.92 =   0.07 ms        @cap-js/postgres - prepare SELECT to_jsonb(ABC.*) as _json_ FROM (SELECT ABC.ID …
   10.09 →  11.18 =   1.09 ms        @cap-js/postgres - stmt.all SELECT to_jsonb(ABC.*) as _json_ FROM (SELECT ABC.ID…
[telemetry] - db.pool:
     size | available | pending
      1/1 |       1/1 |       0
vkozyura commented 4 months ago

closed as not reproducible

xLexip commented 4 months ago

Hey @vkozyura,

here is a stack trace with @cap-js/telemetry v0.2.3 and @cap-js/postgres v1.9.1: image image Do you have any idea what the problem could be here? It's no problem if the database tracing does not work as postgresql is not yet supported. But it would be nice to have an option to disable database tracing that we can use the rest of the telemetry data.

vkozyura commented 4 months ago

@xLexip: Hi, also with your versions I cannot reproduce the issue. I obtain a regular telemetry output for postgres. Before I start disabling database tracíng I really would like to reproduce the problem. Please provide me the output of "cds -v" on your side. Also please provide a small example project and the steps how to reproduce the issue. Best regards

doppelkeks101 commented 4 months ago

Hi @vkozyura,

i work in the same project as @xLexip. We read database content inside the callback of the 'listening' event in a custom server.js file. Below is an example of the usage which results in the error mentions above. When run against an sqlite in-memory database, the code works as expected.

server.js:

const cds = require("@sap/cds");

async function main() {
    const data = await SELECT.from("example.project.Books");
    console.log(data);
}

cds.once("listening", () => {
    main();
});

module.exports = cds.server;

Output of cds -v: @cap-js/cds-types: 0.2.0 @cap-js/postgres: 1.9.1 @cap-js/telemetry: 0.2.3 @sap/cds: 7.9.3 @sap/cds-compiler: 4.6.2 @sap/cds-dk: 7.9.4 @sap/cds-dk (global): 7.9.4 @sap/cds-fiori: 1.2.4 @sap/cds-foss: 5.0.1 @sap/cds-mtxs: 1.18.1 @sap/eslint-plugin-cds: 3.0.4 Node.js: v20.15.1 example-project: 1.0.0 home: /home/user/Documents/example-project/node_modules/@sap/cds

Best regards

vkozyura commented 3 months ago

@xLexip: Hi, With help of your last comment I could reproduce the issue. There was a special config-SQL in PostgreSQL, which lead to the crash. We fixed the issue - the fix to be provided with the next release.