kafka_consumer: add connector failure trace and tracked topics to Connect config events#24309
kafka_consumer: add connector failure trace and tracked topics to Connect config events#24309piochelepiotr wants to merge 12 commits into
Conversation
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 7477f20 | Docs | Datadog PR Page | Give us feedback! |
…nnect config events Surfaces the Kafka Connect status trace for FAILED connectors/tasks and the connector's tracked topics (via the Connect topics-tracking API) in the existing data-streams-message connector config event, so failures and topic usage are visible without cross-referencing the Connect worker directly.
… state - Add CacheHelper.get_cached_json/set_cached_json to remove duplicated try/except read/write boilerplate; existing cache methods now delegate. - Bound the topics fetch/data caches with max_cache_size and prune stale connector entries on write, matching the connector-config cache. - Only mark a connector's topics as fetched when the request succeeds, so a transient failure retries on the next run instead of waiting a full TTL. - Hoist the topics fetch out of _emit_connector_config_events into _collect_rest, alongside the other outbound Connect HTTP calls. - Only report connector_trace when connector_state is FAILED. - Add tests for topics TTL caching and fetch-failure degradation.
Backs off persistently-failing connector topic fetches on the same TTL as successes to avoid a per-run retry storm and log spam. Extracts a _build_connector_event_content helper and a _url_cache_key helper to reduce duplication, sorts task_traces for stable event hashing, and tightens CacheHelper's new JSON cache signatures. Also fixes a line-length formatting violation introduced by the round-1 fix commit.
…nstant, unconditional topic-cache pruning
d3578db to
0a0ed77
Compare
…tor and Any typing
…substring safety net Cover well-known non-sensitive keys from JDBC, Debezium, S3/GCS/BigQuery/ Snowflake/Elasticsearch/HTTP sinks, and MirrorMaker2, so legitimate connector config isn't hidden unnecessarily. Also hide any key matching a credential-shaped substring (password, secret, token, etc.) even if allowlisted, as a backstop against curation mistakes.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de5a375ec9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…n allowlist helper Cap connector topic refreshes at 100 per run so a Connect worker with many expired connectors, or a slow/unsupported topics endpoint, can't stall config/plugin event emission for the rest of the run. Leftover connectors are picked up on a later run since get_items_to_fetch returns oldest-expiry-first. Rename _is_allowed_config_key to is_allowed_config_key: it's a plain module-level predicate, not a reusable-library-module helper, so AGENTS.md's naming convention says no leading underscore.
- task_traces: use a None-safe sort key so a missing task id can't raise TypeError and drop all config/plugin events for the Connect URL. - topics fetch: back off only successful fetches and permanently-unsupported (404/405) endpoints; retry transient failures on the next run, matching _collect_plugins. Only rewrite the topics data cache when it actually changed. - cache: delegate get_items_to_fetch/mark_items_fetched/get_events_to_send to the new get_cached_json/set_cached_json helpers instead of duplicating the read/write-with-try/except blocks. - Move ALLOWED_CONFIG_KEYS/SENSITIVE_KEY_SUBSTRINGS/is_allowed_config_key into connector_config_keys.py so connectors.py stays focused on collection. - Add a .changed changelog fragment for the denylist->allowlist redaction flip. - Add tests for transient-vs-unsupported topics backoff and None task ids. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
|
Validation ReportAll 21 validations passed. Show details
|
|
|
||
| def _fetch_connector_topics(self, url: str, connector_names: list[str]) -> dict[str, list[str]]: | ||
| """Return each connector's tracked topics, refreshing entries whose TTL has expired. | ||
|
|
There was a problem hiding this comment.
What's an expected number of topics per connector?
| except Exception as e: | ||
| if self._is_unsupported_topics_endpoint(e): | ||
| names_to_back_off.append(name) | ||
| self.log.warning("Error fetching topics for connector %s from %s: %s", name, url, e) |
There was a problem hiding this comment.
Should this always be a warning if sometimes we expect a 404 or 405 error?
What does this PR do?
Adds two fields to the existing
data-streams-messageconnector config event emitted by thekafka_consumerintegration's Kafka Connect collector:connector_trace/task_traces: the Kafka Connect statustrace(stack trace text) for a connector or task in theFAILEDstate.topics: the list of topics tracked for the connector via the Kafka Connect topics-tracking REST API (GET /connectors/{name}/topics), fetched in parallel across connectors and refreshed on the same cache/TTL cadence as connector plugin metadata to avoid extra load on the Connect worker.It also changes how connector config is redacted before being included in the event: instead of hiding keys that match known-sensitive patterns, only keys on an allowlist of known-safe Connect framework and popular connector-plugin settings (JDBC, Debezium, S3/GCS/BigQuery/Snowflake/Elasticsearch sinks, HTTP Sink, MirrorMaker2) are shown; everything else is hidden by default. A substring safety net additionally hides any key that looks credential-shaped even if it's allowlisted.
Motivation
A dashboard built on top of
kafka_consumerdata currently has to cross-reference the Connect worker directly to find out why a connector failed or which topics it touches. Surfacing the failure trace and topic list in the config event closes that gap.Switching config redaction from a denylist to an allowlist reduces the risk of leaking a credential through a connector-plugin-specific key we didn't know to redact.
Test plan
Review checklist (to be filled by reviewers)
qa/requiredif this PR needs QA validation, orqa/skip-qaif it does not. Exactly one of the two is required.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is mergedThis PR has been created and validated using the paired-review skill from agent-integrations. Ready for human review.