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

feat(notify): use logrus to capture errors #365

Merged
merged 1 commit into from
Oct 17, 2022

Conversation

orandin
Copy link
Member

@orandin orandin commented Oct 5, 2022

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Improvement: use logrus to capture errors from Send Notify to standardize the errors logs.

  • What is the current behavior? (You can also link to an open issue here)

The error logs aren't standardized.

  • What is the new behavior (if this is a feature change)?

All notify backend use the same function to capture an error and the error logs are standardized.
Also, some fields have been added to simplify the debug.

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

The format of error logs has changed to be standardized with the rest of the application. In case where an alerting system that parses the logs is used, the users need to change their configuration/rules.

  • Other information:

An example of notify_config to test the new behaviour:

"notify_config":{
   "test":{
      "type":"webhook",
      "config":{
         "webhook_url":"http://localhost:7777"
      }
   }
}
# Before
2022/10/05 21:31:43 Post "http://localhost:7777": dial tcp [::1]:7777: connect: connection refused

# After
ERRO[2022-10-05T21:26:22+02:00] Error while sending notification on webhook   error="Post \"http://localhost:7777\": dial tcp [::1]:7777: connect: connection refused" instance_id=8 notification_type=task_state_update notifier_name=test notify_backend=webhook task_id=8f971130-cadf-498d-ac3c-50c799e97d50

rbeuque74
rbeuque74 previously approved these changes Oct 14, 2022
@ovh-cds
Copy link
Collaborator

ovh-cds commented Oct 14, 2022

CDS Report test#1443.0 ✘

  • tests and lint
    • golangci-lint ✘
    • unit-test and integration-tests ✔

@rbeuque74
Copy link
Member

Build is failing on golangci-lint, feel free to run it on your side to find out the issue ;)

Signed-off-by: Simon Delberghe <open-source@orandin.fr>
@orandin
Copy link
Member Author

orandin commented Oct 15, 2022

Now, it should be good! 😉

@rbeuque74
Copy link
Member

Will re-run the CI on monday!

@rbeuque74 rbeuque74 merged commit 82f2ab5 into ovh:master Oct 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants