diff --git a/lib/agent.js b/lib/agent.js index 439353be9d..85d06cdcc9 100644 --- a/lib/agent.js +++ b/lib/agent.js @@ -40,6 +40,7 @@ const { } = require('./util/attributes') const synthetics = require('./synthetics') const Harvester = require('./harvester') +const { createFeatureUsageMetrics } = require('./util/application-logging') // Map of valid states to whether or not data collection is valid const STATES = { @@ -405,7 +406,7 @@ Agent.prototype.startStreaming = function startStreaming() { */ Agent.prototype.onConnect = function onConnect(shouldImmediatelyHarvest, callback) { this.harvester.update(this.config) - generateLoggingSupportMetrics(this) + createFeatureUsageMetrics(this) if (this.config.certificates && this.config.certificates.length > 0) { this.metrics.getOrCreateMetric(NAMES.FEATURES.CERTIFICATES).incrementCallCount() @@ -953,23 +954,4 @@ function generateEventHarvestSupportMetrics(agent, harvestConfig) { } } -/** - * Increments the call counts of application logging supportability metrics - * on every connect cycle - * - * @param {object} agent Instantiation of Node.js agent - */ -function generateLoggingSupportMetrics(agent) { - const loggingConfig = agent.config.application_logging - const logNames = NAMES.LOGGING - - const configKeys = ['metrics', 'forwarding', 'local_decorating'] - configKeys.forEach((configValue) => { - const configFlag = - loggingConfig.enabled && loggingConfig[`${configValue}`].enabled ? 'enabled' : 'disabled' - const metricName = logNames[`${configValue.toUpperCase()}`] - agent.metrics.getOrCreateMetric(`${metricName}${configFlag}`).incrementCallCount() - }) -} - module.exports = Agent diff --git a/lib/aggregators/log-aggregator.js b/lib/aggregators/log-aggregator.js index 2b7016e281..4da319578c 100644 --- a/lib/aggregators/log-aggregator.js +++ b/lib/aggregators/log-aggregator.js @@ -6,6 +6,7 @@ 'use strict' const logger = require('../logger').child({ component: 'logs_aggregator' }) +const { isLogLabelingEnabled } = require('../../lib/util/application-logging') const EventAggregator = require('./event-aggregator') const NAMES = require('../metrics/names') @@ -56,7 +57,12 @@ class LogAggregator extends EventAggregator { return } - const commonAttrs = this.agent.getServiceLinkingMetadata() + let commonAttrs = this.agent.getServiceLinkingMetadata() + + if (isLogLabelingEnabled(this.agent.config)) { + commonAttrs = { ...commonAttrs, ...this.agent.config.loggingLabels } + } + return [ { common: { attributes: commonAttrs }, diff --git a/lib/collector/facts.js b/lib/collector/facts.js index fa2290cbf0..bced9d3420 100644 --- a/lib/collector/facts.js +++ b/lib/collector/facts.js @@ -8,7 +8,6 @@ const fetchSystemInfo = require('../system-info') const defaultLogger = require('../logger').child({ component: 'facts' }) const os = require('os') -const parseLabels = require('../util/label-parser') module.exports = facts @@ -49,7 +48,7 @@ async function facts(agent, callback, { logger = defaultLogger } = {}) { environment: environment, settings: agent.config.publicSettings(), high_security: agent.config.high_security, - labels: parseLabels(agent.config.labels), + labels: agent.config.parsedLabels, metadata: Object.keys(process.env).reduce((obj, key) => { if (key.startsWith('NEW_RELIC_METADATA_')) { obj[key] = process.env[key] diff --git a/lib/config/index.js b/lib/config/index.js index 7eee824785..78c17b7e29 100644 --- a/lib/config/index.js +++ b/lib/config/index.js @@ -24,6 +24,7 @@ const harvestConfigValidator = require('./harvest-config-validator') const mergeServerConfig = new MergeServerConfig() const { boolean: isTruthular } = require('./formatters') const configDefinition = definition() +const parseLabels = require('../util/label-parser') /** * CONSTANTS -- we gotta lotta 'em @@ -195,9 +196,38 @@ function Config(config) { // 9. Set instance attribute filter using updated context this.attributeFilter = new AttributeFilter(this) + + // 10. Setup labels for both `collector/facts` and application logging + this.parsedLabels = parseLabels(this.labels, logger) + this.loggingLabels = this._setApplicationLoggingLabels() } util.inherits(Config, EventEmitter) +/** + * Compares the labels list to the application logging excluded label list and removes any labels that need to be excluded. + * Then prefixing each label with "tags." + * + * assigns labels to `config.loggingLabels` + */ +Config.prototype._setApplicationLoggingLabels = function setApplicationLoggingLabels() { + if (this.application_logging.forwarding.labels.enabled === false) { + return + } + + this.application_logging.forwarding.labels.exclude = + this.application_logging.forwarding.labels.exclude.map((k) => k.toLowerCase()) + + return this.parsedLabels.reduce((filteredLabels, label) => { + if ( + !this.application_logging.forwarding.labels.exclude.includes(label.label_type.toLowerCase()) + ) { + filteredLabels[`tags.${label.label_type}`] = label.label_value + } + + return filteredLabels + }, {}) +} + /** * Because this module and logger depend on each other, the logger needs * a way to inject the actual logger instance once it's constructed. diff --git a/lib/metrics/names.js b/lib/metrics/names.js index 805a9d3b9b..139da8ab5b 100644 --- a/lib/metrics/names.js +++ b/lib/metrics/names.js @@ -337,7 +337,8 @@ const LOGGING = { SENT: `${LOGGING_FORWARDING_PREFIX}/Sent`, FORWARDING: `${LOGGING_FORWARDING_PREFIX}/${NODEJS.PREFIX}`, METRICS: `${SUPPORTABILITY.LOGGING}/Metrics/${NODEJS.PREFIX}`, - LOCAL_DECORATING: `${SUPPORTABILITY.LOGGING}/LocalDecorating/${NODEJS.PREFIX}` + LOCAL_DECORATING: `${SUPPORTABILITY.LOGGING}/LocalDecorating/${NODEJS.PREFIX}`, + LABELS: `${SUPPORTABILITY.LOGGING}/Labels/${NODEJS.PREFIX}` } const KAFKA = { diff --git a/lib/util/application-logging.js b/lib/util/application-logging.js index cd68d2ba20..6d1cff9805 100644 --- a/lib/util/application-logging.js +++ b/lib/util/application-logging.js @@ -75,6 +75,16 @@ utils.isLogForwardingEnabled = function isLogForwardingEnabled(config, agent) { ) } +/** + * Checks if application_logging.forwarding.labels is enabled + * + * @param {object} config agent config + * @returns {boolean} is labeling enabled + */ +utils.isLogLabelingEnabled = function isLogLabelingEnabled(config) { + return !!config.application_logging.forwarding.labels.enabled +} + /** * Increments both `Logging/lines` and `Logging/lines/` call count * @@ -108,3 +118,51 @@ function getLogLevel(level) { } return logLevel } + +/** + * Increments the enabled/disabled metrics for the top line application logging features: + * 1. Supportability/Logging/Metrics/Nodejs/ + * 2. Supportability/Logging/Forwarding/Nodejs/ + * 3. Supportability/Logging/LocalDecorating/Nodejs/ + * 4. Supportability/Logging/Labels/Nodejs/ + * + * Run in `agent.onConnect` + * + * @param {Agent} agent instance + */ +utils.createFeatureUsageMetrics = function createFeatureUsageMetrics(agent) { + const { config, metrics } = agent + const loggingConfig = config.application_logging.enabled + metrics + .getOrCreateMetric( + `${LOGGING.METRICS}${ + loggingConfig && config.application_logging.metrics.enabled ? 'enabled' : 'disabled' + }` + ) + .incrementCallCount() + metrics + .getOrCreateMetric( + `${LOGGING.FORWARDING}${ + loggingConfig && config.application_logging.forwarding.enabled ? 'enabled' : 'disabled' + }` + ) + .incrementCallCount() + metrics + .getOrCreateMetric( + `${LOGGING.LOCAL_DECORATING}${ + loggingConfig && config.application_logging.local_decorating.enabled + ? 'enabled' + : 'disabled' + }` + ) + .incrementCallCount() + metrics + .getOrCreateMetric( + `${LOGGING.LABELS}${ + loggingConfig && config.application_logging.forwarding.labels.enabled + ? 'enabled' + : 'disabled' + }` + ) + .incrementCallCount() +} diff --git a/lib/util/label-parser.js b/lib/util/label-parser.js index 3fc17e8123..ca59044ecf 100644 --- a/lib/util/label-parser.js +++ b/lib/util/label-parser.js @@ -11,10 +11,11 @@ module.exports.fromMap = fromMap // this creates a copy of trim that can be used with map const trim = Function.prototype.call.bind(String.prototype.trim) -const logger = require('../logger').child({ component: 'label-parser' }) const stringify = require('json-stringify-safe') -function parse(labels) { +// pass in parent logger to avoid circular deps +function parse(labels, parentLogger) { + const logger = parentLogger.child({ component: 'label-parser' }) let results if (!labels) { @@ -25,8 +26,8 @@ function parse(labels) { results = fromMap(labels) } - results.warnings.forEach(function logWarnings(messaage) { - logger.warn(messaage) + results.warnings.forEach(function logWarnings(message) { + logger.warn(message) }) return results.labels @@ -106,11 +107,11 @@ function fromMap(map) { try { warnings.unshift('Partially Invalid Label Setting: ' + stringify(map)) } catch (err) { - logger.debug(err, 'Failed to stringify labels') + warnings.unshift('Failed to stringify labels: ' + err.message) } } - return { labels: labels, warnings: warnings } + return { labels, warnings } } function truncate(str, max) { diff --git a/test/unit/agent/agent.test.js b/test/unit/agent/agent.test.js index cd7e46596d..8a43178785 100644 --- a/test/unit/agent/agent.test.js +++ b/test/unit/agent/agent.test.js @@ -949,7 +949,7 @@ test('when event_harvest_config update on connect with a valid config', async (t }) test('logging supportability on connect', async (t) => { - const keys = ['Forwarding', 'Metrics', 'LocalDecorating'] + const keys = ['Forwarding', 'Metrics', 'LocalDecorating', 'Labels'] t.beforeEach((ctx) => { ctx.nr = {} @@ -967,12 +967,13 @@ test('logging supportability on connect', async (t) => { agent.config.application_logging.metrics.enabled = false agent.config.application_logging.forwarding.enabled = false agent.config.application_logging.local_decorating.enabled = false + agent.config.application_logging.forwarding.labels.enabled = false agent.onConnect(false, () => { for (const key of keys) { const disabled = agent.metrics.getMetric(`Supportability/Logging/${key}/Nodejs/disabled`) const enabled = agent.metrics.getMetric(`Supportability/Logging/${key}/Nodejs/enabled`) - assert.equal(disabled.callCount, 1) - assert.equal(enabled, undefined) + assert.equal(disabled.callCount, 1, `${key} should be disabled`) + assert.equal(enabled, undefined, `${key} should not be enabled`) } end() }) @@ -987,12 +988,13 @@ test('logging supportability on connect', async (t) => { agent.config.application_logging.metrics.enabled = true agent.config.application_logging.forwarding.enabled = true agent.config.application_logging.local_decorating.enabled = true + agent.config.application_logging.forwarding.labels.enabled = true agent.onConnect(false, () => { for (const key of keys) { const disabled = agent.metrics.getMetric(`Supportability/Logging/${key}/Nodejs/disabled`) const enabled = agent.metrics.getMetric(`Supportability/Logging/${key}/Nodejs/enabled`) - assert.equal(disabled.callCount, 1) - assert.equal(enabled, undefined) + assert.equal(disabled.callCount, 1, `${key} should be disabled`) + assert.equal(enabled, undefined, `${key} should not be enabled`) } end() }) @@ -1006,12 +1008,13 @@ test('logging supportability on connect', async (t) => { agent.config.application_logging.metrics.enabled = true agent.config.application_logging.forwarding.enabled = true agent.config.application_logging.local_decorating.enabled = true + agent.config.application_logging.forwarding.labels.enabled = true agent.onConnect(false, () => { for (const key of keys) { const disabled = agent.metrics.getMetric(`Supportability/Logging/${key}/Nodejs/disabled`) const enabled = agent.metrics.getMetric(`Supportability/Logging/${key}/Nodejs/enabled`) - assert.equal(enabled.callCount, 1) - assert.equal(disabled, undefined) + assert.equal(enabled.callCount, 1, `${key} should be enabled`) + assert.equal(disabled, undefined, `${key} should not be enabled`) } end() }) diff --git a/test/unit/aggregators/log-aggregator.test.js b/test/unit/aggregators/log-aggregator.test.js index 382182e49d..db2c77fe18 100644 --- a/test/unit/aggregators/log-aggregator.test.js +++ b/test/unit/aggregators/log-aggregator.test.js @@ -10,7 +10,6 @@ const assert = require('node:assert') const LogAggregator = require('../../../lib/aggregators/log-aggregator') const Metrics = require('../../../lib/metrics') const helper = require('../../lib/agent_helper') - const RUN_ID = 1337 const LIMIT = 5 @@ -41,6 +40,21 @@ test('Log Aggregator', async (t) => { harvest_limits: { log_event_data: 42 } + }, + application_logging: { + metrics: { + enabled: true + }, + local_decorating: { + enabled: true + }, + forwarding: { + enabled: true, + labels: { + enabled: true, + exclude: [] + } + } } } } @@ -175,6 +189,38 @@ test('Log Aggregator', async (t) => { logEventAggregator.addBatch(logs, priority) assert.equal(logEventAggregator.getEvents().length, 3) }) + + await t.test('add labels to logs when enabled', (t) => { + const { agent, commonAttrs, logEventAggregator, log } = t.nr + const expectedLabels = { + 'tags.label1': 'value1', + 'tags.LABEL2-ALSO': 'value3' + } + agent.config.loggingLabels = expectedLabels + + logEventAggregator.add(log) + const payload = logEventAggregator._toPayloadSync() + assert.deepStrictEqual(payload, [ + { common: { attributes: { ...commonAttrs, ...expectedLabels } }, logs: [log] } + ]) + }) + + await t.test( + 'should not add labels to logs when `application_logging.forwarding.enabled` is false', + (t) => { + const { agent, commonAttrs, logEventAggregator, log } = t.nr + const expectedLabels = { + 'tags.label1': 'value1', + 'tags.LABEL2-ALSO': 'value3' + } + agent.config.loggingLabels = expectedLabels + agent.config.application_logging.forwarding.labels.enabled = false + + logEventAggregator.add(log) + const payload = logEventAggregator._toPayloadSync() + assert.deepStrictEqual(payload, [{ common: { attributes: { ...commonAttrs } }, logs: [log] }]) + } + ) }) test('big red button', async (t) => { diff --git a/test/unit/collector/facts.test.js b/test/unit/collector/facts.test.js index a4ffb2dd66..2eb7802088 100644 --- a/test/unit/collector/facts.test.js +++ b/test/unit/collector/facts.test.js @@ -15,6 +15,7 @@ const helper = require('../../lib/agent_helper') const sysInfo = require('../../../lib/system-info') const utilTests = require('../../lib/cross_agent_tests/utilization/utilization_json') const bootIdTests = require('../../lib/cross_agent_tests/utilization/boot_id') +const parseLabels = require('../../../lib/util/label-parser') const APP_NAMES = ['a', 'c', 'b'] const DISABLE_ALL_DETECTIONS = { @@ -49,9 +50,13 @@ test('fun facts about apps that New Relic is interested in including', async (t) const logs = { debug: [], - trace: [] + trace: [], + warn: [] } const logger = { + warn(...args) { + logs.warn.push(args) + }, debug(...args) { logs.debug.push(args) }, @@ -195,13 +200,16 @@ test('fun facts about apps that New Relic is interested in including', async (t) }) await t.test('should convert label object to expected format', (t, end) => { - const { agent, facts } = t.nr + const { agent, logger, facts } = t.nr const longKey = '€'.repeat(257) const longValue = '𝌆'.repeat(257) - agent.config.labels = { - a: 'b', - [longKey]: longValue - } + agent.config.parsedLabels = parseLabels( + { + a: 'b', + [longKey]: longValue + }, + { child: () => logger } + ) facts(agent, (result) => { const expected = [ { label_type: 'a', label_value: 'b' }, @@ -213,10 +221,12 @@ test('fun facts about apps that New Relic is interested in including', async (t) }) await t.test('should convert label string to expected format', (t, end) => { - const { agent, facts } = t.nr + const { agent, logger, facts } = t.nr const longKey = '€'.repeat(257) const longValue = '𝌆'.repeat(257) - agent.config.labels = `a: b; ${longKey}: ${longValue}` + agent.config.parsedLabels = parseLabels(`a: b; ${longKey}: ${longValue}`, { + child: () => logger + }) facts(agent, (result) => { const expected = [ { label_type: 'a', label_value: 'b' }, diff --git a/test/unit/config/config.test.js b/test/unit/config/config.test.js index f2bb4e15c9..f2d5a1a7c4 100644 --- a/test/unit/config/config.test.js +++ b/test/unit/config/config.test.js @@ -146,3 +146,80 @@ test('#publicSettings', async (t) => { assert.deepStrictEqual(configuration.applications(), ['test app name']) }) }) + +test('parsedLabels', () => { + const longKey = 'a'.repeat(257) + const longValue = 'b'.repeat(257) + const configuration = Config.initialize({ labels: `a: b; ${longKey}: ${longValue}` }) + assert.deepEqual(configuration.parsedLabels, [ + { label_type: 'a', label_value: 'b' }, + { label_type: 'a'.repeat(255), label_value: 'b'.repeat(255) } + ]) +}) + +test('loggingLabels', async (t) => { + await t.test('should exclude labels regardless of case', () => { + const config = { + labels: { + 'label1': 'value1', + 'LABEL2': 'value2', + 'LABEL2-ALSO': 'value3' + }, + application_logging: { + forwarding: { + labels: { + enabled: true, + exclude: ['LaBeL2'] + } + } + } + } + + const configuration = Config.initialize(config) + const expectedLabels = { + 'tags.label1': 'value1', + 'tags.LABEL2-ALSO': 'value3' + } + + assert.deepEqual(configuration.loggingLabels, expectedLabels) + }) + + await t.test( + 'should not add applicationLabels when `application_logging.forwarding.labels.enabled` is false', + () => { + const config = { + labels: { + 'label1': 'value1', + 'LABEL2': 'value2', + 'LABEL2-ALSO': 'value3' + }, + application_logging: { + forwarding: { + labels: { + enabled: false + } + } + } + } + + const configuration = Config.initialize(config) + assert.deepEqual(configuration.loggingLabels, undefined) + } + ) + + await t.test('should not applicationLabels if no labels defined', () => { + const config = { + labels: {}, + application_logging: { + forwarding: { + labels: { + enabled: true + } + } + } + } + + const configuration = Config.initialize(config) + assert.deepEqual(configuration.loggingLabels, {}) + }) +}) diff --git a/test/unit/feature_flag.test.js b/test/unit/feature_flag.test.js index 12dcff62ae..43859d9873 100644 --- a/test/unit/feature_flag.test.js +++ b/test/unit/feature_flag.test.js @@ -105,6 +105,11 @@ test('should account for all *used* keys', (t) => { test('should warn if released flags are still in config', () => { let called = false Config.prototype.setLogger({ + child() { + return { + warn() {} + } + }, warn() { called = true }, @@ -119,6 +124,11 @@ test('should warn if released flags are still in config', () => { test('should warn if unreleased flags are still in config', () => { let called = false Config.prototype.setLogger({ + child() { + return { + warn() {} + } + }, warn() { called = true }, diff --git a/test/unit/util/application-logging.test.js b/test/unit/util/application-logging.test.js index 2cdc84ad6a..a59cc59f3c 100644 --- a/test/unit/util/application-logging.test.js +++ b/test/unit/util/application-logging.test.js @@ -79,6 +79,9 @@ test('Application Logging Config Tests', async (t) => { enabled: false }, forwarding: { + labels: { + enabled: false + }, enabled: false }, local_decorating: { @@ -124,11 +127,22 @@ test('Application Logging Config Tests', async (t) => { const { config } = t.nr assert.equal(loggingUtils.isApplicationLoggingEnabled(config), false) }) + + await t.test('should be true when application_logging.forwarding.labels is true', (t) => { + const { config } = t.nr + config.application_logging.forwarding.labels.enabled = true + assert.equal(loggingUtils.isLogLabelingEnabled(config), true) + }) + + await t.test('should be false when application_logging.forwarding.labels is false', (t) => { + const { config } = t.nr + config.application_logging.forwarding.labels.enabled = false + assert.equal(loggingUtils.isLogLabelingEnabled(config), false) + }) }) test('incrementLoggingLinesMetrics', async (t) => { t.beforeEach((ctx) => { - console.log('before test') ctx.nr = {} const callCountStub = { incrementCallCount: sinon.stub() } ctx.nr.metricsStub = {