From 9bcd34256221d800ceeb9a0444007a44a7675e30 Mon Sep 17 00:00:00 2001 From: Dylan Yang Date: Wed, 20 Mar 2024 10:58:37 -0400 Subject: [PATCH 1/6] SVLS-4422: support metrics with timestamps when the extension is running --- src/metrics/listener.spec.ts | 38 ++++++++++++++++++++++++++++++++++++ src/metrics/listener.ts | 20 ++++++++++++++++--- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/src/metrics/listener.spec.ts b/src/metrics/listener.spec.ts index 233ad044..15516421 100644 --- a/src/metrics/listener.spec.ts +++ b/src/metrics/listener.spec.ts @@ -137,6 +137,44 @@ describe("MetricsListener", () => { expect(flushScope.isDone()).toBeTruthy(); expect(distributionMock).toHaveBeenCalledWith("my-metric", 10, undefined, ["tag:a", "tag:b"]); }); + + it("only sends metrics with timestamps to the API when the extension is enabled", async () => { + const flushScope = nock(EXTENSION_URL).post("/lambda/flush", JSON.stringify({})).reply(200); + mock({ + "/opt/extensions/datadog-agent": Buffer.from([0]), + }); + nock("https://api.example.com").post("/api/v1/distribution_points?api_key=api-key").reply(200, {}); + + const distributionMock = jest.fn(); + (StatsDClient as any).mockImplementation(() => { + return { + distribution: distributionMock, + close: (callback: any) => callback(undefined), + }; + }); + + jest.spyOn(Date, "now").mockImplementation(() => 1487076708000); + + const metricTimeOneMinuteAgo = new Date(Date.now() - 60000) + const kms = new MockKMS("kms-api-key-decrypted"); + const listener = new MetricsListener(kms as any, { + apiKey: "api-key", + apiKeyKMS: "", + enhancedMetrics: false, + logForwarding: true, + shouldRetryMetrics: false, + localTesting: true, + siteURL, + }); + + await listener.onStartInvocation({}); + listener.sendDistributionMetricWithDate("my-metric-with-a-timestamp", 10, metricTimeOneMinuteAgo, false, "tag:a", "tag:b"); + listener.sendDistributionMetric("my-metric-without-a-timestamp", 10, false, "tag:a", "tag:b"); + await listener.onCompleteInvocation(); + expect(flushScope.isDone()).toBeTruthy(); + expect(nock.isDone()).toBeTruthy(); + expect(distributionMock).toHaveBeenCalledWith("my-metric-without-a-timestamp", 10, undefined, ["tag:a", "tag:b"]); + }); it("logs metrics when logForwarding is enabled with custom timestamp", async () => { const spy = jest.spyOn(process.stdout, "write"); diff --git a/src/metrics/listener.ts b/src/metrics/listener.ts index d75b1023..aabff824 100644 --- a/src/metrics/listener.ts +++ b/src/metrics/listener.ts @@ -140,7 +140,21 @@ export class MetricsListener { forceAsync: boolean, ...tags: string[] ) { + const dist = new Distribution(name, [{ timestamp: metricTime, value }], ...tags); + if (this.isExtensionRunning) { + // Dogstatsd doesn't support distribution metrics with timestamps so we must use the API + const isMetricTimeValid = Date.parse(metricTime.toString()) > 0; + if (isMetricTimeValid) { + if (this.currentProcessor === undefined) { + this.currentProcessor = this.createProcessor(this.config, this.apiKey); + } + this.currentProcessor.then((processor) => { + processor.addMetric(dist); + }); + return; + }; + this.statsDClient?.distribution(name, value, undefined, tags); return; } @@ -149,8 +163,6 @@ export class MetricsListener { return; } - const dist = new Distribution(name, [{ timestamp: metricTime, value }], ...tags); - if (!this.apiKey) { const errorMessage = "api key not configured, see https://dtdg.co/sls-node-metrics"; logError(errorMessage); @@ -167,7 +179,9 @@ export class MetricsListener { } public sendDistributionMetric(name: string, value: number, forceAsync: boolean, ...tags: string[]) { - this.sendDistributionMetricWithDate(name, value, new Date(Date.now()), forceAsync, ...tags); + // The Extension doesn't support distribution metrics with timestamps. Use sendDistributionMetricWithDate instead. + const metricTime = this.isExtensionRunning ? new Date(0) : new Date(Date.now()); + this.sendDistributionMetricWithDate(name, value, metricTime, forceAsync, ...tags); } private async createProcessor(config: MetricsConfig, apiKey: Promise) { From 9afe1a1ebf65341d79497737000aa902e7c79294 Mon Sep 17 00:00:00 2001 From: Dylan Yang Date: Wed, 20 Mar 2024 11:05:02 -0400 Subject: [PATCH 2/6] make pretty --- src/metrics/listener.spec.ts | 13 ++++++++++--- src/metrics/listener.ts | 2 +- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/metrics/listener.spec.ts b/src/metrics/listener.spec.ts index 15516421..93b18b35 100644 --- a/src/metrics/listener.spec.ts +++ b/src/metrics/listener.spec.ts @@ -137,7 +137,7 @@ describe("MetricsListener", () => { expect(flushScope.isDone()).toBeTruthy(); expect(distributionMock).toHaveBeenCalledWith("my-metric", 10, undefined, ["tag:a", "tag:b"]); }); - + it("only sends metrics with timestamps to the API when the extension is enabled", async () => { const flushScope = nock(EXTENSION_URL).post("/lambda/flush", JSON.stringify({})).reply(200); mock({ @@ -155,7 +155,7 @@ describe("MetricsListener", () => { jest.spyOn(Date, "now").mockImplementation(() => 1487076708000); - const metricTimeOneMinuteAgo = new Date(Date.now() - 60000) + const metricTimeOneMinuteAgo = new Date(Date.now() - 60000); const kms = new MockKMS("kms-api-key-decrypted"); const listener = new MetricsListener(kms as any, { apiKey: "api-key", @@ -168,7 +168,14 @@ describe("MetricsListener", () => { }); await listener.onStartInvocation({}); - listener.sendDistributionMetricWithDate("my-metric-with-a-timestamp", 10, metricTimeOneMinuteAgo, false, "tag:a", "tag:b"); + listener.sendDistributionMetricWithDate( + "my-metric-with-a-timestamp", + 10, + metricTimeOneMinuteAgo, + false, + "tag:a", + "tag:b", + ); listener.sendDistributionMetric("my-metric-without-a-timestamp", 10, false, "tag:a", "tag:b"); await listener.onCompleteInvocation(); expect(flushScope.isDone()).toBeTruthy(); diff --git a/src/metrics/listener.ts b/src/metrics/listener.ts index aabff824..6702bd41 100644 --- a/src/metrics/listener.ts +++ b/src/metrics/listener.ts @@ -153,7 +153,7 @@ export class MetricsListener { processor.addMetric(dist); }); return; - }; + } this.statsDClient?.distribution(name, value, undefined, tags); return; From 9146a1c41b7e117a0c797d9e5a26691ece3904b9 Mon Sep 17 00:00:00 2001 From: Dylan Yang Date: Wed, 20 Mar 2024 11:12:53 -0400 Subject: [PATCH 3/6] make linter happy --- src/metrics/listener.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/metrics/listener.ts b/src/metrics/listener.ts index 6702bd41..3df97437 100644 --- a/src/metrics/listener.ts +++ b/src/metrics/listener.ts @@ -149,6 +149,7 @@ export class MetricsListener { if (this.currentProcessor === undefined) { this.currentProcessor = this.createProcessor(this.config, this.apiKey); } + // tslint:disable-next-line: no-floating-promises this.currentProcessor.then((processor) => { processor.addMetric(dist); }); From 8383b3b86b7b0580b81bf018a21294f14ad30e66 Mon Sep 17 00:00:00 2001 From: Dylan Yang Date: Wed, 20 Mar 2024 12:35:48 -0400 Subject: [PATCH 4/6] cleanup --- src/metrics/listener.spec.ts | 2 +- src/metrics/listener.ts | 16 +++++----------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/metrics/listener.spec.ts b/src/metrics/listener.spec.ts index 93b18b35..89f6c7da 100644 --- a/src/metrics/listener.spec.ts +++ b/src/metrics/listener.spec.ts @@ -161,7 +161,7 @@ describe("MetricsListener", () => { apiKey: "api-key", apiKeyKMS: "", enhancedMetrics: false, - logForwarding: true, + logForwarding: false, shouldRetryMetrics: false, localTesting: true, siteURL, diff --git a/src/metrics/listener.ts b/src/metrics/listener.ts index 3df97437..119336e4 100644 --- a/src/metrics/listener.ts +++ b/src/metrics/listener.ts @@ -143,21 +143,15 @@ export class MetricsListener { const dist = new Distribution(name, [{ timestamp: metricTime, value }], ...tags); if (this.isExtensionRunning) { - // Dogstatsd doesn't support distribution metrics with timestamps so we must use the API const isMetricTimeValid = Date.parse(metricTime.toString()) > 0; if (isMetricTimeValid) { - if (this.currentProcessor === undefined) { - this.currentProcessor = this.createProcessor(this.config, this.apiKey); - } - // tslint:disable-next-line: no-floating-promises - this.currentProcessor.then((processor) => { - processor.addMetric(dist); - }); + // Only create the processor to submit metrics to the API when a user provides a valid timestamp as + // Dogstatsd does not support timestamps for distributions. + this.currentProcessor = this.createProcessor(this.config, this.apiKey); + } else { + this.statsDClient?.distribution(name, value, undefined, tags); return; } - - this.statsDClient?.distribution(name, value, undefined, tags); - return; } if (this.config.logForwarding || forceAsync) { writeMetricToStdout(name, value, metricTime, tags); From fb8f7f6a9122ee91b055c36eca8e771eb3a474bd Mon Sep 17 00:00:00 2001 From: Dylan Yang Date: Fri, 22 Mar 2024 13:11:29 -0400 Subject: [PATCH 5/6] adjust processor for extension --- src/metrics/listener.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/metrics/listener.ts b/src/metrics/listener.ts index 7e358a61..3b2bee74 100644 --- a/src/metrics/listener.ts +++ b/src/metrics/listener.ts @@ -135,8 +135,6 @@ export class MetricsListener { forceAsync: boolean, ...tags: string[] ) { - const dist = new Distribution(name, [{ timestamp: metricTime, value }], ...tags); - if (this.isExtensionRunning) { const isMetricTimeValid = Date.parse(metricTime.toString()) > 0; if (isMetricTimeValid) { @@ -153,6 +151,8 @@ export class MetricsListener { return; } + const dist = new Distribution(name, [{ timestamp: metricTime, value }], ...tags); + if (!this.apiKey) { const errorMessage = "api key not configured, see https://dtdg.co/sls-node-metrics"; logError(errorMessage); @@ -175,7 +175,7 @@ export class MetricsListener { } private async createProcessor(config: MetricsConfig, apiKey: Promise) { - if (!this.isExtensionRunning && !this.config.logForwarding) { + if (!this.config.logForwarding) { const { APIClient } = require("./api"); const { Processor } = require("./processor"); From ddd738eb1149c36d46d9405dd1ce1eec71376e52 Mon Sep 17 00:00:00 2001 From: Dylan Yang Date: Fri, 22 Mar 2024 13:13:02 -0400 Subject: [PATCH 6/6] add TODO comment --- src/metrics/listener.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/metrics/listener.ts b/src/metrics/listener.ts index 3b2bee74..c8e4100c 100644 --- a/src/metrics/listener.ts +++ b/src/metrics/listener.ts @@ -131,7 +131,7 @@ export class MetricsListener { public sendDistributionMetricWithDate( name: string, value: number, - metricTime: Date, + metricTime: Date, // TODO: Next breaking change to update to optional or 'Date | undefined'? forceAsync: boolean, ...tags: string[] ) {