implement dedotting of context keys and tag names (#353)

(and destaring and dedoublequoting)

These characters are invalid in elasticsearch field names.

closes #353

Author: beniwohli

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## Unreleased
+
+ * implemented de-dotting of tag names and context keys (#353)
+
 ## v4.0.2
 [Check the diff](https://github.com/elastic/apm-agent-python/compare/v4.0.1...v4.0.2)
 

docs/api.asciidoc

@@ -210,6 +210,9 @@ elasticapm.set_custom_context({'billing_amount': product.price * item_count})
 ----
 
  * `data`: (*required*) A dictionary with the data to be attached. This should be a flat key/value `dict` object.
+ 
+NOTE: `.`, `*`, and `"` are invalid characters for key names and will be replaced with `_`.
+
 
 Errors that happen after this call will also have the custom context attached to them.
 You can call this function multiple times, new context data will be merged with existing data,
@@ -255,3 +258,7 @@ elasticapm.tag(ecommerce=True, dollar_value=47.12)
 Errors that happen after this call will also have the tags attached to them.
 You can call this function multiple times, new tags will be merged with existing tags,
 following the `update()` semantics of Python dictionaries.
+
+The value is converted to a string.
+
+NOTE: `.`, `*`, and `"` are invalid characters for tag names and will be replaced with `_`.

elasticapm/traces.py

@@ -18,7 +18,7 @@
 _time_func = timeit.default_timer
 
 
-TAG_RE = re.compile('^[^.*"]+$')
+TAG_RE = re.compile('[.*"]')
 
 
 try:
@@ -290,10 +290,9 @@ def tag(**tags):
         if not transaction:
             error_logger.warning("Ignored tag %s. No transaction currently active.", name)
             return
-        if TAG_RE.match(name):
-            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)
+        # replace invalid characters for Elasticsearch field names with underscores
+        name = TAG_RE.sub("_", compat.text_type(name))
+        transaction.tags[compat.text_type(name)] = encoding.keyword_field(compat.text_type(value))
 
 
 def set_transaction_name(name, override=True):
@@ -318,6 +317,12 @@ def set_context(data, key="custom"):
         return
     if callable(data) and transaction.is_sampled:
         data = data()
+
+    # remove invalid characters from key names
+    for k in list(data.keys()):
+        if TAG_RE.search(k):
+            data[TAG_RE.sub("_", k)] = data.pop(k)
+
     if key in transaction.context:
         transaction.context[key].update(data)
     else:

tests/instrumentation/transactions_store_tests.py

@@ -220,6 +220,19 @@ def test_tags_merge(elasticapm_client):
     assert transactions[0]["context"]["tags"] == {"foo": "1", "bar": "3", "boo": "biz"}
 
 
+def test_tags_dedot(elasticapm_client):
+    elasticapm_client.begin_transaction("test")
+    elasticapm.tag(**{"d.o.t": "dot"})
+    elasticapm.tag(**{"s*t*a*r": "star"})
+    elasticapm.tag(**{'q"u"o"t"e': "quote"})
+
+    elasticapm_client.end_transaction("test_name", 200)
+
+    transactions = elasticapm_client.events[TRANSACTION]
+
+    assert transactions[0]["context"]["tags"] == {"d_o_t": "dot", "s_t_a_r": "star", "q_u_o_t_e": "quote"}
+
+
 def test_set_transaction_name(elasticapm_client):
     elasticapm_client.begin_transaction("test")
     elasticapm_client.end_transaction("test_name", 200)
@@ -281,6 +294,14 @@ def test_set_user_context_merge(elasticapm_client):
     assert transactions[0]["context"]["user"] == {"username": "foo", "email": "[email protected]", "id": 42}
 
 
+def test_dedot_context_keys(elasticapm_client):
+    elasticapm_client.begin_transaction("test")
+    elasticapm.set_context({"d.o.t": "d_o_t", "s*t*a*r": "s_t_a_r", "q*u*o*t*e": "q_u_o_t_e"})
+    elasticapm_client.end_transaction("foo", 200)
+    transaction = elasticapm_client.events[TRANSACTION][0]
+    assert transaction["context"]["custom"] == {"s_t_a_r": "s_t_a_r", "q_u_o_t_e": "q_u_o_t_e", "d_o_t": "d_o_t"}
+
+
 def test_transaction_name_none_is_converted_to_empty_string(elasticapm_client):
     elasticapm_client.begin_transaction("test")
     transaction = elasticapm_client.end_transaction(None, 200)