Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update kafka exporter dependency #6778

Merged
merged 26 commits into from
Apr 12, 2024
Merged

Conversation

wildum
Copy link
Contributor

@wildum wildum commented Mar 27, 2024

PR Description

The agent is currently using this outdated fork of a popular kafka_exporter.

The fork that we use has not been updated with upstream for years and has a bug with the added lag metrics not being updated on metadata changes (this was reported via a customer escalation). These lag metrics are not in the upstream repo yet (see danielqsj/kafka_exporter#195 (comment)).

I forked the upstream repo, added the lag metrics, fixed them to be updated on metadata changes and merged my fork to a "new" grafana repo that the agent uses with this PR.

I added an integration test that spawns a kafka cluster with a producer and a consumer. The test runs an agent with the prometheus exporter and verifies that the expected metrics are collected correctly.

Fixes grafana/alloy#464

PR Checklist

  • CHANGELOG.md updated
  • Documentation added
  • Tests updated
  • Config converters updated

Copy link
Contributor

@ptodev ptodev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for working on this! I had a quick look and noted a few things in comments.

Btw, it would have been nice if we could document the metrics which the exporter produces. However, we don't seem to do this for any prometheus.exporter component, so it's ok not to do it in this PR.

CHANGELOG.md Outdated Show resolved Hide resolved
@wildum wildum marked this pull request as ready for review April 9, 2024 12:23
@rfratto rfratto added variant/static Related to Grafana Agent Static. variant/flow Relatd to Grafana Agent Flow. outdated-dependency Depdency of the project that is out of date labels Apr 9, 2024
wildum and others added 4 commits April 10, 2024 10:32
@wildum wildum requested a review from ptodev April 10, 2024 12:24
Copy link
Contributor

@ptodev ptodev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Looks great. Do we need to review the exporter fork too?

CHANGELOG.md Outdated Show resolved Hide resolved
@@ -147,14 +188,21 @@ func New(logger log.Logger, c *Config) (integrations.Integration, error) {
return nil, fmt.Errorf("zookeeper lag is enabled but no zookeeper uri was provided")
}

// 30 is the default value
if c.PruneIntervalSeconds != 30 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to print the warning if PruneIntervalSeconds is set to 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if PruneIntervalSeconds is different from 30 then it was explicitly set in the config by the user and that should trigger a warning (even if it is set to 0)

@wildum
Copy link
Contributor Author

wildum commented Apr 11, 2024

Do we need to review the exporter fork too?

The exporter was reviewed by @gaantunes and has been merged to the official grafana fork already. If you have time you can also have a look at it if you want

wildum and others added 3 commits April 11, 2024 14:07
Co-authored-by: Paulin Todev <paulin.todev@gmail.com>
@wildum wildum merged commit 3adb5d3 into main Apr 12, 2024
10 checks passed
@wildum wildum deleted the update-kafka-exporter-dependency branch April 12, 2024 09:40
@wildum wildum restored the update-kafka-exporter-dependency branch April 12, 2024 09:47
ptodev added a commit that referenced this pull request Apr 12, 2024
* update agent to local exporter

* remove prune interval argument

* add kafka exporter integration test

* replace davidmparrott kafka exporter by wildum kafka exporter

* update doc

* update unit test

* update converter

* fix integration test

* integration ci test

* integration ci test

* integration ci test

* add longer sleep

* add notes about metrics rename

* fix copy paste mistake with metric name

* avoid breaking changes by re-introducing pruneIntervalSeconds as a no-op

* change kafka_exporter ref from wildum to grafana

* Update docs/sources/flow/reference/components/prometheus.exporter.kafka.md

Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>

* fix unit tests

* put the pruneIntervalSeconds arg in converter

* Update CHANGELOG.md

Co-authored-by: Paulin Todev <paulin.todev@gmail.com>

* fix doc

---------

Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
Co-authored-by: Paulin Todev <paulin.todev@gmail.com>
ptodev added a commit that referenced this pull request Apr 12, 2024
* Updating dependencies to fix CVEs:
* CVE-2024-27304
* CVE-2024-27289
* CVE-2024-28180
* CVE-2024-24786

* Use stackdriver exporter fork with a fix for histogram sum + count (#6720)

* Update kafka exporter dependency (#6778)

Co-authored-by: kgeckhart <kgeckhart@users.noreply.github.com>
Co-authored-by: William Dumont <william.dumont@grafana.com>
Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
@github-actions github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label May 13, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. outdated-dependency Depdency of the project that is out of date variant/flow Relatd to Grafana Agent Flow. variant/static Related to Grafana Agent Static.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use original fork of kafka_exporter
4 participants