Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

statsd: option to namespace all metrics #1210

Merged
merged 1 commit into from
Dec 31, 2014

Conversation

igor47
Copy link
Contributor

@igor47 igor47 commented Nov 20, 2014

we are currently unable to run datadog on the machines in our staging or
testing clusters. this is because the code in the applications running there is
set to report metrics directly to datadog. those non-production metrics
nontheless become mixed in with production metrics in datadog.

there are two existing solutions:

  • in datadog, always specify an environment tag to exclude non-production metrics
  • in applications, always manually namespace the metrics

the first is difficult. we already have many dashboards and alerts which don't
specify any environment. also, naive examination in, e.g., the metric explorer
will still conflate the metrics and cause confusion.

the second is annoying, because we have to make an identical configuration
change throughout multiple applications.

instead, we propose the mechanism here, which allows statsd metrics being
reported to be automatically namespaced.

i wasn't sure how to test this functionality, since there are no existing tests
for the init function. i am also not sure about grabbing a direct reference
to the api_formatter function elsewhere in the code.

we are currently unable to run datadog on the machines in our staging or
testing clusters. this is because the code in the applications running there is
set to report metrics directly to datadog. those non-production metrics
nontheless become mixed in with production metrics in datadog.

there are two existing solutions:
* in datadog, always specify an environment tag to exclude non-production metrics
* in applications, always manually namespace the metrics

the first is difficult. we already have many dashboards and alerts which don't
specify any environment. also, naive examination in, e.g., the metric explorer
will still conflate the metrics and cause confusion.

the second is annoying, because we have to make an identical configuration
change throughout multiple applications.

instead, we propose the mechanism here, which allows statsd metrics being
reported to be automatically namespaced.

i wasn't sure how to test this functionality, since there are no existing tests
for the `init` function. i am also not sure about grabbing a direct reference
to the api_formatter function elsewhere in the code.
@igor47
Copy link
Contributor Author

igor47 commented Dec 11, 2014

ping. any update on this? it would be really helpful for us

@alq666 alq666 assigned alq666 and remh and unassigned alq666 Dec 11, 2014
@alq666
Copy link
Member

alq666 commented Dec 11, 2014

@remh can you review and consider for inclusion in 5.2.0?

@alq666 alq666 added this to the 5.2.0 milestone Dec 11, 2014
@@ -389,7 +389,27 @@ def init(config_path=None, use_watchdog=False, use_forwarder=False, args=None):
# server and reporting threads.
assert 0 < interval

aggregator = MetricsBucketAggregator(hostname, aggregator_interval, recent_point_threshold=recent_point_threshold)
formatter = api_formatter
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!
Could you put that in a new function something like:

def get_formatter(c):
  formatter = api_formatter

  if c['statsd_metric_namespace']:
    def metric_namespace_formatter_wrapper(*args, **kwargs):
      metric_prefix = c['statsd_metric_namespace']
      if metric_prefix[-1] != '.':
        metric_prefix += '.'

      metric = args[0]
      new_metric = metric_prefix + metric
      new_args = [new_metric] + args[1:]
      return api_formatter(*new_args, **kwargs)

    formatter = metric_namespace_formatter_wrapper
  return formatter

and then call that function in the init() function ?

@remh
Copy link
Contributor

remh commented Dec 11, 2014

Looks good beside the nitpick! Thanks @igor47 we'll merge it for 5.2.0

For tests, you can add some here: https://github.com/DataDog/dd-agent/blob/master/tests/test_dogstatsd.py where you would pass the formatter returned by the get_formatter function to the aggregator.

And then test for the metric names same as we do with other tests in that test module.
Does that make sense?
Thanks!

remh added a commit that referenced this pull request Dec 31, 2014
statsd: option to namespace all metrics
@remh remh merged commit bdc1354 into DataDog:master Dec 31, 2014
miketheman pushed a commit to DataDog/chef-datadog that referenced this pull request Apr 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants