Skip to content

ref(dynamic-sampling): Use already queried data when computing boosted release platform#115792

Merged
cmanallen merged 3 commits into
masterfrom
cmanallen/dynamic-sampling-use-already-queried-data
May 20, 2026
Merged

ref(dynamic-sampling): Use already queried data when computing boosted release platform#115792
cmanallen merged 3 commits into
masterfrom
cmanallen/dynamic-sampling-use-already-queried-data

Conversation

@cmanallen
Copy link
Copy Markdown
Member

@cmanallen cmanallen commented May 19, 2026

This is our top query for a project row (10x second place). But the data already exists in memory at each call site! Let's re-use the already queried project model.

You can see that the transaction typically records a project cache hit as its first op and then queries the project anyway further down. I'm having some difficulty grouping correctly but if you look here you can see that event_manager.dynamic_sampling_observe_latest_release's volume is ~90% of the project-query volume linked above. Since we know it always queries once we can conclude a significant resource savings is possible by using the cached value.

Optionally, we could pass the id and platform as values (which would be best IMO) but since this is a drive-by PR its best to preserve things as much as possible.

@cmanallen cmanallen requested review from a team as code owners May 19, 2026 14:22
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label May 19, 2026
Comment thread src/sentry/dynamic_sampling/rules/helpers/latest_releases.py
Copy link
Copy Markdown
Member

@shellmayr shellmayr left a comment

Choose a reason for hiding this comment

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

LGTM - just a ref, should only reduce the calls we make to the DB without behaviour change

@cmanallen cmanallen merged commit bff1488 into master May 20, 2026
102 of 104 checks passed
@cmanallen cmanallen deleted the cmanallen/dynamic-sampling-use-already-queried-data branch May 20, 2026 12:10
@cmanallen
Copy link
Copy Markdown
Member Author

cmanallen commented May 20, 2026

Initial results. The query no longer executes and can not be found in our traces. No defects have been observed (yet) but I'm not anticipating any.

JonasBa pushed a commit that referenced this pull request May 21, 2026
…d release platform (#115792)

This is our [top
query](https://sentry.sentry.io/explore/traces/?aggregateField=%7B%22groupBy%22%3A%22transaction%22%7D&aggregateField=%7B%22yAxes%22%3A%5B%22count%28span.duration%29%22%5D%7D&mode=aggregate&project=1&query=span.description%3A%22SELECT%20%5C%22sentry_project%5C%22.%5C%22id%5C%22%2C%20%5C%22sentry_project%5C%22.%5C%22slug%5C%22%2C%20%5C%22sentry_project%5C%22.%5C%22name%5C%22%2C%20%5C%22sentry_project%5C%22.%5C%22forced_color%5C%22%2C%20%5C%22sentry_project%5C%22.%5C%22organization_id%5C%22%2C%20%5C%22sentry_project%5C%22.%5C%22public%5C%22%2C%20%5C%22sentry_project%5C%22.%5C%22date_added%5C%22%2C%20%5C%22sentry_project%5C%22.%5C%22status%5C%22%2C%20%5C%22sentry_project%5C%22.%5C%22first_event%5C%22%2C%20%5C%22sentry_project%5C%22.%5C%22external_id%5C%22%2C%20%5C%22sentry_project%5C%22.%5C%22flags%5C%22%2C%20%5C%22sentry_project%5C%22.%5C%22platform%5C%22%20FROM%20%5C%22sentry_project%5C%22%20WHERE%20%5C%22sentry_project%5C%22.%5C%22id%5C%22%20%3D%20%25s%20LIMIT%2021%22&statsPeriod=24h)
for a project row (10x second place). But the data already exists in
memory at each call site! Let's re-use the already queried project
model.

You can see that the
[transaction](https://sentry.sentry.io/explore/traces/trace/48bd65d7d8054efa87ab06ceed977cd3/?aggregateField=%7B%22groupBy%22%3A%22transaction%22%7D&aggregateField=%7B%22yAxes%22%3A%5B%22count%28span.duration%29%22%5D%7D&field=id&field=span.name&field=span.description&field=span.duration&field=transaction&field=timestamp&mode=samples&node=span-8b45bf1b0c1e592b&project=1&query=span.description%3A%22SELECT%20%5C%22sentry_project%5C%22.%5C%22id%5C%22%2C%20%5C%22sentry_project%5C%22.%5C%22slug%5C%22%2C%20%5C%22sentry_project%5C%22.%5C%22name%5C%22%2C%20%5C%22sentry_project%5C%22.%5C%22forced_color%5C%22%2C%20%5C%22sentry_project%5C%22.%5C%22organization_id%5C%22%2C%20%5C%22sentry_project%5C%22.%5C%22public%5C%22%2C%20%5C%22sentry_project%5C%22.%5C%22date_added%5C%22%2C%20%5C%22sentry_project%5C%22.%5C%22status%5C%22%2C%20%5C%22sentry_project%5C%22.%5C%22first_event%5C%22%2C%20%5C%22sentry_project%5C%22.%5C%22external_id%5C%22%2C%20%5C%22sentry_project%5C%22.%5C%22flags%5C%22%2C%20%5C%22sentry_project%5C%22.%5C%22platform%5C%22%20FROM%20%5C%22sentry_project%5C%22%20WHERE%20%5C%22sentry_project%5C%22.%5C%22id%5C%22%20%3D%20%25s%20LIMIT%2021%22%20transaction%3Aspans.consumers.process_segments.process_segment&sort=-span.duration&source=traces&statsPeriod=24h&targetId=9a13383fb2e2c949&timestamp=1779161578)
typically records a project cache hit as its first op and then queries
the project anyway further down. I'm having some difficulty grouping
correctly but if you look
[here](https://sentry.sentry.io/explore/traces/?aggregateField=%7B%22groupBy%22%3A%22span.op%22%7D&aggregateField=%7B%22yAxes%22%3A%5B%22count%28span.duration%29%22%5D%7D&field=id&field=span.name&field=span.description&field=span.duration&field=transaction&field=timestamp&field=span.op&mode=aggregate&project=1&query=transaction%3Aspans.consumers.process_segments.process_segment&statsPeriod=24h)
you can see that
`event_manager.dynamic_sampling_observe_latest_release`'s volume is ~90%
of the project-query volume linked above. Since we know it always
queries once we can conclude a significant resource savings is possible
by using the cached value.

Optionally, we could pass the `id` and `platform` as values (which would
be best IMO) but since this is a drive-by PR its best to preserve things
as much as possible.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants