ibm_ace: fix KeyError when ACE omits resource identifier name (DataDog#24826)

* ibm_ace: don't crash on malformed resourceIdentifier entries

ACE can emit a resourceIdentifier entry with its `name` key replaced by
an empty string instead of being omitted. Handle both cases: parse_tags
no longer raises on a missing `name`, and the empty-string key is
skipped before metric submission.

Includes an anonymized fixture derived from a real captured payload,
plus a regression test that replays it through collect().

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* ibm_ace: log a debug line when skipping a malformed entry

Per review feedback: log the resource type and value when a
resourceIdentifier entry's malformed key is skipped, and assert on it
in the regression test.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* ibm_ace: assert the debug log via caplog instead of a mock

Per review feedback: use pytest's caplog fixture to assert on the
actual rendered log line instead of mocking check.log and inspecting
call args.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

Author: ian28223

ibm_ace/changelog.d/24826.fixed

@@ -0,0 +1 @@
+Fix a `KeyError` crash when ACE omits the `name` field on a resource identifier.

ibm_ace/datadog_checks/ibm_ace/resources.py

@@ -20,7 +20,11 @@ def full_metric_name(self, metric):
         return f'{self.name}.{self.normalized_metric_name(metric)}'
 
     def parse_tags(self, global_tags, metric_data):
-        group = metric_data.pop('name')
+        # ACE can omit `name` for some resourceIdentifier entries.
+        group = metric_data.pop('name', None)
+        if group is None:
+            return list(global_tags)
+
         return [f'group:{group}', *global_tags]
 
     def submit(self, check, metric, value, tags):

ibm_ace/datadog_checks/ibm_ace/subscription.py

@@ -189,6 +189,17 @@ def collect(self):
                     tags = resource.parse_tags(resource_tags, metric_data)
 
                     for metric, value in metric_data.items():
+                        # ACE can emit a malformed entry with an empty key instead of `name`.
+                        if not metric:
+                            self.check.log.debug(
+                                'Skipping resourceIdentifier entry with malformed key for resource %s: '
+                                'value=%r, tags=%s',
+                                resource_data['name'],
+                                value,
+                                tags,
+                            )
+                            continue
+
                         resource.submit(self.check, metric, value, tags)
 
 

ibm_ace/tests/fixtures/resource_statistics_malformed_jdbc.json

@@ -0,0 +1,45 @@
+{
+  "ResourceStatistics": {
+    "brokerLabel": "integration_server",
+    "brokerUUID": "00000000-0000-0000-0000-000000000000",
+    "executionGroupName": "ACESERVER",
+    "executionGroupUUID": "00000000-0000-0000-0000-000000000000",
+    "ResourceType": [
+      {
+        "name": "JDBCConnectionPools",
+        "resourceIdentifier": [
+          {
+            "name": "summary",
+            "NameOfJDBCProvider": "jdbc_DataSourceA",
+            "MaxSizeOfPool": 100,
+            "ActualSizeOfPool": 1,
+            "CumulativeRequests": 891,
+            "CumulativeDelayedRequests": 0,
+            "CumulativeTimedOutRequests": 0,
+            "MaxDelayInMilliseconds": 0
+          },
+          {
+            "": "summary0",
+            "NameOfJDBCProvider": "jdbc_DataSourceB",
+            "MaxSizeOfPool": 100,
+            "ActualSizeOfPool": 1,
+            "CumulativeRequests": 891,
+            "CumulativeDelayedRequests": 0,
+            "CumulativeTimedOutRequests": 0,
+            "MaxDelayInMilliseconds": 0
+          },
+          {
+            "": "summary0",
+            "NameOfJDBCProvider": "jdbc_DataSourceA",
+            "MaxSizeOfPool": 100,
+            "ActualSizeOfPool": 1,
+            "CumulativeRequests": 29,
+            "CumulativeDelayedRequests": 0,
+            "CumulativeTimedOutRequests": 0,
+            "MaxDelayInMilliseconds": 0
+          }
+        ]
+      }
+    ]
+  }
+}

ibm_ace/tests/test_unit.py

@@ -1,9 +1,14 @@
 # (C) Datadog, Inc. 2022-present
 # All rights reserved
 # Licensed under a 3-clause BSD style license (see LICENSE)
+import os
+
 from datadog_checks.ibm_ace.check import IbmAceCheck
+from datadog_checks.ibm_ace.resources import get_resource
 from datadog_checks.ibm_ace.subscription import FlowMonitoringSubscription, ResourceStatisticsSubscription
 
+from .common import HERE
+
 
 def test_flow_monitoring_subscription(instance, global_tags):
     check = IbmAceCheck('ibm_ace', {}, [instance])
@@ -118,3 +123,82 @@ def test_non_truncation_error_given_connection_broken_returns_critical(instance,
     sc_calls = [c for c in check.service_check.call_args_list if c[0][0] == 'mq.subscription']
     assert len(sc_calls) == 1
     assert sc_calls[0][0][1] == ServiceCheck.CRITICAL
+
+
+def test_parse_tags_with_name():
+    resource = get_resource('JDBCConnectionPools')
+    metric_data = {'name': 'MyDataSource', 'NameOfJDBCProvider': 'Oracle'}
+
+    tags = resource.parse_tags(['mq_server:x'], metric_data)
+
+    assert tags == ['group:MyDataSource', 'mq_server:x', 'jdbc_provider:Oracle']
+
+
+def test_parse_tags_without_name():
+    # ACE can omit `name`; this must not raise.
+    resource = get_resource('JDBCConnectionPools')
+    metric_data = {'NameOfJDBCProvider': 'Oracle'}
+
+    tags = resource.parse_tags(['mq_server:x'], metric_data)
+
+    assert tags == ['mq_server:x', 'jdbc_provider:Oracle']
+
+
+def test_collect_survives_malformed_resource_identifier(instance, global_tags, caplog):
+    # ACE 12.0.9 payload where repeated `resourceIdentifier` entries
+    # have their `name` key replaced with an empty string. Must not crash.
+    import logging
+    from unittest.mock import MagicMock, PropertyMock, patch
+
+    caplog.set_level(logging.DEBUG)
+
+    fixture_path = os.path.join(HERE, 'fixtures', 'resource_statistics_malformed_jdbc.json')
+    with open(fixture_path, 'rb') as f:
+        payload = f.read()
+
+    mock_config = MagicMock()
+    mock_config.max_message_length = 65536
+
+    check = IbmAceCheck('ibm_ace', {}, [instance])
+    check.gauge = MagicMock()
+    check.count = MagicMock()
+    check.service_check = MagicMock()
+
+    sub = ResourceStatisticsSubscription(check, global_tags)
+
+    mock_sub = MagicMock()
+    mock_sub.get.side_effect = [payload]
+
+    with (
+        patch.object(type(check), 'config', new_callable=PropertyMock, return_value=mock_config),
+        patch.object(type(sub), 'sub', new_callable=PropertyMock, return_value=mock_sub),
+        patch.object(sub, '_get_elapsed_time', return_value=25),
+    ):
+        sub.collect()  # must not raise
+
+    submitted = check.count.call_args_list + check.gauge.call_args_list
+    submitted_metrics = {c.args[0] for c in submitted}
+
+    # `self.check.count`/`.gauge` are mocked directly, so the `ibm_ace.` namespace
+    # prefix (normally applied inside AgentCheck.count/gauge) isn't present here.
+    assert 'JDBCConnectionPools.CumulativeRequests' in submitted_metrics
+    # The malformed entries' empty-string key must never become a metric name.
+    assert not any(c.args[0].endswith('.') for c in submitted)
+
+    # The well-formed entry still gets its `group` tag; the malformed ones don't.
+    well_formed_call = next(
+        c for c in submitted if 'jdbc_provider:jdbc_DataSourceA' in c.kwargs['tags'] and c.args[1] == 891
+    )
+    assert any(tag.startswith('group:') for tag in well_formed_call.kwargs['tags'])
+
+    malformed_calls = [c for c in submitted if 'jdbc_provider:jdbc_DataSourceB' in c.kwargs['tags']]
+    assert malformed_calls
+    assert not any(tag.startswith('group:') for c in malformed_calls for tag in c.kwargs['tags'])
+
+    # The fixture has two malformed entries; each is logged at debug level when skipped.
+    skip_lines = [r for r in caplog.records if 'Skipping resourceIdentifier entry with malformed key' in r.message]
+    assert len(skip_lines) == 2
+    for record in skip_lines:
+        assert record.levelname == 'DEBUG'
+        assert 'JDBCConnectionPools' in record.message
+        assert "value='summary0'" in record.message