-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[APM] Update aggregations to support script sources #76429
[APM] Update aggregations to support script sources #76429
Conversation
Pinging @elastic/apm-ui (Team:apm) |
size?: number; | ||
missing?: string; | ||
order?: SortOptions; | ||
execution_hint?: 'map' | 'global_ordinals'; | ||
}; | ||
} & MetricsAggregationOptions; |
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.
should we rename this then?
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.
Done
@wylieconlon hold up... I remember why i didn't approve. |
.field, | ||
}, | ||
cardinality: | ||
'field' in projectionSource |
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.
I don't think we (should) expect a script
option here. I feel like we can solve this differently. Do you mind if I push a small change to your branch?
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.
Feel free!
💚 Build SucceededBuild metrics
History
To update your PR or re-run it, just comment with: |
* master: (340 commits) [data.search.SearchSource] Remove legacy ES client APIs. (elastic#75943) [release notes] automatically retry on Github API 5xx errors (elastic#76447) [es_ui_shared] Fix eslint exhaustive deps rule (elastic#76392) [i18n] Integrate 7.9.1 Translations (elastic#76391) [APM] Update aggregations to support script sources (elastic#76429) [Security Solution] Refactor Network Top Countries to use Search Strategy (elastic#76244) Document security settings available on ESS (elastic#76513) [Ingest Manager] Add input revision to the config send to the agent (elastic#76327) [DOCS] Identifies cloud settings for Monitoring (elastic#76579) [DOCS] Identifies Cloud settings in Dev Tools (elastic#76583) [Ingest Manager] Better default value for fleet long polling timeout (elastic#76393) [data.indexPatterns] Fix broken rollup index pattern creation (elastic#76593) [Ingest Manager] Split Registry errors into Connection & Response (elastic#76558) [Security Solution] add an excess validation instead of the exact match (elastic#76472) Introduce TS incremental builds & move src/test_utils to TS project (elastic#76082) fix bad merge (elastic#76629) [Newsfeed] Ensure the version format when calling the API (elastic#76381) remove server_extensions mixin (elastic#76606) Remove legacy applications and legacy mode (elastic#75987) [Discover] Fix sidebar element focus behavior when adding / removing columns (elastic#75749) ...
* [APM] Update aggregations to support script sources * Fix whitespace * Fix checks * Explicitly require field in projection Co-authored-by: Dario Gieselaar <dario.gieselaar@elastic.co> Co-authored-by: Dario Gieselaar <dario.gieselaar@elastic.co>
field
is not always required on aggregations, it's eitherfield
orscript
parameters. This distinction was represented for metric aggregations, but not for bucket aggregations.