From 93e4023058bf4324ad6f287be92960786cc319ad Mon Sep 17 00:00:00 2001 From: Arya <90748009+aryamohanan@users.noreply.github.com> Date: Fri, 13 Dec 2024 16:59:56 +0530 Subject: [PATCH] feat(dynamodb): added endpoint filtering (#1484) refs INSTA-16323 --- .../tracing/cloud/aws-sdk/v2/dynamodb/test.js | 82 +++++++++- .../aws-sdk/v3/dynamodb/test_definition.js | 145 +++++++++++++++++- .../core/src/tracing/backend_mappers/index.js | 5 +- .../src/tracing/backend_mappers/mapper.js | 61 ++++++++ .../tracing/backend_mappers/redis_mapper.js | 29 ---- .../cloud/aws-sdk/v2/dynamodb.js | 2 +- .../cloud/aws-sdk/v3/dynamodb.js | 2 +- packages/core/src/util/spanFilter.js | 26 ++-- .../tracing/backend_mappers/mapper_test.js | 41 ++++- .../core/test/util/normalizeConfig_test.js | 12 +- packages/core/test/util/spanFilter_test.js | 20 ++- 11 files changed, 370 insertions(+), 55 deletions(-) create mode 100644 packages/core/src/tracing/backend_mappers/mapper.js delete mode 100644 packages/core/src/tracing/backend_mappers/redis_mapper.js diff --git a/packages/collector/test/tracing/cloud/aws-sdk/v2/dynamodb/test.js b/packages/collector/test/tracing/cloud/aws-sdk/v2/dynamodb/test.js index 6ae7b35756..4a7cc009cd 100644 --- a/packages/collector/test/tracing/cloud/aws-sdk/v2/dynamodb/test.js +++ b/packages/collector/test/tracing/cloud/aws-sdk/v2/dynamodb/test.js @@ -12,7 +12,7 @@ const { expect } = require('chai'); const { fail } = expect; const supportedVersion = require('@instana/core').tracing.supportedVersion; const config = require('../../../../../../../core/test/config'); -const { retry, stringifyItems, delay } = require('../../../../../../../core/test/test_util'); +const { retry, stringifyItems, delay, expectAtLeastOneMatching } = require('../../../../../../../core/test/test_util'); const ProcessControls = require('../../../../../test_util/ProcessControls'); const globalAgent = require('../../../../../globalAgent'); const { @@ -252,4 +252,84 @@ mochaSuiteFn('tracing/cloud/aws-sdk/v2/dynamodb', function () { }); }); }); + describe('when ignore-endpoints enabled via tracing config', async () => { + let appControls; + before(async () => { + appControls = new ProcessControls({ + dirname: __dirname, + useGlobalAgent: true, + env: { + AWS_DYNAMODB_TABLE_NAME: tableName, + INSTANA_IGNORE_ENDPOINTS: 'dynamodb:list' + } + }); + await appControls.startAndWaitForAgentConnection(); + }); + + beforeEach(async () => { + await agentControls.clearReceivedTraceData(); + }); + + after(async () => { + await appControls.stop(); + }); + + afterEach(async () => { + await appControls.clearIpcMessages(); + }); + + const requestMethod = getNextCallMethod(); + it('should ignore spans for configured ignore endpoints(listTables)', async function () { + const apiPath = `/listTables/${requestMethod}`; + + await appControls.sendRequest({ + method: 'GET', + path: `${apiPath}` + }); + await delay(1000); + const spans = await agentControls.getSpans(); + // 1 x http entry span + // 1 x http client span + expect(spans.length).to.equal(2); + spans.forEach(span => { + expect(span.n).not.to.equal('dynamodb'); + }); + expectAtLeastOneMatching(spans, [ + span => expect(span.n).to.equal('node.http.server'), + span => expect(span.data.http.method).to.equal('GET') + ]); + expectAtLeastOneMatching(spans, [ + span => expect(span.n).to.equal('node.http.client'), + span => expect(span.data.http.method).to.equal('GET') + ]); + }); + it('should not ignore spans for endpoints that are not in the ignore list', async () => { + const apiPath = `/scan/${requestMethod}`; + + await appControls.sendRequest({ + method: 'GET', + path: `${apiPath}` + }); + await delay(1000); + const spans = await agentControls.getSpans(); + + // 1 x http entry span + // 1 x http client span + // 1 x dynamodb span + expect(spans.length).to.equal(3); + + expectAtLeastOneMatching(spans, [ + span => expect(span.n).to.equal('dynamodb'), + span => expect(span.data.dynamodb.op).to.equal('scan') + ]); + expectAtLeastOneMatching(spans, [ + span => expect(span.n).to.equal('node.http.server'), + span => expect(span.data.http.method).to.equal('GET') + ]); + expectAtLeastOneMatching(spans, [ + span => expect(span.n).to.equal('node.http.client'), + span => expect(span.data.http.method).to.equal('GET') + ]); + }); + }); }); diff --git a/packages/collector/test/tracing/cloud/aws-sdk/v3/dynamodb/test_definition.js b/packages/collector/test/tracing/cloud/aws-sdk/v3/dynamodb/test_definition.js index 721b22ada1..c0cff1872b 100644 --- a/packages/collector/test/tracing/cloud/aws-sdk/v3/dynamodb/test_definition.js +++ b/packages/collector/test/tracing/cloud/aws-sdk/v3/dynamodb/test_definition.js @@ -12,7 +12,7 @@ const { expect } = require('chai'); const { fail } = expect; const supportedVersion = require('@instana/core').tracing.supportedVersion; const config = require('@instana/core/test/config'); -const { retry, stringifyItems, delay } = require('@instana/core/test/test_util'); +const { retry, stringifyItems, delay, expectAtLeastOneMatching } = require('@instana/core/test/test_util'); const ProcessControls = require('../../../../../test_util/ProcessControls'); const globalAgent = require('../../../../../globalAgent'); const { @@ -342,6 +342,149 @@ function start(version, requestMethod, reducedTestSuite = false) { }); }); + describe('ignore-endpoints:', function () { + describe('when ignore-endpoints is enabled via agent configuration', () => { + const { AgentStubControls } = require('../../../../../apps/agentStubControls'); + const customAgentControls = new AgentStubControls(); + let controls; + const tableName = createTableName(); + before(async () => { + await customAgentControls.startAgent({ + ignoreEndpoints: { dynamodb: ['listTables'] } + }); + + controls = new ProcessControls({ + agentControls: customAgentControls, + appPath: path.join(__dirname, './app'), + env: { + AWS_DYNAMODB_TABLE_NAME: tableName, + AWS_SDK_CLIENT_DYNAMODB_REQUIRE: version + } + }); + await controls.startAndWaitForAgentConnection(); + }); + + beforeEach(async () => { + await customAgentControls.clearReceivedTraceData(); + }); + + after(async () => { + await customAgentControls.stopAgent(); + await controls.stop(); + }); + after(() => cleanup(tableName)); + + it('should ignore dynamodb spans for ignored endpoints (listTables)', async () => { + const apiPath = `/listTables/${requestMethod}`; + + await controls.sendRequest({ + method: 'GET', + path: `${apiPath}` + }); + await delay(1000); + const spans = await customAgentControls.getSpans(); + // 1 x http entry span + // 1 x http client span + expect(spans.length).to.equal(2); + spans.forEach(span => { + expect(span.n).not.to.equal('dynamodb'); + }); + expectAtLeastOneMatching(spans, [ + span => expect(span.n).to.equal('node.http.server'), + span => expect(span.data.http.method).to.equal('GET') + ]); + expectAtLeastOneMatching(spans, [ + span => expect(span.n).to.equal('node.http.client'), + span => expect(span.data.http.method).to.equal('GET') + ]); + }); + }); + describe('ignore-endpoints enabled via tracing config', async () => { + const tableName = createTableName(); + let appControls; + + before(async () => { + appControls = new ProcessControls({ + useGlobalAgent: true, + appPath: path.join(__dirname, './app'), + env: { + AWS_DYNAMODB_TABLE_NAME: tableName, + AWS_SDK_CLIENT_DYNAMODB_REQUIRE: version, + INSTANA_IGNORE_ENDPOINTS: 'dynamodb:listTables' + } + }); + await appControls.startAndWaitForAgentConnection(); + }); + + beforeEach(async () => { + await agentControls.clearReceivedTraceData(); + }); + + after(async () => { + await appControls.stop(); + }); + + afterEach(async () => { + await appControls.clearIpcMessages(); + }); + + after(() => cleanup(tableName)); + + it('should ignore spans for configured ignore endpoints(listTables)', async function () { + const apiPath = `/listTables/${requestMethod}`; + + await appControls.sendRequest({ + method: 'GET', + path: `${apiPath}` + }); + await delay(1000); + const spans = await agentControls.getSpans(); + // 1 x http entry span + // 1 x http client span + expect(spans.length).to.equal(2); + spans.forEach(span => { + expect(span.n).not.to.equal('dynamodb'); + }); + expectAtLeastOneMatching(spans, [ + span => expect(span.n).to.equal('node.http.server'), + span => expect(span.data.http.method).to.equal('GET') + ]); + expectAtLeastOneMatching(spans, [ + span => expect(span.n).to.equal('node.http.client'), + span => expect(span.data.http.method).to.equal('GET') + ]); + }); + it('should not ignore spans for endpoints that are not in the ignore list', async () => { + const apiPath = `/createTable/${requestMethod}`; + + await appControls.sendRequest({ + method: 'GET', + path: `${apiPath}` + }); + await delay(1000); + const spans = await agentControls.getSpans(); + + // 1 x http entry span + // 1 x http client span + // 1 x dynamodb span + expect(spans.length).to.equal(3); + + expectAtLeastOneMatching(spans, [ + span => expect(span.n).to.equal('dynamodb'), + span => expect(span.data.dynamodb.op).to.equal('createTable') + ]); + expectAtLeastOneMatching(spans, [ + span => expect(span.n).to.equal('node.http.server'), + span => expect(span.data.http.method).to.equal('GET') + ]); + expectAtLeastOneMatching(spans, [ + span => expect(span.n).to.equal('node.http.client'), + span => expect(span.data.http.method).to.equal('GET') + ]); + }); + }); + }); + function verify(controls, response, apiPath, operation, withError, tableName) { return retry( () => diff --git a/packages/core/src/tracing/backend_mappers/index.js b/packages/core/src/tracing/backend_mappers/index.js index 831d0036dd..940a974797 100644 --- a/packages/core/src/tracing/backend_mappers/index.js +++ b/packages/core/src/tracing/backend_mappers/index.js @@ -4,12 +4,13 @@ 'use strict'; +const mapper = require('./mapper'); module.exports = { get transform() { return (/** @type {import('../../core').InstanaBaseSpan} */ span) => { try { - return require(`./${span.n}_mapper`).transform(span); - } catch { + return mapper.transform(span); + } catch (error) { return span; } }; diff --git a/packages/core/src/tracing/backend_mappers/mapper.js b/packages/core/src/tracing/backend_mappers/mapper.js new file mode 100644 index 0000000000..30319af851 --- /dev/null +++ b/packages/core/src/tracing/backend_mappers/mapper.js @@ -0,0 +1,61 @@ +/* + * (c) Copyright IBM Corp. 2024 + */ + +'use strict'; + +/** + * @typedef {Object} FieldMapping + * @typedef {Object} FieldMappings + */ + +/** + * Field mappings for different span types. + * + * This FieldMappings defines how internal span fields are mapped to backend fields + * for various span types (e.g., dynamodb, redis). + * + * As new span types needs to add, simply add their respective mappings + * following the same format (e.g., 'internal-field': 'backend-field'). + * + * @type {FieldMappings} + */ +const fieldMappings = { + dynamodb: { + /// Maps internal field 'operation' to backend field 'op' + operation: 'op' + }, + redis: { + operation: 'command' + } +}; + +/** + * Transforms span data fields to match the backend format. + * + * @param {import('../../core').InstanaBaseSpan} span + * @returns {import('../../core').InstanaBaseSpan} The transformed span. + */ +module.exports.transform = span => { + const spanName = span.n; + const mappings = fieldMappings[spanName]; + // If no mappings exist for the span name or the span data, return the original span + if (!mappings || !span.data[spanName]) return span; + + Object.keys(span.data[spanName]).forEach(internalField => { + // Only proceed if there's a mapping for the internal field in the current span type + if (internalField in mappings) { + const backendField = mappings[internalField]; + + if (backendField) { + span.data[spanName][backendField] = span.data[spanName][internalField]; + delete span.data[spanName][internalField]; + } else { + // If backendField is falsy, remove the internalField from span data + delete span.data[spanName][internalField]; + } + } + }); + + return span; +}; diff --git a/packages/core/src/tracing/backend_mappers/redis_mapper.js b/packages/core/src/tracing/backend_mappers/redis_mapper.js deleted file mode 100644 index e470b011c1..0000000000 --- a/packages/core/src/tracing/backend_mappers/redis_mapper.js +++ /dev/null @@ -1,29 +0,0 @@ -/* - * (c) Copyright IBM Corp. 2024 - */ - -'use strict'; - -const fieldMappings = { - // internal-format: backend-format - operation: 'command' -}; - -/** - * Transforms Redis-related span data fields to match the backend format. - * - * @param {import('../../core').InstanaBaseSpan} span - * @returns {import('../../core').InstanaBaseSpan} The transformed span. - */ -module.exports.transform = span => { - if (span.data?.redis) { - Object.entries(fieldMappings).forEach(([internalField, backendField]) => { - if (span.data.redis[internalField]) { - span.data.redis[backendField] = span.data.redis[internalField]; - delete span.data.redis[internalField]; - } - }); - } - - return span; -}; diff --git a/packages/core/src/tracing/instrumentation/cloud/aws-sdk/v2/dynamodb.js b/packages/core/src/tracing/instrumentation/cloud/aws-sdk/v2/dynamodb.js index e2d2921a68..f8bc477788 100644 --- a/packages/core/src/tracing/instrumentation/cloud/aws-sdk/v2/dynamodb.js +++ b/packages/core/src/tracing/instrumentation/cloud/aws-sdk/v2/dynamodb.js @@ -100,7 +100,7 @@ class InstanaAWSDynamoDB extends InstanaAWSProduct { buildSpanData(ctx, operation, params) { const operationInfo = operationsInfo[operation]; const spanData = { - op: operationInfo.op, + operation: operationInfo.op, region: ctx.config.region }; diff --git a/packages/core/src/tracing/instrumentation/cloud/aws-sdk/v3/dynamodb.js b/packages/core/src/tracing/instrumentation/cloud/aws-sdk/v3/dynamodb.js index 7eb0d0b54b..9fc0d0c876 100644 --- a/packages/core/src/tracing/instrumentation/cloud/aws-sdk/v3/dynamodb.js +++ b/packages/core/src/tracing/instrumentation/cloud/aws-sdk/v3/dynamodb.js @@ -60,7 +60,7 @@ class InstanaAWSDynamoDB extends InstanaAWSProduct { buildSpanData(operation, params) { const spanData = { - op: this.convertOperationName(operation) + operation: this.convertOperationName(operation) }; if (params && params.TableName) { diff --git a/packages/core/src/util/spanFilter.js b/packages/core/src/util/spanFilter.js index b75d205c03..fbb8861704 100644 --- a/packages/core/src/util/spanFilter.js +++ b/packages/core/src/util/spanFilter.js @@ -5,7 +5,7 @@ 'use strict'; // List of span types to allowed to ignore -const IGNORABLE_SPAN_TYPES = ['redis']; +const IGNORABLE_SPAN_TYPES = ['redis', 'dynamodb']; /** * @param {import('../core').InstanaBaseSpan} span @@ -17,15 +17,21 @@ function shouldIgnore(span, endpoints) { if (!IGNORABLE_SPAN_TYPES.includes(span.n)) { return false; } - const operation = span.data?.[span.n]?.operation; - - if (operation && endpoints[span.n]) { - const endpoint = endpoints[span.n]; - if (Array.isArray(endpoint)) { - return endpoint.some(op => op === operation); - } else if (typeof endpoint === 'string') { - return endpoint === operation; - } + const endpoint = endpoints[span.n]; + if (!endpoint) return false; + + // The endpoint already been converted to lowercase during parsing, so here we are normalizing + // the operation for case-insensitive comparison + const operation = span.data?.[span.n]?.operation?.toLowerCase(); + if (!operation) return false; + + // We support both array and string formats for endpoints, but the string format is not shared publicly. + if (Array.isArray(endpoint)) { + return endpoint.some(op => op.toLowerCase() === operation); + } + + if (typeof endpoint === 'string') { + return endpoint.toLowerCase() === operation; } return false; diff --git a/packages/core/test/tracing/backend_mappers/mapper_test.js b/packages/core/test/tracing/backend_mappers/mapper_test.js index 94105c2c96..4a0c731d36 100644 --- a/packages/core/test/tracing/backend_mappers/mapper_test.js +++ b/packages/core/test/tracing/backend_mappers/mapper_test.js @@ -14,12 +14,13 @@ describe('tracing/backend_mappers', () => { span = { n: 'redis', t: '1234567803', s: '1234567892', p: '1234567891', data: { redis: { operation: 'GET' } } }; }); - describe('should invoke transform function', () => { + describe('Redis Mapper', () => { it('should transform redis span using the redis mapper', () => { const result = transform(span); expect(result.data.redis.command).equal('GET'); expect(result.data.redis).to.not.have.property('operation'); }); + it('should not modify fields that need not be transformed in the redis span', () => { span = { data: { redis: { command: 'SET' }, otherField: 'value' } }; @@ -40,4 +41,42 @@ describe('tracing/backend_mappers', () => { expect(transform(span)).to.deep.equal(span); }); }); + + describe('DynamoDB Mapper', () => { + beforeEach(() => { + span = { + n: 'dynamodb', + t: '2234567803', + s: '2234567892', + p: '2234567891', + data: { dynamodb: { operation: 'query' } } + }; + }); + + it('should transform dynamodb span using the dynamodb mapper', () => { + const result = transform(span); + expect(result.data.dynamodb.op).to.equal('query'); + expect(result.data.dynamodb).to.not.have.property('operation'); + }); + + it('should not modify fields that need not be transformed in the dynamodb span', () => { + span = { data: { dynamodb: { op: 'PutItem' }, otherField: 'value' } }; + + const result = transform(span); + expect(result.data.dynamodb).to.not.have.property('operation'); + expect(result.data.dynamodb.op).to.equal('PutItem'); + expect(result.data.otherField).to.equal('value'); + }); + + it('should return the span unmodified if no mapper is found', () => { + span.n = 'http'; + const result = transform(span); + expect(result).to.equal(span); + }); + + it('should cache the mapper after the first load', () => { + transform(span); + expect(transform(span)).to.deep.equal(span); + }); + }); }); diff --git a/packages/core/test/util/normalizeConfig_test.js b/packages/core/test/util/normalizeConfig_test.js index da3bc46c20..fb35c8009f 100644 --- a/packages/core/test/util/normalizeConfig_test.js +++ b/packages/core/test/util/normalizeConfig_test.js @@ -483,13 +483,13 @@ describe('util.normalizeConfig', () => { }); it('should correctly parse INSTANA_IGNORE_ENDPOINTS containing multiple services and endpoints', () => { - process.env.INSTANA_IGNORE_ENDPOINTS = 'redis:get,set; dynamodb:query'; - const config = normalizeConfig(); - expect(config.tracing.ignoreEndpoints).to.deep.equal({ - redis: ['get', 'set'], - dynamodb: ['query'] - }); + process.env.INSTANA_IGNORE_ENDPOINTS = 'redis:get,set; dynamodb:query'; + const config = normalizeConfig(); + expect(config.tracing.ignoreEndpoints).to.deep.equal({ + redis: ['get', 'set'], + dynamodb: ['query'] }); + }); it('should fallback to default if INSTANA_IGNORE_ENDPOINTS is set but has an invalid format', () => { process.env.INSTANA_IGNORE_ENDPOINTS = '"redis=get,set"'; diff --git a/packages/core/test/util/spanFilter_test.js b/packages/core/test/util/spanFilter_test.js index 4bec4e157f..2309569e72 100644 --- a/packages/core/test/util/spanFilter_test.js +++ b/packages/core/test/util/spanFilter_test.js @@ -19,8 +19,9 @@ const span = { } } }; -let ignoreEndpoints = { - redis: ['GET', 'TYPE'] +const ignoreEndpoints = { + redis: ['GET', 'TYPE'], + dynamodb: ['QUERY'] }; describe('util.spanFilter', () => { @@ -39,8 +40,21 @@ describe('util.spanFilter', () => { expect(applyFilter({ span, ignoreEndpoints })).equal(span); }); it('should return span when no ignoreconfiguration', () => { - ignoreEndpoints = {}; span.data.redis.operation = 'GET'; + expect(applyFilter({ span, ignoreEndpoints: {} })).equal(span); + }); + it('should return null when the dynamodb span operation is in the ignore list', () => { + span.n = 'dynamodb'; + span.data = { + dynamodb: { + operation: 'Query' + } + }; + expect(applyFilter({ span, ignoreEndpoints })).equal(null); + }); + + it('should return the dynamodb span when the operation is not in the ignore list', () => { + span.data.dynamodb.operation = 'PutItem'; expect(applyFilter({ span, ignoreEndpoints })).equal(span); }); });