-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Add tests and docs for first/last_over_time and rate #130290
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
Conversation
|
🔍 Preview links for changed docs:
🔔 The preview site may take up to 3 minutes to finish building. These links will become live once it completes. |
|
Pinging @elastic/es-docs (Team:Docs) |
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
|
Pinging @elastic/kibana-esql (ES|QL-ui) |
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.
How easy it is to add a flag to indicate that these are working only with the TS source command? Otherwise I will have to hardcode them at kibana
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.
"type" : "time_series_agg",
The time-series aggregation type is time_series_agg, whereas the type of other aggregations is agg. I can add a new flag too.
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.
Oh I see!! No this will work Nhat! Thanx a ton! I totally missed it 🙈
| examples = { @Example(file = "k8s-timeseries", tag = "rate") } | ||
| ) | ||
| public Rate(Source source, @Param(name = "field", type = { "counter_long", "counter_integer", "counter_double" }) Expression field) { | ||
| this(source, field, new UnresolvedAttribute(source, "@timestamp")); |
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.
We don't need to resolve the timestamp in this field?
kkrik-es
left a comment
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.
Thanks Nhat.
| :::{include} ../_snippets/functions/layout/first_over_time.md | ||
| ::: | ||
|
|
||
| :::{include} ../_snippets/functions/layout/last_over_time.md | ||
| ::: | ||
|
|
||
| :::{include} ../_snippets/functions/layout/rate.md | ||
| ::: |
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.
@dnhatn I see you actually have used applies_to in this PR 👍
BTW curious why you didn't chose to just not include the docs here (thus not publish them) as opposed to publishing them with unavailable markers
It's very early days with the new docs system and just wanna make sure we have somewhat consistent approaches :)
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.
Thanks @leemthompo. I opened #130367 to delay publishing these pages until we are ready.
This PR adds unit tests and docs for first_over_time, last_over_time, and rate. For the rate function, the tests currently only verify that the output is a double, not the actual value.
## Summary Closes #220730 Supports of TS inner agg functions. 1. Retrieves the functions and the docs from elastic/elasticsearch#130290 2. Suggest them only when an agg is already selected and only if the source command is TS  Atm you won't see any changes in the editor. You can test it though if you change the `src/platform/packages/shared/kbn-esql-validation-autocomplete/src/definitions/generated/time_series_agg_functions.ts` `ignoreAsSuggestion` flags to false. ### Checklist - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: Vadim Kibana <[email protected]>
## Summary Closes elastic#220730 Supports of TS inner agg functions. 1. Retrieves the functions and the docs from elastic/elasticsearch#130290 2. Suggest them only when an agg is already selected and only if the source command is TS  Atm you won't see any changes in the editor. You can test it though if you change the `src/platform/packages/shared/kbn-esql-validation-autocomplete/src/definitions/generated/time_series_agg_functions.ts` `ignoreAsSuggestion` flags to false. ### Checklist - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: Vadim Kibana <[email protected]>
This PR adds unit tests and docs for first_over_time, last_over_time, and rate. For the rate function, the tests currently only verify that the output is a double, not the actual value.