-
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
Input v2 compatibility layer #19401
Input v2 compatibility layer #19401
Conversation
Pinging @elastic/integrations-services (Team:Services) |
💚 Build SucceededExpand to view the summary
Build stats
Test stats 🧪
Steps errorsExpand to view the steps failures
|
jenkins run the tests please |
filebeat/input/v2/compat/compat.go
Outdated
log.Infof("Input %v starting", name) | ||
err := r.input.Run( | ||
v2.Context{ | ||
ID: "", // TODO: hmmm.... |
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.
Are you going to resolve this TODO in an upcoming PR?
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.
Answer: hmmm....
😄
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.
There is no common way of how IDs are defined/generated. Let me update this by computing a hash from the configuration object (some parts in libbeat do so).
The cursor based input manager allows you to configure an id
setting per input that allows you to have 2 similar inputs with different ID. I think I will move the setting into this layer (will update this PR), so it can be applied to each input (ID will be added to structured logs + can be set by agent in the future).
filebeat/input/v2/compat/composed.go
Outdated
fallback cfgfile.RunnerFactory | ||
} | ||
|
||
var _ cfgfile.RunnerFactory = composeFactory{} |
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.
What is this for? At least it does not clearly come to my mind so it may require a short comment to clarify.
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.
Ah, this can be removed. I used to export the factory, but added the constructor that returns the expected interface type later. When implementing and exporting a concrete type you still might want the compiler to check that this concrete type implements the supposed interface correctly. Common pattern to do so is:
var _ <interface> = <struct type>{}
or (for interfaces on pointers):
var _ <interface> = (*<struct type>)(nil)
This declares an unused variable, but still triggers the type checker.
An example in the stdlib coming to mind is json.RawMessage
.
The idea is to create the compiler error for the author modifying the type, and not the person using the type.
// signal that the input type is unknown via v2.ErrUnknown. | ||
// | ||
// XXX: This RunnerFactory is used for compining the v2.Loader with the | ||
// existing RunnerFactory for inputs in Filebeat. The Combine function should be removed once the old RunnerFactory is removed. |
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.
Nit: XXX? Also typo in compining
💚 Build SucceededExpand to view the summary
Build stats
Test stats 🧪
Steps errorsExpand to view the steps failures
|
jenkins run the tests please |
This change introduces the input/v2/compat package, that provides a compatibility layer for integration the input v2 API into existing beats functionality. The package provides a function for wrapping v2 inputs into a cfgfile.RunnerFactory (used by autodiscovery, filebet module loading, config file reloading, ....).
74ee468
to
54698d6
Compare
…ne-beats * upstream/master: (105 commits) ci: enable packaging job (elastic#19536) ci: disable upstream trigger on PRs for the packaging job (elastic#19490) Implement memlog on-disk handling (elastic#19408) fix go.mod for PR elastic#19423 (elastic#19521) [MetricBeat] add param `aws_partition` to support aws-cn, aws-us-gov regions (elastic#19423) Input v2 stateless manager (elastic#19406) Input v2 compatibility layer (elastic#19401) [Elastic Agent] Fix artifact downloading to allow endpoint-security to be downloaded (elastic#19503) fix: ignore target changes on scans (elastic#19510) Add more helpers to pipeline/testing package (elastic#19405) Report dependencies in CSV format (elastic#19506) [Filebeat] Fix reference leak in TCP and Unix socket inputs (elastic#19459) Cursor input skeleton (elastic#19378) Add changelog. (elastic#19495) [DOC] Typo in Kerberos (elastic#19265) Remove accidentally commited unused NOTICE template (elastic#19485) [Elastic Agent] Support the install, control, and uninstall of Endpoint (elastic#19248) [Filebeat][httpjson] Add split_events_by config setting (elastic#19246) ci: disabling packaging job until we fix it (elastic#19481) Fix golang.org/x/tools to release1.13 (elastic#19478) ...
(cherry picked from commit c9a9bf5)
(cherry picked from commit c9a9bf5)
What does this PR do?
This change introduces the input/v2/compat package, that provides a
compatibility layer for integration the input v2 API into existing beats
functionality.
The package provides a function for wrapping v2 inputs into a
cfgfile.RunnerFactory (used by autodiscovery, filebet module loading,
config file reloading, ....).
The current state of the full implementation can be seen here and sample inputs based on the new API.
The full list of changes will include:
Test coverage report:
Why is it important?
This PR only introduces the compatiblity layer which will allow use to use new input with existing Beats functionality, but also run new and old architecture in parallel.
Checklist
- [ ] I have made corresponding changes to the documentation- [ ] I have made corresponding change to the default configuration files- [ ] I have added an entry inCHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Related issues