From 3bc890dd815dfcee56e556589bfcfd78592dd563 Mon Sep 17 00:00:00 2001
From: Seth Michael Larson <seth.larson@elastic.co>
Date: Wed, 22 Jul 2020 19:03:07 -0500
Subject: [PATCH] exclude_fields should filter nested extras=...

---
 ecs_logging/_stdlib.py         | 11 ++++++++---
 ecs_logging/_utils.py          | 27 +++++++++++++++++++++++++++
 tests/test_stdlib_formatter.py |  5 ++++-
 tests/test_utils.py            | 28 ++++++++++++++++++++++++++++
 4 files changed, 67 insertions(+), 4 deletions(-)
 create mode 100644 tests/test_utils.py

diff --git a/ecs_logging/_stdlib.py b/ecs_logging/_stdlib.py
index 39e3235..df1b464 100644
--- a/ecs_logging/_stdlib.py
+++ b/ecs_logging/_stdlib.py
@@ -9,6 +9,7 @@
     TYPE_CHECKING,
     collections_abc,
     lru_cache,
+    flatten_dict,
 )
 
 if TYPE_CHECKING:
@@ -152,12 +153,16 @@ def format_to_ecs(self, record):
         # adding 'message' to ``available``, it simplifies the code
         available["message"] = record.getMessage()
 
-        extras = set(available).difference(self._LOGRECORD_DICT)
+        # Pull all extras and flatten them to be sent into '_is_field_excluded'
+        # since they can be defined as 'extras={"http": {"method": "GET"}}'
+        extra_keys = set(available).difference(self._LOGRECORD_DICT)
+        extras = flatten_dict({key: available[key] for key in extra_keys})
+
         # Merge in any keys that were set within 'extra={...}'
-        for field in extras:
+        for field, value in extras.items():
             if self._is_field_excluded(field):
                 continue
-            merge_dicts(de_dot(field, available[field]), result)
+            merge_dicts(de_dot(field, value), result)
 
         # The following is mostly for the ecs format. You can't have 2x
         # 'message' keys in _WANTED_ATTRS, so we set the value to
diff --git a/ecs_logging/_utils.py b/ecs_logging/_utils.py
index 5a685e6..e0c9da9 100644
--- a/ecs_logging/_utils.py
+++ b/ecs_logging/_utils.py
@@ -34,6 +34,33 @@
 ]
 
 
+def flatten_dict(value):
+    # type: (typing.Mapping[str, Any]) -> Dict[str, Any]
+    """Adds dots to all nested fields in dictionaries.
+    Raises an error if there are entries which are represented
+    with different forms of nesting. (ie {"a": {"b": 1}, "a.b": 2})
+    """
+    top_level = {}
+    for key, val in value.items():
+        if not isinstance(val, collections_abc.Mapping):
+            if key in top_level:
+                raise ValueError(
+                    "Duplicate entry for '%s' with different nesting" % key
+                )
+            top_level[key] = val
+        else:
+            val = flatten_dict(val)
+            for vkey, vval in val.items():
+                vkey = "%s.%s" % (key, vkey)
+                if vkey in top_level:
+                    raise ValueError(
+                        "Duplicate entry for '%s' with different nesting" % vkey
+                    )
+                top_level[vkey] = vval
+
+    return top_level
+
+
 def normalize_dict(value):
     # type: (Dict[str, Any]) -> Dict[str, Any]
     """Expands all dotted names to nested dictionaries"""
diff --git a/tests/test_stdlib_formatter.py b/tests/test_stdlib_formatter.py
index 158f75a..1b3f1df 100644
--- a/tests/test_stdlib_formatter.py
+++ b/tests/test_stdlib_formatter.py
@@ -83,7 +83,10 @@ def test_extra_is_merged(time, logger):
     logger.info(
         "hey world",
         extra={
-            "tls": {"cipher": "AES"},
+            "tls": {
+                "cipher": "AES",
+                "client": {"hash": {"md5": "0F76C7F2C55BFD7D8E8B8F4BFBF0C9EC"}},
+            },
             "tls.established": True,
             "tls.client.certificate": "cert",
         },
diff --git a/tests/test_utils.py b/tests/test_utils.py
new file mode 100644
index 0000000..9cfbb5b
--- /dev/null
+++ b/tests/test_utils.py
@@ -0,0 +1,28 @@
+import pytest
+from ecs_logging._utils import flatten_dict, de_dot, normalize_dict
+
+
+def test_flatten_dict():
+    assert flatten_dict(
+        {"a": {"b": 1}, "a.c": {"d.e": {"f": 1}, "d.e.g": [{"f.c": 2}]}}
+    ) == {"a.b": 1, "a.c.d.e.f": 1, "a.c.d.e.g": [{"f.c": 2}]}
+
+    with pytest.raises(ValueError) as e:
+        flatten_dict({"a": {"b": 1}, "a.b": 2})
+
+    assert str(e.value) == "Duplicate entry for 'a.b' with different nesting"
+
+    with pytest.raises(ValueError) as e:
+        flatten_dict({"a": {"b": {"c": 1}}, "a.b": {"c": 2}, "a.b.c": 1})
+
+    assert str(e.value) == "Duplicate entry for 'a.b.c' with different nesting"
+
+
+def test_de_dot():
+    assert de_dot("x.y.z", {"a": {"b": 1}}) == {"x": {"y": {"z": {"a": {"b": 1}}}}}
+
+
+def test_normalize_dict():
+    assert normalize_dict(
+        {"a": {"b": 1}, "a.c": {"d.e": {"f": 1}, "d.e.g": [{"f.c": 2}]}}
+    ) == {"a": {"b": 1, "c": {"d": {"e": {"f": 1, "g": [{"f": {"c": 2}}]}}}}}