From fe37d188e223bbfc56f746e3776b47f4fd1e43a0 Mon Sep 17 00:00:00 2001 From: Yoann Moinet Date: Tue, 19 Nov 2024 15:24:23 -0500 Subject: [PATCH] Use Set instead of arrays for metrics aggregation --- .../telemetry/src/common/aggregator.ts | 113 +++++++++--------- .../telemetry/src/common/metrics/common.ts | 49 +++----- .../telemetry/src/common/output/files.ts | 10 +- .../plugins/telemetry/src/common/sender.ts | 20 +++- packages/plugins/telemetry/src/index.ts | 7 +- .../telemetry/common/aggregator.test.ts | 4 +- .../telemetry/common/output/files.test.ts | 2 +- .../tests/src/plugins/telemetry/index.test.ts | 25 ++-- 8 files changed, 114 insertions(+), 116 deletions(-) diff --git a/packages/plugins/telemetry/src/common/aggregator.ts b/packages/plugins/telemetry/src/common/aggregator.ts index 10540327c..ed88591ad 100644 --- a/packages/plugins/telemetry/src/common/aggregator.ts +++ b/packages/plugins/telemetry/src/common/aggregator.ts @@ -6,10 +6,9 @@ import type { GlobalContext } from '@dd/core/types'; import type { Metric, MetricToSend, OptionsDD, Report } from '@dd/telemetry-plugin/types'; import { getMetric } from './helpers'; -import { getPlugins, getLoaders } from './metrics/common'; +import { addPluginMetrics, addLoaderMetrics } from './metrics/common'; -const getUniversalMetrics = (globalContext: GlobalContext) => { - const metrics: Metric[] = []; +const addUniversalMetrics = (globalContext: GlobalContext, metrics: Set) => { const inputs = globalContext.build.inputs || []; const outputs = globalContext.build.outputs || []; const entries = globalContext.build.entries || []; @@ -48,41 +47,40 @@ const getUniversalMetrics = (globalContext: GlobalContext) => { } // Counts - metrics.push( - { + metrics + .add({ metric: 'assets.count', type: 'count', value: outputs.length, tags: [], - }, - { + }) + .add({ metric: 'entries.count', type: 'count', value: entries.length, tags: [], - }, - { + }) + .add({ metric: 'errors.count', type: 'count', value: nbErrors, tags: [], - }, - { + }) + .add({ metric: 'modules.count', type: 'count', value: inputs.length, tags: [], - }, - { + }) + .add({ metric: 'warnings.count', type: 'count', value: nbWarnings, tags: [], - }, - ); + }); if (duration) { - metrics.push({ + metrics.add({ metric: 'compilation.duration', type: 'duration', value: duration, @@ -106,26 +104,25 @@ const getUniversalMetrics = (globalContext: GlobalContext) => { ...assetsPerInput.get(input.filepath)!.map((assetName) => `assetName:${assetName}`), ); } - metrics.push( - { + metrics + .add({ metric: 'modules.size', type: 'size', value: input.size, tags, - }, - { + }) + .add({ metric: 'modules.dependencies', type: 'count', value: input.dependencies.size, tags, - }, - { + }) + .add({ metric: 'modules.dependents', type: 'count', value: input.dependents.size, tags, - }, - ); + }); } // Assets @@ -139,87 +136,87 @@ const getUniversalMetrics = (globalContext: GlobalContext) => { .map((entryName) => `entryName:${entryName}`), ); } - metrics.push( - { + metrics + .add({ metric: 'assets.size', type: 'size', value: output.size, tags, - }, - { + }) + .add({ metric: 'assets.modules.count', type: 'count', value: output.inputs.length, tags, - }, - ); + }); } // Entries for (const entry of entries) { const tags = [`entryName:${entry.name}`]; - metrics.push( - { + metrics + .add({ metric: 'entries.size', type: 'size', value: entry.size, tags, - }, - { + }) + .add({ metric: 'entries.modules.count', type: 'count', value: entry.inputs.length, tags, - }, - { + }) + .add({ metric: 'entries.assets.count', type: 'count', value: entry.outputs.length, tags, - }, - ); + }); } return metrics; }; -export const getMetrics = ( +export const addMetrics = ( globalContext: GlobalContext, optionsDD: OptionsDD, + metricsToSend: Set, report?: Report, -): MetricToSend[] => { - const metrics: Metric[] = []; +): void => { + const metrics: Set = new Set(); if (report) { const { timings } = report; if (timings) { if (timings.tapables) { - metrics.push(...getPlugins(timings.tapables)); + addPluginMetrics(timings.tapables, metrics); } if (timings.loaders) { - metrics.push(...getLoaders(timings.loaders)); + addLoaderMetrics(timings.loaders, metrics); } } } - metrics.push(...getUniversalMetrics(globalContext)); + addUniversalMetrics(globalContext, metrics); // Format metrics to be DD ready and apply filters - const metricsToSend: MetricToSend[] = metrics - .map((m) => { - let metric: Metric | null = m; - if (optionsDD.filters?.length) { - for (const filter of optionsDD.filters) { - // Could have been filtered out by an early filter. - if (metric) { - metric = filter(metric); - } + for (const metric of metrics) { + if (optionsDD.filters?.length) { + let filteredMetric: Metric | null = metric; + for (const filter of optionsDD.filters) { + // If it's already been filtered out, no need to keep going. + if (!filteredMetric) { + break; } + filteredMetric = filter(metric); } - return metric ? getMetric(metric, optionsDD) : null; - }) - .filter((m) => m !== null) as MetricToSend[]; - - return metricsToSend; + if (filteredMetric) { + metricsToSend.add(getMetric(filteredMetric, optionsDD)); + } + } else { + metricsToSend.add(getMetric(metric, optionsDD)); + } + } }; diff --git a/packages/plugins/telemetry/src/common/metrics/common.ts b/packages/plugins/telemetry/src/common/metrics/common.ts index 050458dfa..62fccad10 100644 --- a/packages/plugins/telemetry/src/common/metrics/common.ts +++ b/packages/plugins/telemetry/src/common/metrics/common.ts @@ -4,10 +4,8 @@ import type { TimingsMap, Metric } from '@dd/telemetry-plugin/types'; -export const getPlugins = (plugins: TimingsMap): Metric[] => { - const metrics: Metric[] = []; - - metrics.push({ +export const addPluginMetrics = (plugins: TimingsMap, metrics: Set): void => { + metrics.add({ metric: 'plugins.count', type: 'count', value: plugins.size, @@ -26,45 +24,39 @@ export const getPlugins = (plugins: TimingsMap): Metric[] => { hookDuration += duration; pluginDuration += duration; } - metrics.push( - { + metrics + .add({ metric: 'plugins.hooks.duration', type: 'duration', value: hookDuration, tags: [`pluginName:${plugin.name}`, `hookName:${hook.name}`], - }, - { + }) + .add({ metric: 'plugins.hooks.increment', type: 'count', value: hook.values.length, tags: [`pluginName:${plugin.name}`, `hookName:${hook.name}`], - }, - ); + }); } - metrics.push( - { + metrics + .add({ metric: 'plugins.duration', type: 'duration', value: pluginDuration, tags: [`pluginName:${plugin.name}`], - }, - { + }) + .add({ metric: 'plugins.increment', type: 'count', value: pluginCount, tags: [`pluginName:${plugin.name}`], - }, - ); + }); } - - return metrics; }; -export const getLoaders = (loaders: TimingsMap): Metric[] => { - const metrics: Metric[] = []; - - metrics.push({ +export const addLoaderMetrics = (loaders: TimingsMap, metrics: Set): void => { + metrics.add({ metric: 'loaders.count', type: 'count', value: loaders.size, @@ -72,21 +64,18 @@ export const getLoaders = (loaders: TimingsMap): Metric[] => { }); for (const loader of loaders.values()) { - metrics.push( - { + metrics + .add({ metric: 'loaders.duration', type: 'duration', value: loader.duration, tags: [`loaderName:${loader.name}`], - }, - { + }) + .add({ metric: 'loaders.increment', type: 'count', value: loader.increment, tags: [`loaderName:${loader.name}`], - }, - ); + }); } - - return metrics; }; diff --git a/packages/plugins/telemetry/src/common/output/files.ts b/packages/plugins/telemetry/src/common/output/files.ts index cd8ef983f..b858fd69d 100644 --- a/packages/plugins/telemetry/src/common/output/files.ts +++ b/packages/plugins/telemetry/src/common/output/files.ts @@ -13,15 +13,15 @@ type FilesToWrite = { [key in Files]?: { content: any }; }; -export const outputFiles = async ( +export const outputFiles: ( data: { report?: Report; - metrics: MetricToSend[]; + metrics: Set; }, outputOptions: OutputOptions, log: Logger, cwd: string, -) => { +) => Promise = async (data, outputOptions, log, cwd) => { // Don't write any file if it's not enabled. if (typeof outputOptions !== 'string' && typeof outputOptions !== 'object' && !outputOptions) { return; @@ -66,8 +66,8 @@ export const outputFiles = async ( }; } - if (metrics && files.metrics) { - filesToWrite.metrics = { content: metrics }; + if (files.metrics) { + filesToWrite.metrics = { content: Array.from(metrics) }; } const proms = Object.entries(filesToWrite).map(async ([filename, file]): Promise => { diff --git a/packages/plugins/telemetry/src/common/sender.ts b/packages/plugins/telemetry/src/common/sender.ts index 243ec9c7e..c72978128 100644 --- a/packages/plugins/telemetry/src/common/sender.ts +++ b/packages/plugins/telemetry/src/common/sender.ts @@ -7,7 +7,7 @@ import type { Logger } from '@dd/core/types'; import type { MetricToSend } from '@dd/telemetry-plugin/types'; export const sendMetrics = ( - metrics: MetricToSend[] | undefined, + metrics: Set, auth: { apiKey?: string; endPoint: string }, log: Logger, ) => { @@ -16,17 +16,25 @@ export const sendMetrics = ( log.warn(`Won't send metrics to Datadog: missing API Key.`); return; } - if (!metrics || metrics.length === 0) { + if (!metrics.size) { log.warn(`No metrics to send.`); return; } - const metricsNames = [...new Set(metrics.map((m) => m.metric))] - .sort() - .map((name) => `${name} - ${metrics.filter((m) => m.metric === name).length}`); + const metricIterations: Map = new Map(); + for (const metric of metrics) { + if (!metricIterations.has(metric.metric)) { + metricIterations.set(metric.metric, 0); + } + metricIterations.set(metric.metric, metricIterations.get(metric.metric)! + 1); + } + + const metricsNames = Array.from(metricIterations.entries()).map( + ([name, count]) => `${name} - ${count}`, + ); log.debug(` -Sending ${metrics.length} metrics. +Sending ${metrics.size} metrics. Metrics: - ${metricsNames.join('\n - ')}`); diff --git a/packages/plugins/telemetry/src/index.ts b/packages/plugins/telemetry/src/index.ts index ec60405e6..6850db23e 100644 --- a/packages/plugins/telemetry/src/index.ts +++ b/packages/plugins/telemetry/src/index.ts @@ -4,7 +4,7 @@ import type { GlobalContext, GetPlugins, PluginOptions, Logger } from '@dd/core/types'; -import { getMetrics } from './common/aggregator'; +import { addMetrics } from './common/aggregator'; import { defaultFilters } from './common/filters'; import { getOptionsDD, validateOptions } from './common/helpers'; import { outputFiles } from './common/output/files'; @@ -16,6 +16,7 @@ import type { BundlerContext, Filter, Metric, + MetricToSend, OptionsWithTelemetry, TelemetryOptions, } from './types'; @@ -72,10 +73,10 @@ export const getPlugins: GetPlugins = ( context.build.duration = context.build.end - context.build.start!; context.build.writeDuration = context.build.end - realBuildEnd; - const metrics = []; + const metrics: Set = new Set(); const optionsDD = getOptionsDD(telemetryOptions); - metrics.push(...getMetrics(context, optionsDD, bundlerContext.report)); + addMetrics(context, optionsDD, metrics, bundlerContext.report); // TODO Extract the files output in an internal plugin. await outputFiles( diff --git a/packages/tests/src/plugins/telemetry/common/aggregator.test.ts b/packages/tests/src/plugins/telemetry/common/aggregator.test.ts index 0377892d2..027459d46 100644 --- a/packages/tests/src/plugins/telemetry/common/aggregator.test.ts +++ b/packages/tests/src/plugins/telemetry/common/aggregator.test.ts @@ -2,14 +2,14 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2019-Present Datadog, Inc. -import { getMetrics } from '@dd/telemetry-plugin/common/aggregator'; +import { addMetrics } from '@dd/telemetry-plugin/common/aggregator'; import { getContextMock } from '@dd/tests/_jest/helpers/mocks'; import { mockOptionsDD, mockReport } from '@dd/tests/plugins/telemetry/testHelpers'; describe('Telemetry Aggregator', () => { test('Should aggregate metrics without throwing.', () => { expect(() => { - getMetrics(getContextMock(), mockOptionsDD, mockReport); + addMetrics(getContextMock(), mockOptionsDD, new Set(), mockReport); }).not.toThrow(); }); }); diff --git a/packages/tests/src/plugins/telemetry/common/output/files.test.ts b/packages/tests/src/plugins/telemetry/common/output/files.test.ts index d4479ca97..7badb15a0 100644 --- a/packages/tests/src/plugins/telemetry/common/output/files.test.ts +++ b/packages/tests/src/plugins/telemetry/common/output/files.test.ts @@ -18,7 +18,7 @@ describe('Telemetry Output Files', () => { await outputFiles( { report: mockReport, - metrics: [], + metrics: new Set(), }, output, mockLogger, diff --git a/packages/tests/src/plugins/telemetry/index.test.ts b/packages/tests/src/plugins/telemetry/index.test.ts index a222fd64e..51bd84b27 100644 --- a/packages/tests/src/plugins/telemetry/index.test.ts +++ b/packages/tests/src/plugins/telemetry/index.test.ts @@ -3,7 +3,7 @@ // Copyright 2019-Present Datadog, Inc. import type { GlobalContext, Options } from '@dd/core/types'; -import { getMetrics } from '@dd/telemetry-plugin/common/aggregator'; +import { addMetrics } from '@dd/telemetry-plugin/common/aggregator'; import type { MetricToSend } from '@dd/telemetry-plugin/types'; import { FAKE_URL, @@ -20,15 +20,17 @@ jest.mock('@dd/telemetry-plugin/common/aggregator', () => { const originalModule = jest.requireActual('@dd/telemetry-plugin/common/aggregator'); return { ...originalModule, - getMetrics: jest.fn(), + addMetrics: jest.fn(), }; }); -const getMetricsMocked = jest.mocked(getMetrics); +const addMetricsMocked = jest.mocked(addMetrics); -const getGetMetricsImplem: (metrics: Record) => typeof getMetrics = - (metrics) => (context, options, report) => { - const originalModule = jest.requireActual('@dd/telemetry-plugin/common/aggregator'); +const getAddMetricsImplem: (metrics: Record) => typeof addMetrics = + (metrics) => (context, options, metricsToSend, report) => { + const originalModule = jest.requireActual< + typeof import('@dd/telemetry-plugin/common/aggregator') + >('@dd/telemetry-plugin/common/aggregator'); context.build.inputs = context.build.inputs?.filter(filterOutParticularities); context.build.entries = context.build.entries?.map((entry) => { return { @@ -36,9 +38,10 @@ const getGetMetricsImplem: (metrics: Record) => typeof g inputs: entry.inputs.filter(filterOutParticularities), }; }); - const originalResult = originalModule.getMetrics(context, options, report); - metrics[context.bundler.fullName] = originalResult; - return originalResult; + + originalModule.addMetrics(context, options, metricsToSend, report); + metrics[context.bundler.fullName] = Array.from(metricsToSend); + return metricsToSend; }; describe('Telemetry Universal Plugin', () => { @@ -126,7 +129,7 @@ describe('Telemetry Universal Plugin', () => { debugFilesPlugins(context), }; // This one is called at initialization, with the initial context. - getMetricsMocked.mockImplementation(getGetMetricsImplem(metrics)); + addMetricsMocked.mockImplementation(getAddMetricsImplem(metrics)); cleanup = await runBundlers( pluginConfig, getComplexBuildOverrides(), @@ -163,7 +166,7 @@ describe('Telemetry Universal Plugin', () => { debugFilesPlugins(context), }; // This one is called at initialization, with the initial context. - getMetricsMocked.mockImplementation(getGetMetricsImplem(metrics)); + addMetricsMocked.mockImplementation(getAddMetricsImplem(metrics)); cleanup = await runBundlers(pluginConfig, getComplexBuildOverrides()); });