From 866942ca51d63838e869eab5374e1f2fbb39cf8c Mon Sep 17 00:00:00 2001 From: Tristan Seligmann Date: Tue, 3 Jul 2018 16:57:01 +0200 Subject: [PATCH 1/8] Add metrics. --- src/fusion_index/lookup.py | 27 +++++---- src/fusion_index/metrics.py | 33 +++++++++++ src/fusion_index/search.py | 107 ++++++++++++++++++++---------------- 3 files changed, 108 insertions(+), 59 deletions(-) create mode 100644 src/fusion_index/metrics.py diff --git a/src/fusion_index/lookup.py b/src/fusion_index/lookup.py index c0f1f39..3f33ccf 100644 --- a/src/fusion_index/lookup.py +++ b/src/fusion_index/lookup.py @@ -4,6 +4,9 @@ from axiom.attributes import AND, bytes, compoundIndex, text from axiom.item import Item +from fusion_index.metrics import ( + METRIC_LOOKUP_INSERT_LATENCY, METRIC_LOOKUP_QUERY_LATENCY) + class LookupEntry(Item): @@ -54,11 +57,12 @@ def get(cls, store, environment, indexType, key): @raises KeyError: if the entry does not exist. """ - return store.findUnique( - cls, - AND(cls.environment == environment, - cls.indexType == indexType, - cls.key == key)).value + with METRIC_LOOKUP_QUERY_LATENCY.labels(environment, indexType).time(): + return store.findUnique( + cls, + AND(cls.environment == environment, + cls.indexType == indexType, + cls.key == key)).value @classmethod @@ -84,9 +88,10 @@ def set(cls, store, environment, indexType, key, value): @type value: L{bytes} @param value: The value to set. """ - item = store.findOrCreate( - cls, - environment=environment, - indexType=indexType, - key=key) - item.value = value + with METRIC_LOOKUP_INSERT_LATENCY.labels(environment, indexType).time(): + item = store.findOrCreate( + cls, + environment=environment, + indexType=indexType, + key=key) + item.value = value diff --git a/src/fusion_index/metrics.py b/src/fusion_index/metrics.py new file mode 100644 index 0000000..417afd8 --- /dev/null +++ b/src/fusion_index/metrics.py @@ -0,0 +1,33 @@ +from prometheus_client import Counter, Histogram + + + +METRIC_LOOKUP_QUERY_LATENCY = Histogram( + 'lookup_query_latency_seconds', + 'Lookup query latency in seconds', + ['environment', 'indexType']) + +METRIC_LOOKUP_INSERT_LATENCY = Histogram( + 'lookup_insert_latency_seconds', + 'Lookup insertion latency in seconds', + ['environment', 'indexType']) + +METRIC_SEARCH_QUERY_LATENCY = Histogram( + 'search_query_latency_seconds', + 'Search query latency in seconds', + ['searchClass', 'environment', 'indexType']) + +METRIC_SEARCH_INSERT_LATENCY = Histogram( + 'search_insert_latency_seconds', + 'Search insertion latency in seconds', + ['searchClass', 'environment', 'indexType']) + +METRIC_SEARCH_DELETE_LATENCY = Histogram( + 'search_delete_latency_seconds', + 'Search deletion latency in seconds', + ['searchClass', 'environment', 'indexType']) + +METRIC_SEARCH_REJECTED = Counter( + 'search_rejected_count', + 'Searches rejected due to being too general', + ['searchClass', 'environment', 'indexType']) diff --git a/src/fusion_index/search.py b/src/fusion_index/search.py index 895cdbf..2723ba7 100644 --- a/src/fusion_index/search.py +++ b/src/fusion_index/search.py @@ -7,13 +7,17 @@ indexes). """ from re import UNICODE, compile -from py2casefold import casefold from unicodedata import normalize from axiom.attributes import AND, compoundIndex, text from axiom.item import Item +from py2casefold import casefold from twisted.python.constants import ValueConstant, Values +from fusion_index.metrics import ( + METRIC_SEARCH_DELETE_LATENCY, METRIC_SEARCH_INSERT_LATENCY, + METRIC_SEARCH_QUERY_LATENCY, METRIC_SEARCH_REJECTED) + class SearchClasses(Values): @@ -97,23 +101,26 @@ def search(cls, store, searchClass, environment, indexType, searchValue, @see: L{SearchEntry} """ - criteria = [] - searchValue = cls._normalize(searchValue) - if searchClass == SearchClasses.EXACT: - criteria.append(SearchEntry.searchValue == searchValue) - elif searchClass == SearchClasses.PREFIX: - criteria.append(SearchEntry.searchValue.startswith(searchValue)) - else: - raise RuntimeError( - 'Invalid search class: {!r}'.format(searchClass)) - criteria.extend([ - SearchEntry.searchClass == searchClass.value, - SearchEntry.environment == environment, - SearchEntry.indexType == indexType, - ]) - if searchType is not None: - criteria.append(SearchEntry.searchType == searchType) - return store.query(SearchEntry, AND(*criteria)).getColumn('result') + with METRIC_SEARCH_QUERY_LATENCY.labels( + searchClass.value, environment, indexType).time(): + criteria = [] + searchValue = cls._normalize(searchValue) + # METRIC_SEARCH_REJECTED + if searchClass == SearchClasses.EXACT: + criteria.append(SearchEntry.searchValue == searchValue) + elif searchClass == SearchClasses.PREFIX: + criteria.append(SearchEntry.searchValue.startswith(searchValue)) + else: + raise RuntimeError( + 'Invalid search class: {!r}'.format(searchClass)) + criteria.extend([ + SearchEntry.searchClass == searchClass.value, + SearchEntry.environment == environment, + SearchEntry.indexType == indexType, + ]) + if searchType is not None: + criteria.append(SearchEntry.searchType == searchType) + return store.query(SearchEntry, AND(*criteria)).getColumn('result') @classmethod @@ -124,30 +131,32 @@ def insert(cls, store, searchClass, environment, indexType, result, @see: L{SearchEntry} """ - searchValue = cls._normalize(searchValue) - entry = store.findUnique( - SearchEntry, - AND(SearchEntry.searchClass == searchClass.value, - SearchEntry.environment == environment, - SearchEntry.indexType == indexType, - SearchEntry.result == result, - SearchEntry.searchType == searchType), - None) - if entry is None: - if searchValue != u'': - SearchEntry( - store=store, - searchClass=searchClass.value, - environment=environment, - indexType=indexType, - result=result, - searchType=searchType, - searchValue=searchValue) - else: - if searchValue == u'': - entry.deleteFromStore() + with METRIC_SEARCH_INSERT_LATENCY.labels( + searchClass.value, environment, indexType).time(): + searchValue = cls._normalize(searchValue) + entry = store.findUnique( + SearchEntry, + AND(SearchEntry.searchClass == searchClass.value, + SearchEntry.environment == environment, + SearchEntry.indexType == indexType, + SearchEntry.result == result, + SearchEntry.searchType == searchType), + None) + if entry is None: + if searchValue != u'': + SearchEntry( + store=store, + searchClass=searchClass.value, + environment=environment, + indexType=indexType, + result=result, + searchType=searchType, + searchValue=searchValue) else: - entry.searchValue = searchValue + if searchValue == u'': + entry.deleteFromStore() + else: + entry.searchValue = searchValue @classmethod @@ -158,10 +167,12 @@ def remove(cls, store, searchClass, environment, indexType, result, @see: L{SearchEntry} """ - store.query( - SearchEntry, - AND(SearchEntry.searchClass == searchClass.value, - SearchEntry.environment == environment, - SearchEntry.indexType == indexType, - SearchEntry.result == result, - SearchEntry.searchType == searchType)).deleteFromStore() + with METRIC_SEARCH_DELETE_LATENCY.labels( + searchClass.value, environment, indexType).time(): + store.query( + SearchEntry, + AND(SearchEntry.searchClass == searchClass.value, + SearchEntry.environment == environment, + SearchEntry.indexType == indexType, + SearchEntry.result == result, + SearchEntry.searchType == searchType)).deleteFromStore() From 2209c935452016734f07fc4771e39c8cecc2b7ab Mon Sep 17 00:00:00 2001 From: Tristan Seligmann Date: Tue, 3 Jul 2018 16:57:09 +0200 Subject: [PATCH 2/8] Expose metrics. --- src/fusion_index/resource.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/fusion_index/resource.py b/src/fusion_index/resource.py index 7c0b673..e04956c 100644 --- a/src/fusion_index/resource.py +++ b/src/fusion_index/resource.py @@ -1,6 +1,7 @@ import json from characteristic import attributes +from prometheus_client.twisted import MetricsResource from toolz.dicttoolz import merge from twisted.web import http from txspinneret.interfaces import ISpinneretResource @@ -37,6 +38,11 @@ def search(self, request, params): return SearchResource(store=self.store, params=params) + @router.route(b'metrics') + def metrics(self, request, params): + return MetricsResource() + + @implementer(ISpinneretResource) @attributes(['store', 'environment', 'indexType', 'key']) From d0d86c658d549c70d4f66241414e6f0a85d632ad Mon Sep 17 00:00:00 2001 From: Tristan Seligmann Date: Tue, 3 Jul 2018 16:59:42 +0200 Subject: [PATCH 3/8] Add dep. --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index 979b33e..98826ea 100644 --- a/setup.py +++ b/setup.py @@ -23,6 +23,7 @@ 'fusion_util', 'toolz', 'py2casefold', + 'prometheus_client', ], license='MIT', packages=find_packages(where='src') + ['twisted.plugins'], From 258a57fda0340d9e7a808d694808b6e916b25cb4 Mon Sep 17 00:00:00 2001 From: Tristan Seligmann Date: Tue, 3 Jul 2018 17:36:56 +0200 Subject: [PATCH 4/8] Fix tests. --- src/fusion_index/test/test_lookup.py | 11 +++++++---- src/fusion_index/test/test_search.py | 11 +++++++++-- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/fusion_index/test/test_lookup.py b/src/fusion_index/test/test_lookup.py index 912a316..0eafd05 100644 --- a/src/fusion_index/test/test_lookup.py +++ b/src/fusion_index/test/test_lookup.py @@ -1,7 +1,7 @@ import string from axiom.store import Store -from hypothesis import given +from hypothesis import given, settings from hypothesis.strategies import binary, characters, lists, text, tuples from testtools import TestCase from testtools.matchers import Equals @@ -9,12 +9,12 @@ from fusion_index.lookup import LookupEntry + def axiom_text(): return text( alphabet=characters( blacklist_categories={'Cs'}, - blacklist_characters={u'\x00'}), - average_size=5) + blacklist_characters={u'\x00'})) _lower_table = dict( @@ -30,12 +30,15 @@ def _lower(s): class LookupTests(TestCase): - @given(lists(tuples(axiom_text(), axiom_text(), axiom_text(), binary()))) + @settings(deadline=500) + @given(lists(tuples(axiom_text(), axiom_text(), axiom_text(), binary()), + max_size=10)) def test_inserts(self, values): """ Test inserting and retrieving arbitrary entries. """ s = Store() + def _tx(): d = {} for e, t, k, v in values: diff --git a/src/fusion_index/test/test_search.py b/src/fusion_index/test/test_search.py index b38d6ea..fd63a17 100644 --- a/src/fusion_index/test/test_search.py +++ b/src/fusion_index/test/test_search.py @@ -1,5 +1,5 @@ from axiom.store import Store -from hypothesis import assume, given +from hypothesis import assume, given, settings, HealthCheck from py2casefold import casefold from testtools import TestCase from testtools.matchers import Annotate, Equals @@ -18,6 +18,7 @@ def punctuated(text): class SearchTests(TestCase): + @settings(suppress_health_check=[HealthCheck.filter_too_much]) @given(axiom_text(), axiom_text(), axiom_text(), axiom_text(), axiom_text()) def test_exactSearches(self, environment, indexType, searchValue, @@ -27,6 +28,7 @@ def test_exactSearches(self, environment, indexType, searchValue, """ assume(SearchEntry._normalize(searchValue) != u'') s = Store() + def _tx(): SearchEntry.insert( s, SearchClasses.EXACT, environment, indexType, result, @@ -58,6 +60,7 @@ def _tx(): s.transact(_tx) + @settings(suppress_health_check=[HealthCheck.filter_too_much]) @given(axiom_text(), axiom_text(), axiom_text(), axiom_text(), axiom_text()) def test_prefixSearches(self, environment, indexType, searchValue, @@ -67,6 +70,7 @@ def test_prefixSearches(self, environment, indexType, searchValue, """ assume(SearchEntry._normalize(searchValue) != u'') s = Store() + def _tx(): SearchEntry.insert( s, SearchClasses.PREFIX, environment, indexType, result, @@ -113,9 +117,10 @@ def test_invalidSearchClass(self): Searching with an invalid search class raises L{RuntimeError}. """ self.assertRaises( - RuntimeError, SearchEntry.search, Store(), 42, u'', u'', u'') + Exception, SearchEntry.search, Store(), 42, u'', u'', u'') + @settings(suppress_health_check=[HealthCheck.filter_too_much]) @given(axiom_text().map(SearchEntry._normalize)) def test_normalization(self, value): """ @@ -124,6 +129,7 @@ def test_normalization(self, value): """ assume(value != u'') s = Store() + def _tx(): SearchEntry.insert( s, SearchClasses.EXACT, u'e', u'i', u'RESULT', u'type', value) @@ -144,6 +150,7 @@ def test_insertEmpty(self): the entry. """ s = Store() + def _tx(): SearchEntry.insert( s, SearchClasses.EXACT, u'e', u'i', u'RESULT', u'type', u'. /') From 8da69e0be08431816f8c8d47cac702b0a8da24b6 Mon Sep 17 00:00:00 2001 From: Tristan Seligmann Date: Tue, 3 Jul 2018 19:17:02 +0200 Subject: [PATCH 5/8] Handle limits and reject empty searches. --- src/fusion_index/search.py | 10 +++++-- src/fusion_index/test/test_search.py | 44 ++++++++++++++++++++++++++-- 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/src/fusion_index/search.py b/src/fusion_index/search.py index 2723ba7..b1f501a 100644 --- a/src/fusion_index/search.py +++ b/src/fusion_index/search.py @@ -95,7 +95,7 @@ def _normalize(cls, value): @classmethod def search(cls, store, searchClass, environment, indexType, searchValue, - searchType=None): + searchType=None, limit=200): """ Return entries matching the given search. @@ -105,7 +105,10 @@ def search(cls, store, searchClass, environment, indexType, searchValue, searchClass.value, environment, indexType).time(): criteria = [] searchValue = cls._normalize(searchValue) - # METRIC_SEARCH_REJECTED + if searchValue == u'': + METRIC_SEARCH_REJECTED.labels( + searchClass.value, environment, indexType).inc() + return [] if searchClass == SearchClasses.EXACT: criteria.append(SearchEntry.searchValue == searchValue) elif searchClass == SearchClasses.PREFIX: @@ -120,7 +123,8 @@ def search(cls, store, searchClass, environment, indexType, searchValue, ]) if searchType is not None: criteria.append(SearchEntry.searchType == searchType) - return store.query(SearchEntry, AND(*criteria)).getColumn('result') + return store.query( + SearchEntry, AND(*criteria), limit=limit).getColumn('result') @classmethod diff --git a/src/fusion_index/test/test_search.py b/src/fusion_index/test/test_search.py index fd63a17..ac87f2f 100644 --- a/src/fusion_index/test/test_search.py +++ b/src/fusion_index/test/test_search.py @@ -1,8 +1,8 @@ from axiom.store import Store -from hypothesis import assume, given, settings, HealthCheck +from hypothesis import HealthCheck, assume, given, settings from py2casefold import casefold from testtools import TestCase -from testtools.matchers import Annotate, Equals +from testtools.matchers import AllMatch, Annotate, Equals, HasLength from fusion_index.search import SearchClasses, SearchEntry from fusion_index.test.test_lookup import axiom_text @@ -68,7 +68,7 @@ def test_prefixSearches(self, environment, indexType, searchValue, """ Test inserting, searching, and removing for the prefix search class. """ - assume(SearchEntry._normalize(searchValue) != u'') + assume(SearchEntry._normalize(searchValue[:3]) != u'') s = Store() def _tx(): @@ -162,3 +162,41 @@ def _tx(): s, SearchClasses.EXACT, u'e', u'i', u'RESULT', u'type', u'. /') self.assertThat(s.query(SearchEntry).count(), Equals(0)) s.transact(_tx) + + + def test_searchEmpty(self): + """ + Searching for a value that is empty after normalization returns nothing. + """ + s = Store() + + def _tx(): + SearchEntry.insert( + s, SearchClasses.PREFIX, u'e', u'i', u'RESULT', u'type', u'yo') + self.assertThat(s.query(SearchEntry).count(), Equals(1)) + self.assertThat([ + list(SearchEntry.search( + s, SearchClasses.PREFIX, u'e', u'i', u'')), + list(SearchEntry.search( + s, SearchClasses.PREFIX, u'e', u'i', u'. .'))], + AllMatch(Equals([]))) + s.transact(_tx) + + + def test_searchLimit(self): + """ + Searching does not return more results than the limit. + """ + s = Store() + + def _tx(): + for x in xrange(50): + SearchEntry.insert( + s, SearchClasses.EXACT, u'e', u'i', u'RESULT', + u'type{}'.format(x), u'yo') + self.assertThat(s.query(SearchEntry).count(), Equals(50)) + self.assertThat( + list(SearchEntry.search( + s, SearchClasses.EXACT, u'e', u'i', u'yo', limit=20)), + HasLength(20)) + s.transact(_tx) From 7d25b3627d19e298d3e30046c5ab5a4408b48af5 Mon Sep 17 00:00:00 2001 From: Tristan Seligmann Date: Tue, 3 Jul 2018 19:54:14 +0200 Subject: [PATCH 6/8] Fix test coverage. --- src/fusion_index/search.py | 8 ++++---- src/fusion_index/test/test_resource.py | 16 ++++++++++++++++ src/fusion_index/test/test_search.py | 4 +++- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/fusion_index/search.py b/src/fusion_index/search.py index b1f501a..18eb4ba 100644 --- a/src/fusion_index/search.py +++ b/src/fusion_index/search.py @@ -105,10 +105,6 @@ def search(cls, store, searchClass, environment, indexType, searchValue, searchClass.value, environment, indexType).time(): criteria = [] searchValue = cls._normalize(searchValue) - if searchValue == u'': - METRIC_SEARCH_REJECTED.labels( - searchClass.value, environment, indexType).inc() - return [] if searchClass == SearchClasses.EXACT: criteria.append(SearchEntry.searchValue == searchValue) elif searchClass == SearchClasses.PREFIX: @@ -116,6 +112,10 @@ def search(cls, store, searchClass, environment, indexType, searchValue, else: raise RuntimeError( 'Invalid search class: {!r}'.format(searchClass)) + if searchValue == u'': + METRIC_SEARCH_REJECTED.labels( + searchClass.value, environment, indexType).inc() + return [] criteria.extend([ SearchEntry.searchClass == searchClass.value, SearchEntry.environment == environment, diff --git a/src/fusion_index/test/test_resource.py b/src/fusion_index/test/test_resource.py index d17bce5..334fff7 100644 --- a/src/fusion_index/test/test_resource.py +++ b/src/fusion_index/test/test_resource.py @@ -460,3 +460,19 @@ def test_invalidSearchClass(self): response = GET( self, agent, b'/search/invalid/e/i/value/') self.assertEqual(response.code, http.NOT_FOUND) + + + +class MetricsTests(SynchronousTestCase): + """ + Test that metrics are published. + """ + def test_metrics(self): + """ + Metrics are published at C{/metrics}. + """ + agent = ResourceTraversalAgent( + IndexRouter(store=Store()).router.resource()) + response = GET( + self, agent, b'/metrics') + self.assertEqual(response.code, http.OK) diff --git a/src/fusion_index/test/test_search.py b/src/fusion_index/test/test_search.py index ac87f2f..11702dc 100644 --- a/src/fusion_index/test/test_search.py +++ b/src/fusion_index/test/test_search.py @@ -116,8 +116,10 @@ def test_invalidSearchClass(self): """ Searching with an invalid search class raises L{RuntimeError}. """ + class junk(object): + value = u'foo' self.assertRaises( - Exception, SearchEntry.search, Store(), 42, u'', u'', u'') + RuntimeError, SearchEntry.search, Store(), junk(), u'', u'', u'') @settings(suppress_health_check=[HealthCheck.filter_too_much]) From 65fc930193957bab462b7fae3144b70b31aedd38 Mon Sep 17 00:00:00 2001 From: Tristan Seligmann Date: Tue, 3 Jul 2018 20:43:50 +0200 Subject: [PATCH 7/8] Update requirements. --- requirements.txt | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/requirements.txt b/requirements.txt index cb3fddf..b5f7bf6 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,32 +2,36 @@ # This file is autogenerated by pip-compile # To update, run: # -# pip-compile --no-annotate --output-file requirements.txt requirements.in +# pip-compile --no-annotate --output-file requirements.txt setup.py # argparse==1.4.0 asn1crypto==0.24.0 attrs==18.1.0 automat==0.7.0 axiom==0.7.5 +cffi==1.11.5 characteristic==14.3.0 constantly==15.1.0 +coverage==4.5.1 cryptography==2.2.2 eliot==1.3.0 enum34==1.1.6 -epsilon==0.7.2 +epsilon==0.7.3 extras==1.0.0 fixtures==3.0.0 fusion-util==1.3.0 hyperlink==18.0.0 -hypothesis==3.61.0 +hypothesis==3.65.1 idna==2.7 incremental==17.5.0 -ipaddress==1.0.21 +ipaddress==1.0.22 linecache2==1.0.0 pbr==4.0.4 +prometheus-client==0.2.0 py2casefold==1.0.1 -pyasn1-modules==0.2.1 +pyasn1-modules==0.2.2 pyasn1==0.4.3 +pycparser==2.18 pyopenssl==18.0.0 pyrsistent==0.14.3 python-mimeparse==1.6.0 From bdf8eeda46e9aeffe391024e9890893c0c0ad3c9 Mon Sep 17 00:00:00 2001 From: Tristan Seligmann Date: Tue, 3 Jul 2018 20:47:03 +0200 Subject: [PATCH 8/8] Fix tox. --- tox.ini | 1 - 1 file changed, 1 deletion(-) diff --git a/tox.ini b/tox.ini index 86a07d4..5129528 100644 --- a/tox.ini +++ b/tox.ini @@ -4,7 +4,6 @@ envlist = py27,pypy [testenv] whitelist_externals = mkdir deps = - coverage -rrequirements.txt commands = pip list