Ottomata has uploaded a new change for review.
https://gerrit.wikimedia.org/r/249997
Change subject: Enable varnishreqstats for all caches
......................................................................
Enable varnishreqstats for all caches
This removes the deprecated varnishreqstats-diamond collector
Change-Id: Id6ce1079386e751bde92e4069396740485399336
---
M modules/role/manifests/cache/maps.pp
M modules/role/manifests/cache/parsoid.pp
M modules/role/manifests/cache/upload.pp
D modules/varnish/manifests/monitoring/varnishreqstats.pp
D modules/varnish/templates/varnishreqstats-diamond.py.erb
5 files changed, 12 insertions(+), 332 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/operations/puppet
refs/changes/97/249997/1
diff --git a/modules/role/manifests/cache/maps.pp
b/modules/role/manifests/cache/maps.pp
index 3ed6f2b..4317bd9 100644
--- a/modules/role/manifests/cache/maps.pp
+++ b/modules/role/manifests/cache/maps.pp
@@ -119,10 +119,9 @@
}
}
- # Parse varnishlogs for request statistics and send to statsd via diamond.
- varnish::monitoring::varnishreqstats { 'MapsFrontend':
- instance_name => 'frontend',
- metric_path => "varnish.${::site}.maps.frontend.request",
- require => Varnish::Instance['maps-frontend'],
+ # Parse varnishlogs for request statistics and send to statsd.
+ varnish::logging::reqstats { 'frontend':
+ metric_prefix => "varnish.${::site}.maps.frontend.request",
+ statsd => hiera('statsd'),
}
}
diff --git a/modules/role/manifests/cache/parsoid.pp
b/modules/role/manifests/cache/parsoid.pp
index 131e144..bc227c0 100644
--- a/modules/role/manifests/cache/parsoid.pp
+++ b/modules/role/manifests/cache/parsoid.pp
@@ -146,10 +146,9 @@
]),
}
- # Parse varnishlogs for request statistics and send to statsd via diamond.
- varnish::monitoring::varnishreqstats { 'ParsoidFrontend':
- instance_name => 'frontend',
- metric_path => "varnish.${::site}.parsoid.frontend.request",
- require => Varnish::Instance['parsoid-frontend'],
+ # Parse varnishlogs for request statistics and send to statsd.
+ varnish::logging::reqstats { 'frontend':
+ metric_prefix => "varnish.${::site}.parsoid.frontend.request",
+ statsd => hiera('statsd'),
}
}
diff --git a/modules/role/manifests/cache/upload.pp
b/modules/role/manifests/cache/upload.pp
index fc5d1c7..ef26c28 100644
--- a/modules/role/manifests/cache/upload.pp
+++ b/modules/role/manifests/cache/upload.pp
@@ -144,10 +144,9 @@
statsd_server => 'statsd.eqiad.wmnet',
}
- # Parse varnishlogs for request statistics and send to statsd via diamond.
- varnish::monitoring::varnishreqstats { 'UploadFrontend':
- instance_name => 'frontend',
- metric_path => "varnish.${::site}.upload.frontend.request",
- require => Varnish::Instance['upload-frontend'],
+ # Parse varnishlogs for request statistics and send to statsd.
+ varnish::logging::reqstats { 'frontend':
+ metric_prefix => "varnish.${::site}.upload.frontend.request",
+ statsd => hiera('statsd'),
}
}
diff --git a/modules/varnish/manifests/monitoring/varnishreqstats.pp
b/modules/varnish/manifests/monitoring/varnishreqstats.pp
deleted file mode 100644
index 5f6b57f..0000000
--- a/modules/varnish/manifests/monitoring/varnishreqstats.pp
+++ /dev/null
@@ -1,38 +0,0 @@
-# == Define varnish::monitoring::varnishreqstats:
-# Installs a Diamond collector that sends varnish request stats to
statsd/graphite.
-#
-# === Parameters
-# [*instance_name*]
-# Name of the varnish instance from which to gather request stats.
-# Default: $name.
-#
-#
-# [*metric_path*]
-# Prefix of the stats that will be sent to statsd/graphite.
-# Default: varnish.$site.$name.request.
-# If $name is text.frontend, this will create graphite keys that look like:
-# servers.cp1052.varnish.eqiad.text.frontend.request.client.status.2xx
-#
-
-# NOTE: This does not work. The combination of diamond + varnishlog ctypes
-# + multiprocessing causes segfaults. This is being removed in favor of
-# varnish::logging::reqstats, which sends directly to statsd rather than
-# via diamond.
-#
-define varnish::monitoring::varnishreqstats(
- $instance_name = $name,
- $metric_path = "varnish.${::site}.${name}.request",
- $ensure = 'absent',
-) {
- # ${collector_name}Collector will be used as the python diamond collector
class name
- # when varnishreqstats-diamond.py.erb is rendered.
- $collector_name = "Varnishreqstats${name}"
- diamond::collector { $collector_name:
- ensure => $ensure,
- content => template('varnish/varnishreqstats-diamond.py.erb'),
- settings => {
- 'varnish_name' => $instance_name,
- 'path' => $metric_path,
- },
- }
-}
diff --git a/modules/varnish/templates/varnishreqstats-diamond.py.erb
b/modules/varnish/templates/varnishreqstats-diamond.py.erb
deleted file mode 100755
index c3c2888..0000000
--- a/modules/varnish/templates/varnishreqstats-diamond.py.erb
+++ /dev/null
@@ -1,279 +0,0 @@
-# -*- coding: utf-8 -*-
-"""
-Diamond collector for counting varnish HTTP requests
-grouped by several dimensions, including:
- - Total requests
- - HTTP Status Code (200, 404, etc.)
- - HTTP Status Code Class (2xx, 3xx, etc.)
- - HTTP Status type (ok, error)
- - HTTP Method (GET, POST, etc.)
-
-If run from the command line, these counts will be
-printed to stdout. Otherwise diamond will send them
-off to statsd/graphite.
-
- Copyright 2015 Andrew Otto <[email protected]>
-
- Licensed under the Apache License, Version 2.0 (the "License");
- you may not use this file except in compliance with the License.
- You may obtain a copy of the License at
-
- http://www.apache.org/licenses/LICENSE-2.0
-
- Unless required by applicable law or agreed to in writing, software
- distributed under the License is distributed on an "AS IS" BASIS,
- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- See the License for the specific language governing permissions and
- limitations under the License.
-
- Author: Andrew Otto
-
-"""
-import argparse
-import datetime
-from diamond.collector import Collector
-from multiprocessing import Manager, Process
-import sys
-import time
-import unittest
-
-from varnishlog import varnishlog
-
-valid_http_methods = (
- 'get',
- 'head',
- 'post',
- 'put',
- 'delete',
- 'trace',
- 'connect',
- 'options',
- 'purge',
-)
-
-# Initialize a dict of default counts
-# we will always report.
-default_counts = {
- 'backend.status.1xx': 0,
- 'backend.status.2xx': 0,
- 'backend.status.3xx': 0,
- 'backend.status.4xx': 0,
- 'backend.status.5xx': 0,
- 'backend.status.ok': 0,
- 'backend.status.error': 0,
- 'backend.total': 0,
-
- 'client.status.1xx': 0,
- 'client.status.2xx': 0,
- 'client.status.3xx': 0,
- 'client.status.4xx': 0,
- 'client.status.5xx': 0,
- 'client.status.ok': 0,
- 'client.status.error': 0,
- 'client.total': 0,
-}
-# Include valid http_methods in default_counts.
-for m in valid_http_methods:
- default_counts['backend.method.' + m] = 0
- default_counts['client.method.' + m] = 0
-
-
-def is_valid_http_method(m):
- """
- Returns True if m is in the list of valid_http_methods.
- """
- return m.lower() in valid_http_methods
-
-
-def is_valid_http_status(s):
- """
- Returns True if s is in a valid HTTP status range.
- """
- try:
- return 100 <= int(s) < 600
- except ValueError:
- return False
-
-
-def print_counts(counts):
- """
- Formats and prints out the contents of the counts dict.
- """
- keys = counts.keys()
- keys.sort()
- print(
- datetime.datetime.now().strftime('%Y-%m-%dT%H:%M:%S').center(31, '-')
- )
- for k in keys:
- print("{0} {1}".format(str(counts[k]).rjust(10), k))
- print('')
-
-
-def count_vsl_entries(vsl_args, counts=default_counts):
- """
- Starts varnishlog and stores counts for http status
- and http method in the counts dict. If varnishlog
- finshes (because vsl_args has -k or -r), counts
- will be returned. Otherwise, pass in a Manager DictProxy
- in as counts and access it in a different process.
- """
-
- def count_vsl_entry_callback(
- transaction_id,
- tag,
- value,
- remote_party
- ):
- if remote_party not in ['backend', 'client']:
- return
-
- # Count the http request method
- if tag in ['TxRequest', 'RxRequest'] and is_valid_http_method(value):
- counts[remote_party + '.method.' + value.lower()] += 1
-
- # Count the http response status
- elif tag in ['RxStatus', 'TxStatus'] and is_valid_http_status(value):
- key_prefix = remote_party + '.status.'
- counts[key_prefix + value[0] + 'xx'] += 1
-
- http_status_key = key_prefix + value
- counts[http_status_key] = (
- counts.setdefault(http_status_key, 0) + 1
- )
- # Increment ok/error status metric.
- if value[0] in '123':
- counts[key_prefix + 'ok'] += 1
- elif value[0] in '45':
- counts[key_prefix + 'error'] += 1
-
- # Thesee indicates we completed a request, count it.
- elif tag in ['ReqEnd', 'BackendClose', 'BackendReuse']:
- counts[remote_party + '.total'] += 1
-
- # Run varnishlog with the count_vsl_entry_callback
- # called for every VSL entry.
- varnishlog(vsl_args, count_vsl_entry_callback)
- return counts
-
-
-def init_counter_process(extra_vsl_args=[]):
- """
- Returns a (Process, DictProxy) tuple. The DictProxy wil
- be updated by Process once Process.start() is called.
- """
-
- shared_counts = Manager().dict(default_counts)
-
- vsl_args = [
- ('i', 'TxRequest'),
- ('i', 'RxStatus'),
- ('i', 'RxRequest'),
- ('i', 'TxStatus'),
- ('i', 'ReqEnd'),
- ('i', 'BackendClose'),
- ('i', 'BackendReuse')
- ]
- vsl_args += extra_vsl_args
-
- counter_process = Process(
- target=count_vsl_entries,
- args=(vsl_args, shared_counts)
- )
- return (counter_process, shared_counts)
-
-
-<% class_name = @collector_name + 'Collector' %>
-class <%= class_name %>(Collector):
-
- def __init__(self, *args, **kwargs):
- super(<%= class_name %>, self).__init__(*args, **kwargs)
-
- # Initialize self.counter_process and self.counts.
- extra_vsl_args = []
- if self.config['varnish_name']:
- extra_vsl_args.append(('n', self.config['varnish_name']))
-
- self.counter_process, self.counts = init_counter_process(
- extra_vsl_args)
- # Start the counter_process.
- self.counter_process.start()
-
- def get_default_config(self):
- """
- Returns the default collector settings.
- """
- config = super(<%= class_name %>, self).get_default_config()
- config.update({
- 'varnish_name': None,
- 'path': 'varnish.request'
- })
-
- return config
-
- def collect(self):
- """
- Publishes items collected in self.counts.
- """
- for key in self.counts.keys():
- self.publish(key, self.counts[key])
-
-
-if __name__ == '__main__':
- parser = argparse.ArgumentParser(
- description=__doc__,
- formatter_class=argparse.RawTextHelpFormatter
- )
- parser.add_argument('--varnish-name', default=None)
- parser.add_argument('--interval', default=1, type=int)
- arguments = parser.parse_args()
-
- extra_vsl_args = []
- if arguments.varnish_name:
- extra_vsl_args.append(('n', arguments.varnish_name))
-
- (counter_process, counts) = init_counter_process(
- extra_vsl_args
- )
- counter_process.start()
-
- while True:
- time.sleep(arguments.interval)
- print_counts(counts)
-
-
-# ##### Tests ######
-# To run:
-# python -m unittest varnishstats-diamond-collector
-#
-# This requires that varnishlog.test.data is present
-# in the current directory. It contains 100 entries
-# spread across 6 transactions. It was collected from
-# a real text varnish server using the varnishlog utility.
-#
-class TestVarnishreqstats(unittest.TestCase):
- varnishlog_test_data_file = 'varnishlog.test.data'
-
- def test_is_valid_http_method(self):
- self.assertTrue(is_valid_http_method('GET'))
- self.assertTrue(is_valid_http_method('get'))
- self.assertTrue(is_valid_http_method('post'))
- self.assertFalse(is_valid_http_method('nogood'))
-
- def test_is_valid_http_status(self):
- self.assertTrue(is_valid_http_status('200'))
- self.assertTrue(is_valid_http_status('404'))
- self.assertTrue(is_valid_http_status(301))
- self.assertFalse(is_valid_http_status('nogood'))
- self.assertFalse(is_valid_http_status('1000'))
-
- def test_count_vsl_entries(self):
- # Test on 100 records from varnishlog.test.data file
- extra_vsl_args = [('r', self.varnishlog_test_data_file)]
- d = count_vsl_entries(extra_vsl_args)
-
- self.assertEqual(d['client.method.post'], 0)
- self.assertEqual(d['client.status.200'], 1)
- self.assertEqual(d['client.status.2xx'], 1)
- self.assertEqual(d['client.status.ok'], 1)
- self.assertEqual(d['client.status.error'], 0)
- self.assertEqual(d['client.total'], 1)
--
To view, visit https://gerrit.wikimedia.org/r/249997
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id6ce1079386e751bde92e4069396740485399336
Gerrit-PatchSet: 1
Gerrit-Project: operations/puppet
Gerrit-Branch: production
Gerrit-Owner: Ottomata <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
