truncate strings that are stored as keywords in ES (#159)

this should avoid schema errors for values that are too long

closes #156

closes #159

Author: beniwohli

CHANGELOG.asciidoc

@@ -13,6 +13,7 @@
 https://github.com/elastic/apm-agent-python/compare/v2.0.0\...master[Check the HEAD diff]
 
  * fixed compatibility issue with aiohttp 3.0 ({pull}157[#157])
+ * Added truncation for fields that have a `maxLength` in the JSON Schema ({pull}159[#159])
 
 [[release-2.0.0]]
 [float]

elasticapm/base.py

@@ -29,7 +29,7 @@
 from elasticapm.utils import compat, is_master_process
 from elasticapm.utils import json_encoder as json
 from elasticapm.utils import stacks, varmap
-from elasticapm.utils.encoding import shorten, transform
+from elasticapm.utils.encoding import keyword_field, shorten, transform
 from elasticapm.utils.module_import import import_string
 
 __all__ = ('Client',)
@@ -111,6 +111,7 @@ def __init__(self, config=None, **defaults):
         self.filter_exception_types_dict = {}
         self._send_timer = None
         self._transports = {}
+        self._service_info = None
 
         self.config = Config(config, default_dict=defaults)
         if self.config.errors:
@@ -327,45 +328,48 @@ def _send_remote(self, url, data, headers=None):
             self.handle_transport_success(url=url)
 
     def get_service_info(self):
+        if self._service_info:
+            return self._service_info
         language_version = platform.python_version()
         if hasattr(sys, 'pypy_version_info'):
             runtime_version = '.'.join(map(str, sys.pypy_version_info[:3]))
         else:
             runtime_version = language_version
         result = {
-            'name': self.config.service_name,
-            'environment': self.config.environment,
-            'version': self.config.service_version,
+            'name': keyword_field(self.config.service_name),
+            'environment': keyword_field(self.config.environment),
+            'version': keyword_field(self.config.service_version),
             'agent': {
                 'name': 'python',
                 'version': elasticapm.VERSION,
             },
             'language': {
                 'name': 'python',
-                'version': platform.python_version(),
+                'version': keyword_field(platform.python_version()),
             },
             'runtime': {
-                'name': platform.python_implementation(),
-                'version': runtime_version,
+                'name': keyword_field(platform.python_implementation()),
+                'version': keyword_field(runtime_version),
             }
         }
         if self.config.framework_name:
             result['framework'] = {
-                'name': self.config.framework_name,
-                'version': self.config.framework_version,
+                'name': keyword_field(self.config.framework_name),
+                'version': keyword_field(self.config.framework_version),
             }
+        self._service_info = result
         return result
 
     def get_process_info(self):
         return {
             'pid': os.getpid(),
             'argv': sys.argv,
-            'title': None,
+            'title': None,  # Note: if we implement this, the value needs to be wrapped with keyword_field
         }
 
     def get_system_info(self):
         return {
-            'hostname': socket.gethostname(),
+            'hostname': keyword_field(socket.gethostname()),
             'architecture': platform.machine(),
             'platform': platform.system().lower(),
         }

elasticapm/contrib/django/client.py

@@ -21,7 +21,7 @@
 
 from elasticapm.base import Client
 from elasticapm.contrib.django.utils import iterate_with_template_sources
-from elasticapm.utils import compat, get_url_dict
+from elasticapm.utils import compat, encoding, get_url_dict
 from elasticapm.utils.module_import import import_string
 from elasticapm.utils.wsgi import get_environ, get_headers
 
@@ -80,11 +80,11 @@ def get_user_info(self, request):
                 else:
                     user_info['is_authenticated'] = bool(user.is_authenticated)
             if hasattr(user, 'id'):
-                user_info['id'] = user.id
+                user_info['id'] = encoding.keyword_field(user.id)
             if hasattr(user, 'get_username'):
-                user_info['username'] = user.get_username()
+                user_info['username'] = encoding.keyword_field(user.get_username())
             elif hasattr(user, 'username'):
-                user_info['username'] = user.username
+                user_info['username'] = encoding.keyword_field(user.username)
 
             if hasattr(user, 'email'):
                 user_info['email'] = user.email

elasticapm/events.py

@@ -14,7 +14,7 @@
 import uuid
 
 from elasticapm.utils import varmap
-from elasticapm.utils.encoding import shorten, to_unicode
+from elasticapm.utils.encoding import keyword_field, shorten, to_unicode
 from elasticapm.utils.stacks import (get_culprit, get_stack_info,
                                      iter_traceback_frames)
 
@@ -112,8 +112,8 @@ def capture(client, exc_info=None, **kwargs):
             'culprit': culprit,
             'exception': {
                 'message': message,
-                'type': str(exc_type),
-                'module': str(exc_module),
+                'type': keyword_field(str(exc_type)),
+                'module': keyword_field(str(exc_module)),
                 'stacktrace': frames,
             }
         }
@@ -146,10 +146,10 @@ def capture(client, param_message=None, message=None, level=None, logger_name=No
         message_data = {
             'id': str(uuid.uuid4()),
             'log': {
-                'level': level or 'error',
-                'logger_name': logger_name or '__root__',
+                'level': keyword_field(level or 'error'),
+                'logger_name': keyword_field(logger_name or '__root__'),
                 'message': message,
-                'param_message': param_message['message'],
+                'param_message': keyword_field(param_message['message']),
             }
         }
         if isinstance(data.get('stacktrace'), dict):

elasticapm/traces.py

@@ -8,7 +8,7 @@
 import uuid
 
 from elasticapm.conf import constants
-from elasticapm.utils import compat, get_name_from_func
+from elasticapm.utils import compat, encoding, get_name_from_func
 
 __all__ = ('capture_span', 'tag', 'set_transaction_name', 'set_custom_context', 'set_user_context')
 
@@ -112,10 +112,10 @@ def to_dict(self):
         self.context['tags'] = self.tags
         result = {
             'id': self.id,
-            'name': self.name,
-            'type': self.transaction_type,
+            'name': encoding.keyword_field(self.name),
+            'type': encoding.keyword_field(self.transaction_type),
             'duration': self.duration * 1000,  # milliseconds
-            'result': str(self.result),
+            'result': encoding.keyword_field(str(self.result)),
             'timestamp': self.timestamp.strftime(constants.TIMESTAMP_FORMAT),
             'sampled': self.is_sampled,
         }
@@ -159,8 +159,8 @@ def fingerprint(self):
     def to_dict(self):
         return {
             'id': self.idx,
-            'name': self.name,
-            'type': self.type,
+            'name': encoding.keyword_field(self.name),
+            'type': encoding.keyword_field(self.type),
             'start': self.start_time * 1000,  # milliseconds
             'duration': self.duration * 1000,  # milliseconds
             'parent': self.parent,
@@ -278,7 +278,7 @@ def tag(**tags):
             error_logger.warning("Ignored tag %s. No transaction currently active.", name)
             return
         if TAG_RE.match(name):
-            transaction.tags[compat.text_type(name)] = compat.text_type(value)
+            transaction.tags[compat.text_type(name)] = encoding.keyword_field(compat.text_type(value))
         else:
             error_logger.warning("Ignored tag %s. Tag names can't contain stars, dots or double quotes.", name)
 
@@ -308,9 +308,9 @@ def set_context(data, key='custom'):
 def set_user_context(username=None, email=None, user_id=None):
     data = {}
     if username is not None:
-        data['username'] = username
+        data['username'] = encoding.keyword_field(username)
     if email is not None:
-        data['email'] = email
+        data['email'] = encoding.keyword_field(email)
     if user_id is not None:
-        data['id'] = user_id
+        data['id'] = encoding.keyword_field(user_id)
     set_context(data, 'user')

elasticapm/utils/__init__.py

@@ -10,13 +10,7 @@
 """
 import os
 
-from elasticapm.utils import compat
-
-try:
-    import urlparse
-except ImportError:
-    import urllib.parse as urlparse
-
+from elasticapm.utils import compat, encoding
 
 default_ports = {
     "https": 433,
@@ -76,19 +70,19 @@ def is_master_process():
 
 
 def get_url_dict(url):
-    scheme, netloc, path, params, query, fragment = urlparse.urlparse(url)
+    scheme, netloc, path, params, query, fragment = compat.urlparse.urlparse(url)
     if ':' in netloc:
         hostname, port = netloc.split(':')
     else:
         hostname, port = (netloc, None)
     url_dict = {
-        'full': url,
+        'full': encoding.keyword_field(url),
         'protocol': scheme + ':',
-        'hostname': hostname,
-        'pathname': path,
+        'hostname': encoding.keyword_field(hostname),
+        'pathname': encoding.keyword_field(path),
     }
     if port:
         url_dict['port'] = port
     if query:
-        url_dict['search'] = '?' + query
+        url_dict['search'] = encoding.keyword_field('?' + query)
     return url_dict

elasticapm/utils/encoding.py

@@ -1,3 +1,5 @@
+# -*- coding: utf-8 -*-
+
 """
 elasticapm.utils.encoding
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -13,6 +15,7 @@
 import uuid
 from decimal import Decimal
 
+from elasticapm.conf.constants import KEYWORD_MAX_LENGTH
 from elasticapm.utils import compat
 
 PROTECTED_TYPES = compat.integer_types + (type(None), float, Decimal, datetime.datetime, datetime.date, datetime.time)
@@ -165,3 +168,9 @@ def shorten(var, list_length=50, string_length=200):
         # TODO: when we finish the above, we should also implement this for dicts
         var = list(var)[:list_length] + ['...', '(%d more elements)' % (len(var) - list_length,)]
     return var
+
+
+def keyword_field(string):
+    if not isinstance(string, compat.string_types) or len(string) <= KEYWORD_MAX_LENGTH:
+        return string
+    return string[:KEYWORD_MAX_LENGTH - 1] + u'…'

tests/client/client_tests.py

@@ -9,9 +9,9 @@
 
 import elasticapm
 from elasticapm.base import Client, ClientState
-from elasticapm.transport.base import Transport, TransportException
-from elasticapm.utils import compat
-from elasticapm.utils.compat import urlparse
+from elasticapm.conf.constants import KEYWORD_MAX_LENGTH
+from elasticapm.transport.base import Transport
+from elasticapm.utils import compat, encoding
 
 
 def test_client_state_should_try_online():
@@ -404,7 +404,7 @@ def test_client_uses_sync_mode_when_master_process(is_master_process):
         secret_token='secret',
         async_mode=True,
     )
-    transport = client._get_transport(urlparse.urlparse('http://exampe.com'))
+    transport = client._get_transport(compat.urlparse.urlparse('http://exampe.com'))
     assert transport.async_mode is False
 
 
@@ -702,3 +702,67 @@ def test_transaction_context_is_used_in_errors(elasticapm_client):
     assert message['context']['tags'] == {'foo': 'baz'}
     assert 'a' in transaction.context['custom']
     assert 'foo' not in transaction.context['custom']
+
+
+def test_transaction_keyword_truncation(sending_elasticapm_client):
+    too_long = 'x' * (KEYWORD_MAX_LENGTH + 1)
+    expected = encoding.keyword_field(too_long)
+    assert too_long != expected
+    assert len(expected) == KEYWORD_MAX_LENGTH
+    assert expected[-1] != 'x'
+    sending_elasticapm_client.begin_transaction(too_long)
+    elasticapm.tag(val=too_long)
+    elasticapm.set_user_context(username=too_long, email=too_long, user_id=too_long)
+    with elasticapm.capture_span(name=too_long, span_type=too_long):
+        pass
+    sending_elasticapm_client.end_transaction(too_long, too_long)
+    sending_elasticapm_client.close()
+    assert sending_elasticapm_client.httpserver.responses[0]['code'] == 202
+    transaction = sending_elasticapm_client.httpserver.payloads[0]['transactions'][0]
+    span = transaction['spans'][0]
+
+    assert transaction['name'] == expected
+    assert transaction['type'] == expected
+    assert transaction['result'] == expected
+
+    assert transaction['context']['user']['id'] == expected
+    assert transaction['context']['user']['username'] == expected
+    assert transaction['context']['user']['email'] == expected
+
+    assert transaction['context']['tags']['val'] == expected
+
+    assert span['type'] == expected
+    assert span['name'] == expected
+
+
+def test_error_keyword_truncation(sending_elasticapm_client):
+    too_long = 'x' * (KEYWORD_MAX_LENGTH + 1)
+    expected = encoding.keyword_field(too_long)
+
+    # let's create a way too long Exception type with a way too long module name
+    WayTooLongException = type(too_long.upper(), (Exception,), {})
+    WayTooLongException.__module__ = too_long
+    try:
+        raise WayTooLongException()
+    except WayTooLongException:
+        sending_elasticapm_client.capture_exception()
+    error = sending_elasticapm_client.httpserver.payloads[0]['errors'][0]
+
+    assert error['exception']['type'] == expected.upper()
+    assert error['exception']['module'] == expected
+
+
+def test_message_keyword_truncation(sending_elasticapm_client):
+    too_long = 'x' * (KEYWORD_MAX_LENGTH + 1)
+    expected = encoding.keyword_field(too_long)
+    sending_elasticapm_client.capture_message(
+        param_message={'message': too_long, 'params': []},
+        logger_name=too_long,
+    )
+
+    error = sending_elasticapm_client.httpserver.payloads[0]['errors'][0]
+
+    assert error['log']['param_message'] == expected
+    assert error['log']['message'] == too_long  # message is not truncated
+
+    assert error['log']['logger_name'] == expected

tests/fixtures.py

@@ -35,6 +35,7 @@ class ValidatingWSGIApp(ContentServer):
     def __init__(self, **kwargs):
         super(ValidatingWSGIApp, self).__init__(**kwargs)
         self.payloads = []
+        self.responses = []
         self.skip_validate = False
 
     def __call__(self, environ, start_response):
@@ -61,6 +62,7 @@ def __call__(self, environ, start_response):
         response.headers.clear()
         response.headers.extend(self.headers)
         response.data = content
+        self.responses.append({'code': code, 'content': content})
         return response(environ, start_response)