diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c712805..f50399e4 100644 --- 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)) diff --git a/engine/apps/metrics_exporter/metrics_collectors.py b/engine/apps/metrics_exporter/metrics_collectors.py index 27d65b12..45ff38ed 100644 --- 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 diff --git a/engine/apps/metrics_exporter/tests/conftest.py b/engine/apps/metrics_exporter/tests/conftest.py index f82ee211..f2868ee4 100644 --- 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: { diff --git a/engine/apps/metrics_exporter/tests/test_metrics_collectors.py b/engine/apps/metrics_exporter/tests/test_metrics_collectors.py index 4dbe7d05..670c590b 100644 --- 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) diff --git a/grafana-plugin/e2e-tests/integrations/integrationsTable.test.ts b/grafana-plugin/e2e-tests/integrations/integrationsTable.test.ts index 85408fbd..28b1a3b9 100644 --- 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, diff --git a/grafana-plugin/src/pages/integrations/Integrations.tsx b/grafana-plugin/src/pages/integrations/Integrations.tsx index f9b102a7..b4c30e26 100644 --- 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 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 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 skipFilterOptionFn: ({ name }) => name === 'integration', })} /> - {isDirectPagingSelectedOnConnectionsTab && ( + {isDirectPagingSelectedOnMonitoringSystemsTab && ( 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 key: 'datasource', render: (item: AlertReceiveChannel) => this.renderDatasource(item, alertReceiveChannelStore), }, - ...(isConnectionsTab + ...(isMonitoringSystemsTab ? [ { width: '10%', @@ -595,7 +595,7 @@ class Integrations extends React.Component ] : []), { - width: isConnectionsTab ? '15%' : '30%', + width: isMonitoringSystemsTab ? '15%' : '30%', title: 'Team', render: (item: AlertReceiveChannel) => this.renderTeam(item, grafanaTeamStore.items), }, diff --git a/grafana-plugin/src/plugin.json b/grafana-plugin/src/plugin.json index b4920482..2dc75d5b 100644 --- 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