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

Add stats_bucket / extended_stats_bucket pipeline aggs #13128

Merged
merged 1 commit into from
Sep 4, 2015

Conversation

polyfractal
Copy link
Contributor

These are the complements to the stats/extended_stats metric aggregations, and can be used
to calculate a variety of statistics over buckets

Pretty straight forward addition, but a few notes:

  • These extend metrics.stats.InternalStats & metrics.stats.extended.ExtendedInternalStats to reduce copy/paste of basically the same code
  • The pipeline aggs register the stream of their specific internal representation (see StatsBucketPipelineAggregator.registerStreams() for example). I thought this was cleaner than cluttering up the SearchModule registration list with 2x registrations for each, but happy to change if this is too indirect
  • Provides two interfaces (StatsBucket & ExtendedStatsBucket) which are basically just identical subclasses of the metric versions. Thought it would still be good to have these for the end-user.

/cc @colings86 if you have some spare time, no rush though.

@clintongormley clintongormley changed the title Aggregations: Add stats_bucket / extended_stats_bucket pipeline aggs Add stats_bucket / extended_stats_bucket pipeline aggs Aug 27, 2015

public class InternalStatsBucket extends InternalStats implements StatsBucket {

public final static Type TYPE = new Type("internal_stats_bucket");
Copy link
Contributor

Choose a reason for hiding this comment

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

I would use the same type name as the StatsPipelineAggregator here so it's easy to see they relate. That's what we do on other aggregations. The contexts in which these are used and deserialised are different so the names would never clash

@colings86
Copy link
Contributor

Left a couple of comments but I think this is looking good.

@polyfractal
Copy link
Contributor Author

Comments addressed!

@colings86
Copy link
Contributor

LGTM

@polyfractal polyfractal force-pushed the feature/stats_pipeline branch 2 times, most recently from 068e9b7 to d90bd80 Compare September 4, 2015 19:17
…gations

These are the complements to the stats/extended_stats metric aggregations, and can be used
to calculate a variety of statistics over buckets
@polyfractal polyfractal force-pushed the feature/stats_pipeline branch from d90bd80 to 397d5be Compare September 4, 2015 19:23
polyfractal added a commit that referenced this pull request Sep 4, 2015
Aggregations: Add stats_bucket / extended_stats_bucket pipeline aggs
@polyfractal polyfractal merged commit 1f8702e into elastic:master Sep 4, 2015
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.

3 participants