-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(explore): Support numeric attributes in count_unique #100895
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
base: master
Are you sure you want to change the base?
feat(explore): Support numeric attributes in count_unique #100895
Conversation
Numeric attributes should be permitted as an argument to count_unique. Requires #100889 Frontend for LOGS-398
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #100895 +/- ##
===========================================
- Coverage 81.15% 81.15% -0.01%
===========================================
Files 8615 8615
Lines 382250 382237 -13
Branches 24033 24029 -4
===========================================
- Hits 310199 310187 -12
+ Misses 71723 71722 -1
Partials 328 328 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: COUNT_UNIQUE Filter Inconsistency
The COUNT_UNIQUE
aggregation in spans.tsx
now supports both 'number' and 'string' column types. However, the filterAggregateParams
function in the same file still only accepts 'string' types for this aggregation. This inconsistency prevents users from selecting valid numeric attributes for COUNT_UNIQUE
operations on spans.
static/app/views/dashboards/datasetConfig/spans.tsx#L110-L111
sentry/static/app/views/dashboards/datasetConfig/spans.tsx
Lines 110 to 111 in d3dfc04
kind: 'column', | |
columnTypes: ['number', 'string'], |
static/app/views/dashboards/datasetConfig/spans.tsx#L274-L285
sentry/static/app/views/dashboards/datasetConfig/spans.tsx
Lines 274 to 285 in d3dfc04
); | |
return {...baseFieldOptions, ...spanTags}; | |
} | |
function _isNotNumericTag(option: FieldValueOption) { | |
// Filter out numeric tags from primary options, they only show up in | |
// the parameter fields for aggregate functions | |
if ('dataType' in option.value.meta) { | |
return option.value.meta.dataType !== 'number'; | |
} | |
return true; |
return aggregation === AggregationKey.COUNT_UNIQUE | ||
? {...storedNumberTags, ...storedStringTags} | ||
: storedNumberTags; | ||
}, [aggregation, storedNumberTags, storedStringTags]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Tag Merging Overwrites Numeric Attributes
For COUNT_UNIQUE
aggregations, merging storedNumberTags
and storedStringTags
with object spread causes string attributes to overwrite numeric ones when keys conflict. This makes numeric variants of attributes unavailable for selection, potentially breaking related functionality.
Numeric attributes should be permitted as an argument to count_unique.
Requires #100889
Closes for LOGS-398