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 data] ability to disable processors #7065

Merged
Merged
Show file tree
Hide file tree
Changes from 2 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
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import './processor_ui';
import pipelineSetupTemplate from '../views/pipeline_setup.html';

const app = uiModules.get('kibana');
function buildProcessorTypeList() {

function buildProcessorTypeList(enabledProcessorTypeIds) {
return _(ProcessorTypes)
.map(Type => {
const instance = new Type();
Expand All @@ -22,6 +23,8 @@ function buildProcessorTypeList() {
};
})
.compact()
.filter((processorType) => enabledProcessorTypeIds.includes(processorType.typeId))
.sortBy('title')
.value();
}

Expand All @@ -36,9 +39,14 @@ app.directive('pipelineSetup', function () {
controller: function ($scope, debounce, Private, Notifier) {
const ingest = Private(IngestProvider);
const notify = new Notifier({ location: `Ingest Pipeline Setup` });
$scope.processorTypes = _.sortBy(buildProcessorTypeList(), 'title');
$scope.sample = {};

//determines which processors are available on the cluster
ingest.getProcessors()
.then((enabledProcessorTypeIds) => {
$scope.processorTypes = buildProcessorTypeList(enabledProcessorTypeIds);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to catch errors here and display a notification if there's a problem, otherwise the user gets an empty processor list and no explanation why.


const pipeline = new Pipeline();
// Loads pre-existing pipeline which will exist if the user returns from
// a later step in the wizard
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import processESIngestProcessorsResponse from '../process_es_ingest_processors_response';
import expect from 'expect.js';
import _ from 'lodash';

describe('processESIngestSimulateResponse', function () {

it('should return a list of strings indicating the enabled processors', function () {
const response = {
nodes: {
node_foo: {
ingest: {
processors: [
{ type: 'proc_foo' },
{ type: 'proc_bar' }
]
}
}
}
};

const expected = [ 'proc_foo', 'proc_bar' ];
const actual = processESIngestProcessorsResponse(response);

expect(actual).to.eql(expected);
Copy link
Contributor

Choose a reason for hiding this comment

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

I still prefer to always use lodash to get deep and strict equality checking expect(_.isEqual(actual, expected)).to.be.ok()

});

it('should return a unique list of processors', function () {
const response = {
nodes: {
node_foo: {
ingest: {
processors: [
{ type: 'proc_foo' },
{ type: 'proc_bar' }
]
}
},
node_bar: {
ingest: {
processors: [
{ type: 'proc_foo' },
{ type: 'proc_bar' }
]
}
}
}
};

const expected = [ 'proc_foo', 'proc_bar' ];
const actual = processESIngestProcessorsResponse(response);

expect(actual).to.eql(expected);
});

it('should combine the available processors from all nodes', function () {
const response = {
nodes: {
node_foo: {
ingest: {
processors: [
{ type: 'proc_foo' }
]
}
},
node_bar: {
ingest: {
processors: [
{ type: 'proc_bar' }
]
}
}
}
};

const expected = [ 'proc_foo', 'proc_bar' ];
const actual = processESIngestProcessorsResponse(response);

expect(actual).to.eql(expected);
});

it('should return an empty array for unexpected response', function () {
expect(processESIngestProcessorsResponse({ nodes: {}})).to.eql([]);
expect(processESIngestProcessorsResponse({})).to.eql([]);
expect(processESIngestProcessorsResponse(undefined)).to.eql([]);
expect(processESIngestProcessorsResponse(null)).to.eql([]);
expect(processESIngestProcessorsResponse('')).to.eql([]);
expect(processESIngestProcessorsResponse(1)).to.eql([]);
});

});
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
const _ = require('lodash');

export default function processESIngestProcessorsResponse(response) {
const nodes = _.get(response, 'nodes');

const results = _.chain(nodes)
.map('ingest.processors')
.reduce((result, processors) => {
return result.concat(processors);
})
.map('type')
.unique()
.value();

return results;
};
2 changes: 2 additions & 0 deletions src/plugins/kibana/server/routes/api/ingest/index.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { registerPost } from './register_post';
import { registerDelete } from './register_delete';
import { registerProcessors } from './register_processors';
import { registerSimulate } from './register_simulate';

export default function (server) {
registerPost(server);
registerDelete(server);
registerProcessors(server);
registerSimulate(server);
}
24 changes: 24 additions & 0 deletions src/plugins/kibana/server/routes/api/ingest/register_processors.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import _ from 'lodash';
import handleESError from '../../../lib/handle_es_error';
import handleResponse from '../../../lib/process_es_ingest_processors_response';
import { keysToCamelCaseShallow, keysToSnakeCaseShallow } from '../../../../common/lib/case_conversion';

export function registerProcessors(server) {
server.route({
path: '/api/kibana/ingest/processors',
Copy link
Contributor

Choose a reason for hiding this comment

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

We're establishing a bad pattern here with simulate and now this. This url could conflict with the ingest config endpoints (which will eventually support GET) if the user creates an index called processors.

We either need to come up with different urls for this endpoint and simulate, or we should move the existing ingest API endpoints to something like /api/kibana/ingest/config/{id}. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Per our discussion, I agree that we can resolve this potential conflict by putting those urls under the config category.

method: 'GET',
handler: function (request, reply) {
const boundCallWithRequest = _.partial(server.plugins.elasticsearch.callWithRequest, request);

return boundCallWithRequest('transport.request', {
path: '/_nodes/ingest',
method: 'GET'
})
.then(handleResponse)
.then(reply)
.catch((error) => {
reply(handleESError(error));
});
}
});
};
12 changes: 12 additions & 0 deletions src/ui/public/ingest/ingest.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,16 @@ export default function IngestProvider($rootScope, $http, config) {
});
};

this.getProcessors = function () {
function unpack(response) {
return response.data;
}

return $http.get(`${ingestAPIPrefix}/processors`)
.then(unpack)
.catch(err => {
throw ('Error communicating with Kibana server');
});
};

}
19 changes: 19 additions & 0 deletions test/unit/api/ingest/_processors.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
define(function (require) {
var Promise = require('bluebird');
var _ = require('intern/dojo/node!lodash');
var expect = require('intern/dojo/node!expect.js');

return function (bdd, scenarioManager, request) {
bdd.describe('processors', () => {

bdd.it('should return 200 for a successful run', function () {
return request.get('/kibana/ingest/processors')
.expect(200)
.then((response) => {
expect(_.isArray(response.body)).to.be(true);
});
});

});
};
});
4 changes: 3 additions & 1 deletion test/unit/api/ingest/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ define(function (require) {
var post = require('./_post');
var del = require('./_del');
var simulate = require('./_simulate');
var processors = require('./processors/index');
var processors = require('./_processors');
var processorTypes = require('./processors/index');

bdd.describe('ingest API', function () {
var scenarioManager = new ScenarioManager(url.format(serverConfig.servers.elasticsearch));
Expand All @@ -27,5 +28,6 @@ define(function (require) {
del(bdd, scenarioManager, request);
simulate(bdd, scenarioManager, request);
processors(bdd, scenarioManager, request);
processorTypes(bdd, scenarioManager, request);
});
});