Merge pull request #159 from ovotech/ae-1869

Remove unwanted tags
ovotech · Feb 20, 2023 · bd9093d · bd9093d
2 parents e097868 + 41c790e
commit bd9093d
Show file tree

Hide file tree

Showing 9 changed files with 22 additions and 57 deletions.
diff --git a/packages/datadog-metrics-tracker/package.json b/packages/datadog-metrics-tracker/package.json
@@ -1,7 +1,7 @@
 {
   "name": "@ovotech/datadog-metrics-tracker",
   "description": "Track metrics and store them in DataDog",
-  "version": "1.3.15",
+  "version": "1.3.16",
   "main": "dist/index.js",
   "source": "src/index.ts",
   "types": "dist/index.d.ts",

diff --git a/packages/datadog-metrics-tracker/src/base.ts b/packages/datadog-metrics-tracker/src/base.ts
@@ -3,7 +3,6 @@ import { StatsD } from 'hot-shots';
 
 interface Point {
   tags: { [name: string]: string };
-  fields: { [name: string]: any };
   measurement: string;
   value?: number;
 }
@@ -19,12 +18,7 @@ export abstract class MetricsTracker {
     this.sendPointsToDatadog = this.sendPointsToDatadog.bind(this);
   }
 
-  protected async trackPoint(
-    measurementName: string,
-    tags: { [name: string]: string },
-    fields: { [name: string]: any },
-    value?: number,
-  ) {
+  protected async trackPoint(measurementName: string, tags: { [name: string]: string }, value?: number) {
     const validTags = this.getValidTags(tags);
     this.logInvalidTags(measurementName, tags);
 
@@ -36,15 +30,13 @@ export abstract class MetricsTracker {
           ...this.staticMeta,
           ...validTags,
         },
-        fields,
         value,
       });
       return;
     } catch (err) {
       this.logger.error('Error tracking Datadog metric', {
         metric: measurementName,
         tags: JSON.stringify(validTags),
-        fields: JSON.stringify(fields),
         error: err,
       });
       return;
@@ -53,9 +45,9 @@ export abstract class MetricsTracker {
 
   private async sendPointsToDatadog(point: Point) {
     this.logger.info(`Sending metrics to Datadog`);
-    const { measurement, tags, fields, value } = point;
+    const { measurement, tags, value } = point;
     //datadog metrics tag
-    this.dogstatsd.distribution(measurement, value || 1, { ...tags, ...fields });
+    this.dogstatsd.distribution(measurement, value || 1, { ...tags });
   }
 
   private getInvalidTagNames(tags: { [name: string]: string }) {

diff --git a/packages/datadog-metrics-tracker/src/external-request.ts b/packages/datadog-metrics-tracker/src/external-request.ts
@@ -7,7 +7,6 @@ export class ExternalRequestMetricsTracker extends MetricsTracker {
     await this.trackPoint(
       ExternalRequestMetricsTracker.externalRequestTimeMeasurementName,
       { requestName, externalServiceName, status: statusCode ? statusCode.toString() : '' },
-      { timeMs: Math.round(timeMs), count: 1 },
       Math.round(timeMs),
     );
   }

diff --git a/packages/datadog-metrics-tracker/src/kafka.ts b/packages/datadog-metrics-tracker/src/kafka.ts
@@ -5,19 +5,11 @@ export class KafkaMetricsTracker extends MetricsTracker {
   private static eventReceivedMeasurementName = 'kafka-event-received';
 
   async trackEventReceived(eventName: string, ageMs: number) {
-    await this.trackPoint(
-      KafkaMetricsTracker.eventReceivedMeasurementName,
-      { eventName },
-      { count: 1, ageMs: Math.round(ageMs) },
-    );
+    await this.trackPoint(KafkaMetricsTracker.eventReceivedMeasurementName, { eventName });
   }
 
   async trackEventProcessed(eventName: string, processingState: ProcessingState) {
-    await this.trackPoint(
-      KafkaMetricsTracker.eventProcessedMeasurementName,
-      { eventName, processingState },
-      { count: 1 },
-    );
+    await this.trackPoint(KafkaMetricsTracker.eventProcessedMeasurementName, { eventName, processingState });
   }
 }
 

diff --git a/packages/datadog-metrics-tracker/src/response.ts b/packages/datadog-metrics-tracker/src/response.ts
@@ -7,7 +7,6 @@ export class ResponseMetricsTracker extends MetricsTracker {
     await this.trackPoint(
       ResponseMetricsTracker.ownResponseTimeMeasurementName,
       { requestName, status: statusCode ? statusCode.toString() : '' },
-      { timeMs: Math.round(timeMs), count: 1 },
       Math.round(timeMs),
     );
   }

diff --git a/packages/datadog-metrics-tracker/test/base.spec.ts b/packages/datadog-metrics-tracker/test/base.spec.ts
@@ -5,8 +5,8 @@ const testMeasurementName = 'test-metrics';
 export class TestTracker extends MetricsTracker {
   private static testMeasurementName = testMeasurementName;
 
-  async trackSomething(tags: { [name: string]: string }, fields: { [name: string]: any }) {
-    await this.trackPoint(TestTracker.testMeasurementName, tags, fields);
+  async trackSomething(tags: { [name: string]: string }) {
+    await this.trackPoint(TestTracker.testMeasurementName, tags);
   }
 }
 
@@ -29,36 +29,36 @@ describe('Base metrics class', () => {
   it('Should track valid tags and write the points to Datadog in a batch call', async () => {
     const tags = { value: 'A string' };
 
-    await tracker.trackSomething(tags, {});
+    await tracker.trackSomething(tags);
     const data = {
       ...metricsMeta,
       ...tags,
     };
-    expect(mockDatadog.distribution).toHaveBeenLastCalledWith(testMeasurementName,1, data);
+    expect(mockDatadog.distribution).toHaveBeenLastCalledWith(testMeasurementName, 1, data);
   });
 
   it('Should track valid metrics and write the points to Datadog in a batch call', async () => {
-    const metrics = { string: 'A string', integer: 3, float: 1.23, boolean: true };
+    const metrics = { string: 'A string' };
 
-    await tracker.trackSomething({}, metrics);
+    await tracker.trackSomething(metrics);
     const data = {
       ...metricsMeta,
       ...metrics,
     };
-    expect(mockDatadog.distribution).toHaveBeenLastCalledWith(testMeasurementName,1, data);
+    expect(mockDatadog.distribution).toHaveBeenLastCalledWith(testMeasurementName, 1, data);
   });
 
   it('Should log rather than track that have empty values', async () => {
     const validTags = { validTag: 'A string' };
     const invalidTags = { invalidTag: '', anotherInvalidTag: '' };
 
-    await tracker.trackSomething({ ...validTags, ...invalidTags }, {});
+    await tracker.trackSomething({ ...validTags, ...invalidTags });
     const data = {
       ...metricsMeta,
       ...validTags,
     };
     jest.runTimersToTime(60000);
-    expect(mockDatadog.distribution).toHaveBeenLastCalledWith(testMeasurementName,1, data);
+    expect(mockDatadog.distribution).toHaveBeenLastCalledWith(testMeasurementName, 1, data);
     expect(mockLogger.warn).toHaveBeenLastCalledWith('Attempted to track tags with no value', {
       metric: testMeasurementName,
       tagNames: 'anotherInvalidTag, invalidTag',

diff --git a/packages/datadog-metrics-tracker/test/external-request.spec.ts b/packages/datadog-metrics-tracker/test/external-request.spec.ts
@@ -20,10 +20,8 @@ describe('Track actions relating to consuming an event from Kafka', () => {
     const data = {
       externalServiceName,
       requestName,
-      count: 1,
-      timeMs: 1234,
     };
-    expect(mockDatadog.distribution).toHaveBeenLastCalledWith('external-request-time',timeMs, data);
+    expect(mockDatadog.distribution).toHaveBeenLastCalledWith('external-request-time', timeMs, data);
   });
 
   it.each([200, 404, 500])('Should track a request time with a status code: %d', async statusCode => {
@@ -36,10 +34,8 @@ describe('Track actions relating to consuming an event from Kafka', () => {
       externalServiceName,
       requestName,
       status: statusCode.toString(10),
-      count: 1,
-      timeMs: 123,
     };
-    expect(mockDatadog.distribution).toHaveBeenLastCalledWith('external-request-time',timeMs, data);
+    expect(mockDatadog.distribution).toHaveBeenLastCalledWith('external-request-time', timeMs, data);
   });
 
   it.each([
@@ -54,8 +50,6 @@ describe('Track actions relating to consuming an event from Kafka', () => {
     const data = {
       externalServiceName,
       requestName,
-      count: 1,
-      timeMs: expectedTrackedTime,
     };
     expect(mockDatadog.distribution).toHaveBeenLastCalledWith('external-request-time', expectedTrackedTime, data);
   });

diff --git a/packages/datadog-metrics-tracker/test/kafka.spec.ts b/packages/datadog-metrics-tracker/test/kafka.spec.ts
@@ -18,10 +18,8 @@ describe('Track actions relating to consuming an event from Kafka', () => {
     await tracker.trackEventReceived(eventName, ageMs);
     const data = {
       eventName,
-      count: 1,
-      ageMs,
     };
-    expect(mockDatadog.distribution).toHaveBeenLastCalledWith('kafka-event-received',1, data);
+    expect(mockDatadog.distribution).toHaveBeenLastCalledWith('kafka-event-received', 1, data);
   });
 
   it.each([
@@ -34,10 +32,8 @@ describe('Track actions relating to consuming an event from Kafka', () => {
     await tracker.trackEventReceived(eventName, exactAge);
     const data = {
       eventName,
-      count: 1,
-      ageMs: expectedTrackedAge,
     };
-    expect(mockDatadog.distribution).toHaveBeenLastCalledWith('kafka-event-received', 1,data);
+    expect(mockDatadog.distribution).toHaveBeenLastCalledWith('kafka-event-received', 1, data);
   });
 
   it.each([ProcessingState.Error, ProcessingState.Success])(
@@ -49,7 +45,6 @@ describe('Track actions relating to consuming an event from Kafka', () => {
       const data = {
         eventName,
         processingState,
-        count: 1,
       };
       expect(mockDatadog.distribution).toHaveBeenLastCalledWith('kafka-event-processed', 1, data);
     },

diff --git a/packages/datadog-metrics-tracker/test/response.spec.ts b/packages/datadog-metrics-tracker/test/response.spec.ts
@@ -18,10 +18,8 @@ describe('Track actions relating to responding to an API request', () => {
     await tracker.trackOwnResponseTime(requestName, timeMs);
     const data = {
       requestName,
-      count: 1,
-      timeMs,
     };
-    expect(mockDatadog.distribution).toHaveBeenLastCalledWith('own-response-time',timeMs, data);
+    expect(mockDatadog.distribution).toHaveBeenLastCalledWith('own-response-time', timeMs, data);
   });
 
   it.each([200, 404, 500])('Should track a response time with a status code: %d', async statusCode => {
@@ -32,10 +30,8 @@ describe('Track actions relating to responding to an API request', () => {
     const data = {
       requestName,
       status: statusCode.toString(10),
-      count: 1,
-      timeMs,
     };
-    expect(mockDatadog.distribution).toHaveBeenLastCalledWith('own-response-time',timeMs, data);
+    expect(mockDatadog.distribution).toHaveBeenLastCalledWith('own-response-time', timeMs, data);
   });
 
   it.each([
@@ -48,9 +44,7 @@ describe('Track actions relating to responding to an API request', () => {
     await tracker.trackOwnResponseTime(requestName, exactTime);
     const data = {
       requestName,
-      count: 1,
-      timeMs: expectedTrackedTime,
     };
-    expect(mockDatadog.distribution).toHaveBeenLastCalledWith('own-response-time',expectedTrackedTime, data);
+    expect(mockDatadog.distribution).toHaveBeenLastCalledWith('own-response-time', expectedTrackedTime, data);
   });
 });