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

[Eric Ziegenhorn]

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.

[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!
>

[lp:~ziggy/graphite/whitelist]

337. By Eric Ziegenhorn on 2011-08-26

Added config options for logging whitelist/blacklist misses/hits.
Moved RegexList class in to writer.py

338. By Eric Ziegenhorn on 2011-08-26

Added a default whitelist entry to allow everything in

339. By Eric Ziegenhorn on 2011-08-26

Add support for comments in whitelist/blacklist files

340. By Eric Ziegenhorn on 2011-08-26

Removed explicit directory chmod as that is not supposed to be part of this tree

[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.

Unmerged revisions

340. By Eric Ziegenhorn on 2011-08-26

Removed explicit directory chmod as that is not supposed to be part of this tree

339. By Eric Ziegenhorn on 2011-08-26

Add support for comments in whitelist/blacklist files

338. By Eric Ziegenhorn on 2011-08-26

Added a default whitelist entry to allow everything in

337. By Eric Ziegenhorn on 2011-08-26

Added config options for logging whitelist/blacklist misses/hits.
Moved RegexList class in to writer.py

336. By Eric Ziegenhorn on 2011-01-10

Work with WHITELIST and/or BLACKLIST commented out in carbon.conf

335. By Eric Ziegenhorn on 2011-01-10

Improved comments in example carbon.conf

334. By Eric Ziegenhorn on 2011-01-10

Added blank whitelist and blacklist files

333. By Eric Ziegenhorn on 2011-01-10

Added whitelist/blacklist functionality

=== modified file 'carbon/bin/carbon-cache.py'
--- carbon/bin/carbon-cache.py	2011-07-11 19:24:57 +0000
+++ carbon/bin/carbon-cache.py	2011-08-26 21:17:16 +0000
@@ -25,6 +25,168 @@
 LIB_DIR = join(ROOT_DIR, 'lib')
 sys.path.insert(0, LIB_DIR)
 
+<<<<<<< TREE
 from carbon.util import run_twistd_plugin
 
 run_twistd_plugin(__file__)
+=======
+# Capture useful debug info for this commonly reported problem
+try:
+  import carbon
+except ImportError:
+  print 'Failed to import carbon, debug information follows.'
+  print 'pwd=%s' % os.getcwd()
+  print 'sys.path=%s' % sys.path
+  print '__file__=%s' % __file__
+  sys.exit(1)
+
+
+# Parse command line options
+parser = optparse.OptionParser(usage='%prog [options] <start|stop|status>')
+parser.add_option('--debug', action='store_true', help='Run in the foreground, log to stdout')
+parser.add_option('--profile', help='Record performance profile data to the given file')
+parser.add_option('--pidfile', default=join(STORAGE_DIR, '%s.pid' % program.split('.')[0]), help='Write pid to the given file')
+parser.add_option('--config', default=join(CONF_DIR, 'carbon.conf'), help='Use the given config file')
+parser.add_option('--logdir', default=LOG_DIR, help="Write logs in the given directory")
+
+(options, args) = parser.parse_args()
+
+if not args:
+  parser.print_usage()
+  raise SystemExit(1)
+
+action = args[0]
+
+if action == 'stop':
+  if not exists(options.pidfile):
+    print 'Pidfile %s does not exist' % options.pidfile
+    raise SystemExit(0)
+
+  pidfile = open(options.pidfile, 'r')
+  try:
+    pid = int( pidfile.read().strip() )
+  except:
+    print 'Could not read pidfile %s' % options.pidfile
+    raise SystemExit(1)
+
+  print 'Deleting %s (contained pid %d)' % (options.pidfile, pid)
+  os.unlink(options.pidfile)
+
+  print 'Sending kill signal to pid %d' % pid
+  os.kill(pid, 15)
+  raise SystemExit(0)
+
+
+elif action == 'status':
+  if not exists(options.pidfile):
+    print '%s is not running' % program
+    raise SystemExit(0)
+
+  pidfile = open(options.pidfile, 'r')
+  try:
+    pid = int( pidfile.read().strip() )
+  except:
+    print 'Failed to read pid from %s' % options.pidfile
+    raise SystemExit(1)
+
+  if exists('/proc/%d' % pid):
+    print "%s is running with pid %d" % (program, pid)
+    raise SystemExit(0)
+  else:
+    print "%s is not running" % program
+    raise SystemExit(0)
+
+elif action != 'start':
+  parser.print_usage()
+  raise SystemExit(1)
+
+
+if exists(options.pidfile):
+  print "Pidfile %s already exists, is %s already running?" % (options.pidfile, program)
+  raise SystemExit(1)
+
+# Import application components
+from carbon.conf import settings
+# Read config (we want failures to occur before daemonizing) and settings is needed in some imported modules below
+settings.readFrom(options.config, 'cache')
+from carbon.log import logToStdout, logToDir
+from carbon.listeners import MetricLineReceiver, MetricPickleReceiver, CacheQueryHandler, startListener
+from carbon.cache import MetricCache
+from carbon.writer import startWriter
+from carbon.instrumentation import startRecordingCacheMetrics
+from carbon.events import metricReceived
+
+use_amqp = settings.get("ENABLE_AMQP", False)
+if use_amqp:
+  from carbon import amqp_listener
+  amqp_host = settings.get("AMQP_HOST", "localhost")
+  amqp_port = settings.get("AMQP_PORT", 5672)
+  amqp_user = settings.get("AMQP_USER", "guest")
+  amqp_password = settings.get("AMQP_PASSWORD", "guest")
+  amqp_verbose  = settings.get("AMQP_VERBOSE", False)
+  amqp_vhost    = settings.get("AMQP_VHOST", "/")
+  amqp_spec     = settings.get("AMQP_SPEC", None)
+  amqp_exchange_name = settings.get("AMQP_EXCHANGE", "graphite")
+
+
+# --debug
+if options.debug:
+  logToStdout()
+
+else:
+  if not isdir(options.logdir):
+    os.makedirs(options.logdir)
+
+  if settings.USER:
+    print "Dropping privileges to become the user %s" % settings.USER
+
+  from carbon.util import daemonize, dropprivs
+  daemonize()
+
+  pidfile = open(options.pidfile, 'w')
+  pidfile.write( str(os.getpid()) )
+  pidfile.close()
+
+  def shutdown():
+    if os.path.exists(options.pidfile):
+      os.unlink(options.pidfile)
+
+  atexit.register(shutdown)
+
+  if settings.USER:
+    pwent = pwd.getpwnam(settings.USER)
+    os.chown(options.pidfile, pwent.pw_uid, pwent.pw_gid)
+    dropprivs(settings.USER)
+
+  logToDir(options.logdir)
+
+# Configure application components
+metricReceived.installHandler(MetricCache.store)
+startListener(settings.LINE_RECEIVER_INTERFACE, settings.LINE_RECEIVER_PORT, MetricLineReceiver)
+startListener(settings.PICKLE_RECEIVER_INTERFACE, settings.PICKLE_RECEIVER_PORT, MetricPickleReceiver)
+startListener(settings.CACHE_QUERY_INTERFACE, settings.CACHE_QUERY_PORT, CacheQueryHandler)
+
+if use_amqp:
+  amqp_listener.startReceiver(amqp_host, amqp_port, amqp_user, amqp_password,
+                              vhost=amqp_vhost, spec=amqp_spec,
+                              exchange_name=amqp_exchange_name,
+                              verbose=amqp_verbose)
+
+startWriter()
+startRecordingCacheMetrics()
+
+
+# Run the twisted reactor
+print "%s running" % program
+
+if options.profile:
+  import cProfile
+
+  if exists(options.profile):
+    os.unlink(options.profile)
+
+  cProfile.run('reactor.run()', options.profile)
+
+else:
+  reactor.run()
+>>>>>>> MERGE-SOURCE
 
=== added file 'carbon/conf/blacklist'
=== modified file 'carbon/conf/carbon.conf.example'
--- carbon/conf/carbon.conf.example	2011-08-15 07:09:56 +0000
+++ carbon/conf/carbon.conf.example	2011-08-26 21:17:16 +0000
@@ -82,10 +82,22 @@
 # degrade performance if logging on the same volume as the whisper data is stored.
 LOG_UPDATES = False
 
+<<<<<<< TREE
 # On some systems it is desirable for whisper to write synchronously.
 # Set this option to True if you'd like to try this. Basically it will
 # shift the onus of buffering writes from the kernel into carbon's cache.
 WHISPER_AUTOFLUSH = False
+=======
+# Adding regular expressions to the whitelist makes carbon accept only matching metrics
+#WHITELIST = /opt/graphite/conf/whitelist
+# Logging metrics that don't match the whitelist can be useful but can degrade performance
+#LOG_WHITELIST = False
+
+# Adding regular expressions to the blacklist prevents carbon from accepting matching metrics
+#BLACKLIST = /opt/graphite/conf/blacklist
+# Logging metrics that match the blacklist can be useful but can degrade performance
+#LOG_BLACKLIST = False
+>>>>>>> MERGE-SOURCE
 
 # Enable AMQP if you want to receve metrics using an amqp broker
 # ENABLE_AMQP = False
 
=== added file 'carbon/conf/whitelist'
--- carbon/conf/whitelist	1970-01-01 00:00:00 +0000
+++ carbon/conf/whitelist	2011-08-26 21:17:16 +0000
@@ -0,0 +1,1 @@
+.*
 
=== modified file 'carbon/lib/carbon/protocols.py'
--- carbon/lib/carbon/protocols.py	2011-08-18 08:48:19 +0000
+++ carbon/lib/carbon/protocols.py	2011-08-26 21:17:16 +0000
@@ -2,13 +2,28 @@
 from twisted.internet.protocol import DatagramProtocol
 from twisted.internet.error import ConnectionDone
 from twisted.protocols.basic import LineOnlyReceiver, Int32StringReceiver
+<<<<<<< TREE
 from carbon import log, events, state, management
+=======
+from carbon.cache import MetricCache
+from carbon.relay import relay
+from carbon.instrumentation import increment
+from carbon.events import metricReceived
+from carbon import log
+from carbon.conf import settings
+import regexlist
+>>>>>>> MERGE-SOURCE
 
 try:
   import cPickle as pickle
 except ImportError:
   import pickle
 
+# Load whitelist and blacklist
+if 'WHITELIST' in settings:
+	whitelist = regexlist.RegexList(settings.WHITELIST)
+if 'BLACKLIST' in settings:
+	blacklist = regexlist.RegexList(settings.BLACKLIST)
 
 class MetricReceiver:
   """ Base class for all metric receiving protocols, handles flow
@@ -60,6 +75,7 @@
       log.listener('invalid line received from client %s, ignoring' % self.peerName)
       return
 
+<<<<<<< TREE
     events.metricReceived(metric, datapoint)
 
 
@@ -76,6 +92,21 @@
 
 
 class MetricPickleReceiver(MetricReceiver, Int32StringReceiver):
+=======
+    if 'BLACKLIST' in settings and blacklist.in_list(metric):
+      log.listener('ignoring metric %s which is blacklisted' % metric)
+      return
+
+    if 'WHITELIST' in settings and not whitelist.in_list(metric):
+      log.listener('ignoring metric %s which is not whitelisted' % metric)
+      return
+
+    increment('metricsReceived')
+    metricReceived(metric, datapoint)
+
+
+class MetricPickleReceiver(LoggingMixin, Int32StringReceiver):
+>>>>>>> MERGE-SOURCE
   MAX_LENGTH = 2 ** 20
 
   def stringReceived(self, data):
@@ -89,6 +120,7 @@
       try:
         datapoint = ( float(datapoint[0]), float(datapoint[1]) ) #force proper types
       except:
+<<<<<<< TREE
         continue
 
       if datapoint[1] == datapoint[1]: # filter out NaN values
@@ -126,6 +158,33 @@
       result = dict(error="Invalid request type \"%s\"" % request['type'])
 
     response = pickle.dumps(result, protocol=-1)
+=======
+        log.listener('invalid pickle received from client %s, ignoring' % self.peerAddr)
+        continue
+
+      if 'BLACKLIST' in settings and blacklist.in_list(metric):
+        log.listener('ignoring metric %s which is blacklisted' % metric)
+        continue
+
+      if 'WHITELIST' in settings and not whitelist.in_list(metric):
+        log.listener('ignoring metric %s which is not whitelisted' % metric)
+        continue
+
+      if datapoint[1] != datapoint[1]: # filter out NaN values
+        log.listener('ingoring metric with NaN value, %s' % metric)
+        continue
+
+      metricReceived(metric, datapoint)
+
+    increment('metricsReceived', len(datapoints))
+
+
+class CacheQueryHandler(LoggingMixin, Int32StringReceiver):
+  def stringReceived(self, metric):
+    values = MetricCache.get(metric, [])
+    log.query('cache query for %s returned %d values' % (metric, len(values)))
+    response = pickle.dumps(values, protocol=-1)
+>>>>>>> MERGE-SOURCE
     self.sendString(response)
 
 
 
=== modified file 'carbon/lib/carbon/writer.py'
--- carbon/lib/carbon/writer.py	2011-08-15 21:16:40 +0000
+++ carbon/lib/carbon/writer.py	2011-08-26 21:17:16 +0000
@@ -15,6 +15,7 @@
 
 import os
 import time
+import re
 from os.path import join, exists, dirname, basename
 from carbon import state
 
@@ -41,6 +42,65 @@
 CACHE_SIZE_LOW_WATERMARK = settings.MAX_CACHE_SIZE * 0.95
 
 
+class RegexList:
+  """ List of regexs """
+
+  def __init__(self, listFile):
+    self.regexs = []
+    self.listFile = listFile
+    self.mtime = None
+    self.lastUpdate = time.time()
+    self.update()
+
+  def update(self):
+    """ If the list file has changed, reload it """
+    self.lastUpdate = time.time()
+    if not os.path.exists(self.listFile):
+      self.regexs = []
+      self.mtime = None
+      return
+    mtime = os.stat(self.listFile).st_mtime
+    if mtime > self.mtime:
+      self.mtime = mtime
+      """ Load the list file """
+      self.regexs = []
+      self.mtime = os.stat(self.listFile).st_mtime
+      file = open(self.listFile)
+      patterns = file.read().split('\n')
+      file.close()
+      for pattern in patterns:
+        p = pattern.strip()
+        if p and p[0] != '#':
+          self.regexs.append(re.compile(p))
+
+  def in_list(self, value):
+    """ Test if regexs is listed """
+    if time.time() - self.lastUpdate > 5:
+      self.update()
+    for regex in self.regexs:
+      if regex.search(value):
+        return True
+    return False
+
+  def matching(self, value):
+    """ Returns the regular expressions that matches a value """
+    matches = []
+    for regex in self.regexs:
+      if regex.search(value):
+        matches.append(regex.pattern)
+    return matches
+
+# Load whitelist and blacklist
+if settings.WHITELIST:
+  whitelist = RegexList(settings.WHITELIST)
+else:
+  whitelist = None
+if settings.BLACKLIST:
+  blacklist = RegexList(settings.BLACKLIST)
+else:
+  blacklist = None
+
+
 def optimalWriteOrder():
   "Generates metrics with the most cached values first and applies a soft rate limit on new metrics"
   global lastCreateInterval
@@ -96,6 +156,16 @@
     for (metric, datapoints, dbFilePath, dbFileExists) in optimalWriteOrder():
       dataWritten = True
 
+      # Skip metrics that are blacklisted or not in the whitelist
+      if blacklist and blacklist.in_list(metric):
+        if settings.LOG_BLACKLIST:
+            log.msg('metric %s is blacklisted' % metric)
+        continue
+      if whitelist and not whitelist.in_list(metric):
+        if settings.LOG_WHITELIST:
+            log.msg('metric %s is not whitelisted' % metric)
+        continue
+
       if not dbFileExists:
         archiveConfig = None