Skip to content

Conversation

@cbrewster
Copy link
Member

Why

The streaming spans were immediately finishing because we were only tracing the construction of the AsyncIterator and not the full iteration.

What changed

  • loop and yield each message of the streaming procedures so the span doesn't finish until the async iterator is disposed.

Test plan

  • Streaming procedure spans should now last the full duration of the stream procedure

@cbrewster
Copy link
Member Author

cbrewster commented Nov 22, 2024

Current dependencies on/for this PR:

This comment was autogenerated by Freephite.

@cbrewster cbrewster requested a review from a team as a code owner November 22, 2024 06:20
@cbrewster cbrewster requested review from blast-hardcheese and removed request for a team November 22, 2024 06:20
@cbrewster cbrewster force-pushed the 11-22-bugfix_make_streaming_spans_last_for_the_entire_duration_of_the_stream branch 2 times, most recently from c00307d to 9c80c89 Compare November 22, 2024 06:29
Copy link
Contributor

@blast-hardcheese blast-hardcheese left a comment

Choose a reason for hiding this comment

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

Do'h, good catch

@cbrewster cbrewster force-pushed the 11-22-bugfix_make_streaming_spans_last_for_the_entire_duration_of_the_stream branch from 9c80c89 to 035439b Compare November 22, 2024 06:47
@cbrewster cbrewster force-pushed the 11-22-bugfix_make_streaming_spans_last_for_the_entire_duration_of_the_stream branch from eca336f to fcea3b5 Compare November 22, 2024 21:46
@cbrewster cbrewster merged commit 4ccbd27 into main Nov 22, 2024
3 checks passed
@cbrewster cbrewster deleted the 11-22-bugfix_make_streaming_spans_last_for_the_entire_duration_of_the_stream branch November 22, 2024 22:03
@cbrewster cbrewster added the bug Something isn't working label Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants