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(CapMan): QueryBuilders + Search tenant_ids #45505

Merged
merged 14 commits into from
Mar 14, 2023

Conversation

rahul-kumar-saini
Copy link
Contributor

@rahul-kumar-saini rahul-kumar-saini commented Mar 7, 2023

Overview

  • All Snuba queries built and executed with QueryBuilder objects now contain org ID in tenant_ids for their Snuba Requests
  • Outcomes Timeseries Queries now have tenant_ids
  • Search queries now have tenant_ids
  • See feat(CapMan): Pass tenant_ids to Snuba #44788 for more context

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Mar 7, 2023
@rahul-kumar-saini rahul-kumar-saini marked this pull request as ready for review March 8, 2023 00:03
@rahul-kumar-saini rahul-kumar-saini requested a review from a team March 8, 2023 00:03
@rahul-kumar-saini rahul-kumar-saini requested a review from a team as a code owner March 8, 2023 00:03
@rahul-kumar-saini rahul-kumar-saini requested a review from a team as a code owner March 8, 2023 00:06
@rahul-kumar-saini rahul-kumar-saini changed the title feat(CapMan): QueryBuilders now produce queries with tenant_ids feat(CapMan): QueryBuilders + Search tenant_ids Mar 8, 2023
Copy link
Member

@volokluev volokluev left a comment

Choose a reason for hiding this comment

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

it seems like these don't have referrers attached to them. How are they populated?

@@ -173,17 +173,18 @@ def get(self, request: Request, organization) -> Response:
Select a field, define a date range, and group or filter by columns.
"""
with self.handle_query_errors():
tenant_ids = {"organization_id": organization.id}
Copy link
Member

Choose a reason for hiding this comment

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

does the referrer get injected somewhere else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since referrer is passed to Snuba in every request, I've updated the Snuba query functions in utils/snuba.py to at least add the referrer per request into tenant ids.

Eg:

if referrer:
kwargs["tenant_ids"] = kwargs.get("tenant_ids") or dict()
kwargs["tenant_ids"]["referrer"] = referrer

@rahul-kumar-saini rahul-kumar-saini requested a review from wmak March 13, 2023 18:12
Copy link
Member

@wmak wmak left a comment

Choose a reason for hiding this comment

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

lgtm

@rahul-kumar-saini rahul-kumar-saini merged commit 911d002 into master Mar 14, 2023
@rahul-kumar-saini rahul-kumar-saini deleted the rahul/feat/querybuilder_tenant_ids branch March 14, 2023 17:32
@github-actions github-actions bot locked and limited conversation to collaborators Mar 30, 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.

3 participants