From 9655a90f236162aa1046e3fc9d9c048952ab5943 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Mon, 26 Aug 2024 13:12:18 -0600 Subject: [PATCH 1/2] Fix plugin sync issues (#4930) # What this PR does - Fix incorrect response for error message on sync - Remove sleep delay from sync (natural latency provides enough delay) ## Which issue(s) this PR closes Related to [issue link here] ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) - [ ] Added the relevant release notes label (see labels prefixed w/ `release:`). These labels dictate how your PR will show up in the autogenerated release notes. --- engine/apps/grafana_plugin/tasks/sync_v2.py | 10 +++------- engine/apps/grafana_plugin/views/sync_v2.py | 7 +++++-- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/engine/apps/grafana_plugin/tasks/sync_v2.py b/engine/apps/grafana_plugin/tasks/sync_v2.py index 0f351e305a..27a3d915f4 100644 --- a/engine/apps/grafana_plugin/tasks/sync_v2.py +++ b/engine/apps/grafana_plugin/tasks/sync_v2.py @@ -1,6 +1,4 @@ import logging -import math -from time import sleep from celery.utils.log import get_task_logger from django.utils import timezone @@ -38,8 +36,7 @@ def sync_organizations_v2(org_ids=None): logger.debug(f"Found {len(active_instance_ids)} active instances") organization_qs = organization_qs.filter(stack_id__in=active_instance_ids) - orgs_per_second = math.ceil(len(organization_qs) / SYNC_PERIOD.seconds) - logger.info(f"Syncing {len(organization_qs)} organizations @ {orgs_per_second} per 1s pause") + logger.info(f"Syncing {len(organization_qs)} organizations") for idx, org in enumerate(organization_qs): if GrafanaAPIClient.validate_grafana_token_format(org.api_token): client = GrafanaAPIClient(api_url=org.grafana_url, api_token=org.api_token) @@ -48,9 +45,8 @@ def sync_organizations_v2(org_ids=None): logger.error( f"Failed to request sync stack_slug={org.stack_slug} status_code={status['status_code']} url={status['url']} message={status['message']}" ) - if idx % orgs_per_second == 0: - logger.info(f"Sleep 1s after {idx + 1} organizations processed") - sleep(1) + if idx % 1000 == 0: + logger.info(f"{idx + 1} organizations processed") else: logger.info(f"Skipping stack_slug={org.stack_slug}, api_token format is invalid or not set") else: diff --git a/engine/apps/grafana_plugin/views/sync_v2.py b/engine/apps/grafana_plugin/views/sync_v2.py index 7df24cfe5c..1c17cef8c3 100644 --- a/engine/apps/grafana_plugin/views/sync_v2.py +++ b/engine/apps/grafana_plugin/views/sync_v2.py @@ -1,5 +1,5 @@ import logging -from dataclasses import asdict +from dataclasses import asdict, is_dataclass from django.conf import settings from rest_framework import status @@ -49,6 +49,9 @@ def post(self, request: Request) -> Response: try: self.do_sync(request) except SyncException as e: - return Response(data=asdict(e.error_data), status=status.HTTP_400_BAD_REQUEST) + return Response( + data=asdict(e.error_data) if is_dataclass(e.error_data) else e.error_data, + status=status.HTTP_400_BAD_REQUEST, + ) return Response(status=status.HTTP_200_OK) From 3269c9b3a7b59b8187a5c705790948e3d80c7033 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Mon, 26 Aug 2024 16:28:38 -0600 Subject: [PATCH 2/2] Fix incorrect IDs being used to lookup user permissions during sync (#4931) # What this PR does - Fixes ID being used to lookup user permissions during sync. - Reduce level on overly chatty log message ## Which issue(s) this PR closes Related to [issue link here] ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) - [ ] Added the relevant release notes label (see labels prefixed w/ `release:`). These labels dictate how your PR will show up in the autogenerated release notes. --- grafana-plugin/pkg/plugin/settings.go | 4 ++-- grafana-plugin/pkg/plugin/users.go | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/grafana-plugin/pkg/plugin/settings.go b/grafana-plugin/pkg/plugin/settings.go index d896b3fd7f..9a3654dd5b 100644 --- a/grafana-plugin/pkg/plugin/settings.go +++ b/grafana-plugin/pkg/plugin/settings.go @@ -168,9 +168,9 @@ func (a *App) OnCallSettingsFromContext(ctx context.Context) (*OnCallPluginSetti return &settings, fmt.Errorf("get GrafanaURL from provisioning failed (not set in jsonData), unable to fallback to grafana cfg") } settings.GrafanaURL = appUrl - log.DefaultLogger.Info(fmt.Sprintf("Using Grafana URL from grafana cfg app url: %s", settings.GrafanaURL)) + log.DefaultLogger.Debug(fmt.Sprintf("Using Grafana URL from grafana cfg app url: %s", settings.GrafanaURL)) } else { - log.DefaultLogger.Info(fmt.Sprintf("Using Grafana URL from provisioning: %s", settings.GrafanaURL)) + log.DefaultLogger.Debug(fmt.Sprintf("Using Grafana URL from provisioning: %s", settings.GrafanaURL)) } settings.RBACEnabled = cfg.FeatureToggles().IsEnabled("accessControlOnCall") diff --git a/grafana-plugin/pkg/plugin/users.go b/grafana-plugin/pkg/plugin/users.go index e4bec0f2f5..99aa97db4a 100644 --- a/grafana-plugin/pkg/plugin/users.go +++ b/grafana-plugin/pkg/plugin/users.go @@ -7,6 +7,7 @@ import ( "net/http" "net/url" "sort" + "strconv" "sync" "sync/atomic" "time" @@ -264,14 +265,15 @@ func (a *App) GetAllUsersWithPermissions(settings *OnCallPluginSettings) ([]OnCa return nil, err } for i := range onCallUsers { - actions, exists := permissions["1"] + userId := strconv.Itoa(onCallUsers[i].ID) + actions, exists := permissions[userId] if exists { onCallUsers[i].Permissions = []OnCallPermission{} for action, _ := range actions { onCallUsers[i].Permissions = append(onCallUsers[i].Permissions, OnCallPermission{Action: action}) } } else { - log.DefaultLogger.Error("Did not find permissions for user", "user", onCallUsers[i].Login) + log.DefaultLogger.Error("Did not find permissions for user", "user", onCallUsers[i].Login, "id", userId) } } }