expose OUTGOING_WEBHOOK_TIMEOUT as env var (#3801)
# What this PR does It adds functionality to be able to configure the outgoing webhook timeout from an environment variable. ## Which issue(s) this PR fixes Running into timeouts when outgoing webhooks take longer than 4 seconds (which is exceptional, but can happen) the webhook reports failure, while it still might have succeeded on the webhook side. ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --------- Co-authored-by: Joey Orlando <joey.orlando@grafana.com> Co-authored-by: Joey Orlando <joseph.t.orlando@gmail.com>
This commit is contained in:
parent
83f0997646
commit
4e3194c106
6 changed files with 13 additions and 18 deletions
|
|
@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
### Added
|
||||
|
||||
- Enable Grafana Alerting V2 feature flag by default
|
||||
- Allow configuration of outgoing webhook timeout via `OUTGOING_WEBHOOK_TIMEOUT` environment variable @kevindw-fluxys ([#3801](https://github.com/grafana/oncall/pull/3801))
|
||||
|
||||
## v1.3.98 (2024-02-01)
|
||||
|
||||
|
|
|
|||
|
|
@ -111,7 +111,8 @@ If no integrations are selected the outgoing webhook will trigger for any integr
|
|||
|
||||
The destination URL the outgoing webhook will make a request to. This must be a FQDN.
|
||||
|
||||
> ⚠️ **Note** the destination server must respond back within 4 seconds or it will result in a timeout
|
||||
> ⚠️ **Note** the destination server must respond back within 4 seconds (by default) or it will result in a timeout
|
||||
> For open source deployments, this timeout is configurable by setting the environment variable OUTGOING_WEBHOOK_TIMEOUT
|
||||
> (this can be seen in the "Response Body" under the "Last Run" section)
|
||||
|
||||
| Required | [Template Accepted](#outgoing-webhook-templates) | Default Value |
|
||||
|
|
|
|||
|
|
@ -14,7 +14,6 @@ from mirage import fields as mirage_fields
|
|||
from requests.auth import HTTPBasicAuth
|
||||
|
||||
from apps.webhooks.utils import (
|
||||
OUTGOING_WEBHOOK_TIMEOUT,
|
||||
InvalidWebhookData,
|
||||
InvalidWebhookHeaders,
|
||||
InvalidWebhookTrigger,
|
||||
|
|
@ -246,17 +245,17 @@ class Webhook(models.Model):
|
|||
|
||||
def make_request(self, url, request_kwargs):
|
||||
if self.http_method == "GET":
|
||||
r = requests.get(url, timeout=OUTGOING_WEBHOOK_TIMEOUT, **request_kwargs)
|
||||
r = requests.get(url, timeout=settings.OUTGOING_WEBHOOK_TIMEOUT, **request_kwargs)
|
||||
elif self.http_method == "POST":
|
||||
r = requests.post(url, timeout=OUTGOING_WEBHOOK_TIMEOUT, **request_kwargs)
|
||||
r = requests.post(url, timeout=settings.OUTGOING_WEBHOOK_TIMEOUT, **request_kwargs)
|
||||
elif self.http_method == "PUT":
|
||||
r = requests.put(url, timeout=OUTGOING_WEBHOOK_TIMEOUT, **request_kwargs)
|
||||
r = requests.put(url, timeout=settings.OUTGOING_WEBHOOK_TIMEOUT, **request_kwargs)
|
||||
elif self.http_method == "DELETE":
|
||||
r = requests.delete(url, timeout=OUTGOING_WEBHOOK_TIMEOUT, **request_kwargs)
|
||||
r = requests.delete(url, timeout=settings.OUTGOING_WEBHOOK_TIMEOUT, **request_kwargs)
|
||||
elif self.http_method == "OPTIONS":
|
||||
r = requests.options(url, timeout=OUTGOING_WEBHOOK_TIMEOUT, **request_kwargs)
|
||||
r = requests.options(url, timeout=settings.OUTGOING_WEBHOOK_TIMEOUT, **request_kwargs)
|
||||
elif self.http_method == "PATCH":
|
||||
r = requests.patch(url, timeout=OUTGOING_WEBHOOK_TIMEOUT, **request_kwargs)
|
||||
r = requests.patch(url, timeout=settings.OUTGOING_WEBHOOK_TIMEOUT, **request_kwargs)
|
||||
else:
|
||||
raise ValueError(f"Unsupported http method: {self.http_method}")
|
||||
return r
|
||||
|
|
|
|||
|
|
@ -1,16 +1,11 @@
|
|||
from unittest.mock import call, patch
|
||||
|
||||
import pytest
|
||||
from django.conf import settings
|
||||
from requests.auth import HTTPBasicAuth
|
||||
|
||||
from apps.webhooks.models import Webhook
|
||||
from apps.webhooks.utils import (
|
||||
OUTGOING_WEBHOOK_TIMEOUT,
|
||||
InvalidWebhookData,
|
||||
InvalidWebhookHeaders,
|
||||
InvalidWebhookTrigger,
|
||||
InvalidWebhookUrl,
|
||||
)
|
||||
from apps.webhooks.utils import InvalidWebhookData, InvalidWebhookHeaders, InvalidWebhookTrigger, InvalidWebhookUrl
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
|
|
@ -236,7 +231,7 @@ def test_make_request(make_organization, make_custom_webhook):
|
|||
webhook.make_request("url", {"foo": "bar"})
|
||||
expected_call = getattr(mock_requests, method.lower())
|
||||
assert expected_call.called
|
||||
assert expected_call.call_args == call("url", timeout=OUTGOING_WEBHOOK_TIMEOUT, foo="bar")
|
||||
assert expected_call.call_args == call("url", timeout=settings.OUTGOING_WEBHOOK_TIMEOUT, foo="bar")
|
||||
|
||||
# invalid
|
||||
webhook = make_custom_webhook(organization=organization, http_method="NOT")
|
||||
|
|
|
|||
|
|
@ -11,8 +11,6 @@ from apps.labels.utils import get_alert_group_labels_dict, get_labels_dict, is_l
|
|||
from apps.schedules.ical_utils import list_users_to_notify_from_ical
|
||||
from common.jinja_templater import apply_jinja_template
|
||||
|
||||
OUTGOING_WEBHOOK_TIMEOUT = 4
|
||||
|
||||
|
||||
class InvalidWebhookUrl(Exception):
|
||||
def __init__(self, message):
|
||||
|
|
|
|||
|
|
@ -91,6 +91,7 @@ GRAFANA_CLOUD_ONCALL_TOKEN = os.environ.get("GRAFANA_CLOUD_ONCALL_TOKEN", None)
|
|||
|
||||
# Outgoing webhook settings
|
||||
DANGEROUS_WEBHOOKS_ENABLED = getenv_boolean("DANGEROUS_WEBHOOKS_ENABLED", default=False)
|
||||
OUTGOING_WEBHOOK_TIMEOUT = getenv_integer("OUTGOING_WEBHOOK_TIMEOUT", default=4)
|
||||
WEBHOOK_RESPONSE_LIMIT = 50000
|
||||
|
||||
# Multiregion settings
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue