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

cumulative sum, moving avg and serial diff pipeline aggregation metric #10033

Merged
merged 4 commits into from
Feb 15, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 0 additions & 128 deletions src/ui/public/agg_types/__tests__/metrics/derivative.js

This file was deleted.

144 changes: 144 additions & 0 deletions src/ui/public/agg_types/__tests__/metrics/parent_pipeline.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
import _ from 'lodash';
import expect from 'expect.js';
import ngMock from 'ng_mock';
import DerivativeProvider from 'ui/agg_types/metrics/derivative';
import CumulativeSumProvider from 'ui/agg_types/metrics/cumulative_sum';
import MovingAvgProvider from 'ui/agg_types/metrics/moving_avg';
import SerialDiffProvider from 'ui/agg_types/metrics/serial_diff';
import VisProvider from 'ui/vis';
import StubbedIndexPattern from 'fixtures/stubbed_logstash_index_pattern';

const metrics = [
Copy link
Contributor

Choose a reason for hiding this comment

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

in the UI, consider using the same division that you used for pipeline aggs. it presents well.

Copy link
Member Author

Choose a reason for hiding this comment

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

i will add it in the bucket PR (will rebase in on master once this gets merged)

{ name: 'derivative', title: 'Derivative', provider: DerivativeProvider },
{ name: 'cumulative_sum', title: 'Cumulative Sum', provider: CumulativeSumProvider },
{ name: 'moving_avg', title: 'Moving Avg', provider: MovingAvgProvider },
{ name: 'serial_diff', title: 'Serial Diff', provider: SerialDiffProvider },
];

describe('parent pipeline aggs', function () {
metrics.forEach(metric => {
describe(`${metric.title} metric`, function () {

let aggDsl;
let metricAgg;
let aggConfig;

function init(settings) {
ngMock.module('kibana');
ngMock.inject(function (Private) {
const Vis = Private(VisProvider);
const indexPattern = Private(StubbedIndexPattern);
metricAgg = Private(metric.provider);

const params = settings || {
metricAgg: '1',
customMetric: null
};

const vis = new Vis(indexPattern, {
title: 'New Visualization',
type: 'metric',
params: {
fontSize: 60,
handleNoResults: true
},
aggs: [
{
id: '1',
type: 'count',
schema: 'metric'
},
{
id: '2',
type: metric.name,
schema: 'metric',
params
}
],
listeners: {}
});

// Grab the aggConfig off the vis (we don't actually use the vis for anything else)
aggConfig = vis.aggs[1];
aggDsl = aggConfig.toDsl();
});
}

it(`should return a label prefixed with ${metric.title} of`, function () {
init();
expect(metricAgg.makeLabel(aggConfig)).to.eql(`${metric.title} of Count`);
});

it(`should return a label ${metric.title} of max bytes`, function () {
init({
metricAgg: 'custom',
customMetric: {
id:'1-orderAgg',
type: 'max',
params: { field: 'bytes' },
schema: 'orderAgg'
}
});
expect(metricAgg.makeLabel(aggConfig)).to.eql(`${metric.title} of Max bytes`);
});

it(`should return a label prefixed with number of ${metric.title.toLowerCase()}`, function () {
init({
metricAgg: 'custom',
customMetric: {
id:'2-orderAgg',
type: metric.name,
params: {
buckets_path: 'custom',
customMetric: {
id:'2-orderAgg-orderAgg',
type: 'count',
schema: 'orderAgg'
}
},
schema: 'orderAgg'
}
});
expect(metricAgg.makeLabel(aggConfig)).to.eql(`2. ${metric.title.toLowerCase()} of Count`);
});

it('should set parent aggs', function () {
init({
metricAgg: 'custom',
customMetric: {
id:'2-metric',
type: 'max',
params: { field: 'bytes' },
schema: 'orderAgg'
}
});
expect(aggDsl[metric.name].buckets_path).to.be('2-metric');
expect(aggDsl.parentAggs['2-metric'].max.field).to.be('bytes');
});

it('should set nested parent aggs', function () {
init({
metricAgg: 'custom',
customMetric: {
id:'2-metric',
type: metric.name,
params: {
buckets_path: 'custom',
customMetric: {
id:'2-metric-metric',
type: 'max',
params: { field: 'bytes' },
schema: 'orderAgg'
}
},
schema: 'orderAgg'
}
});
expect(aggDsl[metric.name].buckets_path).to.be('2-metric');
expect(aggDsl.parentAggs['2-metric'][metric.name].buckets_path).to.be('2-metric-metric');
});

});
});

});
6 changes: 6 additions & 0 deletions src/ui/public/agg_types/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ import AggTypesMetricsCardinalityProvider from 'ui/agg_types/metrics/cardinality
import AggTypesMetricsPercentilesProvider from 'ui/agg_types/metrics/percentiles';
import AggTypesMetricsPercentileRanksProvider from 'ui/agg_types/metrics/percentile_ranks';
import AggTypesMetricsDerivativeProvider from 'ui/agg_types/metrics/derivative';
import AggTypesMetricsCumulativeSumProvider from 'ui/agg_types/metrics/cumulative_sum';
import AggTypesMetricsMovingAvgProvider from 'ui/agg_types/metrics/moving_avg';
import AggTypesMetricsSerialDiffProvider from 'ui/agg_types/metrics/serial_diff';
import AggTypesBucketsDateHistogramProvider from 'ui/agg_types/buckets/date_histogram';
import AggTypesBucketsHistogramProvider from 'ui/agg_types/buckets/histogram';
import AggTypesBucketsRangeProvider from 'ui/agg_types/buckets/range';
Expand All @@ -37,6 +40,9 @@ export default function AggTypeService(Private) {
Private(AggTypesMetricsPercentileRanksProvider),
Private(AggTypesMetricsTopHitProvider),
Private(AggTypesMetricsDerivativeProvider),
Private(AggTypesMetricsCumulativeSumProvider),
Private(AggTypesMetricsMovingAvgProvider),
Private(AggTypesMetricsSerialDiffProvider)
],
buckets: [
Private(AggTypesBucketsDateHistogramProvider),
Expand Down
17 changes: 17 additions & 0 deletions src/ui/public/agg_types/metrics/cumulative_sum.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import AggTypesMetricsMetricAggTypeProvider from 'ui/agg_types/metrics/metric_agg_type';
import ParentPipelineAggHelperProvider from './lib/parent_pipeline_agg_helper';
import { makeNestedLabel } from './lib/make_nested_label';

export default function AggTypeMetricComulativeSumProvider(Private) {
const MetricAggType = Private(AggTypesMetricsMetricAggTypeProvider);
const parentPipelineAggHelper = Private(ParentPipelineAggHelperProvider);

return new MetricAggType({
name: 'cumulative_sum',
title: 'Cumulative Sum',
makeLabel: agg => makeNestedLabel(agg, 'cumulative sum'),
params: [
...parentPipelineAggHelper.params()
Copy link
Contributor

@stacey-gammon stacey-gammon Feb 7, 2017

Choose a reason for hiding this comment

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

Interesting. So how is this different from derivatives? Looks the exact same but with a different name & title. The specific calculation differences must be somewhere. :)

... maybe in makeAgg: function (termsAgg, state) { the termsAgg.vis is different?

It could may be even be refactored further because the only difference between the two is three strings, that could really even be one string if you applied the same normalizations to them (lowercase for nested label, lc and spaces => underscore for name, title as is).

Maybe unnecessary, just a thought. I'm still trying to figure out how choosing cumulative sum matches to the different calculations.

Copy link
Member Author

Choose a reason for hiding this comment

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

so yeah, its pretty much the same, but different ES query will get executed (the name defines the name used in ES query .... so this one will run cumulative_sum instead of derivative ... but apart from that they are the same.

Copy link
Member Author

Choose a reason for hiding this comment

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

why i don't want to refactor this further: at this point they are indeed the same (cumulative sum and derivative) but they actually have different sub options (which we don't expose right now but probably will in the future).

with the current form it'll be really easy to add the parameters that are different + still not be duplicating logic for the parameters that are the same.

]
});
}
51 changes: 3 additions & 48 deletions src/ui/public/agg_types/metrics/derivative.js
Original file line number Diff line number Diff line change
@@ -1,62 +1,17 @@
import AggTypesMetricsMetricAggTypeProvider from 'ui/agg_types/metrics/metric_agg_type';
import metricAggTemplate from 'ui/agg_types/controls/sub_agg.html';
import _ from 'lodash';
import VisAggConfigProvider from 'ui/vis/agg_config';
import VisSchemasProvider from 'ui/vis/schemas';
import ParentPipelineAggHelperProvider from './lib/parent_pipeline_agg_helper';
import { makeNestedLabel } from './lib/make_nested_label';
import { parentPipelineAggController } from './lib/parent_pipeline_agg_controller';
import { parentPipelineAggWritter } from './lib/parent_pipeline_agg_writter';

export default function AggTypeMetricDerivativeProvider(Private) {
const MetricAggType = Private(AggTypesMetricsMetricAggTypeProvider);
const AggConfig = Private(VisAggConfigProvider);
const Schemas = Private(VisSchemasProvider);

const aggFilter = ['!top_hits', '!percentiles', '!percentile_ranks', '!median', '!std_dev'];
const orderAggSchema = (new Schemas([
{
group: 'none',
name: 'orderAgg',
title: 'Order Agg',
aggFilter: aggFilter
}
])).all[0];
const parentPipelineAggHelper = Private(ParentPipelineAggHelperProvider);

return new MetricAggType({
name: 'derivative',
title: 'Derivative',
makeLabel: agg => makeNestedLabel(agg, 'derivative'),
params: [
{
name: 'customMetric',
type: AggConfig,
default: null,
serialize: function (customMetric) {
return customMetric.toJSON();
},
deserialize: function (state, agg) {
return this.makeAgg(agg, state);
},
makeAgg: function (termsAgg, state) {
state = state || { type: 'count' };
state.schema = orderAggSchema;
const metricAgg = new AggConfig(termsAgg.vis, state);
metricAgg.id = termsAgg.id + '-metric';
return metricAgg;
},
write: _.noop
},
{
name: 'buckets_path',
write: _.noop
},
{
name: 'metricAgg',
editor: metricAggTemplate,
default: 'custom',
controller: parentPipelineAggController,
write: parentPipelineAggWritter
}
...parentPipelineAggHelper.params()
]
});
}
Loading