feat: Add OrganizationSpansFrequencyStatsEndpoint for Comparison Workflows project #84765
Conversation
github-actions
bot
added
the
Scope: Backend
❌ 3 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
Mitan
changed the title
| {"attributeDistributions": []} # Empty response matching the expected structure | ||
| ) | ||
|
|
||
| serializer = OrganizationSpansFieldsEndpointSerializer(data=request.GET) |
There was a problem hiding this comment.
Why are we validating our request with this serializer? Looks like you're already doing the dataset validation below and this serializer contains fields not pertinent to this endpoint
There was a problem hiding this comment.
thanks, added a serializer for this endpoint
| class OrganizationSpansFrequencyStatsEndpoint(OrganizationEventsV2EndpointBase): | ||
| snuba_methods = ["GET"] | ||
| publish_status = { | ||
| "GET": ApiPublishStatus.PRIVATE, |
There was a problem hiding this comment.
Let's create a ticket to document this API as a follow up! This API should likely be a public one eventually so it would be good to document it while you have the context!
Co-authored-by: Shruthi <[email protected]>
| dataset = serializers.ChoiceField(["spans", "spansIndexed"], required=False, default="spans") | ||
| # if values are not provided, we will use zeros and then snuba RPC will set the defaults | ||
| # Top number of frequencies to return for each attribute, defaults in snuba to 10 and can't be more than 100 | ||
| max_buckets = serializers.IntegerField(required=False, min_value=0, max_value=100, default=0) |
There was a problem hiding this comment.
let's pick a more reasonable default - like 5 or 10?
| # Top number of frequencies to return for each attribute, defaults in snuba to 10 and can't be more than 100 | ||
| max_buckets = serializers.IntegerField(required=False, min_value=0, max_value=100, default=0) | ||
| # Total number of attributes to return, defaults in snuba to 10_000 | ||
| max_attributes = serializers.IntegerField(required=False, min_value=0, default=0) |
There was a problem hiding this comment.
any reason we're setting a 0 default? let's just not set anything here for default since it defeats the purpose of making it a not required field
|
|
||
|
|
||
| class OrganizationSpansFrequencyStatsEndpointSerializer(serializers.Serializer): | ||
| dataset = serializers.ChoiceField(["spans", "spansIndexed"], required=False, default="spans") |
There was a problem hiding this comment.
spansIndexed should not be a valid choice here
There was a problem hiding this comment.
then i can just remove dataset parameter, since technically we don't need it at all
| class OrganizationSpansFieldsStatsEndpoint(OrganizationEventsV2EndpointBase): | ||
| snuba_methods = ["GET"] | ||
| publish_status = { | ||
| "GET": ApiPublishStatus.PRIVATE, |
There was a problem hiding this comment.
Just to confirm, private means this endpoint will never be useful for customers calling our APIs directly and only used in Sentry frontend. Is that right? If it is marked as private only because it's not stable yet, please make it EXPERIMENTAL
There was a problem hiding this comment.
thanks, yes, we can mark it as experimental
| stats_type = StatsType( | ||
| attribute_distributions=AttributeDistributionsRequest( | ||
| max_buckets=serialized["max_buckets"], | ||
| max_attributes=serialized.get("max_attributes", 0), |
There was a problem hiding this comment.
don't default to 0 here, just set it to None
| max_attributes=serialized.get("max_attributes", 0), | |
| max_attributes=serialized.get("max_attributes"), |
|
|
||
| @region_silo_endpoint | ||
| class OrganizationSpansFieldsStatsEndpoint(OrganizationEventsV2EndpointBase): | ||
| snuba_methods = ["GET"] |
For comparison workflows, we need the count distribution data of all attribute/value pairs (both string and numeric, however we'll focus on string first). This PR adds a sentry backend endpoint for retrieving these stats.
The protos required for this task were already merged here
The snuba RPC was merged here
More info on the project can be found here