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

frauddetection uses child span for messaging #1654

Closed

Conversation

puckpuck
Copy link
Contributor

@puckpuck puckpuck commented Jul 2, 2024

Changes

With #1538, the accountingservice moved from a child span to a linked span from the Kafka queue. We no longer had any child spans from Kafka. This change will set the fraud detection service to generate child spans from Kafka, allowing the demo to showcase both use cases from messaging queues.

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 July 2, 2024 17:14
@github-actions github-actions bot added the helm-update-required Requires an update to the Helm chart when released label Jul 2, 2024
@julianocosta89
Copy link
Member

Not sure if we should add this.
AFAIK the recommended way to do it is via linked spans, as we are doing now.

https://opentelemetry.io/docs/specs/semconv/messaging/messaging-spans/#topic-with-multiple-consumers

@puckpuck
Copy link
Contributor Author

puckpuck commented Jul 2, 2024

I don't see the example listed there, but I believe a child span should be created for single producer/consumer scenarios. I understand we don't have that scenario here at all, but being able to demonstrate trace propagation using a child span is still something I think we should showcase.

@julianocosta89
Copy link
Member

All examples in the page use link.
The recommendation by the Messaging SIG is using link: #1538 (comment)

@puckpuck puckpuck closed this Jul 4, 2024
@puckpuck puckpuck deleted the frauddetection.kafka-spans branch July 4, 2024 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
helm-update-required Requires an update to the Helm chart when released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants