Add missing decrement method to DogStatsDClient (#5330)

I ported an open PR and added additional fixes to the code.

The `NoopDogStatsDClient` gets used by default in unit tests.  However, it
is missing the `decrement` method which causes tests to throw exceptions
when trying to call it.

The `DogStatsDClient` is also missing the decrement method. Instead, the increment method was used by the the custom metric implementation. This is adjusted.

Please check the commit messages for further information.

Fixes #4285
Closes #5241 The `NoopDogStatsDClient` gets used by default in unit tests.  However, it
is missing the `decrement` method which causes tests to throw exceptions
when trying to call it.

---------

Co-authored-by: Justin Johnson <[email protected]>

Author: 2 people

index.d.ts

@@ -813,43 +813,43 @@ declare namespace tracer {
   export interface DogStatsD {
     /**
      * Increments a metric by the specified value, optionally specifying tags.
-     * @param {string} stat The dot-separated metric name.
-     * @param {number} value The amount to increment the stat by.
-     * @param {[tag:string]:string|number} tags Tags to pass along, such as `{ foo: 'bar' }`. Values are combined with config.tags.
+     * @param stat The dot-separated metric name.
+     * @param value The amount to increment the stat by.
+     * @param tags Tags to pass along, such as `{ foo: 'bar' }`. Values are combined with config.tags.
      */
-    increment(stat: string, value?: number, tags?: { [tag: string]: string|number }): void
+    increment(stat: string, value?: number, tags?: Record<string, string|number>): void
 
     /**
      * Decrements a metric by the specified value, optionally specifying tags.
-     * @param {string} stat The dot-separated metric name.
-     * @param {number} value The amount to decrement the stat by.
-     * @param {[tag:string]:string|number} tags Tags to pass along, such as `{ foo: 'bar' }`. Values are combined with config.tags.
+     * @param stat The dot-separated metric name.
+     * @param value The amount to decrement the stat by.
+     * @param tags Tags to pass along, such as `{ foo: 'bar' }`. Values are combined with config.tags.
      */
-    decrement(stat: string, value?: number, tags?: { [tag: string]: string|number }): void
+    decrement(stat: string, value?: number, tags?: Record<string, string|number>): void
 
     /**
      * Sets a distribution value, optionally specifying tags.
-     * @param {string} stat The dot-separated metric name.
-     * @param {number} value The amount to increment the stat by.
-     * @param {[tag:string]:string|number} tags Tags to pass along, such as `{ foo: 'bar' }`. Values are combined with config.tags.
+     * @param stat The dot-separated metric name.
+     * @param value The amount to increment the stat by.
+     * @param tags Tags to pass along, such as `{ foo: 'bar' }`. Values are combined with config.tags.
      */
-    distribution(stat: string, value?: number, tags?: { [tag: string]: string|number }): void
+    distribution(stat: string, value?: number, tags?: Record<string, string|number>): void
 
     /**
      * Sets a gauge value, optionally specifying tags.
-     * @param {string} stat The dot-separated metric name.
-     * @param {number} value The amount to increment the stat by.
-     * @param {[tag:string]:string|number} tags Tags to pass along, such as `{ foo: 'bar' }`. Values are combined with config.tags.
+     * @param stat The dot-separated metric name.
+     * @param value The amount to increment the stat by.
+     * @param tags Tags to pass along, such as `{ foo: 'bar' }`. Values are combined with config.tags.
      */
-    gauge(stat: string, value?: number, tags?: { [tag: string]: string|number }): void
+    gauge(stat: string, value?: number, tags?: Record<string, string|number>): void
 
     /**
      * Sets a histogram value, optionally specifying tags.
-     * @param {string} stat The dot-separated metric name.
-     * @param {number} value The amount to increment the stat by.
-     * @param {[tag:string]:string|number} tags Tags to pass along, such as `{ foo: 'bar' }`. Values are combined with config.tags.
+     * @param stat The dot-separated metric name.
+     * @param value The amount to increment the stat by.
+     * @param tags Tags to pass along, such as `{ foo: 'bar' }`. Values are combined with config.tags.
      */
-    histogram(stat: string, value?: number, tags?: { [tag: string]: string|number }): void
+    histogram(stat: string, value?: number, tags?: Record<string, string|number>): void
 
     /**
      * Forces any unsent metrics to be sent
@@ -871,7 +871,7 @@ declare namespace tracer {
 
     /**
      * Links a failed login event to the current trace.
-     * @param {string} userId The user id of the attemped login.
+     * @param {string} userId The user id of the attempted login.
      * @param {boolean} exists If the user id exists.
      * @param {[key: string]: string} metadata Custom fields to link to the login failure event.
      *

packages/dd-trace/src/dogstatsd.js

@@ -14,6 +14,10 @@ const TYPE_GAUGE = 'g'
 const TYPE_DISTRIBUTION = 'd'
 const TYPE_HISTOGRAM = 'h'
 
+/**
+ * @import { DogStatsD } from "../../../index.d.ts"
+ * @implements {DogStatsD}
+ */
 class DogStatsDClient {
   constructor (options = {}) {
     if (options.metricsProxyUrl) {
@@ -39,6 +43,10 @@ class DogStatsDClient {
     this._add(stat, value, TYPE_COUNTER, tags)
   }
 
+  decrement (stat, value, tags) {
+    this._add(stat, -value, TYPE_COUNTER, tags)
+  }
+
   gauge (stat, value, tags) {
     this._add(stat, value, TYPE_GAUGE, tags)
   }
@@ -77,7 +85,7 @@ class DogStatsDClient {
           // we're not getting a 200 from the proxy endpoint. If it's a 404,
           // then we know we'll never have the endpoint, so just clear out the
           // options. Either way, we can give UDP a try.
-          this._httpOptions = null
+          this._httpOptions = undefined
         }
         this._sendUdp(queue)
       }
@@ -185,7 +193,11 @@ class DogStatsDClient {
   }
 }
 
-// This is a simplified user-facing proxy to the underlying DogStatsDClient instance
+/**
+ * This is a simplified user-facing proxy to the underlying DogStatsDClient instance
+ *
+ * @implements {DogStatsD}
+ */
 class CustomMetrics {
   constructor (config) {
     const clientConfig = DogStatsDClient.generateClientConfig(config)
@@ -208,9 +220,9 @@ class CustomMetrics {
   }
 
   decrement (stat, value = 1, tags) {
-    return this.dogstatsd.increment(
+    return this.dogstatsd.decrement(
       stat,
-      value * -1,
+      value,
       CustomMetrics.tagTranslator(tags)
     )
   }

packages/dd-trace/src/noop/dogstatsd.js

@@ -1,6 +1,12 @@
+/**
+ * @import { DogStatsD } from "../../../../index.d.ts"
+ * @implements {DogStatsD}
+ */
 module.exports = class NoopDogStatsDClient {
   increment () { }
 
+  decrement () { }
+
   gauge () { }
 
   distribution () { }

packages/dd-trace/test/dogstatsd.spec.js

@@ -155,11 +155,12 @@ describe('dogstatsd', () => {
 
     client.gauge('test.avg', 10)
     client.increment('test.count', 10)
+    client.decrement('test.count', 5)
     client.flush()
 
     expect(udp4.send).to.have.been.called
-    expect(udp4.send.firstCall.args[0].toString()).to.equal('test.avg:10|g\ntest.count:10|c\n')
-    expect(udp4.send.firstCall.args[2]).to.equal(30)
+    expect(udp4.send.firstCall.args[0].toString()).to.equal('test.avg:10|g\ntest.count:10|c\ntest.count:-5|c\n')
+    expect(udp4.send.firstCall.args[2]).to.equal(46)
   })
 
   it('should support tags', () => {

packages/dd-trace/test/proxy.spec.js

@@ -75,6 +75,7 @@ describe('TracerProxy', () => {
 
     noopDogStatsDClient = {
       increment: sinon.spy(),
+      decrement: sinon.spy(),
       gauge: sinon.spy(),
       distribution: sinon.spy(),
       histogram: sinon.spy(),
@@ -654,6 +655,8 @@ describe('TracerProxy', () => {
       it('should not throw when calling noop methods', () => {
         proxy.dogstatsd.increment('inc')
         expect(noopDogStatsDClient.increment).to.have.been.calledWith('inc')
+        proxy.dogstatsd.decrement('dec')
+        expect(noopDogStatsDClient.decrement).to.have.been.calledWith('dec')
         proxy.dogstatsd.distribution('dist')
         expect(noopDogStatsDClient.distribution).to.have.been.calledWith('dist')
         proxy.dogstatsd.histogram('hist')