fix(elasticsearch): omit large body tag values [backport #5229 to 1.8] (#5438)

brettlangdon · Kyle-Verhoog · web-flow · commit 9a60d28a7aa5 · 2023-03-30T16:07:28.000-04:00
Backport of #5229 to 1.8 The elasticsearch body tag value in practice can be very large. Since the value is not truncated, it can easily result in traces exceeding the max size limit which results in dropped traces. The proposed solution is to omit the value. This is because obfuscation logic lives in the Agent and so the body cannot be truncated as sensitive data would be leaked. No body tag is better than no trace. Ideally the obfuscation logic should exist in the client so that the body can be truncated. ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines) are followed. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] Author is aware of the performance implications of this PR as reported in the benchmarks PR comment. ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer is aware of, and discussed the performance implications of this PR as reported in the benchmarks PR comment. Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>
diff --git a/ddtrace/_tracing/__init__.py b/ddtrace/_tracing/__init__.py
diff --git a/ddtrace/_tracing/_limits.py b/ddtrace/_tracing/_limits.py
@@ -0,0 +1,6 @@
+"""
+Limits for trace data.
+"""
+
+MAX_SPAN_META_KEY_LEN = 200
+MAX_SPAN_META_VALUE_LEN = 25000
diff --git a/ddtrace/contrib/elasticsearch/patch.py b/ddtrace/contrib/elasticsearch/patch.py
@@ -1,6 +1,7 @@
 from importlib import import_module
 
 from ddtrace import config
+from ddtrace._tracing import _limits
 from ddtrace.contrib.trace_utils import ext_service
 from ddtrace.vendor.wrapt import wrap_function_wrapper as _w
 
@@ -95,7 +96,19 @@ def _perform_request(func, instance, args, kwargs):
                 span.set_tag_str(http.QUERY_STRING, encoded_params)
 
             if method in ["GET", "POST"]:
-                span.set_tag_str(metadata.BODY, instance.serializer.dumps(body))
+                ser_body = instance.serializer.dumps(body)
+                # Elasticsearch request bodies can be very large resulting in traces being too large
+                # to send.
+                # When this occurs, drop the value.
+                # Ideally the body should be truncated, however we cannot truncate as the obfuscation
+                # logic for the body lives in the agent and truncating would make the body undecodable.
+                if len(ser_body) <= _limits.MAX_SPAN_META_VALUE_LEN:
+                    span.set_tag_str(metadata.BODY, ser_body)
+                else:
+                    span.set_tag_str(
+                        metadata.BODY,
+                        "<body size %s exceeds limit of %s>" % (len(ser_body), _limits.MAX_SPAN_META_VALUE_LEN),
+                    )
             status = None
 
             # set analytics sample rate
diff --git a/releasenotes/notes/elasticsearch-body-75bfd855099e9e5a.yaml b/releasenotes/notes/elasticsearch-body-75bfd855099e9e5a.yaml
@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    elasticsearch: Omit large ``elasticsearch.body`` tag values that are
+      greater than 25000 characters to prevent traces from being too large to
+      send.
diff --git a/tests/contrib/elasticsearch/test_elasticsearch.py b/tests/contrib/elasticsearch/test_elasticsearch.py
@@ -1,6 +1,8 @@
 import datetime
 from importlib import import_module
 
+import pytest
+
 from ddtrace import Pin
 from ddtrace import config
 from ddtrace.constants import ANALYTICS_SAMPLE_RATE_KEY
@@ -272,3 +274,24 @@ def _get_es(self):
 
     def _get_index_args(self):
         return {"index": self.ES_INDEX, "doc_type": self.ES_TYPE}
+
+    @pytest.mark.skipif(
+        (7, 0, 0) <= elasticsearch.__version__ <= (7, 1, 0), reason="test isn't compatible these elasticsearch versions"
+    )
+    def test_large_body(self):
+        """
+        Ensure large bodies are omitted to prevent large traces from being produced.
+        """
+        args = self._get_index_args()
+        body = {
+            "query": {"range": {"created": {"gte": "asdf" * 25000}}},
+        }
+        # it doesn't matter if the request fails, so long as a span is generated
+        try:
+            self.es.search(size=100, body=body, **args)
+        except Exception:
+            pass
+        spans = self.get_spans()
+        self.reset()
+        assert len(spans) == 1
+        assert len(spans[0].get_tag("elasticsearch.body")) < 25000

-Original file line number
+Diff line change
@@ @@ -0,0 +1,6 @@ @@
 +---
 +fixes:
 +  - |
 +    elasticsearch: Omit large ``elasticsearch.body`` tag values that are
 +      greater than 25000 characters to prevent traces from being too large to
 +      send.