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

[ML] Data frame analytics analysis stats #53788

Conversation

dimitris-athanasiou
Copy link
Contributor

Adds parsing and indexing of analysis instrumentation stats.
The latest one is also returned from the get-stats API.

Note that we chose to duplicate objects even where they are currently
similar. There are already ideas on how these will diverge in the future
and while the duplication looks ugly at the moment, it is the option
that offers the highest flexibility.

Adds parsing and indexing of analysis instrumentation stats.
The latest one is also returned from the get-stats API.

Note that we chose to duplicate objects even where they are currently
similar. There are already ideas on how these will diverge in the future
and while the duplication looks ugly at the moment, it is the option
that offers the highest flexibility.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core (:ml)

@benwtrent benwtrent self-requested a review March 19, 2020 14:03
Copy link
Member

@benwtrent benwtrent left a comment

Choose a reason for hiding this comment

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

I have an overall concern with HLRC.

By making these fields ctor parse fields unboxed and mandatory, we restrict hlrc compatibility as it cannot ever handle those fields becoming optional or being null.

Do you think we should all these to be optional (at least when parsing from xcontent). We can still make them mandatory in the object by unboxing to zero values.

@dimitris-athanasiou
Copy link
Contributor Author

@benwtrent I addressed your comments.

For now I made fields optional where necessary and kept them simply boxed.

Copy link
Member

@benwtrent benwtrent left a comment

Choose a reason for hiding this comment

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

two things, both I think are just copy-paste type errors. Once those are cool, should be good to go :D

@dimitris-athanasiou dimitris-athanasiou merged commit 04aee0a into elastic:master Mar 20, 2020
@dimitris-athanasiou dimitris-athanasiou deleted the df-analytics-analysis-stats branch March 20, 2020 08:36
dimitris-athanasiou pushed a commit to dimitris-athanasiou/elasticsearch that referenced this pull request Mar 20, 2020
dimitris-athanasiou added a commit that referenced this pull request Mar 20, 2020
dimitris-athanasiou added a commit that referenced this pull request Mar 20, 2020
Adds parsing and indexing of analysis instrumentation stats.
The latest one is also returned from the get-stats API.

Note that we chose to duplicate objects even where they are currently
similar. There are already ideas on how these will diverge in the future
and while the duplication looks ugly at the moment, it is the option
that offers the highest flexibility.

Backport of #53788
dimitris-athanasiou added a commit to dimitris-athanasiou/elasticsearch that referenced this pull request Mar 20, 2020
dimitris-athanasiou added a commit that referenced this pull request Mar 20, 2020
@jakelandis jakelandis removed the v8.0.0 label Jul 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants