Skip to content
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

Add support of Graylog GELF Alerter #1050

Merged
merged 16 commits into from
Jan 3, 2023
Merged

Conversation

malinkinsa
Copy link
Contributor

@malinkinsa malinkinsa commented Dec 12, 2022

Description

Checklist

  • I have reviewed the contributing guidelines.
  • I have included unit tests for my changes or additions.
  • I have successfully run make test-docker with my changes.
  • I have manually tested all relevant modes of the change in this PR.
  • I have updated the documentation.
  • I have updated the changelog.

@jertel
Copy link
Owner

jertel commented Dec 17, 2022

Thanks for the submission. I recommend looking at the tests/alerters/ directory, as you'll probably find a template there to use for adding unit test coverage.

@malinkinsa
Copy link
Contributor Author

Yep, till the end of the week will do it

Minor fixes in GelfAlerter
Add tests
@malinkinsa
Copy link
Contributor Author

@jertel
Done

@jertel
Copy link
Owner

jertel commented Jan 2, 2023

Thanks for adding the unit tests. Will you please also update the schema.yaml file, with the expected parameter data types? Search for "discord" or "alerta" for examples of what is needed.

@malinkinsa
Copy link
Contributor Author

malinkinsa commented Jan 2, 2023

update the schema.yaml file

Ready.

@nsano-rururu many thanks for your edits, with them it is better.

nsano-rururu
nsano-rururu previously approved these changes Jan 2, 2023
self.additional_headers = self.rule.get('gelf_http_headers')
self.ca_cert = self.rule.get('gelf_ca_cert')
self.http_ignore_ssl_errors = self.rule.get('http_ignore_ssl_errors', False)
self.timeout = self.rule.get('timeout', 30)
Copy link
Collaborator

Choose a reason for hiding this comment

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

About settings Won't you add validation settings that are referenced when rules are executed? If the data type and input value variations are fixed, add settings.
https://github.com/jertel/elastalert2/blob/master/elastalert/schema.yaml

example

gelf_ignore_ssl_errors: {type: boolean}
gelf_type: {type: string, enum: ['http', 'tcp']}
gelf_version: {type: string}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is gelf_log_level a number? Have you decided on a range? If the minimum and maximum are fixed, we recommend adding the following settings to schema.yaml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

About gelf_version - i didn't want to add a specified version list to schemas. The latest version is 1.1 and it is default now and this version was released in 2013.

About gelf_log_level - good point. I'm adding a range now.

@jertel jertel merged commit 3631473 into jertel:master Jan 3, 2023
@malinkinsa malinkinsa deleted the gelf_http branch January 15, 2023 15:37
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants