Merge lp:~mleinartas/graphite/whitelist into lp:~graphite-dev/graphite/main

[Michael Leinartas]

We would be completely overwhelmed by the volume of metrics flowing into Graphite if we accepted everything that came our way. I can't control what is sent to Graphite, but I can control what is accepted into Graphite. To implement that, we've created whitelist and blacklist functionality that respectively allows or denies metrics entry into Graphite. This code is based on code that has been running in production for months with several thousand metrics per minute after filtering.

Each list is a file with one regular expression per line. If a metric matches a regex in the blacklist it is dropped. Only if a metric matches a regex in the whitelist will it be accepted by carbon.

For example, a whitelist consisting of the line:
^server\.load\.
will allow any metric that starts with "server.load." into carbon, unless it also matches a regex in the blacklist.

To test, uncomment the WHITELIST and/or BLACKLIST lines in carbon.conf and add one or more regular expressions to graphite/conf/whitelist and/or graphite/conf/blacklist. Restart carbon for your changes to take effect. Metrics which are rejected are logged in listener.log.

**UPDATE**
I've submitted a branch with trunk merged back in and with some changes from the original patch. This should get another review before merging. This will still be compatible with the patch our friends at Orbitz are running, but will require them to add and enable the global USE_WHITELIST flag.

Some things to validate:
 * whitelist and blacklist configs are by default in CONF_DIR/whitelist and CONF_DIR/blacklist. This seems like a good place, but would it make more sense to use a .conf file for consistency with the rest of the files in that directory?
* I moved the RegexList class to it's own file - regexlist.py
* Functionality can be globally enabled/disabled with the USE_WHITELIST flag in carbon.conf. Removing the conf files also effectively disables it, but an additional global flag feels appropriate

Also note that this makes some changes to config - I can merge this in with the megacarbon branch separately after it's merged to trunk, or I could branch it off of megacarbon instead if that's better.

[Jon Stevens (latchkey-gmail)]

I personally think that this should be done through a separate proxy sitting in front of carbon-cache.py. It seems like by the time carbon accepts the request and parses it, you might as well just record it.

[Eric Ziegenhorn (ziggy)]

But that takes storage space, the time to create it, etc. I agree that
would be a good architecture though if a single proxy could handle the flow
in real time, and I'd guess it could in most cases. Note that filtering
happens right when the request hits carbon though, not after much
processing. It never gets to the metric queue if it isn't whitelisted.

On Mon, Jan 10, 2011 at 12:13 PM, Jon Stevens <email address hidden> wrote:

> I personally think that this should be done through a separate proxy
> sitting in front of carbon-cache.py. It seems like by the time carbon
> accepts the request and parses it, you might as well just record it.
> --
> https://code.launchpad.net/~ziggy/graphite/whitelist/+merge/45710<https://code.launchpad.net/%7Eziggy/graphite/whitelist/+merge/45710>
> You are the owner of lp:~ziggy/graphite/whitelist.
>

[chrismd (chrismd)]

Sorry for the delayed response. Actually carbon-cache already has a whitelist feature, it broke a while back because a default value for its config option went away somehow but I just fixed that in trunk. Anyways, it uses an exact list of metric names for whitelisting and doesn't provide blacklisting so your patch definitely adds value.

However you use a module I've never heard of, regexlist. I would prefer to not add another thirdparty dependency if possible and in this case I think it would be pretty simple to write a quick function to read the regexes from a file into a list. Also the diffs against carbon-cache.py no longer apply since Sidnei's refactoring to use twistd will be merged soon.

Thanks for submitting this, I'm sure many will find it useful once we get it merged in

review: Needs Fixing

[Eric Ziegenhorn (ziggy)]

I wrote regexlist module specifically for this feature. I could in-line it
somewhere if that is preferrable. I can update it to apply to trunk in the
not-too-distant future if there is nothing else to re-work first, just let
me know. Thanks!

On Sun, Jul 10, 2011 at 5:22 PM, chrismd <email address hidden> wrote:

> Review: Needs Fixing
> Sorry for the delayed response. Actually carbon-cache already has a
> whitelist feature, it broke a while back because a default value for its
> config option went away somehow but I just fixed that in trunk. Anyways, it
> uses an exact list of metric names for whitelisting and doesn't provide
> blacklisting so your patch definitely adds value.
>
> However you use a module I've never heard of, regexlist. I would prefer to
> not add another thirdparty dependency if possible and in this case I think
> it would be pretty simple to write a quick function to read the regexes from
> a file into a list. Also the diffs against carbon-cache.py no longer apply
> since Sidnei's refactoring to use twistd will be merged soon.
>
> Thanks for submitting this, I'm sure many will find it useful once we get
> it merged in
> --
> https://code.launchpad.net/~ziggy/graphite/whitelist/+merge/45710<https://code.launchpad.net/%7Eziggy/graphite/whitelist/+merge/45710>
> You are the owner of lp:~ziggy/graphite/whitelist.
>

[chrismd (chrismd)]

Sure just inline the function. The only thing I'd change is the if 'BLACKLIST' in settings check being done for each line received, ideally that lookup only need to be done once outside the lineReceived function.

> I wrote regexlist module specifically for this feature. I could in-line it
> somewhere if that is preferrable. I can update it to apply to trunk in the
> not-too-distant future if there is nothing else to re-work first, just let
> me know. Thanks!
>

[Eric Ziegenhorn (ziggy)]

I in-lined the RegexList class in writer.py as per your request. I changed the checks in writeCachedDataPoints() to try and make them faster especially in the case you're not using a whitelist or blacklist, but I don't see a way to get rid of them entirely. Feel free to enlighten me. :) I added more config settings for logging related to the whitelist or blacklist because it can be very useful to know where your metrics are going but sometimes it is too slow. I can tweak/get rid of that as you'd prefer. I can write proper documentation for this for the wiki if and when you want. I have this refactoring running in dev, but will push out to staging and production for further testing if I'm heading in the right direction for getting it upstream.

Additionally, I made a small standalone web interface for checking a metric to see if it would be whitelisted or blacklisted and if so, which regexes it matches. This has been helpful because when you get lots of whitelist or blacklist entries it can be hard to tell if a given metric matches or not. It's ~130 lines of python (+~2700 lines for the bottle.py micro-framework). Let me know if that'd be useful to include or reference with Graphite.

[chrismd (chrismd)]

Hi Ziggy, thanks for the updates. Unfortunately this patch doesn't apply cleanly to trunk because of all the recent carbon refactoring (ie. carbon-cache.py on trunk is 100% different than the source this branch derives from). It probably won't take much to sync it with current trunk but that should definitely happen as a trunk merge into this branch rather than the other way around, at least at first. Can you 'bzr merge lp:graphite' into your branch, resolve the conflicts and verify everything still works? If we can get this branch into a state where it cleanly merges into trunk then we'll be good to go. Thanks.

[lp:~mleinartas/graphite/whitelist]

623. By Michael Leinartas on 2011-12-14

add whitelist documentation

624. By Michael Leinartas on 2011-12-14

add instrumentation for whitelist/blacklist matches

[Eric Ziegenhorn (ziggy)]

I've been trying to get around to this but haven't found time yet...thanks for getting it cleaned up and ready for trunk Michael!

[lp:~mleinartas/graphite/whitelist]

625. By Michael Leinartas on 2012-01-04

use .conf extention for whitelist and blacklist for consistency

=== added file 'carbon/conf/blacklist.conf.example'
--- carbon/conf/blacklist.conf.example	1970-01-01 00:00:00 +0000
+++ carbon/conf/blacklist.conf.example	2012-01-04 18:43:36 +0000
@@ -0,0 +1,4 @@
+# This file takes a single regular expression per line
+# If USE_WHITELIST is set to True in carbon.conf, any metrics received which
+# match one of these expressions will be dropped
+^some\.noisy\.metric\.prefix\..*

=== modified file 'carbon/conf/carbon.conf.example'
--- carbon/conf/carbon.conf.example	2011-12-03 08:12:58 +0000
+++ carbon/conf/carbon.conf.example	2012-01-04 18:43:36 +0000
@@ -92,6 +92,11 @@
 # shift the onus of buffering writes from the kernel into carbon's cache.
 WHISPER_AUTOFLUSH = False
 
+# Set this to True to enable whitelisting and blacklisting of metrics in
+# CONF_DIR/whitelist and CONF_DIR/blacklist. If the whitelist is missing or
+# empty, all metrics will pass through
+# USE_WHITELIST = False
+
 # Enable AMQP if you want to receve metrics using an amqp broker
 # ENABLE_AMQP = False
 
@@ -180,6 +185,11 @@
 # data until the send queues fall below 80% MAX_QUEUE_SIZE.
 USE_FLOW_CONTROL = True
 
+# Set this to True to enable whitelisting and blacklisting of metrics in
+# CONF_DIR/whitelist and CONF_DIR/blacklist. If the whitelist is missing or
+# empty, all metrics will pass through
+# USE_WHITELIST = False
+
 
 [aggregator]
 LINE_RECEIVER_INTERFACE = 0.0.0.0
@@ -227,3 +237,8 @@
 # each metric. Aggregation only happens for datapoints that fall in
 # the past MAX_AGGREGATION_INTERVALS * intervalSize seconds.
 MAX_AGGREGATION_INTERVALS = 5
+
+# Set this to True to enable whitelisting and blacklisting of metrics in
+# CONF_DIR/whitelist and CONF_DIR/blacklist. If the whitelist is missing or
+# empty, all metrics will pass through
+# USE_WHITELIST = False

=== added file 'carbon/conf/whitelist.conf.example'
--- carbon/conf/whitelist.conf.example	1970-01-01 00:00:00 +0000
+++ carbon/conf/whitelist.conf.example	2012-01-04 18:43:36 +0000
@@ -0,0 +1,5 @@
+# This file takes a single regular expression per line
+# If USE_WHITELIST is set to True in carbon.conf, only metrics received which
+# match one of these expressions will be persisted. If this file is empty or
+# missing, all metrics will pass through
+.*

=== modified file 'carbon/lib/carbon/conf.py'
--- carbon/lib/carbon/conf.py	2011-12-19 18:02:23 +0000
+++ carbon/lib/carbon/conf.py	2012-01-04 18:43:36 +0000
@@ -59,6 +59,7 @@
   DESTINATIONS=[],
   USE_FLOW_CONTROL=True,
   USE_INSECURE_UNPICKLER=False,
+  USE_WHITELIST=False,
 )
 
 
@@ -158,6 +159,8 @@
         ["config", "c", None, "Use the given config file."],
         ["instance", "", "a", "Manage a specific carbon instance."],
         ["logdir", "", None, "Write logs to the given directory."],
+        ["whitelist", "", None, "List of metric patterns to allow."],
+        ["blacklist", "", None, "List of metric patterns to disallow."],
         ]
 
     def postOptions(self):
@@ -217,6 +220,14 @@
                     os.makedirs(logdir)
                 log.logToDir(logdir)
 
+        if self["whitelist"] is None:
+            self["whitelist"] = join(settings["CONF_DIR"], "whitelist.conf")
+        settings["whitelist"] = self["whitelist"]
+
+        if self["blacklist"] is None:
+            self["blacklist"] = join(settings["CONF_DIR"], "blacklist.conf")
+        settings["blacklist"] = self["blacklist"]
+
     def parseArgs(self, *action):
         """If an action was provided, store it for further processing."""
         if len(action) == 1:
@@ -346,6 +357,14 @@
         default=None,
         help="Use the given config file")
     parser.add_option(
+      "--whitelist",
+      default=None,
+      help="Use the given whitelist file")
+    parser.add_option(
+      "--blacklist",
+      default=None
+      help="Use the given blacklist file")
+    parser.add_option(
         "--logdir",
         default=None,
         help="Write logs in the given directory")

=== modified file 'carbon/lib/carbon/protocols.py'
--- carbon/lib/carbon/protocols.py	2011-10-06 09:22:43 +0000
+++ carbon/lib/carbon/protocols.py	2012-01-04 18:43:36 +0000
@@ -4,6 +4,7 @@
 from twisted.protocols.basic import LineOnlyReceiver, Int32StringReceiver
 from carbon import log, events, state, management
 from carbon.conf import settings
+from carbon.regexlist import WhiteList, BlackList
 from carbon.util import pickle, get_unpickler
 
 
@@ -46,6 +47,12 @@
     events.resumeReceivingMetrics.removeHandler(self.resumeReceiving)
 
   def metricReceived(self, metric, datapoint):
+    if BlackList and metric in BlackList:
+      instrumentation.increment('blackListMatches')
+      return
+    if WhiteList and metric not in WhiteList:
+      instrumentation.increment('whiteListMatches')
+      return
     if datapoint[1] == datapoint[1]: # filter out NaN values
       events.metricReceived(metric, datapoint)
 

=== added file 'carbon/lib/carbon/regexlist.py'
--- carbon/lib/carbon/regexlist.py	1970-01-01 00:00:00 +0000
+++ carbon/lib/carbon/regexlist.py	2012-01-04 18:43:36 +0000
@@ -0,0 +1,62 @@
+import time
+import re
+import os.path
+from carbon import log
+from twisted.internet.task import LoopingCall
+
+
+class RegexList:
+  """ Maintain a list of regex for matching whitelist and blacklist """
+
+  def __init__(self):
+    self.regex_list = []
+    self.list_file = None
+    self.read_task = LoopingCall(self.read_list)
+    self.rules_last_read = 0.0
+
+  def read_from(self, list_file):
+    self.list_file = list_file
+    self.read_list()
+    self.read_task.start(10, now=False)
+
+  def read_list(self):
+    # Clear rules and move on if file isn't there
+    if not os.path.exists(self.list_file):
+      self.regex_list = []
+      return
+
+    try:
+      mtime = os.path.getmtime(self.list_file)
+    except:
+      log.err("Failed to get mtime of %s" % self.list_file)
+      return
+
+    if mtime <= self.rules_last_read:
+      return
+
+    # Begin read
+    new_regex_list = []
+    for line in open(self.list_file):
+      pattern = line.strip()
+      if line.startswith('#') or not line:
+        continue
+      try:
+        new_regex_list.append(re.compile(pattern))
+      except:
+        log.err("Failed to parse '%s' in '%s'. Ignoring line" % (pattern, self.list_file))
+
+    self.regex_list = new_regex_list
+    self.rules_last_read = mtime
+
+  def __contains__(self, value):
+    for regex in self.regex_list:
+      if regex.search(value):
+        return True
+    return False
+
+  def __nonzero__(self):
+    return bool(self.regex_list)
+
+
+WhiteList = RegexList()
+BlackList = RegexList()

=== modified file 'carbon/lib/carbon/service.py'
--- carbon/lib/carbon/service.py	2011-09-18 09:55:35 +0000
+++ carbon/lib/carbon/service.py	2012-01-04 18:43:36 +0000
@@ -94,6 +94,11 @@
                             interface=settings.MANHOLE_INTERFACE)
         service.setServiceParent(root_service)
 
+    if settings.USE_WHITELIST:
+      from carbon.regexlist import WhiteList,BlackList
+      WhiteList.read_from(settings["whitelist"])
+      BlackList.read_from(settings["blacklist"])
+
     # Instantiate an instrumentation service that will record metrics about
     # this service.
     from carbon.instrumentation import InstrumentationService

=== modified file 'docs/config-carbon.rst'
--- docs/config-carbon.rst	2011-09-20 17:12:37 +0000
+++ docs/config-carbon.rst	2012-01-04 18:43:36 +0000
@@ -152,3 +152,13 @@
 aggregate metric 'prod.applications.apache.all.requests' would be calculated
 by summing their values.
 
+whitelist and blacklist
+-----------------------
+The whitelist functionality allows any of the carbon daemons to only accept metrics that are explicitly
+whitelisted and/or to reject blacklisted metrics. The functionality can be enabled in carbon.conf with
+the ``USE_WHITELIST`` flag. This can be useful when too many metrics are being sent to a Graphite
+instance or when there are metric senders sending useless or invalid metrics.
+
+The whitelist and blacklist files are located in ``GRAPHITE_CONF_DIR``. Each file contains one regular
+expressions per line to match against metric values. If the whitelist configuration is missing or empty,
+all metrics will be passed through.