Skip to content

Commit

Permalink
Use promises instead of callbacks (#125)
Browse files Browse the repository at this point in the history
The `flush()` method and the `report(series)` method on reporters now use promises instead of callbacks, moving to a more standardized model for async operations. The old callbacks are still supported, but deprecated, and will be removed in the future. Fixes #84.

Other items to note here:
- The global error handler now *always* runs for errors (it used to only run if there was no error callback for `flush`). This is probably how it always should have been, and fits better with the promise model.
- The promise returned from `flush()` is a little special — it won’t throw an async error if you don’t handle errors, so it’s safe to simply call and not use `await` or a `.catch()` handler (unlike normal async functions).
- Moved HTTP-related tests to a new reporters suite; the logger tests now use a mock reporter.
- Bumps version to v0.12.0-dev since this is a major feature.
  • Loading branch information
Mr0grog authored Nov 28, 2024
1 parent 3f4d1d4 commit 8f88c9f
Show file tree
Hide file tree
Showing 7 changed files with 358 additions and 200 deletions.
3 changes: 3 additions & 0 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@
"files": ["test/**/*.js"],
"env": {
"mocha": true
},
"rules": {
"max-statements": ["off"]
}
}
]
Expand Down
54 changes: 39 additions & 15 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,7 @@ Where `options` is an object and can contain the following:
* Set tags that are common to all metrics.
* `onError`: A function to call when there are asynchronous errors seding
buffered metrics to Datadog. It takes one argument (the error). (optional)
* If the error was not handled (either by setting this option or by
specifying a handler when manually calling `flush()`), the error will be
logged to stdout.
* If this option is not set, the error will be logged to stderr.
* `histogram`: An object with default options for all histograms. This has the
same properties as the options object on the `histogram()` method. Options
specified when calling the method are layered on top of this object.
Expand Down Expand Up @@ -170,16 +168,14 @@ Send metrics to a totally different service instead of Datadog:
```js
metrics.init({
reporter: {
report(series, onSuccess, onError) {
async report(series) {
// `series` is an array of metrics objects, formatted basically how the
// Datadog v1 metrics API and v1 distributions API want them.
fetch('https://my-datadog-like-api.com/series', {
method: 'POST',
body: JSON.stringify({ series })
})
.then(response => response.json())
.then(() => onSuccess())
.catch(onError);
const response = await fetch('https://my-datadog-like-api.com/series', {
method: 'POST',
body: JSON.stringify({ series })
});
return await response.json();
}
}
});
Expand Down Expand Up @@ -277,15 +273,40 @@ metrics.distribution('test.service_time', 0.248);

### Flushing

`metrics.flush([onSuccess[, onError]])`
`metrics.flush()`

Calling `flush` sends any buffered metrics to Datadog. Unless you set
`flushIntervalSeconds` to 0 it won't be necessary to call this function.
Calling `flush` sends any buffered metrics to Datadog and returns a promise.
This function will be called automatically unless you set `flushIntervalSeconds`
to `0`.

It can be useful to trigger a manual flush by calling if you want to
make sure pending metrics have been sent before you quit the application
process, for example.

⚠️ This method used to take two callback arguments for handling successes and
errors. That form is deprecated and will be removed in a future update:

```js
// Deprecated:
metrics.flush(
() => console.log('Flush suceeded!'),
(error) => console.log('Flush error:', error)
);

// Current, using `await`:
try {
await metrics.flush();
console.log('Flush suceeded!');
} catch (error) {
console.log('Flush error:', error);
}

