Merge #206

206: Limit search results and add metrics r=jonathanj a=mithrandi

Closes #10.



Co-authored-by: Tristan Seligmann <[email protected]>

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

setup.py

@@ -23,6 +23,7 @@
         'fusion_util',
         'toolz',
         'py2casefold',
+        'prometheus_client',
         ],
     license='MIT',
     packages=find_packages(where='src') + ['twisted.plugins'],

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

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'])

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'])

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):
@@ -91,29 +95,36 @@ 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.
 
         @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)
+            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))
+            if searchValue == u'':
+                METRIC_SEARCH_REJECTED.labels(
+                    searchClass.value, environment, indexType).inc()
+                return []
+            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), limit=limit).getColumn('result')
 
 
     @classmethod
@@ -124,30 +135,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 +171,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()

src/fusion_index/test/test_lookup.py

@@ -1,20 +1,20 @@
 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
 
 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:

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)