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

feat(trace-explorer): Experiment with sorting by timestamp #73119

Merged
merged 8 commits into from
Jun 24, 2024

Conversation

Zylphrex
Copy link
Member

This is an experiment to try supporting sorting traces by timestamp.

This is an experiment to try supporting sorting traces by timestamp.
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jun 21, 2024
Copy link

codecov bot commented Jun 21, 2024

Codecov Report

Attention: Patch coverage is 90.00000% with 13 lines in your changes missing coverage. Please review.

Project coverage is 77.99%. Comparing base (0e395d1) to head (8241a8b).
Report is 36 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master   #73119       +/-   ##
===========================================
+ Coverage   56.91%   77.99%   +21.07%     
===========================================
  Files        6611     6626       +15     
  Lines      295232   295748      +516     
  Branches    50848    50931       +83     
===========================================
+ Hits       168026   230655    +62629     
+ Misses     122482    58814    -63668     
- Partials     4724     6279     +1555     
Files Coverage Δ
src/sentry/features/temporary.py 100.00% <100.00%> (ø)
src/sentry/options/defaults.py 100.00% <100.00%> (ø)
src/sentry/snuba/referrer.py 100.00% <100.00%> (+0.38%) ⬆️
src/sentry/api/endpoints/organization_traces.py 90.65% <89.51%> (+68.92%) ⬆️

... and 2008 files with indirect coverage changes

limit=self.limit,
timestamp_column=timestamp_column,
max_execution_seconds=30,
max_parallel_queries=3,
Copy link
Member

Choose a reason for hiding this comment

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

Good call on parallel to speed it up a bit, are we sure this won't spam Snuba and hit rate limits?

Copy link
Member Author

Choose a reason for hiding this comment

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

3 x 8 hour queries should stay within the rate limits of snuba but I'll confirm before merging. And I explicitly moved this to a different referrer to help alleviate this concern slightly.

@Zylphrex Zylphrex marked this pull request as ready for review June 24, 2024 15:17
@Zylphrex Zylphrex requested a review from a team as a code owner June 24, 2024 15:17
@Zylphrex Zylphrex merged commit 30dfaf2 into master Jun 24, 2024
49 checks passed
@Zylphrex Zylphrex deleted the txiao/feat/experiment-with-sorting-by-timestamp branch June 24, 2024 15:18
Copy link

sentry-io bot commented Jun 25, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ AssertionError: {'data': [{'breakdowns': [{'duration': 60100, 'end': 1719273060100, 'isRoot': False, 'kind': 'pro... pytest.runtest.protocol tests/sentry/api/endpoi... View Issue

Did you find this useful? React with a 👍 or 👎

@github-actions github-actions bot locked and limited conversation to collaborators Jul 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants