-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[Metricbeat] Support processors defined for light modules #15923
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mtojek for working on this!
I have added some comments, the main issues I see:
- Processors defined in the manifest should be executed before processors defined in the configuration. This will be important to know what to expect from the metricset itself, and what fields are available for processors in the configuration.
- The link between the metricsets and the connectors, based on a map with the metricset name, and the content of a field in the event, looks a bit weak to me. It'd be nice if we could find a more explicit way of connecting the metricsets with the runners.
Have you considered to create one wrapper per metricset when needed? Not sure about the implications. But this would allow to create also a runner per metricset without modifying the current runner. For these cases Factory.Create
could return a runner group struct that implements the Runner
interface and contains all the runners generated for a configuration.
Or maybe we need to have multiple outputs in each wrapper, and attach them to each one of its specific clients.
I fixed all your concerns and got rid of map of clients. A runner group combining all wrappers has been applied. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can continue with this approach, good job!
1c3d68a
to
3185a0c
Compare
... and it passed. I will proceed with merging the PR. |
) * Adjust test cases first * Setup processors for light modules * Fix: comments, mage check * Adjust code * Fix: missing comment * Fix: mage check * Test light modules * Test connector * Test runner * Fix: ToLower * Adjust code after review * Fix: mage check * Adjust code after review * Adjust code after review * Fix: check error * Fix: imports * Increase test coverage * Add unit tests * Fix: hound * beater: use factory * Beater: modules * Fix: system tests * Fix: implements interface * Add changelog entry * Verify if processors are setup (cherry picked from commit a70d6e8)
…15992) * Adjust test cases first * Setup processors for light modules * Fix: comments, mage check * Adjust code * Fix: missing comment * Fix: mage check * Test light modules * Test connector * Test runner * Fix: ToLower * Adjust code after review * Fix: mage check * Adjust code after review * Adjust code after review * Fix: check error * Fix: imports * Increase test coverage * Add unit tests * Fix: hound * beater: use factory * Beater: modules * Fix: system tests * Fix: implements interface * Add changelog entry * Verify if processors are setup (cherry picked from commit a70d6e8)
Pinging @elastic/integrations (Team:Integrations) |
Since metricbeat light modules support processors (elastic#15923), module initialization requires a publisher in the beat so modules can attach their processors. `metricbeat test modules` is not initializing as normal metricbeat commands, and it is not initializing any output or publisher pipeline, so metricbeat panics when trying to initialize modules with the new method. This change adds a dummy publisher for this case, and fixes also a condition that was adding a `nil` module option, causing additional panics. A test that reproduced the issue is also added.
Since metricbeat light modules support processors (#15923), module initialization requires a publisher in the beat so modules can attach their processors. `metricbeat test modules` is not initializing as normal metricbeat commands, and it is not initializing any output or publisher pipeline, so metricbeat panics when trying to initialize modules with the new method. This change adds a dummy publisher for this case, and fixes also a condition that was adding a `nil` module option, causing additional panics. A test that reproduced the issues is also added.
Since metricbeat light modules support processors (elastic#15923), module initialization requires a publisher in the beat so modules can attach their processors. `metricbeat test modules` is not initializing as normal metricbeat commands, and it is not initializing any output or publisher pipeline, so metricbeat panics when trying to initialize modules with the new method. This change adds a dummy publisher for this case, and fixes also a condition that was adding a `nil` module option, causing additional panics. A test that reproduced the issues is also added. (cherry picked from commit 25b8bf1)
Since metricbeat light modules support processors (elastic#15923), module initialization requires a publisher in the beat so modules can attach their processors. `metricbeat test modules` is not initializing as normal metricbeat commands, and it is not initializing any output or publisher pipeline, so metricbeat panics when trying to initialize modules with the new method. This change adds a dummy publisher for this case, and fixes also a condition that was adding a `nil` module option, causing additional panics. A test that reproduced the issues is also added. (cherry picked from commit 25b8bf1)
Since metricbeat light modules support processors (elastic#15923), module initialization requires a publisher in the beat so modules can attach their processors. `metricbeat test modules` is not initializing as normal metricbeat commands, and it is not initializing any output or publisher pipeline, so metricbeat panics when trying to initialize modules with the new method. This change adds a dummy publisher for this case, and fixes also a condition that was adding a `nil` module option, causing additional panics. A test that reproduced the issues is also added. (cherry picked from commit 25b8bf1)
Since metricbeat light modules support processors (#15923), module initialization requires a publisher in the beat so modules can attach their processors. `metricbeat test modules` is not initializing as normal metricbeat commands, and it is not initializing any output or publisher pipeline, so metricbeat panics when trying to initialize modules with the new method. This change adds a dummy publisher for this case, and fixes also a condition that was adding a `nil` module option, causing additional panics. A test that reproduced the issues is also added. (cherry picked from commit 25b8bf1)
Since metricbeat light modules support processors (#15923), module initialization requires a publisher in the beat so modules can attach their processors. `metricbeat test modules` is not initializing as normal metricbeat commands, and it is not initializing any output or publisher pipeline, so metricbeat panics when trying to initialize modules with the new method. This change adds a dummy publisher for this case, and fixes also a condition that was adding a `nil` module option, causing additional panics. A test that reproduced the issues is also added. (cherry picked from commit 25b8bf1)
…8853) Since metricbeat light modules support processors (#15923), module initialization requires a publisher in the beat so modules can attach their processors. `metricbeat test modules` is not initializing as normal metricbeat commands, and it is not initializing any output or publisher pipeline, so metricbeat panics when trying to initialize modules with the new method. This change adds a dummy publisher for this case, and fixes also a condition that was adding a `nil` module option, causing additional panics. A test that reproduced the issues is also added. (cherry picked from commit 25b8bf1) Add nil pipeline from #16715, required for the fix. (cherry picked from commit 7a1b524) Co-authored-by: Steffen Siering <[email protected]>
…es` (elastic#18853) Since metricbeat light modules support processors (elastic#15923), module initialization requires a publisher in the beat so modules can attach their processors. `metricbeat test modules` is not initializing as normal metricbeat commands, and it is not initializing any output or publisher pipeline, so metricbeat panics when trying to initialize modules with the new method. This change adds a dummy publisher for this case, and fixes also a condition that was adding a `nil` module option, causing additional panics. A test that reproduced the issues is also added. (cherry picked from commit 316793d) Add nil pipeline from elastic#16715, required for the fix. (cherry picked from commit 257fdfc) Co-authored-by: Steffen Siering <[email protected]>
This PR adds processor support for light modules. Processors can be enabled using definitions in
manifest.yml
.Issue: #14740
Let me start with draft and if we're fine with design and implementation, I will cover it with tests.
How to test this PR
(Remove
metrics_filters
)docker-compose up
to boot up IBM MQ module. The portlocalhost:9157
should be exposed.ibmmq
metricbeat (x-pack) module.metricbeat
with-e -d processors
and observe events flow.Filtering should be applied. To make sure that the processor is used, make a mistake in the configuration. It should appear in the processor's debug logs.