// Or, using Promise callbacks:
metrics.flush()
.then(() => console.log('Flush succeeded'))
.catch((error) => console.log('Flush error:', error)) ;
```

## Logging

Datadog-metrics uses the [debug](https://github.com/visionmedia/debug)
Expand All @@ -312,7 +333,10 @@ Contributions are always welcome! For more info on how to contribute or develop

**New Features:**

TBD
* Asynchronous actions now use promises instead of callbacks. In places where `onSuccess` and `onError` callbacks were used, they are now deprecated. Instead, those methods return promises (callbacks still work, but support will be removed in a future release). This affects:

* The `flush()` method now returns a promise.
* The `report(series)` method on any custom reporters should now return a promise. For now, datadog-metrics will use the old callback-based behavior if the method signature has callbacks listed after `series` argument.

**Bug Fixes:**

Expand Down
89 changes: 71 additions & 18 deletions lib/loggers.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
'use strict';
const { logDebug, logDeprecation } = require('./logging');
const { logDebug, logDeprecation, logError } = require('./logging');
const Aggregator = require('./aggregators').Aggregator;
const { DatadogReporter } = require('./reporters');
const Gauge = require('./metrics').Gauge;
Expand All @@ -24,10 +24,19 @@ const Distribution = require('./metrics').Distribution;
*/

/**
* @typedef {object} ReporterType
* @typedef {object} CallbackReporterType
* @property {(series: any[], onSuccess?: Function, onError?: Function) => void} report
*/

/**
* @typedef {object} PromiseReporterType
* @property {(series: any[]) => Promise} report
*/

/**
* @typedef {PromiseReporterType|CallbackReporterType} ReporterType
*/

/**
* @typedef {object} BufferedMetricsLoggerOptions
* @property {string} [apiKey] Datadog API key
Expand Down Expand Up @@ -88,7 +97,7 @@ class BufferedMetricsLogger {

/** @private */
this.aggregator = opts.aggregator || new Aggregator(opts.defaultTags);
/** @private */
/** @private @type {ReporterType} */
this.reporter = opts.reporter || new DatadogReporter(opts.apiKey, opts.appKey, opts.site);
/** @private */
this.host = opts.host;
Expand Down Expand Up @@ -239,26 +248,70 @@ class BufferedMetricsLogger {
}

/**
* Calling `flush` sends any buffered metrics to Datadog. Unless you set
* `flushIntervalSeconds` to 0 it won't be necessary to call this function.
* Send buffered metrics to Datadog.
*
* Unless you've set `flushIntervalSeconds` to 0, this will be called
* automatically for you. However, you may want to call it manually when
* your application quits to send any remaining metrics.
*
* It can be useful to trigger a manual flush by calling if you want to
* make sure pending metrics have been sent before you quit the application
* process, for example.
* @param {() => void} [onSuccess]
* @param {(error: Error) => void} [onError]
* Returns a promise indicating when sending has completed. Older support
* for success and error callbacks as arguments to this method is deprecated
* and will be removed in the future. Please switch to promises!
*
* @param {() => void} [onSuccess] DEPRECATED! This argument will be removed
* soon. Please use the returned promise instead.
* @param {(error: Error) => void} [onError] DEPRECATED! This argument will
* be removed soon. Please use the returned promise instead.
* @returns {Promise}
*/
flush(onSuccess, onError) {
const series = this.aggregator.flush();
if (series.length > 0) {
logDebug('Flushing %d metrics to Datadog', series.length);
this.reporter.report(series, onSuccess, onError || this.onError);
} else {
logDebug('Nothing to flush');
if (typeof onSuccess === 'function') {
onSuccess();
const result = new Promise((resolve, reject) => {
const series = this.aggregator.flush();
if (series.length > 0) {
logDebug('Flushing %d metrics to Datadog', series.length);

if (this.reporter.report.length > 1) {
logDeprecation(
'Callback arguments on the `report()` method of a ' +
'reporter are deprecated and will stop working in a ' +
'future release. Please update your reporter to ' +
'return a promise instead.'
);
this.reporter.report(series, resolve, reject);
} else {
// @ts-expect-error TS can't figure out we have a promise here.
this.reporter.report(series).then(resolve, reject);
}
} else {
logDebug('Nothing to flush');
resolve();
}
});

if (onSuccess || onError) {
logDeprecation(
'The `onSuccess` and `onError` callback arguments for ' +
'BufferedMetricsLogger.flush() are deprecated and will be ' +
'removed in a future release. Please use the promise object that ' +
'`flush()` returns instead.'
);
result.then(
typeof onSuccess === 'function' ? onSuccess : () => null,
typeof onError === 'function' ? onError : () => null
);
}

// Notify global handler *and* ensure a simple call to `logger.flush()`
// without error handling doesn't throw an unhandled rejection error.
result.catch((error) => {
if (this.onError) {
this.onError(error);
} else {
logError('failed to send metrics (err=%s)', error);
}
});

return result;
}
}

Expand Down
57 changes: 26 additions & 31 deletions lib/reporters.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,15 @@
'use strict';
const datadogApiClient = require('@datadog/datadog-api-client');
const { AuthorizationError } = require('./errors');
const { logDebug, logDeprecation, logError } = require('./logging');
const { logDebug, logDeprecation } = require('./logging');

/**
* A Reporter that throws away metrics instead of sending them to Datadog. This
* is useful for disabling metrics in your application and for tests.
*/
class NullReporter {
report(series, onSuccess) {
async report(_series) {
// Do nothing.
if (typeof onSuccess === 'function') {
onSuccess();
}
}
}

Expand Down Expand Up @@ -55,7 +52,12 @@ class DatadogReporter {
datadogClients.set(this, new datadogApiClient.v1.MetricsApi(configuration));
}

report(series, onSuccess, onError) {
/**
* Send an array of serialized metrics to Datadog.
* @param {any[]} series
* @returns {Promise}
*/
async report(series) {
logDebug('Calling report with %j', series);

// Distributions must be submitted via a different method than other
Expand Down Expand Up @@ -84,31 +86,24 @@ class DatadogReporter {
}));
}

Promise.all(submissions)
.then(() => {
logDebug('sent metrics successfully');
if (typeof onSuccess === 'function') {
onSuccess();
}
})
.catch((error) => {
if (error.code === 403) {
error = new AuthorizationError(
'Your Datadog API key is not authorized to send ' +
'metrics. Check to make sure the DATADOG_API_KEY ' +
'environment variable or the `apiKey` option is set ' +
'to a valid API key for your Datadog account, and ' +
'that it is not an *application* key. For more, see: ' +
'https://docs.datadoghq.com/account_management/api-app-keys/',
{ cause: error }
);
}
if (typeof onError === 'function') {
onError(error);
} else {
logError('failed to send metrics (err=%s)', error);
}
});
try {
await Promise.all(submissions);
logDebug('sent metrics successfully');
} catch (error) {
if (error.code === 403) {
throw new AuthorizationError(
'Your Datadog API key is not authorized to send ' +
'metrics. Check to make sure the DATADOG_API_KEY ' +
'environment variable or the `apiKey` init option is set ' +
'to a valid API key for your Datadog account, and ' +
'that it is not an *application* key. For more, see: ' +
'https://docs.datadoghq.com/account_management/api-app-keys/',
{ cause: error }
);
}

throw error;
}
}
}

Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "datadog-metrics",
"version": "0.11.5-dev",
"version": "0.12.0-dev",
"description": "Buffered metrics reporting via the Datadog HTTP API",
"main": "index.js",
"types": "dist/index.d.ts",
Expand Down Expand Up @@ -31,6 +31,7 @@
"devDependencies": {
"@types/node": "^12.20.55",
"chai": "4.3.6",
"chai-as-promised": "^7.1.2",
"chai-string": "1.5.0",
"eslint": "^8.24.0",
"mocha": "9.2.2",
Expand Down
Loading

0 comments on commit 8f88c9f

Please sign in to comment.