Skip to content

feat(starfish): Add endpoint returns series for top 5 facets#48665

Merged
shruthilayaj merged 6 commits into
masterfrom
shruthi/feat/return-top-stats-of-facets
May 8, 2023
Merged

feat(starfish): Add endpoint returns series for top 5 facets#48665
shruthilayaj merged 6 commits into
masterfrom
shruthi/feat/return-top-stats-of-facets

Conversation

@shruthilayaj

@shruthilayaj shruthilayaj commented May 5, 2023

Copy link
Copy Markdown
Member

Also return count delta to maybe allow us to prioritize
the facet that has had an increase in throughput (in the
case of a regression caused by a sudden increase in a certain
facet)

Series will be used to show sparklines in the frontend

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label May 5, 2023
@shruthilayaj shruthilayaj changed the title feat(starfish): Endpoint returns series for top 5 facet feat(starfish): Add endpoint returns series for top 5 facets May 5, 2023
@shruthilayaj shruthilayaj marked this pull request as ready for review May 5, 2023 21:38
@shruthilayaj shruthilayaj requested a review from a team as a code owner May 5, 2023 21:38
@shruthilayaj shruthilayaj requested review from a team May 5, 2023 21:38
@codecov

codecov Bot commented May 5, 2023

Copy link
Copy Markdown

Codecov Report

Merging #48665 (c7d7f92) into master (b037d8c) will increase coverage by 0.01%.
The diff coverage is 91.07%.

❗ Current head c7d7f92 differs from pull request most recent head 3be3b51. Consider uploading reports for the commit 3be3b51 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #48665      +/-   ##
==========================================
+ Coverage   80.94%   80.96%   +0.01%     
==========================================
  Files        4780     4781       +1     
  Lines      201784   201920     +136     
  Branches    11450    11450              
==========================================
+ Hits       163333   163483     +150     
+ Misses      38196    38182      -14     
  Partials      255      255              
Impacted Files Coverage Δ
static/app/views/replays/detail/network/index.tsx 0.00% <ø> (ø)
...ts/organization_events_facets_stats_performance.py 88.63% <88.63%> (ø)
...ndpoints/organization_events_facets_performance.py 92.30% <100.00%> (+0.45%) ⬆️
src/sentry/api/urls.py 100.00% <100.00%> (ø)

... and 24 files with indirect coverage changes

@wmak wmak left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

small nits, lgtm

Comment on lines +408 to +410
count_column = "count_range({condition}, {boundary})"
count_range_1 = tag_query.resolve_function(
count_column.format(condition="lessOrEquals", boundary=middle),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
count_column = "count_range({condition}, {boundary})"
count_range_1 = tag_query.resolve_function(
count_column.format(condition="lessOrEquals", boundary=middle),
count_range_1 = tag_query.resolve_function(
f"count_range(lessOrEquals, {middle})",

tag_key = TAG_ALIASES.get(tag_key)

with sentry_sdk.start_span(op="discover.endpoint", description="discover_query"):
referrer = "api.organization-events-facets-stats-performance.top-tags"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should use the referrer module in snuba,
ie. from sentry.snuba.referrer import Referrer

@shruthilayaj shruthilayaj merged commit 4b62d91 into master May 8, 2023
@shruthilayaj shruthilayaj deleted the shruthi/feat/return-top-stats-of-facets branch May 8, 2023 20:14
@github-actions github-actions Bot locked and limited conversation to collaborators May 24, 2023
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.

2 participants