Skip to content
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

feat: wrap client if @dynatrace/oneagent-sdk is present #777

Merged
merged 4 commits into from
Sep 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 91 additions & 0 deletions hana/lib/drivers/dynatrace.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
// COPIED AS IS (excluding unused code) FROM @sap/cds
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a long term or temporary solution ? As native support from @sap/hana-client was requested.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sebastian mentioned that Daniel agreed to implement that on our side as observability colleagues claimed the feature degradation to the legacy hana service.

I'm afraid that this will stay on us for a longer time.


const dynatrace = {}
try {
dynatrace.sdk = require('@dynatrace/oneagent-sdk')
dynatrace.api = dynatrace.sdk.createInstance()
} catch {
// If module was not required, do not do anything
}

const isDynatraceEnabled = () => {
return dynatrace.sdk !== undefined && !process.env.CDS_SKIP_DYNATRACE
}

const _dynatraceResultCallback = function (tracer, cb) {
return function (err, ...args) {
const results = args.shift()
if (err) {
tracer.error(err)
} else {
tracer.setResultData({
rowsReturned: (results && results.length) || results
})
}
tracer.end(cb, err, results, ...args)
}
}

const _execUsingDynatrace = (client, execFn, dbInfo) => {
// args = [sql, options, callback] --> options is optional
return function (...args) {
const cb = args[args.length - 1]

const tracer = dynatrace.api.traceSQLDatabaseRequest(dbInfo, {
statement: args[0]
})

tracer.startWithContext(execFn, client, ...args.slice(0, args.length - 1), _dynatraceResultCallback(tracer, cb))
}
}

const _preparedStmtUsingDynatrace = function (client, prepareFn, dbInfo) {
// args = [sql, options, callback] --> options is optional
return function (...args) {
const cb = args[args.length - 1]

const tracer = dynatrace.api.traceSQLDatabaseRequest(dbInfo, {
statement: args[0]
})

tracer.startWithContext(prepareFn, client, ...args.slice(0, args.length - 1), (err, stmt) => {
if (err) {
tracer.error(err)
tracer.end(cb, err)
} else {
// same here. hana-client does not like decorating
const originalExecFn = stmt.exec
stmt.exec = function (...args) {
const stmtCb = args[args.length - 1]
originalExecFn.call(stmt, ...args.slice(0, args.length - 1), _dynatraceResultCallback(tracer, stmtCb))
}
cb(null, stmt)
}
})
}
}

const dynatraceClient = (client, credentials, tenant) => {
const dbInfo = {
name: `SAPHANA${tenant ? `-${tenant}` : ''}`,
vendor: dynatrace.sdk.DatabaseVendor.HANADB,
host: credentials.host,
port: Number(credentials.port)
}

// hana-client does not like decorating.
// because of that, we need to override the fn and pass the original fn for execution
const originalExecFn = client.exec
client.exec = _execUsingDynatrace(client, originalExecFn, dbInfo)
const originalPrepareFn = client.prepare
if (client.name === '@sap/hana-client') {
// client.prepare = ... doesn't work for hana-client
Object.defineProperty(client, 'prepare', { value: _preparedStmtUsingDynatrace(client, originalPrepareFn, dbInfo) })
} else {
client.prepare = _preparedStmtUsingDynatrace(client, originalPrepareFn, dbInfo)
}

return client
}

module.exports = { dynatraceClient, isDynatraceEnabled }
2 changes: 2 additions & 0 deletions hana/lib/drivers/hana-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const { Readable, Stream } = require('stream')
const cds = require('@sap/cds')
const hdb = require('@sap/hana-client')
const { driver, prom, handleLevel } = require('./base')
const { isDynatraceEnabled: dt_sdk_is_present, dynatraceClient: wrap_client } = require('./dynatrace')
const LOG = cds.log('@sap/hana-client')
if (process.env.NODE_ENV === 'production' && !process.env.HDB_NODEJS_THREADPOOL_SIZE && !process.env.UV_THREADPOOL_SIZE) LOG.warn("When using @sap/hana-client, it's strongly recommended to adjust its thread pool size with environment variable `HDB_NODEJS_THREADPOOL_SIZE`, otherwise it might lead to performance issues.\nLearn more: https://help.sap.com/docs/SAP_HANA_CLIENT/f1b440ded6144a54ada97ff95dac7adf/31a8c93a574b4f8fb6a8366d2c758f21.html")

Expand Down Expand Up @@ -43,6 +44,7 @@ class HANAClientDriver extends driver {

super(creds)
this._native = hdb.createConnection(creds)
if (dt_sdk_is_present()) this._native = wrap_client(this._native, creds, creds.tenant)
this._native.setAutoCommit(false)
}

Expand Down
2 changes: 2 additions & 0 deletions hana/lib/drivers/hdb.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const hdb = require('hdb')
const iconv = require('iconv-lite')

const { driver, prom, handleLevel } = require('./base')
const { isDynatraceEnabled: dt_sdk_is_present, dynatraceClient: wrap_client } = require('./dynatrace')

const credentialMappings = [
{ old: 'certificate', new: 'ca' },
Expand Down Expand Up @@ -33,6 +34,7 @@ class HDBDriver extends driver {

super(creds)
this._native = hdb.createClient(creds)
if (dt_sdk_is_present()) this._native = wrap_client(this._native, creds, creds.tenant)
this._native.setAutoCommit(false)
this._native.on('close', () => this.destroy?.())

Expand Down