Remove the get_dict methods from the clients

[FlorentClarret]

What does this PR do?

Motivation

Additional Notes

Review checklist (to be filled by reviewers)

• Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
• PR title must be written as a CHANGELOG entry (see why)
• Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
• PR must have changelog/ and integration/ labels attached
• If the PR doesn't need to be tested during QA, please add a qa/skip-qa label.

[FlorentClarret]

Current dependencies on/for this PR:

• master
  • PR Revamp Kafka consumer check #13918
    • PR Implement OAuth #14184
    • PR Add a tls e2e env and implement it #14137
      • PR Add a kerberos e2e env #14120
    • PR Implement OAuth config #14247
    • PR Remove some unused functions #14145
    • PR Update all the tests to use the kafka_instance instead of a custom one #14144
    • PR Refactor the confluent kafka client #14158
    • PR Implement regex for consumer_groups #14193
    • PR Implement the request_metadata_update method #14152
    • PR Remove the get_dict methods from the clients #14149 👈
    • PR Manually build confluent-kafka in the test env #14173

This comment was auto-generated by Graphite.

[FlorentClarret]

To reviewer: This test with max_partition_contexts set to 1 does not make sense to me because:

• We mocked get_consumer_offsets_dict to return {("consumer_group1", "topic1", "partition1"): 2}, so with a len to 1
• We mocked get_highwater_offsets_dict to return {("topic1", "partition1"): 3, ("topic2", "partition2"): 3}
• However, if len(get_consumer_offsets_dict) > limit with limit=1, we do not even run the get_highwater_offsets methods, so get_highwater_offsets_dict should return an empty dict in that case.
• See

  integrations-core/kafka_consumer/datadog_checks/kafka_consumer/kafka_consumer.py

  Lines 46 to 51 in dc5d5cb

  	highwater_offsets = {}
  	try:
  	if len(consumer_offsets) < self._context_limit:
  	highwater_offsets = self.client.get_highwater_offsets(consumer_offsets)
  	else:
  	self.warning("Context limit reached. Skipping highwater offset collection.")

To me, this test should run both methods and return more contexts than we should handle from the highwater method and then, skip some contexts. So here we now have 3 contexts (1 consumer, 2 highwaters) with a limit set to 2, we should only emit 2 metrics

[yzhan289]

Ah that is a good catch! I should have made an assert here to verify that get_highwater_offsets() was actually called in the first place rather than just returning a mocked get_highwater_offsets_dict().

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (AI-2904/kafka-consumer-revamp@65751ea). Click here to learn what that means.
The diff coverage is n/a.

Flag	Coverage Δ
kafka_consumer	91.30% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.