Skip to content

Commit

Permalink
chore: resolved review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
aryamohanan committed Dec 4, 2024
1 parent fdfc94b commit 575654e
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 23 deletions.
10 changes: 7 additions & 3 deletions packages/collector/src/announceCycle/unannounced.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,13 @@ function applySpanBatchingConfiguration(agentResponse) {
agentOpts.config.tracing.spanBatchingEnabled = true;
}
}

/**
* - The agent configuration currently uses a pipe ('|') as a separator for endpoints.
* - This function splits the string by both pipe ('|') and comma (',') to ensure compatibility.
* - Additionally, it supports the `string[]` format for backward compatibility,
* as this was the previously used standard.
*
* @param {AgentAnnounceResponse} agentResponse
*/
function applyIgnoreEndpointsConfiguration(agentResponse) {
Expand All @@ -233,9 +239,7 @@ function applyIgnoreEndpointsConfiguration(agentResponse) {
Object.entries(endpointTracingConfigFromAgent).map(([service, endpoints]) => {
let normalizedEndpoints = null;
if (typeof endpoints === 'string') {
normalizedEndpoints = endpoints
.split(/[|,]/) // From agent, the commands are separated by a pipe ('|')
.map(endpoint => endpoint?.trim()?.toLowerCase());
normalizedEndpoints = endpoints.split(/[|,]/).map(endpoint => endpoint?.trim()?.toLowerCase());
} else if (Array.isArray(endpoints)) {
normalizedEndpoints = endpoints.map(endpoint => endpoint?.toLowerCase());
}
Expand Down
2 changes: 1 addition & 1 deletion packages/collector/test/apps/agentStub.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const enableSpanBatching = process.env.ENABLE_SPANBATCHING === 'true';
const kafkaTraceCorrelation = process.env.KAFKA_TRACE_CORRELATION
? process.env.KAFKA_TRACE_CORRELATION === 'true'
: null;
const ignoreEndpoints = process.env.INSTANA_IGNORE_ENDPOINTS && JSON.parse(process.env.INSTANA_IGNORE_ENDPOINTS);
const ignoreEndpoints = process.env.IGNORE_ENDPOINTS && JSON.parse(process.env.IGNORE_ENDPOINTS);

let discoveries = {};
let rejectAnnounceAttempts = 0;
Expand Down
2 changes: 1 addition & 1 deletion packages/collector/test/apps/agentStubControls.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class AgentStubControls {
}
}
if (opts.ignoreEndpoints) {
env.INSTANA_IGNORE_ENDPOINTS = JSON.stringify(opts.ignoreEndpoints);
env.IGNORE_ENDPOINTS = JSON.stringify(opts.ignoreEndpoints);
}

this.agentStub = spawn('node', [path.join(__dirname, 'agentStub.js')], {
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/tracing/backend_mappers/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

'use strict';

Object.defineProperty(module.exports, 'transform', {
get() {
module.exports = {
get transform() {
return (/** @type {import('../../core').InstanaBaseSpan} */ span) => {
try {
return require(`./${span.n}_mapper`).transform(span);
Expand All @@ -14,4 +14,4 @@ Object.defineProperty(module.exports, 'transform', {
}
};
}
});
};
6 changes: 2 additions & 4 deletions packages/core/src/tracing/backend_mappers/redis_mapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const fieldMappings = {
* @param {import('../../core').InstanaBaseSpan} span
* @returns {import('../../core').InstanaBaseSpan} The transformed span.
*/
function transform(span) {
module.exports.transform = span => {
if (span.data?.redis) {
Object.entries(fieldMappings).forEach(([internalField, backendField]) => {
if (span.data.redis[internalField]) {
Expand All @@ -26,6 +26,4 @@ function transform(span) {
}

return span;
}

module.exports = { transform };
};
28 changes: 19 additions & 9 deletions packages/core/src/util/normalizeConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -685,13 +685,26 @@ function normalizeIgnoreEndpoints(config) {
if (!config.tracing.ignoreEndpoints) {
config.tracing.ignoreEndpoints = {};
}
if (Object.keys(config.tracing.ignoreEndpoints).length) {
for (const [service, endpoints] of Object.entries(config.tracing.ignoreEndpoints)) {

const ignoreEndpoints = config.tracing.ignoreEndpoints;

if (typeof ignoreEndpoints !== 'object' || Array.isArray(ignoreEndpoints)) {
logger.warn(
`Invalid tracing.ignoreEndpoints configuration. Expected an object, but received: ${JSON.stringify(
ignoreEndpoints
)}`
);
config.tracing.ignoreEndpoints = {};
return;
}

if (Object.keys(ignoreEndpoints).length) {
for (const [service, endpoints] of Object.entries(ignoreEndpoints)) {
const normalizedService = service.toLowerCase();

if (!Array.isArray(endpoints)) {
logger.warn(
`Invalid configuration for ${normalizedService}: ignoredEndpoints.${normalizedService} is not an array. The value will be ignored: ${JSON.stringify(
`Invalid configuration for ${normalizedService}: tracing.ignoreEndpoints.${normalizedService} is not an array. The value will be ignored: ${JSON.stringify(
endpoints
)}`
);
Expand All @@ -700,14 +713,11 @@ function normalizeIgnoreEndpoints(config) {
config.tracing.ignoreEndpoints[normalizedService] = endpoints.map(endpoint => endpoint?.toLowerCase());
}
}
return;
}

if (process.env.INSTANA_IGNORE_ENDPOINTS) {
} else if (process.env.INSTANA_IGNORE_ENDPOINTS) {
// The environment variable name and its format are still under discussion.
// It is currently private and will not be documented or publicly shared.
try {
logger.info('Ignore endpoints have been added via environment variable INSTANA_IGNORE_ENDPOINTS.');
config.tracing.ignoreEndpoints = JSON.parse(process.env.INSTANA_IGNORE_ENDPOINTS);
return;
} catch (error) {
logger.warn(
`Failed to parse INSTANA_IGNORE_ENDPOINTS: ${process.env.INSTANA_IGNORE_ENDPOINTS}. Error: ${error.message}`
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/util/spanFilter.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ function shouldIgnore(span, endpoints) {
const operation = span.data?.[span.n]?.operation;

if (operation && endpoints[span.n]) {
return endpoints[span.n].includes(operation);
return endpoints[span.n].some(op => op === operation);
}

return false;
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/tracing/backend_mappers/mapper_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
const expect = require('chai').expect;
const { transform } = require('../../../src/tracing/backend_mappers');

describe('BE span transformation test', () => {
describe('tracing/backend_mappers', () => {
let span;

beforeEach(() => {
Expand Down

0 comments on commit 575654e

Please sign in to comment.