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

Kafka Contact Point: Fix v3 API URL when used against a Kafka Rest Proxy #40

Merged
merged 10 commits into from Jan 13, 2023
Merged

Conversation

ghost
Copy link

@ghost ghost commented Jan 13, 2023

Context

Support for using v3 APIs when producing to kafka was added in this PR: #28
However, I only tested this against a Confluent Kafka Cluster from confluent.io
Turns out that the API paths are slightly different for the Kafka Rest Proxy and the confluent kafka cluster. Thanks @yuri-tceretian for bringing this to my attention!

This commit fixes the URL to work with the Kafka Rest Proxy. Confluent Kafka users can make use of this by add a "/kafka" suffix to the endpoint. We will add this as a hint on the UI.

Testing

  1. Added a test and modified existing tests.
  2. Verified that I am able to send a test notification to both Kafka Rest Proxy and Confluent Kafka Cluster.

@yuri-tceretian
Copy link
Contributor

Thanks for addressing this problem! Now we just need to add a hint to users to use http://localhost:0000/kafka for Endpoint url

@ghost ghost marked this pull request as ready for review January 13, 2023 20:12
@ghost
Copy link
Author

ghost commented Jan 13, 2023

Thanks for addressing this problem! Now we just need to add a hint to users to use http://localhost:0000/kafka for Endpoint url

Yep! On it!

@ghost ghost changed the title [Draft] Kafka Contact Point: Fix v3 API URL when used against a Kafka Rest Proxy Kafka Contact Point: Fix v3 API URL when used against a Kafka Rest Proxy Jan 13, 2023
@ghost
Copy link
Author

ghost commented Jan 13, 2023

Updated the UI here: grafana/grafana@412278f

@yuri-tceretian yuri-tceretian merged commit b9b3de8 into grafana:main Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants