feat(outcomes) Support key tsdb models#15380
Conversation
| `group_on_time`: whether to add a GROUP BY clause on the 'time' field. | ||
| `group_on_model`: whether to add a GROUP BY clause on the primary model. | ||
| """ | ||
| # XXX: to counteract the hack in project_key_stats.py |
There was a problem hiding this comment.
The hack I'm referring to: https://github.com/getsentry/sentry/blob/master/src/sentry/api/endpoints/project_key_stats.py#L36-L38
There was a problem hiding this comment.
This throws if the key is a string not containing a number. Are you sure keys are always numbers?
There was a problem hiding this comment.
Yea. The key based models are only ever called from project_key_stats so feel pretty safe about that.
fpacifici
left a comment
fpacifici
left a comment
There was a problem hiding this comment.
just one clarification, otherwise it is ok
| `group_on_time`: whether to add a GROUP BY clause on the 'time' field. | ||
| `group_on_model`: whether to add a GROUP BY clause on the primary model. | ||
| """ | ||
| # XXX: to counteract the hack in project_key_stats.py |
There was a problem hiding this comment.
This throws if the key is a string not containing a number. Are you sure keys are always numbers?
fpacifici
left a comment
fpacifici
left a comment
There was a problem hiding this comment.
Please verify my last comment, anyway that would not be dangerous since we are measuring the mismatch still and in the worst case scenario it would restrict the result and not allow to read results the user is not supposed to see.
| project_key = ProjectKey.objects.get(pk=key_ids[0]) | ||
| organization_id = project_key.project.organization_id |
There was a problem hiding this comment.
Was it possible before to load metrics across organizations ? This one defaults to the organization of the first project.
There was a problem hiding this comment.
Nope. It was always a single organization.
2 participants
In addition to
projectandorganizationtsdb models, outcomes should also support the follow project key tsdb models:key_total_receivedkey_total_rejectedkey_total_blacklistedI missed this in the initial implementation so adding it now.
Test plan