Fix OTLP Count Metric Serialization (#856)

* Fix metric filtering in OTLP encoding

* Add regression test for duplicate metrics

* Make error message more clear

* Add pylint ignore C0123

* Add explanation comment

* Linting fixups

newrelic/core/otlp_utils.py

@@ -84,7 +84,7 @@
 
 
 def otlp_encode(payload):
-    if type(payload) is dict:
+    if type(payload) is dict:  # pylint: disable=C0123
         _logger.warning(
             "Using OTLP integration while protobuf is not installed. This may result in larger payload sizes and data loss."
         )
@@ -162,7 +162,9 @@ def stats_to_otlp_metrics(metric_data, start_time, end_time):
     separate the types and report multiple metrics, one for each type.
     """
     for name, metric_container in metric_data:
-        if any(isinstance(metric, CountStats) for metric in metric_container.values()):
+        # Types are checked here using type() instead of isinstance, as CountStats is a subclass of TimeStats.
+        # Imporperly checking with isinstance will lead to count metrics being encoded and reported twice.
+        if any(type(metric) is CountStats for metric in metric_container.values()):  # pylint: disable=C0123
             # Metric contains Sum metric data points.
             yield Metric(
                 name=name,
@@ -177,11 +179,11 @@ def stats_to_otlp_metrics(metric_data, start_time, end_time):
                             attributes=create_key_values_from_iterable(tags),
                         )
                         for tags, value in metric_container.items()
-                        if isinstance(value, CountStats)
+                        if type(value) is CountStats  # pylint: disable=C0123
                     ],
                 ),
             )
-        if any(isinstance(metric, TimeStats) for metric in metric_container.values()):
+        if any(type(metric) is TimeStats for metric in metric_container.values()):  # pylint: disable=C0123
             # Metric contains Summary metric data points.
             yield Metric(
                 name=name,
@@ -194,7 +196,7 @@ def stats_to_otlp_metrics(metric_data, start_time, end_time):
                             attributes=create_key_values_from_iterable(tags),
                         )
                         for tags, value in metric_container.items()
-                        if isinstance(value, TimeStats)
+                        if type(value) is TimeStats  # pylint: disable=C0123
                     ]
                 ),
             )

tests/agent_features/test_dimensional_metrics.py

@@ -24,19 +24,17 @@
     validate_transaction_metrics,
 )
 
+import newrelic.core.otlp_utils
 from newrelic.api.application import application_instance
 from newrelic.api.background_task import background_task
 from newrelic.api.transaction import (
     record_dimensional_metric,
     record_dimensional_metrics,
 )
 from newrelic.common.metric_utils import create_metric_identity
-
-import newrelic.core.otlp_utils
 from newrelic.core.config import global_settings
 from newrelic.packages import six
 
-
 try:
     # python 2.x
     reload
@@ -55,7 +53,7 @@ def otlp_content_encoding(request):
     _settings.debug.otlp_content_encoding = request.param
     reload(newrelic.core.otlp_utils)
     assert newrelic.core.otlp_utils.otlp_content_setting == request.param, "Content encoding mismatch."
-    
+
     yield
 
     _settings.debug.otlp_content_encoding = prev
@@ -177,7 +175,7 @@ def _test():
         ("Metric.NotPresent", None, None),
     ],
 )
-def test_dimensional_metric_payload():
+def test_dimensional_metrics_payload():
     @background_task(name="test_dimensional_metric_payload")
     def _test():
         record_dimensional_metrics(
@@ -197,3 +195,30 @@ def _test():
     app = application_instance()
     core_app = app._agent.application(app.name)
     core_app.harvest()
+
+
+@reset_core_stats_engine()
+@validate_dimensional_metric_payload(
+    summary_metrics=[
+        ("Metric.Summary", None, 1),
+        ("Metric.Count", None, None),  # Should NOT be present
+    ],
+    count_metrics=[
+        ("Metric.Count", None, 1),
+        ("Metric.Summary", None, None),  # Should NOT be present
+    ],
+)
+def test_dimensional_metrics_no_duplicate_encodings():
+    @background_task(name="test_dimensional_metric_payload")
+    def _test():
+        record_dimensional_metrics(
+            [
+                ("Metric.Summary", 1),
+                ("Metric.Count", {"count": 1}),
+            ]
+        )
+
+    _test()
+    app = application_instance()
+    core_app = app._agent.application(app.name)
+    core_app.harvest()

tests/testing_support/validators/validate_dimensional_metric_payload.py

@@ -117,7 +117,7 @@ def _bind_params(method, payload=(), *args, **kwargs):
                 if not count:
                     if metric in sent_summary_metrics:
                         data_points = data_points_to_dict(sent_summary_metrics[metric]["summary"]["data_points"])
-                        assert tags not in data_points, "(%s, %s) Found." % (metric, tags and dict(tags))
+                        assert tags not in data_points, "(%s, %s) Unexpected but found." % (metric, tags and dict(tags))
                 else:
                     assert metric in sent_summary_metrics, "%s Not Found. Got: %s" % (
                         metric,
@@ -153,7 +153,7 @@ def _bind_params(method, payload=(), *args, **kwargs):
                 if not count:
                     if metric in sent_count_metrics:
                         data_points = data_points_to_dict(sent_count_metrics[metric]["sum"]["data_points"])
-                        assert tags not in data_points, "(%s, %s) Found." % (metric, tags and dict(tags))
+                        assert tags not in data_points, "(%s, %s) Unexpected but found." % (metric, tags and dict(tags))
                 else:
                     assert metric in sent_count_metrics, "%s Not Found. Got: %s" % (
                         metric,