Support for disabled metrics #1048
Conversation
arteam
force-pushed
the
support_for_disabled_metrics
branch
from
dc7816c
to
6f2ec88
Compare
arteam
changed the title
|
I think this should be good to merge. Could anybody give a review? |
|
Looks good, except that metric types are meter, timer, counter, gauge… |
|
And is there a reason to go with |
I don't like the inverted logic too. But I think it makes sense from the the user's perspective. The user reports some metrics to its time-series database and sees that some of them are useless, because he/she doesn't use them. The user wants to disable them. For instance, |
|
Is this only for the 3.2 branch or would it go into master/4.0 too? See the conversation in #837 and #178 - the idea that each "Metric" has a set of "Attributes" (name/value) off it. I'm not sure on the status of those but maybe instead of a MetricType enum, allow filtering by String based "attribute names" and just provide a helper class with the list of standard/known attribute names that currently exist (what is present in the MetricType enum currently). For 3.2 the enum makes sense as there's a fixed number of attributes, for 4.0 if #837 becomes a thing then this would just want to be changed to a String (suppose there'd be so many other breaking changes that it could just be worried about at that point). Anyway, thought of it when I saw the comment about "metric types". |
|
@ryanrupp Good point. This would only go to 3.2, for exactly the reasons you mention. Also there will be some inversion of control with regard to reporters in v4 to make this sort of filtering easier to implement. @arteam Ryan reminded me that we're calling these 'Attributes', can we rename MetricType and disabledMetricTypes to MetricAttribute and disabledMetricAttributes? |
arteam
force-pushed
the
support_for_disabled_metrics
branch
from
83a5deb
to
5a72bcf
Compare
|
That's a good suggestion. Renamed |
arteam
force-pushed
the
support_for_disabled_metrics
branch
2 times, most recently
from
fa558b5
to
d09483f
Compare
(like all p999 across every metric). Allows fine-grained filtering of sent metrics and therefore effective load reduction.
Add a facility to disable set of metrics which should not be reported to a remote system. `ScheduledReporter` only saves them. The decision to how to filter the metric is delegated to concrete reporters.
This is a more common terminology for this enumeration and it was introduced earlier in #837.
arteam
force-pushed
the
support_for_disabled_metrics
branch
from
d09483f
to
3ccd7a1
Compare
|
I fixed the build (it was affected by relevant changes from the 3.1 branch moved to 3.2), now it should be good to merge. |
This is for based on the work of @phauer in #1008. The set of disabled metrics is implemented in
ScheduledReporter, so all reporters have an opportunity to implement this feature, but it's not necessarily (for instanceConsoleReporterdoesn't really need it). Along with the Graphite reporter, the Ganglia reporter is also updated to support of disabling sending metrics.