-
Notifications
You must be signed in to change notification settings - Fork 25.6k
[7.x] Add analytics plugin usage stats to _xpack/usage (#54911) #55162
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
[7.x] Add analytics plugin usage stats to _xpack/usage (#54911) #55162
Conversation
Adds analytics plugin usage stats to _xpack/usage. Closes elastic#54847
|
Pinging @elastic/es-analytics-geo (:Analytics/Aggregations) |
polyfractal
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.
Left another serialization question, but otherwise LGTM I think, given what we chatted about and what needs forward-porting to master.
Yay for having tests now so it doesn't go pear-shaped in the future :)
| if (out.getVersion().onOrAfter(Version.V_7_7_0)) { | ||
| out.writeVLong(counters.get(Item.BOXPLOT)); | ||
| } | ||
| out.writeZLong(counters.get(Item.CUMULATIVE_CARDINALITY)); |
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 this be a VLong not ZLong?
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.
It was ZLong in 7.6, got partially changed by mistake in 7.7. So, I am returning to back to the original serialization method and I will open a PR to fix it in 7.7. Good catch, thanks for reminding me!
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 opened #55183. Not sure if it will make it into 7.7.0 or not.
Fixes incorrect serialization of cumulativeCardinalityUsage Relates to elastic#55162
Fixes incorrect serialization of cumulativeCardinalityUsage Relates to #55162
Adds analytics plugin usage stats to _xpack/usage.
Closes #54847
@nik9000, @polyfractal I will have to do some change to #54911 once this PR is merged to make it backward compatible. I discovered some issues during porting. Basically I will have to forward port these changes in AnalyticsStatsAction.java. Since there are significant changes in this backport I would love to get a review from one of you.