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

Migrate processor to the new global registry #7396

Closed

Conversation

ph
Copy link
Contributor

@ph ph commented Jun 21, 2018

depends on #7392

@ph ph added in progress Pull request is currently in progress. libbeat labels Jun 21, 2018
})
}

type Constructor func(config *common.Config) (Processor, error)

Choose a reason for hiding this comment

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

exported type Constructor should have comment or be unexported

constr Constructor
}

func Plugin(name string, c Constructor) map[string][]interface{} {

Choose a reason for hiding this comment

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

exported function Plugin should have comment or be unexported

)

// Feature exposes add_host_metadata.
var Feature = processors.Feature(processorName, newHostMetadataProcessor, feature.Stable)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this variable here used for something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes this is used by the global registry, see comment #7392 (comment) for a bit more details about the usage.

@ph ph force-pushed the feature/processor-new-plugin branch from 9e1eb99 to 4d20a63 Compare June 27, 2018 19:48
@ph ph changed the base branch from master to feature/move-to-new-registry June 27, 2018 19:49
@ph ph changed the title [WIP] Migrate processor to the new global registry Migrate processor to the new global registry Jun 27, 2018
@ph ph force-pushed the feature/processor-new-plugin branch from 4d20a63 to 054fbdf Compare June 27, 2018 19:50
@ph ph added in progress Pull request is currently in progress. and removed in progress Pull request is currently in progress. review labels Jun 27, 2018
@urso
Copy link

urso commented Jun 29, 2018

A little sad the Namespaces are gone from processors. Namespaces allow plugins to be further namespaced (used in x-exec branch). E.g. have common namespaces for common plugin types.

The way it was used:

processors:
- lookup.exec:
    ...

- lookup.file:
    ...

The Namespace ensured lookup is a namespaced feature/plugin with it's own set of plugins. I wonder if we want to re-introduce this functionality with outlook of turning metricbeat/filebeat modules into features as well.

@ph
Copy link
Contributor Author

ph commented Jun 29, 2018

@urso I think this can still be done with a flat structure and handled at the processor level instead.
If I look at your comments the "lookup" processor will have 2 plugins: file and exec.

in a flat view the namespace will be "libbeat.processor.lookup", so when we create the processor "lookup", he can create the exec or file. We cannot assume that exec or file actually implements the processor interface it could be something else, like in the add_kubernetes_metadata processor.

But, we can provide code to wrap a nested processor.

@ph
Copy link
Contributor Author

ph commented Jun 29, 2018

Another way to look at it, is `lookup' is a bundle of two features file and exec.

@ph ph force-pushed the feature/processor-new-plugin branch from 054fbdf to e4fd51f Compare July 27, 2018 18:03
@ph ph added review and removed in progress Pull request is currently in progress. labels Jul 27, 2018
@ph
Copy link
Contributor Author

ph commented Jul 27, 2018

@urso I've moved the code to use feature.NewDetails and rebased the code, all the "actions" processors expose a Bundle that we can register in the include package. All the other processors expose a unique feature.

This is ready for review and be merged into the feature branch.

@ph ph closed this Jul 27, 2018
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