-
Notifications
You must be signed in to change notification settings - Fork 21
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
Unify Sync #578
Merged
Merged
Unify Sync #578
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
Rough pass on the SyncSchedular, it handles all the timing of when a sync happens. Only ElastAlert is using it at this time. Updated existing tests with consistent variable naming, added forgotten calls to ctrl.Finish, and to reflect changes to ElastAlertEngine's struct. Added missing copyright comments to a few files.
SyncScheduler now builds the logger for the sync process ensuring that every sync has an Id and the engine name. The scheduler now also times the duration of syncs (including the tailing integrity check). Refactored Strelka to use the SyncScheduler. Added logging to elastalert that mirrors strelka and vice versa. Now both engines log every rule they process and report added/updated/unchanged/deleted.
659e3d0
to
a1742c8
Compare
Test coverage went from 56.5% to 76.2% for the elastalert package.
a1742c8
to
731f97d
Compare
Test coverage went from 59.0% to 78.8%
d7fb725
to
96bc476
Compare
Test coverage went from 73.7% to 78.1% Updated an elastalert test to better test sigmaToElastAlert with overrides. Updated a strelka test to use forcesync.
IntegrityCheck now supports adaptive logging. If a logger is passed in the logger gets a new field (intCheckId) and is used. If a nil logger is passed in a new one is built with default fields. This allows IntegrityChecks after a Sync to have the syncId AND an intCheckId. The IntegrityCheck report log has been condensed. The two lists of IDs enabledButNotDeployed and deployedButNotEnabled are now reported on the same log line as the pass/fail text. Failures are logged as warnings now.
TOoSmOotH
approved these changes
Jul 5, 2024
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Refactor sync to eliminate duplicate code and processes shared between the engines. The timing of when syncs happen are all managed the same way and have been simplified with the new SyncScheduler. This also introduces syncIds to the log statements so every individual sync can have it's logs correlated. Logging between the engines has also been made more consistent.