Reduce dependencies in Crawler #17653
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The crawler creates active inputs for static configuration, starts config file reloading, and starts the module loader. With this change the crawler has no direct dependency (well, reduced) on input.Input anymore, but will use the `Runner` interface, even for statically configured inputs. This also reduces dependencies, as most plumbing is already done by the inputs.RunnerFactory and must not be duplicated by the crawler anymore. The input.Runner used to compute a 'ID' by hashing the inputs configuration. The ID was public, to be used by the crawler only. Instead of having the input compute the ID, it is the crawler who will compute the input ID now. Note: the cfgfile RunnerList maintains its own set of IDs. The crawler and RunnerList each used to use the ID to check for 'duplicate' configurations, but because the IDs are not 'shred' duplication detection is not across the Beat. ID detection is actively used by input config file reloading and auto discvovery only, in order to check if an input still needs to be running, are shall shut down.
urso
added
review
refactoring
Filebeat
|
Pinging @elastic/integrations-services (Team:Services) |
urso
commented
Apr 9, 2020
| return fmt.Errorf("Error while initializing input: %+v", err) | ||
| } | ||
| if inputRunner, ok := runner.(*input.Runner); ok { | ||
| inputRunner.Once = c.once |
There was a problem hiding this comment.
input.New returns a partially initialized type. The once flag must only be set for static inputs, but the factory is used for static and dynamic inputs. As I expect the crawler to go away in the future I wanted keep the scope rather small for what I need for the integration of the new input API, I still keep this small piece of tech debt here for now.
3 tasks
kvch
approved these changes
Apr 14, 2020
1 task
urso
added
v7.8.0
and removed
needs_backport
urso
pushed a commit
to urso/beats
that referenced
this pull request
Apr 15, 2020
* Reduce dependencies in Crawler The crawler creates active inputs for static configuration, starts config file reloading, and starts the module loader. With this change the crawler has no direct dependency (well, reduced) on input.Input anymore, but will use the `Runner` interface, even for statically configured inputs. This also reduces dependencies, as most plumbing is already done by the inputs.RunnerFactory and must not be duplicated by the crawler anymore. The input.Runner used to compute a 'ID' by hashing the inputs configuration. The ID was public, to be used by the crawler only. Instead of having the input compute the ID, it is the crawler who will compute the input ID now. Note: the cfgfile RunnerList maintains its own set of IDs. The crawler and RunnerList each used to use the ID to check for 'duplicate' configurations, but because the IDs are not 'shred' duplication detection is not across the Beat. ID detection is actively used by input config file reloading and auto discvovery only, in order to check if an input still needs to be running, are shall shut down. * fix import formatting in crawler * Update log message test looks for (cherry picked from commit 52fa265)
urso
pushed a commit
that referenced
this pull request
Apr 23, 2020
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
The crawler creates active inputs for static configuration, starts
config file reloading, and starts the module loader.
With this change the crawler has no direct dependency (well, reduced) on
input.Input anymore, but will use the
Runnerinterface, even forstatically configured inputs.
This also reduces dependencies, as most plumbing is already done by the
inputs.RunnerFactory and must not be duplicated by the crawler anymore.
The input.Runner used to compute a 'ID' by hashing the inputs
configuration. The ID was public, to be used by the crawler only.
Instead of having the input compute the ID, it is the crawler who will
compute the input ID now.
While reducing dependencies, I replaced
logp.Infocalls with an internallogp.Loggerinstance for logging purposes.Note: the cfgfile RunnerList maintains its own set of IDs. The crawler
and RunnerList each used to use the ID to check for 'duplicate'
configurations, but because the IDs are not 'shred' duplication
detection is not across the Beat.
ID detection is actively used by input config file reloading and auto
discvovery only, in order to check if an input still needs to be
running, are shall shut down.
Why is it important?
Reduce dependencies and responsibilities in the crawler, while centering input configuration and running more around the RunnerFactory and Runner interfaces only. This changes makes it easier to integrate with alternative RunnerFactory implementations in the future.
Checklist
- [ ] I have commented my code, particularly in hard-to-understand areas- [ ] I have made corresponding changes to the documentation- [ ] I have made corresponding change to the default configuration files- [ ] I have added tests that prove my fix is effective or that my feature works- [ ] I have added an entry inCHANGELOG.next.asciidocorCHANGELOG-developer.next.asciidoc.