diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ce43e09..72acf229 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/). ### Fixed +- Readability of span names for DB and Cloud SDK traces - Peer dependency allows `@sap/cds^10` ## Version 2.0.0 - 2026-06-29 diff --git a/lib/tracing/cloud_sdk.js b/lib/tracing/cloud_sdk.js index 77a2c9b7..792472a4 100644 --- a/lib/tracing/cloud_sdk.js +++ b/lib/tracing/cloud_sdk.js @@ -1,8 +1,20 @@ const { SpanKind } = require('@opentelemetry/api') +const cds = require('@sap/cds') const trace = require('./trace') const wrap = require('./wrap') +const { _append_url_path } = cds.env.requires.telemetry.tracing +const APPEND_URL_PATH = _append_url_path && _append_url_path !== 'false' + +function _cloudSdkSpanName(destination, requestConfig) { + const method = requestConfig?.method || 'GET' + if (!APPEND_URL_PATH) return method + const url = destination.url || requestConfig?.url || '' + const path = url.replace(/^https?:\/\/[^/]+/, '').split('?')[0] + return `${method}${path ? ' ' + path : ''}` +} + // REVISIT: unverified! module.exports = () => { try { @@ -16,9 +28,7 @@ module.exports = () => { cloudSDK.executeHttpRequest = wrap(_execute, { wrapper: function executeHttpRequest(destination, requestConfig) { return trace( - `${destination?.name ? destination?.name + ' ' : ''}${requestConfig?.method || 'GET'} ${ - destination.url || requestConfig?.url - }`, + _cloudSdkSpanName(destination, requestConfig), _execute, this, arguments, @@ -29,9 +39,7 @@ module.exports = () => { cloudSDK.executeHttpRequestWithOrigin = wrap(_executeWithOrigin, { wrapper: function executeHttpRequestWithOrigin(destination, requestConfig) { return trace( - `${destination?.name ? destination?.name + ' ' : ''}${requestConfig?.method || 'GET'} ${ - destination.url || requestConfig?.url - }`, + _cloudSdkSpanName(destination, requestConfig), _executeWithOrigin, this, arguments, diff --git a/lib/tracing/trace.js b/lib/tracing/trace.js index f7914446..dc9d70db 100644 --- a/lib/tracing/trace.js +++ b/lib/tracing/trace.js @@ -41,9 +41,9 @@ const MASK_HEADERS = (cds.env.log.mask_headers || ['/authorization/i', '/cookie/ const CRUD = { CREATE: 1, READ: 1, UPDATE: 1, DELETE: 1 } -const { hrtime, adjust_root_name, _truncate_span_name } = cds.env.requires.telemetry.tracing +const { hrtime, _append_url_path, _truncate_span_name } = cds.env.requires.telemetry.tracing const HRTIME = hrtime && hrtime !== 'false' -const ADJUST_ROOT_NAME = adjust_root_name && adjust_root_name !== 'false' +const APPEND_URL_PATH = _append_url_path && _append_url_path !== 'false' const $hrnow = Symbol('@cap-js/telemetry:hrnow') const $adjusted = Symbol('@cap-js/telemetry:adjusted') @@ -283,7 +283,7 @@ function trace(req, fn, that, args, opts = {}) { _setAttributes(parent, _getRequestAttributes()) const ctx = cds.context if (ctx?.http?.req?.[$hrnow]) parent.startTime = ctx.http.req[$hrnow] - if (ADJUST_ROOT_NAME && parent.attributes[ATTR_URL_PATH]) parent.name += ' ' + parent.attributes[ATTR_URL_PATH] + if (APPEND_URL_PATH && parent.attributes[ATTR_URL_PATH]) parent.name += ' ' + parent.attributes[ATTR_URL_PATH] } let name = typeof req === 'string' ? req : _getSpanName(req, fn, that) @@ -308,9 +308,19 @@ function trace(req, fn, that, args, opts = {}) { LOG._warn && LOG.warn('Failed to determine attributes:', err) } - // REVISIT: with _hana_prom = true, the sql is not yet in the span name - if (name.match(/^@cap-js\/\w+ - \w+$/) && options.attributes[ATTR_DB_QUERY_TEXT]) { - name += ' ' + options.attributes[ATTR_DB_QUERY_TEXT] + // Normalize DB span names built in cds.js (prepare/exec/stmt.*) so raw SQL + // doesn't leak into the name; the SQL is preserved in db.query.text. + // Matches "@cap-js/ - " optionally followed by " ", + // where is prepare | exec | stmt.. Covers both the sqlite/pg case + // (SQL is already baked in) and the HANA-promisified case (SQL not yet appended). + const dbNameMatch = name.match(/^(@cap-js\/\w+ - (?:prepare|exec|stmt\.\w+))(?:\s.*)?$/) + if (dbNameMatch && (options.attributes[ATTR_DB_OPERATION_NAME] || options.attributes[ATTR_DB_SQL_TABLE])) { + const SQL_VERB = { READ: 'SELECT', CREATE: 'INSERT' } + const op = options.attributes[ATTR_DB_OPERATION_NAME] + const verb = SQL_VERB[op] ?? op + const table = options.attributes[ATTR_DB_SQL_TABLE] + const tail = [verb, table].filter(Boolean).join(' ') + if (tail) name = `${dbNameMatch[1]} ${tail}` } if (name.length > 80 && _truncate_span_name !== false) name = name.substring(0, 79) + '…' diff --git a/test/tracing-span-names.test.js b/test/tracing-span-names.test.js new file mode 100644 index 00000000..8cd1d7a8 --- /dev/null +++ b/test/tracing-span-names.test.js @@ -0,0 +1,93 @@ +const cds = require('@sap/cds') +const { expect, data } = cds.test(__dirname + '/bookshop', '--profile', 'tracing-attributes') +const http = require('http') + +describe('span names', () => { + beforeEach(data.reset) + + const log = jest.spyOn(console, 'dir') + beforeEach(log.mockClear) + + const getSpans = () => log.mock.calls.map(c => c[0]).filter(Boolean) + + // Spans from our tracer only (excludes HTTP instrumentation spans) + const getCapSpans = () => getSpans().filter(s => s.instrumentationScope?.name === '@cap-js/telemetry') + const capSpanNames = () => getCapSpans().map(s => s.name) + + describe('db', () => { + test('SELECT: inner DB span name uses operation+table, not SQL', async () => { + await SELECT.from('sap.capire.bookshop.Books').where('title !=', 'DUMMY') + const names = capSpanNames() + // outer CAP span: event+target (unchanged) + expect(names).to.include('db - READ sap.capire.bookshop.Books') + // inner SQLite prepare span: SQL verb + table appended, not raw SQL + expect(names.some(n => /^@cap-js\/\w+ - prepare SELECT sap\.capire\.bookshop\.Books$/.test(n))).to.be.true + // no span name should carry raw SQL (function calls, column lists, etc.) + // but a bare "SELECT " is the intended shape + for (const name of names) { + expect(name, `span "${name}" contains raw SQL`).not.to.match( + /SELECT\s+\S+,|SELECT\s+\w*\(|json_insert|INSERT\s+INTO|UPDATE\s+\S+\s+SET|DELETE\s+FROM/ + ) + } + }) + + test('SELECT: inner DB span has db.query.text attribute with SQL', async () => { + await SELECT.from('sap.capire.bookshop.Books').where('title !=', 'DUMMY') + const spans = getCapSpans() + const prepareSpan = spans.find(s => /^@cap-js\/\w+ - prepare /.test(s.name)) + expect(prepareSpan).to.exist + // SQL lives in the attribute, not the name + expect(prepareSpan.attributes['db.query.text']).to.match(/SELECT/) + expect(prepareSpan.attributes['db.operation.name']).to.equal('READ') + expect(prepareSpan.attributes['db.sql.table']).to.equal('sap.capire.bookshop.Books') + }) + + test('raw SQL via db.run: span name preserves the SQL string', async () => { + const db = await cds.connect.to('db') + await db.run('SELECT ID, title FROM sap_capire_bookshop_Books WHERE ID = 201') + const names = capSpanNames() + // _getSpanName: event === undefined path -> "db - " + expect(names.some(n => n.startsWith('db - SELECT'))).to.be.true + }) + + test('INSERT: inner DB span name uses operation+table, not SQL', async () => { + await INSERT.into('sap.capire.bookshop.Books').entries([{ ID: 999 }]) + const names = capSpanNames() + expect(names).to.include('db - CREATE sap.capire.bookshop.Books') + for (const name of names) { + expect(name, `span "${name}" contains SQL`).not.to.match(/INSERT\s+INTO|json_insert/) + } + }) + }) + + describe('cloud sdk', () => { + let server, port + + beforeAll(done => { + server = http.createServer((req, res) => { + res.writeHead(200, { 'Content-Type': 'application/json' }) + res.end(JSON.stringify({ value: [] })) + }) + server.listen(0, () => { + port = server.address().port + done() + }) + }) + + afterAll(() => new Promise(resolve => server.close(resolve))) + + test('Cloud SDK span name uses destination+method, not URL', async () => { + // Cloud SDK resilience module resolution issues in cds < 9 + if (Number(cds.version.split('.')[0]) < 9) return + + cds.env.requires.TestRemote = { kind: 'odata', credentials: { url: `http://localhost:${port}` } } + const remote = await cds.connect.to('TestRemote') + await remote.send({ method: 'GET', path: '/test' }) + + // no CAP-level span name should contain a URL + for (const name of capSpanNames()) { + expect(name, `span "${name}" contains URL`).not.to.match(/https?:\/\//) + } + }) + }) +})