fix: guard against django cache result errors when setting row_count [backport #5723 to 1.12] (#5789)

Backport of #5723 to 1.12

Patch for issue: #5460.
Added try except block to catch potential errors that may occur. Added
tests to verify patch works as intended to catch errors.

## 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] PR description includes explicit acknowledgement/acceptance 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 has explicitly acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment.

Co-authored-by: William Conti <[email protected]>

Author: ZStriker19

ddtrace/contrib/django/patch.py

@@ -177,12 +177,16 @@ def traced_cache(django, pin, func, instance, args, kwargs):
                 db.ROWCOUNT, sum(1 for doc in result if doc) if result and isinstance(result, Iterable) else 0
             )
         elif command_name == "get":
-            # if valid result and check for special case for Django~3.0 that returns an empty Sentinel object as
-            # missing key
-            if result is not None and result != getattr(instance, "_missing_key", None):
-                span.set_metric(db.ROWCOUNT, 1)
-            # else result is invalid or None, set row count to 0
-            else:
+            try:
+                # check also for special case for Django~3.2 that returns an empty Sentinel object for empty results
+                # also check if result is Iterable first since some iterables return ambiguous truth results with ``==``
+                if result is None or (
+                    not isinstance(result, Iterable) and result == getattr(instance, "_missing_key", None)
+                ):
+                    span.set_metric(db.ROWCOUNT, 0)
+                else:
+                    span.set_metric(db.ROWCOUNT, 1)
+            except (AttributeError, NotImplementedError, ValueError):
                 span.set_metric(db.ROWCOUNT, 0)
         return result
 

releasenotes/notes/guard-against-django-cache-result-errors-6ed2fe989f96eaf9.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    django: Adds catch to guard against a ValueError, AttributeError, or NotImplementedError from being thrown when evaluating
+    a django cache result for ``db.row_count`` tag.

tests/contrib/django/test_django.py

@@ -811,6 +811,130 @@ def test_cache_get_rowcount_missing_key_with_default(test_spans):
     assert_dict_issuperset(span.get_metrics(), {"db.row_count": 1})
 
 
+class RaiseNotImplementedError:
+    def __eq__(self, _):
+        raise NotImplementedError
+
+
+class RaiseValueError:
+    def __eq__(self, _):
+        raise ValueError
+
+
+class RaiseAttributeError:
+    def __eq__(self, _):
+        raise AttributeError
+
+
+def test_cache_get_rowcount_throws_attribute_and_value_error(test_spans):
+
+    # get the default cache
+    cache = django.core.cache.caches["default"]
+
+    cache.set(1, RaiseNotImplementedError())
+    cache.set(2, RaiseValueError())
+    cache.set(3, RaiseAttributeError())
+
+    # This is the diff with `test_cache_get_rowcount_missing_key`,
+    # we are setting a default value to be returned in case of a cache miss
+    result = cache.get(1)
+    assert isinstance(result, RaiseNotImplementedError)
+
+    result = cache.get(2)
+    assert isinstance(result, RaiseValueError)
+
+    result = cache.get(3)
+    assert isinstance(result, RaiseAttributeError)
+
+    spans = test_spans.get_spans()
+    assert len(spans) == 6
+
+    set_1 = spans[0]
+    set_2 = spans[1]
+    set_3 = spans[2]
+    assert set_1.resource == "django.core.cache.backends.locmem.set"
+    assert set_2.resource == "django.core.cache.backends.locmem.set"
+    assert set_3.resource == "django.core.cache.backends.locmem.set"
+
+    get_1 = spans[3]
+    assert get_1.service == "django"
+    assert get_1.resource == "django.core.cache.backends.locmem.get"
+    assert_dict_issuperset(get_1.get_metrics(), {"db.row_count": 0})
+
+    get_2 = spans[4]
+    assert get_2.service == "django"
+    assert get_2.resource == "django.core.cache.backends.locmem.get"
+    assert_dict_issuperset(get_2.get_metrics(), {"db.row_count": 0})
+
+    get_3 = spans[5]
+    assert get_3.service == "django"
+    assert get_3.resource == "django.core.cache.backends.locmem.get"
+    assert_dict_issuperset(get_3.get_metrics(), {"db.row_count": 0})
+
+
+class MockDataFrame:
+    def __init__(self, data):
+        self.data = data
+
+    def __eq__(self, other):
+        if isinstance(other, str):
+            return MockDataFrame([item == other for item in self.data])
+        else:
+            return MockDataFrame([row == other for row in self.data])
+
+    def __bool__(self):
+        raise ValueError("Cannot determine truthiness of comparison result for DataFrame.")
+
+    def __iter__(self):
+        return iter(self.data)
+
+
+@pytest.mark.skipif(django.VERSION < (2, 0, 0), reason="")
+def test_cache_get_rowcount_iterable_ambiguous_truthiness(test_spans):
+    # get the default cache
+
+    data = {"col1": 1, "col2": 2, "col3": 3}
+
+    cache = django.core.cache.caches["default"]
+    cache.set(1, MockDataFrame(data))
+    cache.set(2, None)
+
+    # throw error to verify that mock class has ambigiuous truthiness, django patch will do a similar
+    # set of comparisons when trying to get rowcount
+    with pytest.raises(ValueError):
+        df = MockDataFrame(data)
+        assert df is not None
+        check_result = df == "some_result"
+        if check_result:
+            print("This should never print.")
+
+    # Test to ensure that a DF does not throw a bool error when trying to
+    # determine if the result was valid.
+    result = cache.get(1)
+    assert isinstance(result, MockDataFrame)
+
+    result = cache.get(2)
+    assert result is None
+
+    spans = test_spans.get_spans()
+    assert len(spans) == 4
+
+    set_1 = spans[0]
+    set_2 = spans[1]
+    assert set_1.resource == "django.core.cache.backends.locmem.set"
+    assert set_2.resource == "django.core.cache.backends.locmem.set"
+
+    get_1 = spans[2]
+    assert get_1.service == "django"
+    assert get_1.resource == "django.core.cache.backends.locmem.get"
+    assert_dict_issuperset(get_1.get_metrics(), {"db.row_count": 1})
+
+    get_2 = spans[3]
+    assert get_2.service == "django"
+    assert get_2.resource == "django.core.cache.backends.locmem.get"
+    assert_dict_issuperset(get_2.get_metrics(), {"db.row_count": 0})
+
+
 def test_cache_get_unicode(test_spans):
     # get the default cache
     cache = django.core.cache.caches["default"]