Skip to content

fix(kafka): cancel cluster ID fetch goroutine on Close to prevent leak#4531

Merged
robcarlan-datadog merged 1 commit into
rob.carlan/dsm-kafka-cluster-idfrom
ben.db/fix-kafka-cluster-id-goroutine-leak
Mar 12, 2026
Merged

fix(kafka): cancel cluster ID fetch goroutine on Close to prevent leak#4531
robcarlan-datadog merged 1 commit into
rob.carlan/dsm-kafka-cluster-idfrom
ben.db/fix-kafka-cluster-id-goroutine-leak

Conversation

@genesor

@genesor genesor commented Mar 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

  • Replace standalone goroutine fields (clusterIDCancel/clusterIDDone) with a closeAsync []func() slice on both Consumer and Producer
  • Extract startClusterIDFetch helper that launches the cluster ID fetch goroutine and returns a cancel+wait closure — context cancellation unblocks the ClusterID call immediately on Close()
  • Switch from standalone kafka.NewAdminClient to derived kafka.NewAdminClientFromConsumer/NewAdminClientFromProducer — eliminates go.* config key filtering
  • Apply identical changes to both kafka/ (v1) and kafka.v2/ (v2)
  • Port missing test coverage to v1: add TestConsumerFunctionalWithClusterID and assert ext.MessagingKafkaClusterID span tag in TestConsumerFunctional

To merge in #4470 to fix CI leak

@genesor
genesor marked this pull request as ready for review March 12, 2026 17:12
@genesor
genesor requested review from a team as code owners March 12, 2026 17:12
@github-actions github-actions Bot added the apm:ecosystem contrib/* related feature requests or bugs label Mar 12, 2026
@datadog-official

datadog-official Bot commented Mar 12, 2026 •
edited by datadog-prod-us1-5 Bot
Loading

Copy link
Copy Markdown
Contributor

✅ Tests

🎉 All green!

❄️ No new flaky tests detected
🧪 All tests passed

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: debda11 | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback!

@codecov

codecov Bot commented Mar 12, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.04%. Comparing base (202bc0d) to head (7c42ce7).
⚠️ Report is 20 commits behind head on rob.carlan/dsm-kafka-cluster-id.

Additional details and impacted files

see 14 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pr-commenter

pr-commenter Bot commented Mar 12, 2026 •
edited
Loading

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2026-03-12 20:50:11

Comparing candidate commit debda11 in PR branch ben.db/fix-kafka-cluster-id-goroutine-leak with baseline commit b1e5582 in branch rob.carlan/dsm-kafka-cluster-id.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 155 metrics, 9 unstable metrics.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

@robcarlan-datadog
robcarlan-datadog force-pushed the rob.carlan/dsm-kafka-cluster-id branch from ea80cd7 to b1e5582 Compare March 12, 2026 18:48
@genesor
genesor force-pushed the ben.db/fix-kafka-cluster-id-goroutine-leak branch from 7c42ce7 to debda11 Compare March 12, 2026 20:30
@robcarlan-datadog
robcarlan-datadog merged commit 9327968 into rob.carlan/dsm-kafka-cluster-id Mar 12, 2026
165 of 262 checks passed
@robcarlan-datadog
robcarlan-datadog deleted the ben.db/fix-kafka-cluster-id-goroutine-leak branch March 12, 2026 23:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@robcarlan-datadog robcarlan-datadog Awaiting requested review from robcarlan-datadog

Assignees

No one assigned

Labels

apm:ecosystem contrib/* related feature requests or bugs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@genesor @robcarlan-datadog