Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
8 changes: 5 additions & 3 deletions packages/collector/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ const instanaSharedMetrics = require('@instana/shared-metrics');
require('./tracing'); // load additional instrumentations

const log = require('./logger');
const normalizeCollectorConfig = require('./util/normalizeConfig');
const collectorConfig = require('./util/normalizeConfig');
const experimental = require('./experimental');

// NOTE: Default collector logger && config for cases like `preinit`.
Expand Down Expand Up @@ -158,10 +158,12 @@ function init(userConfig = {}) {
log.init(userConfig);
}

const collectorConfig = normalizeCollectorConfig(userConfig);
collectorConfig.init(config);
Comment thread
kirrg001 marked this conversation as resolved.
Outdated

const finalCollectorConfig = collectorConfig.normalizeConfig(userConfig);
config = instanaNodeJsCore.coreConfig.normalize({
userConfig,
finalConfigBase: collectorConfig
finalConfigBase: finalCollectorConfig
});

agentConnection = require('./agentConnection');
Expand Down
122 changes: 98 additions & 24 deletions packages/collector/src/util/normalizeConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,21 @@
* (c) Copyright Instana Inc. and contributors 2019
*/

/* eslint-disable dot-notation */

'use strict';

const util = require('@instana/core/src/config/util');

/** @type {import('@instana/core/src/core').GenericLogger} */
let logger;
Comment thread
aryamohanan marked this conversation as resolved.
Outdated

/**
* @param {import('@instana/core/src/config').InstanaConfig} config
*/
exports.init = function init(config) {
logger = config.logger;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 High: The init function uses config.logger without validation. If config is null, undefined, or doesn't have a logger property, this will throw a TypeError. Add defensive checks: if (!config || !config.logger) before attempting to access config.logger.

Prompt for Bob
Your task is to check if the following issue is valid. If so, understand the root cause and fix it.

<issue>
🟠 High: The `init` function uses `config.logger` without validation. If `config` is null, undefined, or doesn't have a `logger` property, this will throw a TypeError. Add defensive checks: `if (!config || !config.logger)` before attempting to access `config.logger`.
</issue>

You can find the issue in this file:
<path>
packages/collector/src/util/normalizeConfig.js
</path>

Around this line:
<line>
17
</line>

util.init(logger);
};

const defaults = {
agentHost: '127.0.0.1',
agentPort: 42699,
Expand All @@ -16,38 +27,101 @@ const defaults = {

/**
* Merges the config that was passed to the init function with environment variables and default values.
* @param {import('../types/collector').CollectorConfig} config
* @param {import('../types/collector').CollectorConfig} userConfig
* @returns {import('../types/collector').CollectorConfig}
*/
module.exports = function normalizeConfig(config = {}) {
config.agentHost = config.agentHost || process.env.INSTANA_AGENT_HOST || defaults.agentHost;
config.agentPort = config.agentPort || parseToPositiveInteger(process.env.INSTANA_AGENT_PORT, defaults.agentPort);
config.agentRequestTimeout =
config.agentRequestTimeout ||
parseToPositiveInteger(process.env.INSTANA_AGENT_REQUEST_TIMEOUT, defaults.agentRequestTimeout);
function normalizeConfig(userConfig = {}) {
const finalConfig = {};

config.autoProfile = config.autoProfile || process.env.INSTANA_AUTO_PROFILE || defaults.autoProfile;
config.tracing = config.tracing || {};
// NOTE: This function only normalizes collector-specific configuration fields.
// Other userConfig fields (like serviceName, tracing, etc.) are passed through as-is
// and will be normalized later by core/config when this collector config is passed
// as extraFinalConfig to core's normalize function.
finalConfig.agentHost = normalizeAgentHost(userConfig, defaults);
Comment thread
kirrg001 marked this conversation as resolved.
finalConfig.agentPort = normalizeAgentPort(userConfig, defaults);
finalConfig.agentRequestTimeout = normalizeAgentRequestTimeout(userConfig, defaults);
finalConfig.autoProfile = normalizeAutoProfile(userConfig, defaults);
finalConfig.reportUnhandledPromiseRejections = normalizeUnhandledRejections(userConfig);
finalConfig.tracing = userConfig.tracing || {};

if (config.reportUnhandledPromiseRejections == null) {
config.reportUnhandledPromiseRejections = false;
}
return finalConfig;
}

return config;
};
// Export both init and normalizeConfig
// For backward compatibility, also make normalizeConfig the default export
module.exports = normalizeConfig;
module.exports.init = exports.init;
module.exports.normalizeConfig = normalizeConfig;
Comment thread
aryamohanan marked this conversation as resolved.
Outdated

/**
* @param {import('../types/collector').CollectorConfig} userConfig
* @param {{ agentHost: string }} defaultConfig
* @returns {string}
*/
function normalizeAgentHost(userConfig, defaultConfig) {
Comment thread
kirrg001 marked this conversation as resolved.
return resolveConfig(process.env.INSTANA_AGENT_HOST, userConfig.agentHost, defaultConfig.agentHost);
}

/**
* @param {string | number} value
* @param {number} defaultValue
* @param {import('../types/collector').CollectorConfig} userConfig
* @param {{ agentPort: number }} defaultConfig
* @returns {number}
*/
function parseToPositiveInteger(value, defaultValue) {
if (typeof value !== 'string') {
return defaultValue;
function normalizeAgentPort(userConfig, defaultConfig) {
return util.resolveNumericConfig({
envVar: 'INSTANA_AGENT_PORT',
configValue: userConfig.agentPort,
defaultValue: defaultConfig.agentPort,
configPath: 'config.agentPort'
});
}

/**
* @param {import('../types/collector').CollectorConfig} userConfig
* @param {{ agentRequestTimeout: number }} defaultConfig
* @returns {number}
*/
function normalizeAgentRequestTimeout(userConfig, defaultConfig) {
return util.resolveNumericConfig({
envVar: 'INSTANA_AGENT_REQUEST_TIMEOUT',
configValue: userConfig.agentRequestTimeout,
defaultValue: defaultConfig.agentRequestTimeout,
configPath: 'config.agentRequestTimeout'
});
}

/**
* @param {import('../types/collector').CollectorConfig} userConfig
* @param {{ autoProfile: string | boolean }} defaultConfig
* @returns {string | boolean}
*/
function normalizeAutoProfile(userConfig, defaultConfig) {
return resolveConfig(process.env.INSTANA_AUTO_PROFILE, userConfig.autoProfile, defaultConfig.autoProfile);
}

/**
* @param {import('../types/collector').CollectorConfig} userConfig
* @returns {boolean}
*/
function normalizeUnhandledRejections(userConfig) {
return userConfig.reportUnhandledPromiseRejections ?? false;
}

/**
* @template T
* @param {T | undefined} envValue
* @param {T | undefined} configValue
* @param {T} defaultValue
* @returns {T}
*/
function resolveConfig(envValue, configValue, defaultValue) {
if (configValue != null) {
return configValue;
}
value = parseInt(value, 10);
if (!isNaN(value)) {
return Math.abs(Math.round(value));

if (envValue != null) {
return envValue;
}

return defaultValue;
}
19 changes: 11 additions & 8 deletions packages/collector/test/unit/src/util/normalizeConfig.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,17 @@ const expect = require('chai').expect;
const normalizeConfig = require('@_local/collector/src/util/normalizeConfig');

describe('util.normalizeConfig', () => {
before(() => {
normalizeConfig.init({
logger: {
Comment thread
aryamohanan marked this conversation as resolved.
Outdated
debug: () => {},
info: () => {},
warn: () => {},
error: () => {}
}
});
});

beforeEach(resetEnv);
afterEach(resetEnv);

Expand Down Expand Up @@ -84,14 +95,6 @@ describe('util.normalizeConfig', () => {
expect(config.agentPort).to.equal(42699);
});

it('should normalize negative env values', () => {
Comment thread
kirrg001 marked this conversation as resolved.
process.env.INSTANA_AGENT_PORT = '-3000';

const config = normalizeConfig();

expect(config.agentPort).to.equal(3000);
});
Comment thread
kirrg001 marked this conversation as resolved.

it('should fallback to default for invalid env value', () => {
process.env.INSTANA_AGENT_PORT = 'invalid';

Expand Down