From de73a585e61b9d7c42631c49bddc4ee46842f598 Mon Sep 17 00:00:00 2001 From: Mauro Stettler Date: Mon, 9 Mar 2020 15:05:03 -0300 Subject: [PATCH 1/5] log grafana dashboard/panel id headers --- webapp/graphite/render/evaluator.py | 49 +++++++++++++++++------ webapp/graphite/render/views.py | 4 +- webapp/graphite/storage.py | 11 ++++++ webapp/tests/test_render.py | 61 ++++++++++++++++++++++++++++- 4 files changed, 111 insertions(+), 14 deletions(-) diff --git a/webapp/graphite/render/evaluator.py b/webapp/graphite/render/evaluator.py index c5cb1a257..a268468de 100644 --- a/webapp/graphite/render/evaluator.py +++ b/webapp/graphite/render/evaluator.py @@ -104,18 +104,7 @@ def evaluateTokens(requestContext, tokens, replacements=None, pipedArg=None): def handleInvalidParameters(e): if not getattr(handleInvalidParameters, 'alreadyLogged', False): - log.warning( - 'Received invalid parameters ({msg}): {func} ({args})'.format( - msg=str(e), - func=tokens.call.funcname, - args=', '.join( - argList - for argList in [ - ', '.join(repr(arg) for arg in args), - ', '.join('{k}={v}'.format(k=str(k), v=repr(v)) for k, v in kwargs.items()), - ] if argList - ) - )) + log.warning(invalidParamLogMsg(requestContext, str(e), tokens.call.funcname, args, kwargs)) # only log invalid parameters once setattr(handleInvalidParameters, 'alreadyLogged', True) @@ -139,6 +128,42 @@ def handleInvalidParameters(e): return evaluateScalarTokens(tokens) +def invalidParamLogMsg(requestContext, exception, func, args, kwargs): + source = '' + + if 'sourceIdHeaders' in requestContext: + if 'HTTP_X_GRAFANA_ORG_ID' in requestContext['sourceIdHeaders']: + source += 'org-id: {id}'.format( + id=requestContext['sourceIdHeaders']['HTTP_X_GRAFANA_ORG_ID']) + if 'HTTP_X_DASHBOARD_ID' in requestContext['sourceIdHeaders']: + if source: + source += ', ' + source += 'dashboard-id: {id}'.format( + id=requestContext['sourceIdHeaders']['HTTP_X_DASHBOARD_ID']) + if 'HTTP_X_PANEL_ID' in requestContext['sourceIdHeaders']: + if source: + source += ', ' + source += 'panel-id: {id}'.format( + id=requestContext['sourceIdHeaders']['HTTP_X_PANEL_ID']) + + logMsg = 'Received invalid parameters ({msg}): {func} ({args})'.format( + msg=exception, + func=func, + args=', '.join( + argList + for argList in [ + ', '.join(repr(arg) for arg in args), + ', '.join('{k}={v}'.format(k=str(k), v=repr(v)) for k, v in kwargs.items()), + ] if argList + )) + + if not source: + return logMsg + + logMsg += '; source: ({source})'.format(source=source) + return logMsg + + def evaluateScalarTokens(tokens): if tokens.number: if tokens.number.integer: diff --git a/webapp/graphite/render/views.py b/webapp/graphite/render/views.py index 5624d96dc..3d2794e3f 100755 --- a/webapp/graphite/render/views.py +++ b/webapp/graphite/render/views.py @@ -25,7 +25,7 @@ from graphite.errors import InputParameterError, handleInputParameterError from graphite.user_util import getProfileByUsername from graphite.util import json, unpickle, pickle, msgpack, BytesIO -from graphite.storage import extractForwardHeaders +from graphite.storage import extractForwardHeaders, extractSourceIdHeaders from graphite.logger import log from graphite.render.evaluator import evaluateTarget from graphite.render.attime import parseATTime @@ -68,6 +68,7 @@ def renderView(request): 'template' : requestOptions['template'], 'tzinfo' : requestOptions['tzinfo'], 'forwardHeaders': requestOptions['forwardHeaders'], + 'sourceIdHeaders': requestOptions['sourceIdHeaders'], 'data' : [], 'prefetched' : {}, 'xFilesFactor' : requestOptions['xFilesFactor'], @@ -352,6 +353,7 @@ def parseOptions(request): cacheTimeout = int( queryParams.get('cacheTimeout', settings.DEFAULT_CACHE_DURATION) ) requestOptions['targets'] = [] requestOptions['forwardHeaders'] = extractForwardHeaders(request) + requestOptions['sourceIdHeaders'] = extractSourceIdHeaders(request) # Extract the targets out of the queryParams mytargets = [] diff --git a/webapp/graphite/storage.py b/webapp/graphite/storage.py index f8ce0edc3..282f5956c 100755 --- a/webapp/graphite/storage.py +++ b/webapp/graphite/storage.py @@ -517,6 +517,17 @@ def extractForwardHeaders(request): return headers +# headers which get set by Grafana when issuing queries, identifying where the query came from +def extractSourceIdHeaders(request): + source_id_headers = ['HTTP_X_GRAFANA_ORG_ID', 'HTTP_X_DASHBOARD_ID', 'HTTP_X_PANEL_ID'] + headers = {} + for header in source_id_headers: + value = request.META.get(header) + if value is not None: + headers[header] = value + return headers + + def write_index(index=None): if not index: index = settings.INDEX_FILE diff --git a/webapp/tests/test_render.py b/webapp/tests/test_render.py index 953786518..d2d2bf11a 100644 --- a/webapp/tests/test_render.py +++ b/webapp/tests/test_render.py @@ -12,7 +12,7 @@ from graphite.render.datalib import TimeSeries from graphite.render.hashing import ConsistentHashRing, hashRequest, hashData -from graphite.render.evaluator import evaluateTarget, extractPathExpressions, evaluateScalarTokens +from graphite.render.evaluator import evaluateTarget, extractPathExpressions, evaluateScalarTokens, invalidParamLogMsg from graphite.render.functions import NormalizeEmptyResultError from graphite.render.grammar import grammar from graphite.render.views import renderViewJson @@ -925,6 +925,65 @@ def test_template_pathExpression_variables(self): data = json.loads(response.content)[0] self.assertEqual(data['target'], 'sumSeries(hosts.worker*.cpu)') + def test_invalid_parameter_log_message(self): + # first testing without source headers + requestContext = {} + exception = 'exception details' + func = 'test_func' + args = ['arg1'] + kwargs = {} + + msg = invalidParamLogMsg(requestContext, exception, func, args, kwargs) + self.assertEqual( + msg, + 'Received invalid parameters (exception details): test_func (\'arg1\')' + ) + + args = [] + kwargs = {'arg': 'testvalue'} + msg = invalidParamLogMsg(requestContext, exception, func, args, kwargs) + self.assertEqual( + msg, + 'Received invalid parameters (exception details): test_func (arg=\'testvalue\')' + ) + + kwargs = {'arg': '3.3'} + msg = invalidParamLogMsg(requestContext, exception, func, args, kwargs) + self.assertEqual( + msg, + 'Received invalid parameters (exception details): test_func (arg=\'3.3\')' + ) + + args = [float('INF')] + kwargs = {'kwarg1': True} + msg = invalidParamLogMsg(requestContext, exception, func, args, kwargs) + self.assertEqual( + msg, + 'Received invalid parameters (exception details): test_func (inf, kwarg1=True)' + ) + + args = [1] + kwargs = {} + requestContext['sourceIdHeaders'] = { + 'HTTP_X_GRAFANA_ORG_ID': 123, + } + msg = invalidParamLogMsg(requestContext, exception, func, args, kwargs) + self.assertEqual( + msg, + 'Received invalid parameters (exception details): test_func (1); source: (org-id: 123)' + ) + + requestContext['sourceIdHeaders'] = { + 'HTTP_X_GRAFANA_ORG_ID': 123, + 'HTTP_X_DASHBOARD_ID': 9, + 'HTTP_X_PANEL_ID': 38, + } + msg = invalidParamLogMsg(requestContext, exception, func, args, kwargs) + self.assertEqual( + msg, + 'Received invalid parameters (exception details): test_func (1); source: (org-id: 123, dashboard-id: 9, panel-id: 38)' + ) + class ConsistentHashRingTest(TestCase): def test_chr_compute_ring_position(self): From eb316bd86cb49c3d19141e9e617d771c9a519865 Mon Sep 17 00:00:00 2001 From: Mauro Stettler Date: Mon, 9 Mar 2020 21:43:16 -0300 Subject: [PATCH 2/5] move extractSourceHeaders to more appropriate place --- webapp/graphite/render/views.py | 13 ++++++++++++- webapp/graphite/storage.py | 11 ----------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/webapp/graphite/render/views.py b/webapp/graphite/render/views.py index 3d2794e3f..e01a4e18f 100755 --- a/webapp/graphite/render/views.py +++ b/webapp/graphite/render/views.py @@ -25,7 +25,7 @@ from graphite.errors import InputParameterError, handleInputParameterError from graphite.user_util import getProfileByUsername from graphite.util import json, unpickle, pickle, msgpack, BytesIO -from graphite.storage import extractForwardHeaders, extractSourceIdHeaders +from graphite.storage import extractForwardHeaders from graphite.logger import log from graphite.render.evaluator import evaluateTarget from graphite.render.attime import parseATTime @@ -463,6 +463,17 @@ def parseOptions(request): return (graphOptions, requestOptions) +# headers which get set by Grafana when issuing queries, identifying where the query came from +def extractSourceIdHeaders(request): + source_id_headers = ['HTTP_X_GRAFANA_ORG_ID', 'HTTP_X_DASHBOARD_ID', 'HTTP_X_PANEL_ID'] + headers = {} + for header in source_id_headers: + value = request.META.get(header) + if value is not None: + headers[header] = value + return headers + + connectionPools = {} diff --git a/webapp/graphite/storage.py b/webapp/graphite/storage.py index 282f5956c..f8ce0edc3 100755 --- a/webapp/graphite/storage.py +++ b/webapp/graphite/storage.py @@ -517,17 +517,6 @@ def extractForwardHeaders(request): return headers -# headers which get set by Grafana when issuing queries, identifying where the query came from -def extractSourceIdHeaders(request): - source_id_headers = ['HTTP_X_GRAFANA_ORG_ID', 'HTTP_X_DASHBOARD_ID', 'HTTP_X_PANEL_ID'] - headers = {} - for header in source_id_headers: - value = request.META.get(header) - if value is not None: - headers[header] = value - return headers - - def write_index(index=None): if not index: index = settings.INDEX_FILE From fe9535a936dedc6e496f346d269e02d93564775a Mon Sep 17 00:00:00 2001 From: Mauro Stettler Date: Mon, 9 Mar 2020 22:05:53 -0300 Subject: [PATCH 3/5] allow user to use custom headers to ID bad queries --- webapp/graphite/render/evaluator.py | 17 ++++++----------- webapp/graphite/render/views.py | 22 +++++++++++++++++----- webapp/graphite/settings.py | 7 +++++++ webapp/tests/test_render.py | 12 ++++++------ 4 files changed, 36 insertions(+), 22 deletions(-) diff --git a/webapp/graphite/render/evaluator.py b/webapp/graphite/render/evaluator.py index a268468de..9a73a7ae9 100644 --- a/webapp/graphite/render/evaluator.py +++ b/webapp/graphite/render/evaluator.py @@ -132,19 +132,14 @@ def invalidParamLogMsg(requestContext, exception, func, args, kwargs): source = '' if 'sourceIdHeaders' in requestContext: - if 'HTTP_X_GRAFANA_ORG_ID' in requestContext['sourceIdHeaders']: - source += 'org-id: {id}'.format( - id=requestContext['sourceIdHeaders']['HTTP_X_GRAFANA_ORG_ID']) - if 'HTTP_X_DASHBOARD_ID' in requestContext['sourceIdHeaders']: + headers = list(requestContext['sourceIdHeaders'].keys()) + headers.sort() + for name in headers: if source: source += ', ' - source += 'dashboard-id: {id}'.format( - id=requestContext['sourceIdHeaders']['HTTP_X_DASHBOARD_ID']) - if 'HTTP_X_PANEL_ID' in requestContext['sourceIdHeaders']: - if source: - source += ', ' - source += 'panel-id: {id}'.format( - id=requestContext['sourceIdHeaders']['HTTP_X_PANEL_ID']) + source += '{name}: {value}'.format( + name=name, + value=requestContext['sourceIdHeaders'][name]) logMsg = 'Received invalid parameters ({msg}): {func} ({args})'.format( msg=exception, diff --git a/webapp/graphite/render/views.py b/webapp/graphite/render/views.py index e01a4e18f..230229577 100755 --- a/webapp/graphite/render/views.py +++ b/webapp/graphite/render/views.py @@ -463,14 +463,26 @@ def parseOptions(request): return (graphOptions, requestOptions) -# headers which get set by Grafana when issuing queries, identifying where the query came from +# extract headers which get set by Grafana when issuing queries, to help identifying where a query came from. +# user-defined headers from settings.INPUT_VALIDATION_SOURCE_ID_HEADERS also get extracted and mixed +# with the standard Grafana headers. def extractSourceIdHeaders(request): - source_id_headers = ['HTTP_X_GRAFANA_ORG_ID', 'HTTP_X_DASHBOARD_ID', 'HTTP_X_PANEL_ID'] headers = {} - for header in source_id_headers: - value = request.META.get(header) + + if 'HTTP_X_GRAFANA_ORG_ID' in request.META: + headers['grafana-org-id'] = request.META['HTTP_X_GRAFANA_ORG_ID'] + + if 'HTTP_X_DASHBOARD_ID' in request.META: + headers['dashboard-id'] = request.META['HTTP_X_DASHBOARD_ID'] + + if 'HTTP_X_PANEL_ID' in request.META: + headers['panel-id'] = request.META['HTTP_X_PANEL_ID'] + + for name in settings.INPUT_VALIDATION_SOURCE_ID_HEADERS: + value = request.META.get('HTTP_%s' % name.upper().replace('-', '_')) if value is not None: - headers[header] = value + headers[settings.INPUT_VALIDATION_SOURCE_ID_HEADERS[name]] = value + return headers diff --git a/webapp/graphite/settings.py b/webapp/graphite/settings.py index 657af8441..fdd957958 100644 --- a/webapp/graphite/settings.py +++ b/webapp/graphite/settings.py @@ -189,6 +189,13 @@ # provided arguments and return an error message to the user ENFORCE_INPUT_VALIDATION = False +# headers which shall be added to log statements informing about invalid queries, +# this is useful to identify where a query came from. +# The dict is keyed by the header name and the associated value is a short description +# of the header which will be used in the log statement, for example: +# {'X-FORWARD-FOR': 'forwarded-for'} +INPUT_VALIDATION_SOURCE_ID_HEADERS = {} + # Django 1.5 requires this to be set. Here we default to prior behavior and allow all ALLOWED_HOSTS = [ '*' ] diff --git a/webapp/tests/test_render.py b/webapp/tests/test_render.py index d2d2bf11a..e45fc9126 100644 --- a/webapp/tests/test_render.py +++ b/webapp/tests/test_render.py @@ -965,23 +965,23 @@ def test_invalid_parameter_log_message(self): args = [1] kwargs = {} requestContext['sourceIdHeaders'] = { - 'HTTP_X_GRAFANA_ORG_ID': 123, + 'grafana-org-id': 123, } msg = invalidParamLogMsg(requestContext, exception, func, args, kwargs) self.assertEqual( msg, - 'Received invalid parameters (exception details): test_func (1); source: (org-id: 123)' + 'Received invalid parameters (exception details): test_func (1); source: (grafana-org-id: 123)' ) requestContext['sourceIdHeaders'] = { - 'HTTP_X_GRAFANA_ORG_ID': 123, - 'HTTP_X_DASHBOARD_ID': 9, - 'HTTP_X_PANEL_ID': 38, + 'grafana-org-id': 123, + 'dashboard-id': 9, + 'panel-id': 38, } msg = invalidParamLogMsg(requestContext, exception, func, args, kwargs) self.assertEqual( msg, - 'Received invalid parameters (exception details): test_func (1); source: (org-id: 123, dashboard-id: 9, panel-id: 38)' + 'Received invalid parameters (exception details): test_func (1); source: (dashboard-id: 9, grafana-org-id: 123, panel-id: 38)' ) From 4f4458c21ba9df8c8c0f42aa21dde947ca7a0b50 Mon Sep 17 00:00:00 2001 From: Mauro Stettler Date: Tue, 10 Mar 2020 18:03:11 -0300 Subject: [PATCH 4/5] refactor code a bit to make it nicer --- webapp/graphite/render/views.py | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/webapp/graphite/render/views.py b/webapp/graphite/render/views.py index 230229577..0ecfa7b60 100755 --- a/webapp/graphite/render/views.py +++ b/webapp/graphite/render/views.py @@ -467,21 +467,18 @@ def parseOptions(request): # user-defined headers from settings.INPUT_VALIDATION_SOURCE_ID_HEADERS also get extracted and mixed # with the standard Grafana headers. def extractSourceIdHeaders(request): - headers = {} - - if 'HTTP_X_GRAFANA_ORG_ID' in request.META: - headers['grafana-org-id'] = request.META['HTTP_X_GRAFANA_ORG_ID'] - - if 'HTTP_X_DASHBOARD_ID' in request.META: - headers['dashboard-id'] = request.META['HTTP_X_DASHBOARD_ID'] + source_headers = { + 'X-Grafana-Org-ID': 'grafana-org-id', + 'X-Dashboard-ID': 'dashboard-id', + 'X-Panel-ID': 'panel-id', + } + source_headers.update(settings.INPUT_VALIDATION_SOURCE_ID_HEADERS) - if 'HTTP_X_PANEL_ID' in request.META: - headers['panel-id'] = request.META['HTTP_X_PANEL_ID'] - - for name in settings.INPUT_VALIDATION_SOURCE_ID_HEADERS: - value = request.META.get('HTTP_%s' % name.upper().replace('-', '_')) - if value is not None: - headers[settings.INPUT_VALIDATION_SOURCE_ID_HEADERS[name]] = value + headers = {} + for hdr_name, log_name in source_headers.items(): + value = request.META.get('HTTP_' + hdr_name.upper().replace('-', '_')) + if value: + headers[log_name] = value return headers From 29bb1111924b9850a1b6bcf789bfccb20db42e7b Mon Sep 17 00:00:00 2001 From: Mauro Stettler Date: Fri, 13 Mar 2020 16:43:33 -0300 Subject: [PATCH 5/5] allow floats in scaleToSeconds() --- webapp/graphite/render/functions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapp/graphite/render/functions.py b/webapp/graphite/render/functions.py index 9490d5464..d478d0e63 100644 --- a/webapp/graphite/render/functions.py +++ b/webapp/graphite/render/functions.py @@ -1315,7 +1315,7 @@ def scaleToSeconds(requestContext, seriesList, seconds): scaleToSeconds.group = 'Transform' scaleToSeconds.params = [ Param('seriesList', ParamTypes.seriesList, required=True), - Param('seconds', ParamTypes.integer, required=True), + Param('seconds', ParamTypes.float, required=True), ]