Skip to content

packetbeat application layer analyzers -> pluggable modules#1101

Merged
ruflin merged 2 commits intoelastic:masterfrom
urso:enh/config-protos
Mar 4, 2016
Merged

packetbeat application layer analyzers -> pluggable modules#1101
ruflin merged 2 commits intoelastic:masterfrom
urso:enh/config-protos

Conversation

@urso
Copy link
Copy Markdown

@urso urso commented Mar 3, 2016

No description provided.

@urso urso added enhancement in progress Pull request is currently in progress. Packetbeat labels Mar 3, 2016
@urso urso force-pushed the enh/config-protos branch from 15a4306 to 6f11b8c Compare March 3, 2016 16:43
@urso urso added review and removed in progress Pull request is currently in progress. labels Mar 3, 2016
@monicasarbu
Copy link
Copy Markdown
Contributor

Great work! LGTM.

"github.com/elastic/beats/packetbeat/beater"

// import support protocol modules
_ "github.com/elastic/beats/packetbeat/protos/amqp"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should automate this in the future. Also on my list for metricbeat, so perhaps we will be able to use the same script.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

not sure I'd want to full automate this. nice thing about having imports in main.go only is, one can import a very basic packetbeat as is, without any modules (for example to have a flowsbeat connection only flow information).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

By automate I mean the generation of this list based on the directories that exist. In metricbeat I put the list in a different package: https://github.com/elastic/beats/tree/master/metricbeat/include

The advantage of this if someone creates a beat with only one module he can either include all other modules and metricsets he wants one by one or he can just link to `_ "github.com/elastic/beats/metricbeat/include" and gets all the official modules of metricbeat without having to update it all the time.

@ruflin
Copy link
Copy Markdown
Contributor

ruflin commented Mar 3, 2016

This change feels really good. It is quite large and which makes it hard to follow as lots of things moved around. I focused more on if the tests stayed the same as there were much fewer changes on the test side. It LGTM and I suggest to merge this one as soon as the builds go green.

@urso
Copy link
Copy Markdown
Author

urso commented Mar 3, 2016

Yeah, was a little surprised about this of PR, but I have had to do all modules at once.

For review concentrate on packetbeat/protos/protos.go and registry.go + maybe one module (e.g. http) initialization. Updates to all modules are basically similar:

  • create New instead of init
  • load config into copy of defaultConfig
  • call init (+simplify setFromConfig)

ruflin added a commit that referenced this pull request Mar 4, 2016
packetbeat application layer analyzers -> pluggable modules
@ruflin ruflin merged commit 34cbe9d into elastic:master Mar 4, 2016
This was referenced Mar 4, 2016
@urso urso deleted the enh/config-protos branch April 27, 2016 12:23
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