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

[checkout] fix kafka restart #1590

Merged

Conversation

puckpuck
Copy link
Contributor

@puckpuck puckpuck commented Jun 1, 2024

Changes

If Kafka is restarted, the checkout service will start to report very long response/acknowledgment times from Kafka. This may be due to the demo's unique configuration, which only has a single broker. To get around the issue, the Kafka/Sarama SDK is set to respond without full acknowledgments. This may lead to missed errors in certain situations but will allow the demo to recover from Kafka restarts and continue working without issues.

Part of this PR is to do proper handling of the Kafka message in a success or error scenario and also add attributes to the Kafka producer span related to details about the interaction with Kafka. A docs update will be required.

Merge Requirements

For new features contributions please make sure you have completed the following
essential items:

  • CHANGELOG.md updated to document new feature additions
  • Appropriate documentation updates in the docs
    * [ ] Appropriate Helm chart updates in the helm-charts

Maintainers will not merge until the above have been completed. If you're unsure
which docs need to be changed ping the
@open-telemetry/demo-approvers.

@puckpuck puckpuck requested a review from a team June 1, 2024 02:50
@puckpuck
Copy link
Contributor Author

puckpuck commented Jun 6, 2024

@julianocosta89 I moved to use semantic conventions or attribute names that align closer to what already exists. Let me know if we still need to update them.

Copy link
Member

@julianocosta89 julianocosta89 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @puckpuck!

@julianocosta89 julianocosta89 merged commit cc29172 into open-telemetry:main Jun 6, 2024
29 checks passed
@puckpuck puckpuck deleted the checkout.fix-kafka-restart branch June 7, 2024 01:07
AlexPSplunk pushed a commit to splunk/edu-opentelemetry-demo that referenced this pull request Jul 10, 2024
* fix kafka acks

* Update src/checkoutservice/main.go

Co-authored-by: Juliano Costa <julianocosta89@outlook.com>

* align attribute names with semantic conventions

* checkout: recover from kafka restart

---------

Co-authored-by: Juliano Costa <julianocosta89@outlook.com>
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.

2 participants