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

Fix asyncpg spans #3639

Merged
merged 13 commits into from
Oct 15, 2024
Merged

Fix asyncpg spans #3639

merged 13 commits into from
Oct 15, 2024

Conversation

sentrivana
Copy link
Contributor

@sentrivana sentrivana commented Oct 10, 2024

  • Change span.set_data to span.set_attribute

  • Change start_transactions in the tests to start_spans

  • Stringify db.params, db.paramstyle, db.cursor

  • ❗ Change how adding query source works.
    Previously, we first let the DB span finish and then looked at its start and end timestamps to figure out if the query was slow enough to attach query data.

    With the switch to OTel, we can no longer put stuff on a finished span, meaning the slow query span still has to be alive when adding data to it. So instead of looking at the end timestamp of a finished span, we keep the span alive and instead look at current time at the point when the query is finished.

    This means the DB span will be longer than the actual query (since it now includes adding query data).

Copy link

codecov bot commented Oct 10, 2024

❌ 2925 Tests Failed:

Tests completed Failed Passed Skipped
20066 2925 17141 4443
View the top 3 failed tests by shortest run time
tests.integrations.litestar.test_litestar test_middleware_partial_receive_send
Stack Traces | 0s run time
No failure message available
tests.integrations.litestar.test_litestar test_middleware_partial_receive_send
Stack Traces | 0s run time
No failure message available
tests.integrations.litestar.test_litestar test_span_origin
Stack Traces | 0s run time
No failure message available

To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard

Comment on lines +82 to +83
with capture_internal_exceptions():
add_query_source(span)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the query source change from the PR description

@sentrivana sentrivana marked this pull request as ready for review October 11, 2024 09:09
@sl0thentr0py sl0thentr0py merged commit ef7c39c into potel-base Oct 15, 2024
16 of 122 checks passed
@sl0thentr0py sl0thentr0py deleted the ivana/potel/fix-asyncpg branch October 15, 2024 14:45
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