From 5f9757c9d210e21162aee180b702b2945e7808a3 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 27 Feb 2025 18:30:12 +0100 Subject: [PATCH] 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 --- index.d.ts | 42 ++++++++++++------------ packages/dd-trace/src/dogstatsd.js | 20 ++++++++--- packages/dd-trace/src/noop/dogstatsd.js | 6 ++++ packages/dd-trace/test/dogstatsd.spec.js | 5 +-- packages/dd-trace/test/proxy.spec.js | 3 ++ 5 files changed, 49 insertions(+), 27 deletions(-) diff --git a/index.d.ts b/index.d.ts index 771988ce788..e7691d64dcc 100644 --- a/index.d.ts +++ b/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): 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): 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): 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): 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): 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. * diff --git a/packages/dd-trace/src/dogstatsd.js b/packages/dd-trace/src/dogstatsd.js index fcaaf8bf742..5978a25e1ec 100644 --- a/packages/dd-trace/src/dogstatsd.js +++ b/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) ) } diff --git a/packages/dd-trace/src/noop/dogstatsd.js b/packages/dd-trace/src/noop/dogstatsd.js index 899ac11e228..7487a5362c5 100644 --- a/packages/dd-trace/src/noop/dogstatsd.js +++ b/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 () { } diff --git a/packages/dd-trace/test/dogstatsd.spec.js b/packages/dd-trace/test/dogstatsd.spec.js index 1e961d87e7d..832c16b9b32 100644 --- a/packages/dd-trace/test/dogstatsd.spec.js +++ b/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', () => { diff --git a/packages/dd-trace/test/proxy.spec.js b/packages/dd-trace/test/proxy.spec.js index fc1b42b15d2..510fb70a4b7 100644 --- a/packages/dd-trace/test/proxy.spec.js +++ b/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')