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

Add an otelcol.exporter.kafka component #717

Merged
merged 5 commits into from
Jun 6, 2024
Merged

Conversation

ptodev
Copy link
Contributor

@ptodev ptodev commented Apr 30, 2024

PR Description

Adding a new otelcol.exporter.kafka component based on the OTel Collector's kafka exporter.

Which issue(s) this PR fixes

Fixes #290

PR Checklist

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

@hainenber
Copy link
Contributor

I wonder if we still need to implement an equivalent otelcol-to-Alloy converter, similar to these? 🤔

@ptodev ptodev force-pushed the ptodev/add-kafka-exporter branch from b40405e to ebc2dda Compare May 23, 2024 10:30
@mattiasnensen
Copy link

Something that is worth looking at is whether or not the fix exists in Grafana Alloy so sasl mechanism AWS_MSK_IAM actually works. Please look at this issue: open-telemetry/opentelemetry-collector-contrib#32500

@ptodev ptodev force-pushed the ptodev/add-kafka-exporter branch 3 times, most recently from 613aefb to 674a729 Compare June 5, 2024 09:51
@ptodev ptodev marked this pull request as ready for review June 5, 2024 09:51
@ptodev ptodev requested a review from clayton-cornell as a code owner June 5, 2024 09:51
@ptodev
Copy link
Contributor Author

ptodev commented Jun 5, 2024

@hainenber True, I added the converters now :)

@mattiasnensen Unfortunately I think Alloy will only pick up the change once the upstream exporter has implemented it. If you need to reach out to the Otel maintainers you could open an issue in the Contrib repo or send a message on the CNCF slack. As a last resort you could also just open another PR which is inspired by the closed one that you are interested in. The Alloy team also contributes upstream, but in this case it doesn't seem to be a critical issue and more of a missing feature? In that case I honestly don't think we'd have bandwidth for it, since we're a bit preoccupied with more urgent work :/

@mattiasnensen
Copy link

@hainenber True, I added the converters now :)

@mattiasnensen Unfortunately I think Alloy will only pick up the change once the upstream exporter has implemented it. If you need to reach out to the Otel maintainers you could open an issue in the Contrib repo or send a message on the CNCF slack. As a last resort you could also just open another PR which is inspired by the closed one that you are interested in. The Alloy team also contributes upstream, but in this case it doesn't seem to be a critical issue and more of a missing feature? In that case I honestly don't think we'd have bandwidth for it, since we're a bit preoccupied with more urgent work :/

It was reported here but it was closed: open-telemetry/opentelemetry-collector-contrib#19747

Then this issue was created which allegedly fixed the issue, but the contribution wasn't accepted as the contributor didn't have a signed CLA: open-telemetry/opentelemetry-collector-contrib#32500

Best regards,

@wildum
Copy link
Contributor

wildum commented Jun 5, 2024

One additional comment: can you update the link to the otel component in your PR please? It points to the v0.96.0 version but the one implemented here is the v0.101.0

ptodev and others added 2 commits June 6, 2024 11:24
Co-authored-by: William Dumont <william.dumont@grafana.com>
@ptodev ptodev force-pushed the ptodev/add-kafka-exporter branch from b6c3855 to 37f3eb9 Compare June 6, 2024 10:24
Copy link
Contributor

@wildum wildum left a comment

Choose a reason for hiding this comment

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

LGTM!

@ptodev ptodev force-pushed the ptodev/add-kafka-exporter branch from 37f3eb9 to 772d9ab Compare June 6, 2024 11:17
Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
@clayton-cornell clayton-cornell added the type/docs Docs Squad label across all Grafana Labs repos label Jun 6, 2024
Copy link
Contributor

@clayton-cornell clayton-cornell left a comment

Choose a reason for hiding this comment

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

Docs look OK to me :-)

@clayton-cornell
Copy link
Contributor

Looks good. I know the descriptions in the shared content are unlikely to be seen anywhere, but... good habit to be consistent if we can and you never know what search results will show up.

@ptodev ptodev merged commit 0b9862d into main Jun 6, 2024
18 checks passed
@ptodev ptodev deleted the ptodev/add-kafka-exporter branch June 6, 2024 17:18
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age type/docs Docs Squad label across all Grafana Labs repos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Send observability signals to Kafka
5 participants