diff --git a/webapp/graphite/render/evaluator.py b/webapp/graphite/render/evaluator.py index c5cb1a257..9a73a7ae9 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,37 @@ def handleInvalidParameters(e): return evaluateScalarTokens(tokens) +def invalidParamLogMsg(requestContext, exception, func, args, kwargs): + source = '' + + if 'sourceIdHeaders' in requestContext: + headers = list(requestContext['sourceIdHeaders'].keys()) + headers.sort() + for name in headers: + if source: + source += ', ' + source += '{name}: {value}'.format( + name=name, + value=requestContext['sourceIdHeaders'][name]) + + 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/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), ] diff --git a/webapp/graphite/render/views.py b/webapp/graphite/render/views.py index 5624d96dc..0ecfa7b60 100755 --- a/webapp/graphite/render/views.py +++ b/webapp/graphite/render/views.py @@ -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 = [] @@ -461,6 +463,26 @@ def parseOptions(request): return (graphOptions, requestOptions) +# 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_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) + + 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 + + connectionPools = {} 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 953786518..e45fc9126 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'] = { + 'grafana-org-id': 123, + } + msg = invalidParamLogMsg(requestContext, exception, func, args, kwargs) + self.assertEqual( + msg, + 'Received invalid parameters (exception details): test_func (1); source: (grafana-org-id: 123)' + ) + + requestContext['sourceIdHeaders'] = { + '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: (dashboard-id: 9, grafana-org-id: 123, panel-id: 38)' + ) + class ConsistentHashRingTest(TestCase): def test_chr_compute_ring_position(self):