chore: fine tuning

Author: aryamohanan

packages/collector/src/announceCycle/unannounced.js

@@ -43,7 +43,7 @@ const maxRetryDelay = 60 * 1000; // one minute
  * @typedef {Object} TracingConfig
  * @property {Array.<string>} [extra-http-headers]
  * @property {KafkaTracingConfig} [kafka]
- * @property {IgnoreEndpoints} [ignore-endpoints]
+ * @property {Object.<string, Array.<string>>} [ignore-endpoints]
  * @property {boolean} [span-batching-enabled]
  */
 
@@ -52,11 +52,6 @@ const maxRetryDelay = 60 * 1000; // one minute
  * @property {boolean} [trace-correlation]
  */
 
-/**
- * @typedef {Object} IgnoreEndpoints
- * @property {Object.<string, Array.<string>>} [endpoints]
- */
-
 module.exports = {
   /**
    * @param {import('./').AnnounceCycleContext} ctx
@@ -231,16 +226,17 @@ function applySpanBatchingConfiguration(agentResponse) {
  * @param {AgentAnnounceResponse} agentResponse
  */
 function applyIgnoreEndpointsConfiguration(agentResponse) {
-  if (agentResponse.tracing && agentResponse.tracing['ignore-endpoints']) {
+  if (agentResponse?.tracing?.['ignore-endpoints']) {
     const endpointTracingConfigFromAgent = agentResponse.tracing['ignore-endpoints'];
     const endpointTracingConfig = {};
     // eslint-disable-next-line no-restricted-syntax
     for (const [service, actions] of Object.entries(endpointTracingConfigFromAgent)) {
       // From agent, the commands are separated by a pipe ('|')
       // @ts-ignore
-      endpointTracingConfig[service.toLowerCase()] =
-        // @ts-ignore
-        actions != null ? actions?.split('|')?.map(action => action?.trim()?.toLowerCase()) : [];
+      endpointTracingConfig[service.toLowerCase()] = actions
+        ? // @ts-ignore
+          actions?.split(/[|,]/).map(action => action?.trim()?.toLowerCase())
+        : [];
     }
 
     ensureNestedObjectExists(agentOpts.config, ['tracing', 'ignoreEndpoints']);

packages/collector/test/announceCycle/unannounced_test.js

@@ -213,7 +213,7 @@ describe('unannounced state', () => {
         }
       });
     });
-    it('should apply the configuration to ignore specified endpoints for a single package', done => {
+    it('should apply the configuration to ignore specified endpoint for a single package', done => {
       prepareAnnounceResponse({
         tracing: {
           'ignore-endpoints': {
@@ -234,6 +234,27 @@ describe('unannounced state', () => {
         }
       });
     });
+    it('should apply the configuration to ignore multiple endpoints for a single package', done => {
+      prepareAnnounceResponse({
+        tracing: {
+          'ignore-endpoints': {
+            redis: 'TYPE|GET'
+          }
+        }
+      });
+      unannouncedState.enter({
+        transitionTo: () => {
+          expect(agentOptsStub.config).to.deep.equal({
+            tracing: {
+              ignoreEndpoints: {
+                redis: ['type', 'get']
+              }
+            }
+          });
+          done();
+        }
+      });
+    });
     it('should apply the tracing configuration to ignore multiple endpoints across different packages', done => {
       prepareAnnounceResponse({
         tracing: {

packages/core/src/tracing/spanBuffer.js

@@ -6,7 +6,7 @@
 'use strict';
 
 const tracingMetrics = require('./metrics');
-const { filterSpan } = require('../util/filterSpan');
+const { applyFilter } = require('../util/spanFilter');
 const { transform } = require('./backend_mappers');
 
 /** @type {import('../core').GenericLogger} */
@@ -189,8 +189,8 @@ exports.addSpan = function (span) {
     return;
   }
 
-  // Transform the span
-  const processedSpan = manageSpan(span);
+  // Process the span, apply any transformations, and implement filtering if necessary.
+  const processedSpan = processSpan(span);
   if (!processedSpan) {
     logger.warn('Span of type %s has no trace ID. Not transmitting this span', span.n);
     return;
@@ -505,23 +505,24 @@ function removeSpansIfNecessary() {
  * @param {import('../core').InstanaBaseSpan} span
  * @returns {import('../core').InstanaBaseSpan} span
  */
-function manageSpan(span) {
+function processSpan(span) {
   if (!span || typeof span.n !== 'string') {
     return span;
   }
 
-  // Currently only filter if ignoreEndpoint is configured
+  // Currently, filter only if the ignoreEndpoint is configured.
+  // In the next phase, this check will be removed.
   if (ignoreEndpoints) {
     // @ts-ignore
-    span = filterSpan({ span, ignoreEndpoints });
+    span = applyFilter({ span, ignoreEndpoints });
 
     if (!span) {
       return null;
     }
   }
   return transform(span);
 }
-// export the manageSpan function for use in test.
+// export the processSpan function for use in test.
 if (process.env.NODE_ENV === 'test') {
-  module.exports.manageSpan = manageSpan;
+  module.exports.processSpan = processSpan;
 }

packages/core/src/util/normalizeConfig.js

@@ -27,7 +27,7 @@ const constants = require('../tracing/constants');
  * @property {boolean} [disableW3cTraceCorrelation]
  * @property {KafkaTracingOptions} [kafka]
  * @property {boolean} [allowRootExitSpan]
- * @property {Object} [ignoreEndpoints]
+ * @property {Object.<string, Array.<string>>} [ignoreEndpoints]
  */
 
 /**
@@ -62,11 +62,6 @@ const constants = require('../tracing/constants');
  * @property {number} [timeBetweenHealthcheckCalls]
  */
 
-/**
- * @typedef {Object} IgnoreEndpoints
- * @property {Object.<string, Array.<string>>} [endpoints]
- */
-
 /**
  * @typedef {Object} AgentConfig
  * @property {AgentTracingConfig} [tracing]
@@ -231,7 +226,7 @@ function normalizeTracingConfig(config) {
   normalizeDisableW3cTraceCorrelation(config);
   normalizeTracingKafka(config);
   normalizeAllowRootExitSpan(config);
-  normalizeIgnoredEndpoints(config);
+  normalizeIgnoreEndpoints(config);
 }
 
 /**
@@ -691,15 +686,15 @@ function normalizeSingleValue(configValue, defaultValue, configPath, envVarKey)
 /**
  * @param {InstanaConfig} config
  */
-function normalizeIgnoredEndpoints(config) {
-  if (!config.tracing.ignoreEndpoints) {
+function normalizeIgnoreEndpoints(config) {
+  if (!config.tracing?.ignoreEndpoints) {
     config.tracing.ignoreEndpoints = {};
   }
 
   for (const [service, methods] of Object.entries(config.tracing.ignoreEndpoints)) {
     const normalizedService = service.toLowerCase();
     if (!Array.isArray(methods)) {
-      console.warn(
+      logger.warn(
         `Invalid configuration for ${normalizedService}: ignoredEndpoints.${normalizedService} is not an array, the value will be ignored: ${JSON.stringify(
           methods
         )}`
@@ -708,7 +703,7 @@ function normalizeIgnoredEndpoints(config) {
       config.tracing.ignoreEndpoints[normalizedService] = [];
     } else {
       // @ts-ignore
-      config.tracing.ignoreEndpoints[normalizedService] = methods.map(method => method.toLowerCase());
+      config.tracing.ignoreEndpoints[normalizedService] = methods.map(method => method?.toLowerCase());
     }
   }
 }

packages/core/src/util/filterSpan.js renamed to packages/core/src/util/spanFilter.js

@@ -5,8 +5,7 @@
 'use strict';
 
 /** @type {import('../core').GenericLogger} */
-let logger;
-logger = require('../logger').getLogger('tracing/util/filterSpan', newLogger => {
+let logger = require('../logger').getLogger('util/spanFilter', newLogger => {
   logger = newLogger;
 });
 /**
@@ -26,15 +25,15 @@ function shouldIgnore(span, ignoreEndpoints) {
  * @param {{ span: import('../core').InstanaBaseSpan, ignoreEndpoints: { [key: string]: Array<string> } }} params
  * @returns {import('../core').InstanaBaseSpan | null}
  */
-function filterSpan({ span, ignoreEndpoints }) {
+function applyFilter({ span, ignoreEndpoints }) {
   if (ignoreEndpoints && shouldIgnore(span, ignoreEndpoints[span.n])) {
     logger.debug('Span ignored due to matching ignore endpoint', {
       spanType: span.n,
-      ignoredOperation: span.data[span.n]?.operation
+      ignoreOperation: span.data[span.n]?.operation
     });
     return null;
   }
   return span;
 }
 
-module.exports = { filterSpan };
+module.exports = { applyFilter };

packages/core/test/tracing/spanBuffer_test.js

@@ -566,7 +566,7 @@ describe('tracing/spanBuffer', () => {
         verifyNoBatching(span1, span2);
       });
     });
-    describe('manageSpan test', () => {
+    describe('processSpan test', () => {
       before(() => {
         spanBuffer.init(
           {
@@ -597,16 +597,16 @@ describe('tracing/spanBuffer', () => {
         }
       };
       it('should return the span if span is not valid', () => {
-        expect(spanBuffer.manageSpan(null)).to.equal(null);
+        expect(spanBuffer.processSpan(null)).to.equal(null);
       });
 
       it('should filter out the span when command is listed in ignoreEndpoints config', () => {
-        expect(spanBuffer.manageSpan(span)).to.equal(null);
+        expect(spanBuffer.processSpan(span)).to.equal(null);
       });
 
       it('should transform and return the span  for command not specified in ignoreEndpoints config', () => {
         span.data.redis.operation = 'set';
-        const result = spanBuffer.manageSpan(span);
+        const result = spanBuffer.processSpan(span);
         expect(result.data.redis.command).to.equal('set');
         expect(result.data.redis).to.not.have.property('operation');
       });

packages/core/test/util/normalizeConfig_test.js

@@ -484,7 +484,14 @@ describe('util.normalizeConfig', () => {
     });
     expect(config.tracing.ignoreEndpoints).to.deep.equal({ redis: ['get'] });
   });
-
+  it('should apply multiple ignore endpoints via config', () => {
+    const config = normalizeConfig({
+      tracing: {
+        ignoreEndpoints: { redis: ['GET', 'TYPE'] }
+      }
+    });
+    expect(config.tracing.ignoreEndpoints).to.deep.equal({ redis: ['get', 'type'] });
+  });
   it('should apply ignore endpoints via config for multiple packages', () => {
     const config = normalizeConfig({
       tracing: {

packages/core/test/util/filterSpan_test.js renamed to packages/core/test/util/spanFilter_test.js

@@ -5,7 +5,7 @@
 'use strict';
 
 const expect = require('chai').expect;
-const { filterSpan } = require('../../src/util/filterSpan');
+const { applyFilter } = require('../../src/util/spanFilter');
 
 const span = {
   t: '1234567803',
@@ -23,24 +23,24 @@ let ignoreEndpoints = {
   redis: ['GET', 'TYPE']
 };
 
-describe('util.filterSpan', () => {
+describe('util.spanFilter', () => {
   it('should return null when the span should be ignored', () => {
     span.data.redis.operation = 'GET';
-    expect(filterSpan({ span, ignoreEndpoints })).equal(null);
+    expect(applyFilter({ span, ignoreEndpoints })).equal(null);
   });
 
   it('should return the span when command is not in the ignore list', () => {
     span.data.redis.operation = 'HGET';
-    expect(filterSpan({ span, ignoreEndpoints })).equal(span);
+    expect(applyFilter({ span, ignoreEndpoints })).equal(span);
   });
 
   it('should return the span when span.n does not match any endpoint in config', () => {
     span.n = 'node.http.client';
-    expect(filterSpan({ span, ignoreEndpoints })).equal(span);
+    expect(applyFilter({ span, ignoreEndpoints })).equal(span);
   });
   it('should return span when no ignoreconfiguration', () => {
     ignoreEndpoints = {};
     span.data.redis.operation = 'GET';
-    expect(filterSpan({ span, ignoreEndpoints })).equal(span);
+    expect(applyFilter({ span, ignoreEndpoints })).equal(span);
   });
 });