Skip to content

Init OTEL plugin - #1

Closed
schiwekM wants to merge 6 commits into
mainfrom
dev-init
Closed

Init OTEL plugin#1
schiwekM wants to merge 6 commits into
mainfrom
dev-init

Conversation

@schiwekM

Copy link
Copy Markdown
Contributor
  • Adds OTEL library
  • Test cases definitely not final (+ how to test that metrics + traces actually end up in Dynatrace / other collectors

Comment thread package.json
Comment thread package.json Outdated
Comment thread package.json
@@ -0,0 +1,67 @@
{

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rework based on audit-logging

Comment thread metrics/CDSConsoleMetricsExporter.js
Comment thread plugin/trace.js
@@ -0,0 +1,204 @@
const cds = require('@sap/cds')
const LOG = cds.log('trace')

@sjvans sjvans Oct 6, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we really need to use multiple module names? if no, it should be otel. if yes, something like otel - xxx...

Comment thread plugin/CDSConsoleExporter.js
@@ -0,0 +1,18 @@
const cds = require('@sap/cds')
const LOG = cds.log('trace')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

@@ -0,0 +1,359 @@
const cds = require('@sap/cds'),
xsenv = require('@sap/xsenv')
const LOG = cds.log('otel')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

Comment on lines +51 to +52
if (!cds.env.trace) cds.env.trace = {} //Ensures that later code does not break
if (!cds.env.metrics) cds.env.metrics = {} //Ensures that later code does not break

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there should be a common root. also, defaults should be provided in package.json, which makes these ensures unnecessary

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would disagree as cds.env.logs also exists and this way it is more consistent

tracerProvider: provider,
instrumentations: instrumentations
})
if (!cds.env.tracer) cds.env.tracer = trace.getTracer(cds.env.trace.name, cds.env.trace.version)

@sjvans sjvans Oct 6, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is cds.env.tracer? cds.env is for config

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cds.env.tracer is the tracer class from Otel used in trace.js for actual tracing as the reference has to be handed over. Moved it to cds.env.trace.tracer

Co-authored-by: sjvans <30337871+sjvans@users.noreply.github.com>
@schiwekM

Copy link
Copy Markdown
Contributor Author

Closed in favour for the other PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants