Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Add Graphite backend configuration flag to connect only when there are metrics to send #345

Open
wants to merge 3 commits into from

5 participants

@urlgrey

The Statsd Graphite backend in the master branch will connect to a Graphite carbon-relay or carbon-cache server even when there are no metrics to report. This can be useful for indicating that the Statsd process is alive and well, and capable of sending metrics if/when they occur; however, the frequency & volume of connections to Graphite could become a problem in some circumstances.

In my case, I have a large number of Amazon EC2 instances that can perform various tasks. Statsd metrics are generated for just one type of task. Using the master branch of Statsd, all EC2 instances connect to the Graphite server at 1 second intervals, usually reporting no metrics. I would prefer to have just the instances handling the task that produces metrics connect to Graphite and report stats. My applications produces several Statsd gauge values while the task is running.

I have added a 'quiet' boolean flag to the Statsd Graphite configuration. It has a default value of 'false', which will cause the Graphite backend to perform identically to the current master branch (always connect to Graphite). If the 'quiet' flag is set to 'true', then a connection will be made to Graphite only when there are metrics to report.

Here's an example Statsd config.js file showing its usage:

{
  graphitePort: "2013"
, graphiteHost: "x.x.x.x"
, flushInterval: "1000"
, port: 8125
, backends: [ "./backends/graphite" ]
, graphite: { quiet: true }
, deleteIdleStats: true
}
@urlgrey urlgrey Adding 'quiet' boolean flag to Graphite backend connector to avoid co…
…nnecting to Graphite when there are not metrics to report.
510a3a1
@mrtazz
Owner

Interesting patch, do you see any downsides of this just being the normal behaviour? I don't think connecting without any metrics is useful in any situation. So if you can't think of any problems that might arise, I'd say let's make this the default (and only) way of connecting.

@urlgrey

It might be useful to have Statsd connect to Graphite at the configured flush interval to indicate that it's alive & well. This would help distinguish between an absence of application metrics because there are no metrics to report versus no metrics because the Statsd service is down. In my case, the application metrics are not considered mission critical. It's preferable to have Statsd stay quiet when no application metrics need to be pushed to Graphite.

I like the idea of making 'quiet=true' the default, and think that it's worth allowing that behavior to be controlled through the quiet configuration option.

@draco2003
urlgrey added some commits
@urlgrey urlgrey Applied feedback from pull-request. The quiet option is effective onl…
…y when the deleteIdleStats option is enabled. The check for empty stats associative arrays has been made more efficient.
6506508
@urlgrey urlgrey Updated unit test to include deleteIdleStats value of true 9468677
@urlgrey

Thanks for the excellent feedback! I've updated the pull-request to perform the 'quiet' operation only when the 'deleteIdleStats' option is enabled. The documentation in the example configuration file has bee updated accordingly. I also modified the check for empty stats associative arrays to be more efficient.

@draco2003

Thanks for the PR and for updating the PR!

Just a heads up. I'm looking into how we can have this PR and #364 coexist.
I think both options valid use cases and would be good to include.

Thinking we move to a graphite connection_type config option so we can set quiet , standard(?) or persistent, and then handle the cases accordingly. Debating about including the current case as the "standard" option to support backwards compatibility.

Other option would be to try and do some combination of both automagically. Persisting the connection while regularly flushing metrics, but timeout and don't reconnect if we have a flushInterval that doesn't have metrics to send. Need to think this option through a little more, but like the idea of less config options for everyone :)

@draco2003

Just added a PR for Persistent connections for high traffic statsd servers.
If that gets the :+1: i think that we'll just need to wrap the
this.end() with an if quiet check in your PR so we only call end when in quiet mode, otherwise we reuse the connection, then merge this in as well.

https://github.com/urlgrey/statsd/blob/9468677584f8b45ea863a257f6eca7f3df32c13e/backends/graphite.js#L74

@urlgrey

That sounds great, I'm looking forward to seeing both features be a part of the Statsd project!

@urlgrey

On second thought, I'm not sure that we should close the connection when the 'quiet' flag is true. In my case I'd like to keep the connection open, if possible, in the interest of efficiency. Does the persistent-connection pull-request handle the case where the connection is left open & idle for an extended period of time (i.e. > 5 minutes)?

@draco2003
@skidder
@mrtazz
Owner

Hey is this synced with the changes in #364 ? Or do we need to sync both up to merge this?

@cPanelScott

This feature looks useful to me. I've incorporated urlgrey's work into an up to date fork (I hope that's okay) so that I can get the latest version of statsd along with this feature. I'm willing to help where I can if there's outstanding work required to get this merged into mainline.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 25, 2013
  1. @urlgrey

    Adding 'quiet' boolean flag to Graphite backend connector to avoid co…

    urlgrey authored
    …nnecting to Graphite when there are not metrics to report.
Commits on Oct 10, 2013
  1. @urlgrey

    Applied feedback from pull-request. The quiet option is effective onl…

    urlgrey authored
    …y when the deleteIdleStats option is enabled. The check for empty stats associative arrays has been made more efficient.
  2. @urlgrey
This page is out of date. Refresh to see the latest.
View
36 backends/graphite.js
@@ -21,6 +21,8 @@ var net = require('net'),
var l;
var debug;
+var quiet = false;
+var deleteIdleStats = false;
var flushInterval;
var graphiteHost;
var graphitePort;
@@ -98,6 +100,19 @@ var flush_stats = function graphite_flush(ts, metrics) {
var timer_data = metrics.timer_data;
var statsd_metrics = metrics.statsd_metrics;
+ if (quiet
+ && deleteIdleStats
+ && (counters === null || isEmptyObject(counters))
+ && (timer_data === null || isEmptyObject(timer_data))
+ && (gauges === null || isEmptyObject(gauges))
+ && (sets === null || isEmptyObject(sets))
+ ) {
+ if (debug) {
+ l.log("Not sending stats, no metrics to report");
+ }
+ return;
+ }
+
for (key in counters) {
var namespace = counterNamespace.concat(key);
var value = counters[key];
@@ -190,14 +205,18 @@ exports.init = function graphite_init(startup_time, config, events) {
prefixSet = config.graphite.prefixSet;
globalSuffix = config.graphite.globalSuffix;
legacyNamespace = config.graphite.legacyNamespace;
+ quiet = config.graphite.quiet;
+ deleteIdleStats = config.deleteIdleStats;
// set defaults for prefixes & suffix
- globalPrefix = globalPrefix !== undefined ? globalPrefix : "stats";
- prefixCounter = prefixCounter !== undefined ? prefixCounter : "counters";
- prefixTimer = prefixTimer !== undefined ? prefixTimer : "timers";
- prefixGauge = prefixGauge !== undefined ? prefixGauge : "gauges";
- prefixSet = prefixSet !== undefined ? prefixSet : "sets";
+ globalPrefix = globalPrefix !== undefined ? globalPrefix : "stats";
+ prefixCounter = prefixCounter !== undefined ? prefixCounter : "counters";
+ prefixTimer = prefixTimer !== undefined ? prefixTimer : "timers";
+ prefixGauge = prefixGauge !== undefined ? prefixGauge : "gauges";
+ prefixSet = prefixSet !== undefined ? prefixSet : "sets";
legacyNamespace = legacyNamespace !== undefined ? legacyNamespace : true;
+ quiet = quiet !== undefined ? quiet : false;
+ deleteIdleStats = deleteIdleStats !== undefined ? deleteIdleStats : false;
// In order to unconditionally add this string, it either needs to be
// a single space if it was unset, OR surrounded by a . and a space if
@@ -247,3 +266,10 @@ exports.init = function graphite_init(startup_time, config, events) {
return true;
};
+
+function isEmptyObject(obj) {
+ for (var name in obj) {
+ return false;
+ }
+ return true;
+}
View
2  exampleConfig.js
@@ -64,6 +64,8 @@ Optional Variables:
graphite:
legacyNamespace: use the legacy namespace [default: true]
+ quiet: connect to the Graphite server only when there are metrics to report
+ AND deleteIdleStats is set to 'true' [default: false]
globalPrefix: global prefix to use for sending stats to graphite [default: "stats"]
prefixCounter: graphite prefix for counter metrics [default: "counters"]
prefixTimer: graphite prefix for timer metrics [default: "timers"]
View
3  test/graphite_tests.js
@@ -80,7 +80,8 @@ module.exports = {
, port: 8125\n\
, dumpMessages: false \n\
, debug: false\n\
- , graphite: { legacyNamespace: false }\n\
+ , deleteIdleStats: true\n\
+ , graphite: { legacyNamespace: false, quiet: true }\n\
, graphitePort: " + this.testport + "\n\
, graphiteHost: \"127.0.0.1\"}";
Something went wrong with that request. Please try again.