Skip to content
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

Convert metric aggs docs runtime fields #71260

Merged
merged 4 commits into from
Apr 5, 2021

Conversation

nik9000
Copy link
Member

@nik9000 nik9000 commented Apr 2, 2021

This replaces the script docs for bucket aggregations with runtime
fields. We expect runtime fields to be nicer to work with because you
can also fetch them or filter on them. We expect them to be faster
because their don't need this sort of instanceof tree:

Relates to #69291

This replaces the `script` docs for bucket aggregations with runtime
fields. We expect runtime fields to be nicer to work with because you
can also fetch them or filter on them. We expect them to be faster
because their don't need this sort of `instanceof` tree:
https://github.com/elastic/elasticsearch/blob/a92a647b9f17d1bddf5c707490a19482c273eda3/server/src/main/java/org/elasticsearch/search/aggregations/support/values/ScriptDoubleValues.java#L42

Relates to elastic#69291
@elasticmachine elasticmachine added Team:Docs Meta label for docs team Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) labels Apr 2, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-docs (Team:Docs)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo (Team:Analytics)

Copy link
Member

@not-napoleon not-napoleon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for jumping on this.

Copy link
Contributor

@jrodewig jrodewig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I left a few suggestions to avoid second person (mostly "we") wording. Those are non-blocking, and you can ignore them if wanted. Thanks for handling this @nik9000.

Copy link
Contributor

@lockewritesdocs lockewritesdocs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Started a review with non-blocking changes while James was reviewing, so might be some overlap.

Comment on lines 78 to 81
If you need to create a boxplot for values that aren't indexed exactly you
should create a <<runtime,runtime field>> and get the boxplot of that. For
example, if our load times are in milliseconds but we want values calculated
in seconds, we could use a runtime field to convert them on-the-fly:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If you need to create a boxplot for values that aren't indexed exactly you
should create a <<runtime,runtime field>> and get the boxplot of that. For
example, if our load times are in milliseconds but we want values calculated
in seconds, we could use a runtime field to convert them on-the-fly:
If you need to create a boxplot for values that differ from the indexed values,
create a <<runtime,runtime field>> and then compute the boxplot. For
example, if your load times are in milliseconds but you want values calculated
in seconds, use a runtime field to convert values within a query:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that "on-the-fly" was in these docs before you made changes, but we try to avoid idioms like these because they don't translate well beyond English.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@nik9000
Copy link
Member Author

nik9000 commented Apr 5, 2021

Thanks for the reviews folks!

@nik9000 nik9000 merged commit 6a1220e into elastic:master Apr 5, 2021
nik9000 added a commit to nik9000/elasticsearch that referenced this pull request Apr 5, 2021
This replaces the `script` docs for bucket aggregations with runtime
fields. We expect runtime fields to be nicer to work with because you
can also fetch them or filter on them. We expect them to be faster
because their don't need this sort of `instanceof` tree:
https://github.com/elastic/elasticsearch/blob/a92a647b9f17d1bddf5c707490a19482c273eda3/server/src/main/java/org/elasticsearch/search/aggregations/support/values/ScriptDoubleValues.java#L42

Relates to elastic#69291

Co-authored-by: James Rodewig <[email protected]>
Co-authored-by: Adam Locke <[email protected]>
nik9000 added a commit to nik9000/elasticsearch that referenced this pull request Apr 5, 2021
This replaces the `script` docs for bucket aggregations with runtime
fields. We expect runtime fields to be nicer to work with because you
can also fetch them or filter on them. We expect them to be faster
because their don't need this sort of `instanceof` tree:
https://github.com/elastic/elasticsearch/blob/a92a647b9f17d1bddf5c707490a19482c273eda3/server/src/main/java/org/elasticsearch/search/aggregations/support/values/ScriptDoubleValues.java#L42

Relates to elastic#69291

Co-authored-by: James Rodewig <[email protected]>
Co-authored-by: Adam Locke <[email protected]>
nik9000 added a commit that referenced this pull request Apr 5, 2021
This replaces the `script` docs for bucket aggregations with runtime
fields. We expect runtime fields to be nicer to work with because you
can also fetch them or filter on them. We expect them to be faster
because their don't need this sort of `instanceof` tree:
https://github.com/elastic/elasticsearch/blob/a92a647b9f17d1bddf5c707490a19482c273eda3/server/src/main/java/org/elasticsearch/search/aggregations/support/values/ScriptDoubleValues.java#L42

Relates to #69291

Co-authored-by: James Rodewig <[email protected]>
Co-authored-by: Adam Locke <[email protected]>
nik9000 added a commit that referenced this pull request Apr 5, 2021
This replaces the `script` docs for bucket aggregations with runtime
fields. We expect runtime fields to be nicer to work with because you
can also fetch them or filter on them. We expect them to be faster
because their don't need this sort of `instanceof` tree:
https://github.com/elastic/elasticsearch/blob/a92a647b9f17d1bddf5c707490a19482c273eda3/server/src/main/java/org/elasticsearch/search/aggregations/support/values/ScriptDoubleValues.java#L42

Relates to #69291

Co-authored-by: James Rodewig <[email protected]>
Co-authored-by: Adam Locke <[email protected]>

Co-authored-by: James Rodewig <[email protected]>
Co-authored-by: Adam Locke <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Aggregations Aggregations >docs General docs changes Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) Team:Docs Meta label for docs team v7.12.1 v7.13.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants