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

otel: ensure traces include span parenting for publish->subscription calls #2258

Closed
safeer opened this issue Aug 5, 2024 · 0 comments · Fixed by #2287
Closed

otel: ensure traces include span parenting for publish->subscription calls #2258

safeer opened this issue Aug 5, 2024 · 0 comments · Fixed by #2287
Assignees

Comments

@safeer
Copy link
Contributor

safeer commented Aug 5, 2024

Subscription span should be a child of the publisher's span. Check whether this is the case, and if not, create & propagate a new child span for the subscriber.

@safeer safeer self-assigned this Aug 5, 2024
@safeer safeer mentioned this issue Aug 5, 2024
15 tasks
@ftl-robot ftl-robot mentioned this issue Aug 5, 2024
github-merge-queue bot pushed a commit that referenced this issue Aug 8, 2024
This adds an `otel_context` column to both `topic_events` and
`async_calls`.
- on publish, the current otel context is stored in `topic_events`
- `otel_context` is propagated to the `async_calls` row on dequeue and
retries
- on execution of a row in `async_calls`, the current otel trace is
replaced with `otel_context`

Pros:
- we can see related calls all in the same trace
- pattern can be reused for all async calls (i.e. fsm)

Cons:
- storing otel specific context in the db, feels a bit gross
- there is no root span tying the retries / subscriptions together,
since the root would be the controller lifecycle. screenshot below.

An alternative would be to persist the `requestKey` in the tables
instead of `otel_context` and attach that to subscribers / retries, and
use that as a filter in DD, etc.

<img width="774" alt="Screenshot 2024-08-07 at 12 59 43 PM"
src="https://github.com/user-attachments/assets/2cd61392-0ff8-4032-8ca4-20e4aec033de">

Closes #2258
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 a pull request may close this issue.

1 participant