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

Filter per metric and expansion #83

Open
fabienrenaud opened this issue Jul 17, 2017 · 8 comments
Open

Filter per metric and expansion #83

fabienrenaud opened this issue Jul 17, 2017 · 8 comments

Comments

@fabienrenaud
Copy link

fabienrenaud commented Jul 17, 2017

metric-datadog allows filtering metrisc by name using a MetricFilter. This feature should be extended to allow filtering of expansions per metric.

Example

  • meter: "httpResCode[endpoint={endpoint},resCode={resCode}]"
  • timer: "httpRequests[endpoint={endpoint}]"
  • whitelisted default expansions: count, p95, p99

The reporter will send to datadog:

  • httpRequests.count[endpoint={endpoint}]
  • httpRequests.p95[endpoint={endpoint}]
  • httpRequests.p99[endpoint={endpoint}]
  • httpResCode.count[endpoint={endpoint},resCode={resCode}]

However, httpRequests.count is unnecessary since i can get that count by summing the counts of all resCodes meters (and do that in datadog).

MetricFilter only provides filtering by names defined in the metrics API. It would be more convenient to filter by metric name that will be sent to datadog (just before sending it). Filtering by tag could also be more powerful if the filtering is done near the end of the funnel.

@nickdella
Copy link

Thanks for the write up, @fabienrenaud ! Could you better state your motivation for this request? As stated, I don't see a big benefit in filtering httpRequests.count just because it's redundant. That said, I do see tag filtering as useful, especially if you have an application producing a large or unbounded set of tag values for which you'd like to whitelist only the top 10-20 values.

In any case, it's unlikely I'll personally have any time to work on this in the next couple months. PRs are welcome, of course. The codebase is fairly well-test and readable IMO :)

@qualidafial
Copy link

Datadog pricing is the issue where I work. Every VM or container is allocated a certain number of unique metrics (including tags!). Dropwizard produces so many metrics out of the box that we're nearly over budget before adding any metrics of our own.

@adamhonen
Copy link

A possible solution one can use is implementing a Transport class that wraps another Transport (& Request implementation) and only forwards the addGauge/addCounter when the metric name fits whatever requirements you define. The metric names at that point are fully expanded.

We use this for precisely the reasons @qualidafial mentioned - to reduce the amount of DataDog metrics we report from each JVM.

e.g.:

import org.coursera.metrics.datadog.model.DatadogCounter;
import org.coursera.metrics.datadog.model.DatadogGauge;
import org.coursera.metrics.datadog.transport.Transport;

import java.io.IOException;

public class FilteredTransport implements Transport {

	private static final String[] FORBIDDEN_METRICS = new String[]{"5MinuteRate", "15MinuteRate", "meanRate", "stddev", "min", "max", "mean", "p95", "p98"};
	private final Transport _delegate;


	public FilteredTransport(final Transport delegate) {
		_delegate = delegate;
	}

	private static boolean filterMetrics(final String metricName) {
		for (final String forbiddenSuffix : FORBIDDEN_METRICS) {
			if (metricName.endsWith(forbiddenSuffix)) {
				return false;
			}
		}

		return true;
	}

	@Override
	public Request prepare() throws IOException {
		return new FilteredRequest(_delegate.prepare());
	}

	@Override
	public void close() throws IOException {
		_delegate.close();
	}

	private static class FilteredRequest implements Request {
		private final Request _delegate;

		public FilteredRequest(final Request request) {
			_delegate = request;
		}

		@Override
		public void addGauge(final DatadogGauge gauge) throws IOException {
			if (FilteredTransport.filterMetrics(gauge.getMetric())) {
				_delegate.addGauge(gauge);
			}
		}

		@Override
		public void addCounter(final DatadogCounter counter) throws IOException {
			if (FilteredTransport.filterMetrics(counter.getMetric())) {
				_delegate.addCounter(counter);
			}
		}

		@Override
		public void send() throws Exception {
			_delegate.send();
		}
	}
}

A bit on the hacky side, but it's simple and it works.

@qualidafial
Copy link

@adamhonen you can already restrict the metric suffixes with the expansions config option.

@qualidafial
Copy link

Example config:

metrics:
  reporters:
    - type: datadog
      expansions:
        - COUNT
        - MEAN
        - MEDIAN
        - P99

@adamhonen
Copy link

Yeah, I guess my example is simple enough that it could be handled that way.
I was thinking of more elaborate cases where you might want different expansions for different metrics.
That's at least how I understood the initial comment.

@qualidafial
Copy link

I agree.

I think I'd prefer a solution that used the existing MetricFilter scaffolding, and just exposed a couple out-of-the-box filters we could chain together--e.g. whitelist, blacklist, regex matching.

@qualidafial
Copy link

So.. it looks like this is already provided by dropwizard-metrics:

metrics:
  reporters:
    - type: datadog
      expansions:
      - COUNT
      - MEAN
      - MEDIAN
      - P99
      useRegexFilters: true
      includes:
      - "myapp.*"
      - "jvm\\.attribute\\.uptime.*"
      - "jvm\\.gc.*"
      - "jvm\\.memory.*"
      excludes:
      - "jvm\\.memory\\.pools.*"

The include and excludes are matched against the entire metric name (including tags), so you want to add a .* prefix and/or suffix depending on whether you want to match metric names that start with, end with, or just contain some pattern.

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

No branches or pull requests

4 participants