From 434fe17617ef96a6bee98b4e338920070348c37d Mon Sep 17 00:00:00 2001 From: Innokentii Konstantinov Date: Thu, 25 Aug 2022 13:34:19 +0500 Subject: [PATCH 1/5] user_id -> linked_user_id (#410) --- engine/apps/api/views/user.py | 8 ++++---- engine/apps/social_auth/pipeline.py | 4 ++-- engine/apps/telegram/models/verification/personal.py | 4 ++-- engine/common/insight_log/chatops_insight_logs.py | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/engine/apps/api/views/user.py b/engine/apps/api/views/user.py index ed3f633b..7911acd2 100644 --- a/engine/apps/api/views/user.py +++ b/engine/apps/api/views/user.py @@ -361,8 +361,8 @@ class UserView( author=request.user, event_name=ChatOpsEvent.USER_UNLINKED, chatops_type=ChatOpsType.TELEGRAM, - user=user.username, - user_id=user.public_primary_key, + linked_user=user.username, + linked_user_id=user.public_primary_key, ) except TelegramToUserConnector.DoesNotExist: return Response(status=status.HTTP_400_BAD_REQUEST) @@ -383,8 +383,8 @@ class UserView( author=request.user, event_name=ChatOpsEvent.USER_UNLINKED, chatops_type=backend.backend_id, - user=user.username, - user_id=user.public_primary_key, + linked_user=user.username, + linked_user_id=user.public_primary_key, ) except ObjectDoesNotExist: return Response(status=status.HTTP_400_BAD_REQUEST) diff --git a/engine/apps/social_auth/pipeline.py b/engine/apps/social_auth/pipeline.py index 1babe21d..a271d056 100644 --- a/engine/apps/social_auth/pipeline.py +++ b/engine/apps/social_auth/pipeline.py @@ -72,8 +72,8 @@ def connect_user_to_slack(response, backend, strategy, user, organization, *args author=user, event_name=ChatOpsEvent.USER_LINKED, chatops_type=ChatOpsType.SLACK, - user=user.username, - user_id=user.public_primary_key, + linked_user=user.username, + linked_user_id=user.public_primary_key, ) user.slack_user_identity = slack_user_identity user.save(update_fields=["slack_user_identity"]) diff --git a/engine/apps/telegram/models/verification/personal.py b/engine/apps/telegram/models/verification/personal.py index 9dc49cbc..43823312 100644 --- a/engine/apps/telegram/models/verification/personal.py +++ b/engine/apps/telegram/models/verification/personal.py @@ -36,8 +36,8 @@ class TelegramVerificationCode(models.Model): author=user, event_name=ChatOpsEvent.USER_LINKED, chatops_type=ChatOpsType.TELEGRAM, - user=user.username, - user_id=user.public_primary_key, + linked_user=user.username, + linked_user_id=user.public_primary_key, ) return connector, created diff --git a/engine/common/insight_log/chatops_insight_logs.py b/engine/common/insight_log/chatops_insight_logs.py index aae90ed3..5999b655 100644 --- a/engine/common/insight_log/chatops_insight_logs.py +++ b/engine/common/insight_log/chatops_insight_logs.py @@ -36,7 +36,7 @@ def write_chatops_insight_log(author, event_name: ChatOpsEvent, chatops_type: Ch user_id = author.public_primary_key username = json.dumps(author.username) - log_line = f'tenant_id={tenant_id} author_id={user_id} author={username} action_type="chat_ops" action_name={event_name.value} chat_ops_type={chatops_type.value}' # noqa + log_line = f"tenant_id={tenant_id} author_id={user_id} author={username} action_type=chat_ops action_name={event_name.value} chat_ops_type={chatops_type.value}" # noqa for k, v in kwargs.items(): log_line += f" {k}={json.dumps(v)}" From 8428848844ea530efe6a44c02a6f669bde99cbe8 Mon Sep 17 00:00:00 2001 From: Vadim Stepanov Date: Thu, 25 Aug 2022 14:02:32 +0100 Subject: [PATCH 2/5] Telegram docs/instructions update (#406) * update Telegram Setup section for open-source.md * update channel connection instruction * update Telegram configuration docs --- .../chat-options/configure-telegram.md | 26 ++++++++++--------- docs/sources/open-source.md | 12 ++++----- .../TelegramIntegrationButton.tsx | 8 ++++-- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/docs/sources/chat-options/configure-telegram.md b/docs/sources/chat-options/configure-telegram.md index e119aff4..daef6ab6 100644 --- a/docs/sources/chat-options/configure-telegram.md +++ b/docs/sources/chat-options/configure-telegram.md @@ -16,15 +16,22 @@ weight: 300 # Telegram integration for Grafana OnCall -You can use Telegram to deliver alert group notifications to a dedicated channel, and allow users to perform notification actions. +You can manage alerts either directly in your personal Telegram DMs or in a dedicated team channel. -Each alert group notification is assigned a dedicated discussion. Users can perform notification actions (acknowledge, resolve, silence), create reports, and discuss alerts in the comments section of the discussions. +## Configure Telegram user settings in Grafana OnCall -In case an integration route is not configured to use a Telegram channel, users will receive messages with alert group contents, logs and actions in their DMs. +To receive alert group contents, escalation logs and to be able to perform actions (acknowledge, resolve, silence) in Telegram DMs, please refer to the following steps: -## Connect to Telegram +1. In your profile, find the Telegram setting and click **Connect**. +1. Click **Connect automatically** for the bot to message you and to bring up your telegram account. +1. Click **Start** when the OnCall bot messages you and wait for the connection confirmation. +1. Done! Now you can receive alerts directly to your Telegram DMs. -Connect your organization's Telegram account to your Grafana OnCall instance by following the instructions provided in OnCall. You can use the following steps as a reference. +If you want to connect manually, you can click the URL provided and then **SEND MESSAGE**. In your Telegram account, click **Start**. + +## (Optional) Connect to a Telegram channel + +In case you want to manage alerts in a dedicated Telegram channel, please use the following steps as a reference. > **NOTE:** Only Grafana users with the administrator role can configure OnCall settings. @@ -42,10 +49,5 @@ Connect your organization's Telegram account to your Grafana OnCall instance by 1. In OnCall, send the provided verification code to the channel. 1. Make sure users connect to Telegram in their OnCall user profile. -## Configure Telegram user settings in OnCall - -1. In your profile, find the Telegram setting and click **Connect**. -1. Click **Connect automatically** for the bot to message you and to bring up your telegram account. -1. Click **Start** when the OnCall bot messages you. - -If you want to connect manually, you can click the URL provided and then **SEND MESSAGE**. In your Telegram account, click **Start**. +Each alert group is assigned a dedicated discussion. Users can perform actions (acknowledge, resolve, silence), and discuss alerts in the comments section of the discussions. +In case an integration route is not configured to use a Telegram channel, users will receive messages with alert group contents, logs and actions in their DMs. diff --git a/docs/sources/open-source.md b/docs/sources/open-source.md index fb02b50f..b2e6c1df 100644 --- a/docs/sources/open-source.md +++ b/docs/sources/open-source.md @@ -166,13 +166,11 @@ lt --port 8080 -s pretty-turkey-83 --print-requests The Telegram integration for Grafana OnCall is designed for collaborative team work and improved incident response. Refer to the following steps to configure the Telegram integration: -1. Ensure your OnCall environment is up and running. - -1. Request [BotFather](https://t.me/BotFather) for a key, then add your key in `TELEGRAM_TOKEN` in your Grafana OnCall **Env Variables**. - -1. Set `TELEGRAM_WEBHOOK_HOST` with your external URL for your Grafana OnCall. - -1. From the **ChatOps** tab in Grafana OnCall, click **Telegram**. +1. Ensure your Grafana OnCall environment is up and running. +2. Create a Telegram bot using [BotFather](https://t.me/BotFather) and save the token provided by BotFather. Please make sure to disable **Group Privacy** for the bot (Bot Settings -> Group Privacy -> Turn off). +3. Paste the token provided by BotFather to the `TELEGRAM_TOKEN` variable on the **Env Variables** page of your Grafana OnCall instance. +4. Set the `TELEGRAM_WEBHOOK_HOST` variable to the external address of your Grafana OnCall instance. Please note that `TELEGRAM_WEBHOOK_HOST` must start with `https://` and be publicly available (meaning that it can be reached by Telegram servers). If your host is private or local, consider using a reverse proxy (e.g. [ngrok](https://ngrok.com)). +5. Now you can connect Telegram accounts on the **Users** page and receive alert groups to Telegram direct messages. Alternatively, in case you want to connect Telegram channels to your Grafana OnCall environment, navigate to the **ChatOps** tab. ## Grafana OSS-Cloud Setup diff --git a/grafana-plugin/src/containers/TelegramIntegrationButton/TelegramIntegrationButton.tsx b/grafana-plugin/src/containers/TelegramIntegrationButton/TelegramIntegrationButton.tsx index f4b66692..256a2a16 100644 --- a/grafana-plugin/src/containers/TelegramIntegrationButton/TelegramIntegrationButton.tsx +++ b/grafana-plugin/src/containers/TelegramIntegrationButton/TelegramIntegrationButton.tsx @@ -147,12 +147,16 @@ const TelegramModal = (props: TelegramModalProps) => { {' '} - , to the channel. + , to the channel and wait for the confirmation message.
- 8. Make sure users connect to Telegram in their OnCall user profile. + 8. Make sure users connect their Telegram accounts in their OnCall user profile. +
+ +
+ 9. Done! Now you can manage alerts in your Telegram workspace.
From 356aa336ad91e3375fc1e7dbad72d8fee43a1c3f Mon Sep 17 00:00:00 2001 From: Innokentii Konstantinov Date: Fri, 26 Aug 2022 13:46:50 +0500 Subject: [PATCH 3/5] Remove auto-recreating logic for UserNotificationPolicy (#414) * Remove auto-recreating logic for UserNotificationPolicy It's removed to get rid of select_for_update on User on each notify_user_task * Fix and add tests * remove get_user_policies method --- .../incident_log_builder.py | 4 +-- engine/apps/alerts/tasks/notify_group.py | 2 +- engine/apps/alerts/tasks/notify_user.py | 4 +-- .../api/views/user_notification_policy.py | 10 ++++--- .../base/models/user_notification_policy.py | 27 ++++++------------- .../tests/test_user_notification_policy.py | 23 ++++++++++++++++ .../views/personal_notifications.py | 6 ++++- engine/apps/user_management/models/user.py | 3 +++ engine/common/exceptions/__init__.py | 7 ++++- engine/common/exceptions/exceptions.py | 4 +++ engine/conftest.py | 3 +++ 11 files changed, 62 insertions(+), 31 deletions(-) diff --git a/engine/apps/alerts/incident_log_builder/incident_log_builder.py b/engine/apps/alerts/incident_log_builder/incident_log_builder.py index c1582551..ca5ae047 100644 --- a/engine/apps/alerts/incident_log_builder/incident_log_builder.py +++ b/engine/apps/alerts/incident_log_builder/incident_log_builder.py @@ -659,9 +659,7 @@ class IncidentLogBuilder: # last passed step order + 1 notification_policy_order = last_user_log.notification_policy.order + 1 - notification_policies = UserNotificationPolicy.objects.get_or_create_for_user( - user=user_to_notify, important=important - ) + notification_policies = UserNotificationPolicy.objects.filter(user=user_to_notify, important=important) for notification_policy in notification_policies: future_notification = notification_policy.order >= notification_policy_order diff --git a/engine/apps/alerts/tasks/notify_group.py b/engine/apps/alerts/tasks/notify_group.py index d18c31b1..9803affb 100644 --- a/engine/apps/alerts/tasks/notify_group.py +++ b/engine/apps/alerts/tasks/notify_group.py @@ -58,7 +58,7 @@ def notify_group_task(alert_group_pk, escalation_policy_snapshot_order=None): if not user.is_notification_allowed: continue - notification_policies = UserNotificationPolicy.objects.get_or_create_for_user( + notification_policies = UserNotificationPolicy.objects.filter( user=user, important=escalation_policy_step == EscalationPolicy.STEP_NOTIFY_GROUP_IMPORTANT, ) diff --git a/engine/apps/alerts/tasks/notify_user.py b/engine/apps/alerts/tasks/notify_user.py index 57d902b2..a9ba1d15 100644 --- a/engine/apps/alerts/tasks/notify_user.py +++ b/engine/apps/alerts/tasks/notify_user.py @@ -73,9 +73,7 @@ def notify_user_task( user_has_notification = UserHasNotification.objects.filter(pk=user_has_notification.pk).select_for_update()[0] if previous_notification_policy_pk is None: - notification_policy = UserNotificationPolicy.objects.get_or_create_for_user( - user=user, important=important - ).first() + notification_policy = UserNotificationPolicy.objects.filter(user=user, important=important).first() # Here we collect a brief overview of notification steps configured for user to send it to thread. collected_steps_ids = [] next_notification_policy = notification_policy.next() diff --git a/engine/apps/api/views/user_notification_policy.py b/engine/apps/api/views/user_notification_policy.py index ae7e4bee..7231bcc5 100644 --- a/engine/apps/api/views/user_notification_policy.py +++ b/engine/apps/api/views/user_notification_policy.py @@ -26,6 +26,7 @@ from apps.base.models.user_notification_policy import BUILT_IN_BACKENDS, Notific from apps.user_management.models import User from common.api_helpers.exceptions import BadRequest from common.api_helpers.mixins import UpdateSerializerMixin +from common.exceptions import UserNotificationPolicyCouldNotBeDeleted from common.insight_log import EntityEvent, write_resource_insight_log @@ -55,14 +56,14 @@ class UserNotificationPolicyView(UpdateSerializerMixin, ModelViewSet): except ValueError: raise BadRequest(detail="Invalid user param") if user_id is None or user_id == self.request.user.public_primary_key: - queryset = self.model.objects.get_or_create_for_user(user=self.request.user, important=important) + queryset = self.model.objects.filter(user=self.request.user, important=important) else: try: target_user = User.objects.get(public_primary_key=user_id) except User.DoesNotExist: raise BadRequest(detail="User does not exist") - queryset = self.model.objects.get_or_create_for_user(user=target_user, important=important) + queryset = self.model.objects.filter(user=target_user, important=important) queryset = self.serializer_class.setup_eager_loading(queryset) @@ -111,7 +112,10 @@ class UserNotificationPolicyView(UpdateSerializerMixin, ModelViewSet): def perform_destroy(self, instance): user = instance.user prev_state = user.insight_logs_serialized - instance.delete() + try: + instance.delete() + except UserNotificationPolicyCouldNotBeDeleted: + raise BadRequest(detail="Can't delete last user notification policy") new_state = user.insight_logs_serialized write_resource_insight_log( instance=user, diff --git a/engine/apps/base/models/user_notification_policy.py b/engine/apps/base/models/user_notification_policy.py index e0a62275..b6444995 100644 --- a/engine/apps/base/models/user_notification_policy.py +++ b/engine/apps/base/models/user_notification_policy.py @@ -4,13 +4,14 @@ from typing import Tuple from django.conf import settings from django.core.exceptions import ValidationError from django.core.validators import MinLengthValidator -from django.db import models, transaction +from django.db import models from django.db.models import Q, QuerySet from django.utils import timezone from ordered_model.models import OrderedModel from apps.base.messaging import get_messaging_backends from apps.user_management.models import User +from common.exceptions import UserNotificationPolicyCouldNotBeDeleted from common.public_primary_keys import generate_public_primary_key, increase_public_primary_key_length @@ -69,24 +70,6 @@ def validate_channel_choice(value): class UserNotificationPolicyQuerySet(models.QuerySet): - def get_or_create_for_user(self, user: User, important: bool) -> "QuerySet[UserNotificationPolicy]": - with transaction.atomic(): - User.objects.select_for_update().get(pk=user.pk) - return self._get_or_create_for_user(user, important) - - def _get_or_create_for_user(self, user: User, important: bool) -> "QuerySet[UserNotificationPolicy]": - notification_policies = super().filter(user=user, important=important) - - if notification_policies.exists(): - return notification_policies - - if important: - policies = self.create_important_policies_for_user(user) - else: - policies = self.create_default_policies_for_user(user) - - return policies - def create_default_policies_for_user(self, user: User) -> "QuerySet[UserNotificationPolicy]": model = self.model @@ -197,6 +180,12 @@ class UserNotificationPolicy(OrderedModel): else: return "Not set" + def delete(self): + if UserNotificationPolicy.objects.filter(important=self.important, user=self.user).count() == 1: + raise UserNotificationPolicyCouldNotBeDeleted("Can't delete last user notification policy") + else: + super().delete() + class NotificationChannelOptions: """ diff --git a/engine/apps/base/tests/test_user_notification_policy.py b/engine/apps/base/tests/test_user_notification_policy.py index 5d0e1df7..41354491 100644 --- a/engine/apps/base/tests/test_user_notification_policy.py +++ b/engine/apps/base/tests/test_user_notification_policy.py @@ -9,6 +9,7 @@ from apps.base.models.user_notification_policy import ( validate_channel_choice, ) from apps.base.tests.messaging_backend import TestOnlyBackend +from common.exceptions import UserNotificationPolicyCouldNotBeDeleted @pytest.mark.parametrize( @@ -80,3 +81,25 @@ def test_extra_messaging_backends_details(): ) assert validate_channel_choice(channel_choice) is None + + +@pytest.mark.django_db +def test_unable_to_delete_last_notification_policy( + make_organization, + make_user_for_organization, + make_user_notification_policy, +): + organization = make_organization() + user = make_user_for_organization(organization) + + first_policy = make_user_notification_policy( + user, UserNotificationPolicy.Step.NOTIFY, notify_by=UserNotificationPolicy.NotificationChannel.SLACK + ) + + second_policy = make_user_notification_policy( + user, UserNotificationPolicy.Step.WAIT, wait_delay=timedelta(minutes=5) + ) + + first_policy.delete() + with pytest.raises(UserNotificationPolicyCouldNotBeDeleted): + second_policy.delete() diff --git a/engine/apps/public_api/views/personal_notifications.py b/engine/apps/public_api/views/personal_notifications.py index 44b251a3..b1288b4b 100644 --- a/engine/apps/public_api/views/personal_notifications.py +++ b/engine/apps/public_api/views/personal_notifications.py @@ -12,6 +12,7 @@ from apps.user_management.models import User from common.api_helpers.exceptions import BadRequest from common.api_helpers.mixins import RateLimitHeadersMixin, UpdateSerializerMixin from common.api_helpers.paginators import FiftyPageSizePaginator +from common.exceptions import UserNotificationPolicyCouldNotBeDeleted from common.insight_log import EntityEvent, write_resource_insight_log @@ -74,7 +75,10 @@ class PersonalNotificationView(RateLimitHeadersMixin, UpdateSerializerMixin, Mod def perform_destroy(self, instance): user = self.request.user prev_state = user.insight_logs_serialized - instance.delete() + try: + instance.delete() + except UserNotificationPolicyCouldNotBeDeleted: + raise BadRequest(detail="Can't delete last user notification policy") new_state = user.insight_logs_serialized write_resource_insight_log( instance=user, diff --git a/engine/apps/user_management/models/user.py b/engine/apps/user_management/models/user.py index c1a00669..041a0ec5 100644 --- a/engine/apps/user_management/models/user.py +++ b/engine/apps/user_management/models/user.py @@ -260,6 +260,9 @@ class User(models.Model): # TODO: check whether this signal can be moved to save method of the model @receiver(post_save, sender=User) def listen_for_user_model_save(sender, instance, created, *args, **kwargs): + if created: + instance.notification_policies.create_default_policies_for_user(instance) + instance.notification_policies.create_important_policies_for_user(instance) drop_cached_ical_for_custom_events_for_organization.apply_async( (instance.organization_id,), ) diff --git a/engine/common/exceptions/__init__.py b/engine/common/exceptions/__init__.py index d191b8f4..ec922fb4 100644 --- a/engine/common/exceptions/__init__.py +++ b/engine/common/exceptions/__init__.py @@ -1 +1,6 @@ -from .exceptions import MaintenanceCouldNotBeStartedError, TeamCanNotBeChangedError, UnableToSendDemoAlert # noqa: F401 +from .exceptions import ( # noqa: F401 + MaintenanceCouldNotBeStartedError, + TeamCanNotBeChangedError, + UnableToSendDemoAlert, + UserNotificationPolicyCouldNotBeDeleted, +) diff --git a/engine/common/exceptions/exceptions.py b/engine/common/exceptions/exceptions.py index 69318bd5..9adf0b47 100644 --- a/engine/common/exceptions/exceptions.py +++ b/engine/common/exceptions/exceptions.py @@ -17,3 +17,7 @@ class TeamCanNotBeChangedError(OperationCouldNotBePerformedError): class UnableToSendDemoAlert(OperationCouldNotBePerformedError): pass + + +class UserNotificationPolicyCouldNotBeDeleted(OperationCouldNotBePerformedError): + pass diff --git a/engine/conftest.py b/engine/conftest.py index 85fb9a3d..68ef50d5 100644 --- a/engine/conftest.py +++ b/engine/conftest.py @@ -68,6 +68,7 @@ from apps.telegram.tests.factories import ( TelegramVerificationCodeFactory, ) from apps.twilioapp.tests.factories import PhoneCallFactory, SMSFactory +from apps.user_management.models.user import User, listen_for_user_model_save from apps.user_management.tests.factories import OrganizationFactory, TeamFactory, UserFactory from common.constants.role import Role @@ -150,7 +151,9 @@ def make_organization(): @pytest.fixture def make_user_for_organization(): def _make_user_for_organization(organization, role=Role.ADMIN, **kwargs): + post_save.disconnect(listen_for_user_model_save, sender=User) user = UserFactory(organization=organization, role=role, **kwargs) + post_save.disconnect(listen_for_user_model_save, sender=User) return user return _make_user_for_organization From 76901a564b6a8d18302d3a1c85cf0a31dd35a561 Mon Sep 17 00:00:00 2001 From: Innokentii Konstantinov Date: Fri, 26 Aug 2022 14:26:01 +0500 Subject: [PATCH 4/5] Handle case when user somehow deleted all their notification policies --- engine/apps/alerts/tasks/notify_user.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/engine/apps/alerts/tasks/notify_user.py b/engine/apps/alerts/tasks/notify_user.py index a9ba1d15..425eea16 100644 --- a/engine/apps/alerts/tasks/notify_user.py +++ b/engine/apps/alerts/tasks/notify_user.py @@ -74,6 +74,11 @@ def notify_user_task( if previous_notification_policy_pk is None: notification_policy = UserNotificationPolicy.objects.filter(user=user, important=important).first() + if notification_policy is None: + task_logger.info( + f"notify_user_task: Failed to notify. No notification policies. user_id={user_pk} alert_group_id={alert_group_pk} important={important}" + ) + return # Here we collect a brief overview of notification steps configured for user to send it to thread. collected_steps_ids = [] next_notification_policy = notification_policy.next() From 25c3d19a050c7e73bf40b0cd97eb6e9509cfd398 Mon Sep 17 00:00:00 2001 From: Innokentii Konstantinov Date: Fri, 26 Aug 2022 16:06:21 +0500 Subject: [PATCH 5/5] Update CHANGELOG.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46c127f9..186fc7ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Change Log +## v1.0.26 (2022-08-26) +- Insight log's format fixes +- Remove UserNotificationPolicy auto-recreating + ## v1.0.25 (2022-08-24) - Bug fixes