v1.3.72

2023-12-05 12:15:08 -05:00 · 2023-12-05 12:15:08 -05:00 · 2d47fdd5c2
commit 2d47fdd5c2
parent 185822a5fa a1edc20cbe
7 changed files with 64 additions and 45 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

 ## Unreleased

+## v1.3.72 (2023-12-05)
+
+### Fixed
+
+- Address metrics calculation issue which occurred when `USE_REDIS_CLUSTER` env var was set by @joeyorlando ([#3510](https://github.com/grafana/oncall/pull/3510))
+
 ## v1.3.71 (2023-12-05)

 ### Added
@ -16,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed

 - Disallow creating and deleting direct paging integrations by @vadimkerr ([#3475](https://github.com/grafana/oncall/pull/3475))
+- Renamed "Connections" tab to "Monitoring Systems" and "Direct Paging" to "Manual Direct Paging" on Integrations page

 ## v1.3.70 (2023-12-01)

@ -63,9 +70,9 @@ Minor bugfixes + dependency updates :)
 ### Added

 - Add ability to use Grafana Service Account Tokens for OnCall API (This is only enabled for resolution_notes
-endpoint currently) @mderynck ([#3189](https://github.com/grafana/oncall/pull/3189))
+  endpoint currently) @mderynck ([#3189](https://github.com/grafana/oncall/pull/3189))
 - Add ability for webhook presets to mask sensitive headers @mderynck
-([#3189](https://github.com/grafana/oncall/pull/3189))
+  ([#3189](https://github.com/grafana/oncall/pull/3189))

 ### Changed

@ -74,7 +81,7 @@ endpoint currently) @mderynck ([#3189](https://github.com/grafana/oncall/pull/31
 ### Fixed

 - Fixed issue that blocked saving webhooks with presets if the preset is controlling the URL @mderynck
-([#3189](https://github.com/grafana/oncall/pull/3189))
+  ([#3189](https://github.com/grafana/oncall/pull/3189))
 - User filter doesn't display current value on Alert Groups page ([1714](https://github.com/grafana/oncall/issues/1714))
 - Remove displaying rotation modal for Terraform/API based schedules
 - Filters polishing ([3183](https://github.com/grafana/oncall/issues/3183))
--- a/engine/apps/metrics_exporter/metrics_collectors.py
+++ b/engine/apps/metrics_exporter/metrics_collectors.py
@ -28,9 +28,13 @@ from apps.metrics_exporter.tasks import start_calculate_and_cache_metrics, start
 application_metrics_registry = CollectorRegistry()


-RE_ALERT_GROUPS_TOTAL = re.compile(r"{}_(\d+)".format(ALERT_GROUPS_TOTAL))
-RE_ALERT_GROUPS_RESPONSE_TIME = re.compile(r"{}_(\d+)".format(ALERT_GROUPS_RESPONSE_TIME))
-RE_USER_WAS_NOTIFIED_OF_ALERT_GROUPS = re.compile(r"{}_(\d+)".format(USER_WAS_NOTIFIED_OF_ALERT_GROUPS))
+# _RE_BASE_PATTERN allows for optional curly-brackets around the metric name as in some cases this may occur
+# see common.cache.ensure_cache_key_allocates_to_the_same_hash_slot for more details regarding this
+_RE_BASE_PATTERN = r"{{?{}}}?_(\d+)"
+
+RE_ALERT_GROUPS_TOTAL = re.compile(_RE_BASE_PATTERN.format(ALERT_GROUPS_TOTAL))
+RE_ALERT_GROUPS_RESPONSE_TIME = re.compile(_RE_BASE_PATTERN.format(ALERT_GROUPS_RESPONSE_TIME))
+RE_USER_WAS_NOTIFIED_OF_ALERT_GROUPS = re.compile(_RE_BASE_PATTERN.format(USER_WAS_NOTIFIED_OF_ALERT_GROUPS))


 # https://github.com/prometheus/client_python#custom-collectors
--- a/engine/apps/metrics_exporter/tests/conftest.py
+++ b/engine/apps/metrics_exporter/tests/conftest.py
@ -22,11 +22,11 @@ METRICS_TEST_USER_USERNAME = "Alex"
@pytest.fixture()
 def mock_cache_get_metrics_for_collector(monkeypatch):
    def _mock_cache_get(key, *args, **kwargs):
-        if key.startswith(ALERT_GROUPS_TOTAL):
+        if ALERT_GROUPS_TOTAL in key:
            key = ALERT_GROUPS_TOTAL
-        elif key.startswith(ALERT_GROUPS_RESPONSE_TIME):
+        elif ALERT_GROUPS_RESPONSE_TIME in key:
            key = ALERT_GROUPS_RESPONSE_TIME
-        elif key.startswith(USER_WAS_NOTIFIED_OF_ALERT_GROUPS):
+        elif USER_WAS_NOTIFIED_OF_ALERT_GROUPS in key:
            key = USER_WAS_NOTIFIED_OF_ALERT_GROUPS
        test_metrics = {
            ALERT_GROUPS_TOTAL: {
--- a/engine/apps/metrics_exporter/tests/test_metrics_collectors.py
+++ b/engine/apps/metrics_exporter/tests/test_metrics_collectors.py
@ -1,6 +1,7 @@
 from unittest.mock import patch

 import pytest
+from django.test import override_settings
 from prometheus_client import CollectorRegistry, generate_latest

 from apps.alerts.constants import AlertGroupState
@ -12,29 +13,34 @@ from apps.metrics_exporter.constants import (
 from apps.metrics_exporter.metrics_collectors import ApplicationMetricsCollector


+# redis cluster usage modifies the cache keys for some operations, so we need to test both cases
+# see common.cache.ensure_cache_key_allocates_to_the_same_hash_slot for more details
+@pytest.mark.parametrize("use_redis_cluster", [True, False])
@patch("apps.metrics_exporter.metrics_collectors.get_organization_ids", return_value=[1])
@patch("apps.metrics_exporter.metrics_collectors.start_calculate_and_cache_metrics.apply_async")
@pytest.mark.django_db
 def test_application_metrics_collector(
-    mocked_org_ids, mocked_start_calculate_and_cache_metrics, mock_cache_get_metrics_for_collector
+    mocked_org_ids, mocked_start_calculate_and_cache_metrics, mock_cache_get_metrics_for_collector, use_redis_cluster
 ):
    """Test that ApplicationMetricsCollector generates expected metrics from cache"""
-    collector = ApplicationMetricsCollector()
-    test_metrics_registry = CollectorRegistry()
-    test_metrics_registry.register(collector)
-    for metric in test_metrics_registry.collect():
-        if metric.name == ALERT_GROUPS_TOTAL:
-            # integration with labels for each alert group state
-            assert len(metric.samples) == len(AlertGroupState)
-        elif metric.name == ALERT_GROUPS_RESPONSE_TIME:
-            # integration with labels for each value in collector's bucket + _count and _sum histogram values
-            assert len(metric.samples) == len(collector._buckets) + 2
-        elif metric.name == USER_WAS_NOTIFIED_OF_ALERT_GROUPS:
-            # metric with labels for each notified user
-            assert len(metric.samples) == 1
-    result = generate_latest(test_metrics_registry).decode("utf-8")
-    assert result is not None
-    assert mocked_org_ids.called
-    # Since there is no recalculation timer for test org in cache, start_calculate_and_cache_metrics must be called
-    assert mocked_start_calculate_and_cache_metrics.called
-    test_metrics_registry.unregister(collector)
+
+    with override_settings(USE_REDIS_CLUSTER=use_redis_cluster):
+        collector = ApplicationMetricsCollector()
+        test_metrics_registry = CollectorRegistry()
+        test_metrics_registry.register(collector)
+        for metric in test_metrics_registry.collect():
+            if metric.name == ALERT_GROUPS_TOTAL:
+                # integration with labels for each alert group state
+                assert len(metric.samples) == len(AlertGroupState)
+            elif metric.name == ALERT_GROUPS_RESPONSE_TIME:
+                # integration with labels for each value in collector's bucket + _count and _sum histogram values
+                assert len(metric.samples) == len(collector._buckets) + 2
+            elif metric.name == USER_WAS_NOTIFIED_OF_ALERT_GROUPS:
+                # metric with labels for each notified user
+                assert len(metric.samples) == 1
+        result = generate_latest(test_metrics_registry).decode("utf-8")
+        assert result is not None
+        assert mocked_org_ids.called
+        # Since there is no recalculation timer for test org in cache, start_calculate_and_cache_metrics must be called
+        assert mocked_start_calculate_and_cache_metrics.called
+        test_metrics_registry.unregister(collector)
--- a/grafana-plugin/e2e-tests/integrations/integrationsTable.test.ts
+++ b/grafana-plugin/e2e-tests/integrations/integrationsTable.test.ts
@ -2,7 +2,9 @@ import { test } from '../fixtures';
 import { generateRandomValue } from '../utils/forms';
 import { createIntegration, searchIntegrationAndAssertItsPresence } from '../utils/integrations';

-test('Integrations table shows data in Connections and Direct Paging tabs', async ({ adminRolePage: { page } }) => {
+test('Integrations table shows data in Monitoring Systems and Direct Paging tabs', async ({
+  adminRolePage: { page },
+}) => {
  const ID = generateRandomValue();
  const WEBHOOK_INTEGRATION_NAME = `Webhook-${ID}`;
  const ALERTMANAGER_INTEGRATION_NAME = `Alertmanager-${ID}`;
@ -22,7 +24,7 @@ test('Integrations table shows data in Connections and Direct Paging tabs', asyn
  await page.getByRole('tab', { name: 'Tab Integrations' }).click();

  // Create 1 Direct Paging integration if it doesn't exist
-  await page.getByRole('tab', { name: 'Tab Direct Paging' }).click();
+  await page.getByRole('tab', { name: 'Tab Manual Direct Paging' }).click();
  const integrationsTable = page.getByTestId('integrations-table');
  await page.waitForTimeout(2000);
  const isDirectPagingAlreadyCreated = (await integrationsTable.getByText('Direct paging').count()) >= 1;
@ -37,7 +39,7 @@ test('Integrations table shows data in Connections and Direct Paging tabs', asyn
  }
  await page.getByRole('tab', { name: 'Tab Integrations' }).click();

-  // By default Connections tab is opened and newly created integrations are visible except Direct Paging one
+  // By default Monitoring Systems tab is opened and newly created integrations are visible except Direct Paging one
  await searchIntegrationAndAssertItsPresence({ page, integrationsTable, integrationName: WEBHOOK_INTEGRATION_NAME });
  await searchIntegrationAndAssertItsPresence({
    page,
@ -52,7 +54,7 @@ test('Integrations table shows data in Connections and Direct Paging tabs', asyn
  });

  // Then after switching to Direct Paging tab only Direct Paging integration is visible
-  await page.getByRole('tab', { name: 'Tab Direct Paging' }).click();
+  await page.getByRole('tab', { name: 'Tab Manual Direct Paging' }).click();
  await searchIntegrationAndAssertItsPresence({
    page,
    integrationsTable,
--- a/grafana-plugin/src/pages/integrations/Integrations.tsx
+++ b/grafana-plugin/src/pages/integrations/Integrations.tsx
@ -58,7 +58,7 @@ import { PAGE, TEXT_ELLIPSIS_CLASS } from 'utils/consts';
 import styles from './Integrations.module.scss';

 enum TabType {
-  Connections = 'connections',
+  MonitoringSystems = 'monitoring-systems',
  DirectPaging = 'direct-paging',
 }

@ -66,11 +66,11 @@ const TAB_QUERY_PARAM_KEY = 'tab';

 const TABS = [
  {
-    label: 'Connections',
-    value: TabType.Connections,
+    label: 'Monitoring Systems',
+    value: TabType.MonitoringSystems,
  },
  {
-    label: 'Direct Paging',
+    label: 'Manual Direct Paging',
    value: TabType.DirectPaging,
  },
 ];
@ -106,7 +106,7 @@ class Integrations extends React.Component<IntegrationsProps, IntegrationsState>
      integrationsFilters: { searchTerm: '', integration_ne: ['direct_paging'] },
      errorData: initErrorDataState(),
      confirmationModal: undefined,
-      activeTab: props.query[TAB_QUERY_PARAM_KEY] || TabType.Connections,
+      activeTab: props.query[TAB_QUERY_PARAM_KEY] || TabType.MonitoringSystems,
    };
  }

@ -204,8 +204,8 @@ class Integrations extends React.Component<IntegrationsProps, IntegrationsState>
    const { alertReceiveChannelStore } = store;

    const { count, results, page_size } = alertReceiveChannelStore.getPaginatedSearchResult();
-    const isDirectPagingSelectedOnConnectionsTab =
-      activeTab === TabType.Connections && integrationsFilters.integration?.includes('direct_paging');
+    const isDirectPagingSelectedOnMonitoringSystemsTab =
+      activeTab === TabType.MonitoringSystems && integrationsFilters.integration?.includes('direct_paging');

    return (
      <>
@ -253,7 +253,7 @@ class Integrations extends React.Component<IntegrationsProps, IntegrationsState>
                  skipFilterOptionFn: ({ name }) => name === 'integration',
                })}
              />
-              {isDirectPagingSelectedOnConnectionsTab && (
+              {isDirectPagingSelectedOnMonitoringSystemsTab && (
                <Alert
                  className={cx('goToDirectPagingAlert')}
                  severity="info"
@ -556,7 +556,7 @@ class Integrations extends React.Component<IntegrationsProps, IntegrationsState>
      alertReceiveChannelStore,
      filtersStore: { applyLabelFilter },
    } = this.props.store;
-    const isConnectionsTab = this.state.activeTab === TabType.Connections;
+    const isMonitoringSystemsTab = this.state.activeTab === TabType.MonitoringSystems;

    const columns = [
      {
@ -578,7 +578,7 @@ class Integrations extends React.Component<IntegrationsProps, IntegrationsState>
        key: 'datasource',
        render: (item: AlertReceiveChannel) => this.renderDatasource(item, alertReceiveChannelStore),
      },
-      ...(isConnectionsTab
+      ...(isMonitoringSystemsTab
        ? [
            {
              width: '10%',
@ -595,7 +595,7 @@ class Integrations extends React.Component<IntegrationsProps, IntegrationsState>
          ]
        : []),
      {
-        width: isConnectionsTab ? '15%' : '30%',
+        width: isMonitoringSystemsTab ? '15%' : '30%',
        title: 'Team',
        render: (item: AlertReceiveChannel) => this.renderTeam(item, grafanaTeamStore.items),
      },
--- a/grafana-plugin/src/plugin.json
+++ b/grafana-plugin/src/plugin.json
@ -57,7 +57,7 @@
    {
      "type": "page",
      "name": "Integrations",
-      "path": "/a/grafana-oncall-app/integrations?tab=connections",
+      "path": "/a/grafana-oncall-app/integrations?tab=monitoring-systems",
      "role": "Viewer",
      "action": "grafana-oncall-app.integrations:read",
      "addToNav": true