From ea4ff43ec766c9175a1e0a64e86406cd4989773c Mon Sep 17 00:00:00 2001 From: shariff-6 Date: Tue, 3 Dec 2024 16:41:16 +0300 Subject: [PATCH] [Integration][New Relic] NoneType object has no attribute get error during resync (#1187) --- integrations/newrelic/CHANGELOG.md | 9 + .../core/service_levels.py | 35 ++- integrations/newrelic/pyproject.toml | 2 +- .../newrelic/tests/test_service_levels.py | 255 ++++++++++++++++++ 4 files changed, 290 insertions(+), 11 deletions(-) create mode 100644 integrations/newrelic/tests/test_service_levels.py diff --git a/integrations/newrelic/CHANGELOG.md b/integrations/newrelic/CHANGELOG.md index 70966c418b..22480f2639 100644 --- a/integrations/newrelic/CHANGELOG.md +++ b/integrations/newrelic/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 +## 0.1.101 (2024-12-3) + + +### Improvements + +- Improved error handling and type safety in NewRelic's service levels class, including null-checking for responses. +- Adds unit tests for the service level class. + + ## 0.1.100 (2024-11-25) diff --git a/integrations/newrelic/newrelic_integration/core/service_levels.py b/integrations/newrelic/newrelic_integration/core/service_levels.py index 99a4344f2f..ad0ffc2a70 100644 --- a/integrations/newrelic/newrelic_integration/core/service_levels.py +++ b/integrations/newrelic/newrelic_integration/core/service_levels.py @@ -10,6 +10,7 @@ render_query, ) from newrelic_integration.core.paging import send_paginated_graph_api_request +from loguru import logger SLI_OBJECT = "__SLI" BATCH_SIZE = 50 @@ -22,6 +23,10 @@ def __init__(self, http_client: httpx.AsyncClient): async def get_service_level_indicator_value( self, http_client: httpx.AsyncClient, nrql: str ) -> dict[Any, Any]: + if not nrql: + logger.debug("Empty NRQL provided, skipping graph api request...") + return {} + query = await render_query( GET_SLI_BY_NRQL_QUERY, nrql_query=nrql, @@ -30,6 +35,11 @@ async def get_service_level_indicator_value( response = await send_graph_api_request( http_client, query, request_type="get_service_level_indicator_value" ) + + if not response: + logger.warning("Empty response from API, skipping...") + return {} + service_levels = ( response.get("data", {}) .get("actor", {}) @@ -37,21 +47,22 @@ async def get_service_level_indicator_value( .get("nrql", {}) .get("results", []) ) - if service_levels: - return service_levels[0] - return {} + return service_levels[0] if service_levels else {} async def enrich_slo_with_sli_and_tags( self, service_level: dict[str, Any] ) -> dict[str, Any]: # Get the NRQL which is used to build the actual SLI result - nrql = ( - service_level.get("serviceLevel", {}) - .get("indicators", [])[0] - .get("resultQueries", {}) - .get("indicator", {}) - .get("nrql") - ) + indicators = service_level.get("serviceLevel", {}).get("indicators", []) + if not indicators: + logger.warning( + "No indicators found in service level, returning the raw service level" + ) + service_level[SLI_OBJECT] = {} + format_tags(service_level) + return service_level + + nrql = indicators[0].get("resultQueries", {}).get("indicator", {}).get("nrql") service_level[SLI_OBJECT] = await self.get_service_level_indicator_value( self.http_client, nrql ) @@ -80,6 +91,10 @@ async def _extract_service_levels( response: dict[Any, Any] ) -> Tuple[Optional[str], list[dict[Any, Any]]]: """Extract service levels from the response. used by send_paginated_graph_api_request""" + if not response: + logger.debug("Empty response in extract_service_levels") + return None, [] + results = ( response.get("data", {}) .get("actor", {}) diff --git a/integrations/newrelic/pyproject.toml b/integrations/newrelic/pyproject.toml index dfac683659..7fe05d094c 100644 --- a/integrations/newrelic/pyproject.toml +++ b/integrations/newrelic/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "newrelic" -version = "0.1.100" +version = "0.1.101" description = "New Relic Integration" authors = ["Tom Tankilevitch "] diff --git a/integrations/newrelic/tests/test_service_levels.py b/integrations/newrelic/tests/test_service_levels.py new file mode 100644 index 0000000000..e0e30217b0 --- /dev/null +++ b/integrations/newrelic/tests/test_service_levels.py @@ -0,0 +1,255 @@ +import pytest +from unittest.mock import AsyncMock, MagicMock, patch +import httpx +from typing import Dict, Any, List, Optional, AsyncGenerator + +from port_ocean.context.ocean import initialize_port_ocean_context +from port_ocean.exceptions.context import PortOceanContextAlreadyInitializedError +from newrelic_integration.core.service_levels import ServiceLevelsHandler, SLI_OBJECT + + +@pytest.fixture(autouse=True) +def mock_ocean_context() -> None: + """Fixture to initialize the port_ocean context.""" + try: + mock_app = MagicMock() + mock_app.config.integration.config = {"new_relic_account_id": "test_account"} + initialize_port_ocean_context(mock_app) + except PortOceanContextAlreadyInitializedError: + pass + + +@pytest.fixture +def mock_http_client() -> AsyncMock: + """Fixture to create a mocked HTTP client.""" + return AsyncMock(spec=httpx.AsyncClient) + + +@pytest.mark.asyncio +class TestServiceLevelsHandler: + @pytest.fixture + def service_levels_handler( + self, mock_http_client: AsyncMock + ) -> ServiceLevelsHandler: + """Fixture to create an instance of ServiceLevelsHandler.""" + return ServiceLevelsHandler(mock_http_client) + + async def test_get_service_level_indicator_value_none_response( + self, + service_levels_handler: ServiceLevelsHandler, + mock_http_client: AsyncMock, + ) -> None: + """Test handling of None response in get_service_level_indicator_value.""" + # Patch render_query and send_graph_api_request to handle None + with ( + patch( + "newrelic_integration.core.service_levels.render_query", + return_value="mocked-query", + ), + patch( + "newrelic_integration.core.service_levels.send_graph_api_request", + return_value=None, + ), + ): + # Test the method + result = await service_levels_handler.get_service_level_indicator_value( + mock_http_client, "test_nrql" + ) + + # Assertions + assert result == {} + + async def test_get_service_level_indicator_value_successful_response( + self, + service_levels_handler: ServiceLevelsHandler, + mock_http_client: AsyncMock, + ) -> None: + """Test successful response in get_service_level_indicator_value.""" + # Prepare a mock successful response + mock_successful_response: Dict[str, Any] = { + "data": { + "actor": { + "account": { + "nrql": { + "results": [{"result": 95.5, "count": 1000, "total": 50}] + } + } + } + } + } + + # Patch render_query and send_graph_api_request to return mock data + with ( + patch( + "newrelic_integration.core.service_levels.render_query", + return_value="mocked-query", + ), + patch( + "newrelic_integration.core.service_levels.send_graph_api_request", + return_value=mock_successful_response, + ), + ): + # Test the method + result = await service_levels_handler.get_service_level_indicator_value( + mock_http_client, "test_nrql" + ) + + # Assertions + assert result == {"result": 95.5, "count": 1000, "total": 50} + + async def test_enrich_slo_with_sli_and_tags_none_nrql( + self, + service_levels_handler: ServiceLevelsHandler, + mock_http_client: AsyncMock, + ) -> None: + """Test enriching service level object with None or missing NRQL.""" + # Test cases with type hint + test_cases: List[Dict[str, Any]] = [ + # Case 1: Missing indicators + {"serviceLevel": {}}, + # Case 2: Empty indicators + {"serviceLevel": {"indicators": []}}, + # Case 3: Missing resultQueries + {"serviceLevel": {"indicators": [{}]}}, + # Case 4: Missing NRQL + {"serviceLevel": {"indicators": [{"resultQueries": {"indicator": {}}}]}}, + ] + + for service_level in test_cases: + # Mock SLI value retrieval to handle potential errors + with patch.object( + service_levels_handler, + "get_service_level_indicator_value", + return_value={}, + ): + # Patch format_tags to do nothing + with patch( + "newrelic_integration.core.service_levels.format_tags" + ) as mock_format_tags: + # Test the method + result = await service_levels_handler.enrich_slo_with_sli_and_tags( + service_level + ) + + # Assertions + assert result.get(SLI_OBJECT, {}) == {} + mock_format_tags.assert_called_once_with(service_level) + + async def test_extract_service_levels_none_response( + self, + service_levels_handler: ServiceLevelsHandler, + ) -> None: + """Test extracting service levels from a None or malformed response.""" + # Test cases with explicit type hint + test_cases: List[Optional[Dict[str, Any]]] = [ + # None response + None, + # Empty dictionary + {}, + # Partial dictionary + {"data": {}}, + {"data": {"actor": {}}}, + {"data": {"actor": {"entitySearch": {}}}}, + {"data": {"actor": {"entitySearch": {"results": {}}}}}, + ] + + for mock_response in test_cases: + # Test the static method + cursor, entities = await ServiceLevelsHandler._extract_service_levels( + mock_response or {} + ) + + # Assertions + assert cursor is None + assert entities == [] + + async def test_extract_service_levels_successful_response( + self, + service_levels_handler: ServiceLevelsHandler, + ) -> None: + """Test extracting service levels from a successful response.""" + # Prepare a mock successful response + mock_successful_response: Dict[str, Any] = { + "data": { + "actor": { + "entitySearch": { + "results": { + "nextCursor": "next_page_token", + "entities": [ + {"guid": "service_level_1"}, + {"guid": "service_level_2"}, + ], + } + } + } + } + } + + # Test the static method + cursor, entities = await ServiceLevelsHandler._extract_service_levels( + mock_successful_response + ) + + # Assertions + assert cursor == "next_page_token" + assert len(entities) == 2 + assert entities[0]["guid"] == "service_level_1" + assert entities[1]["guid"] == "service_level_2" + + async def test_list_service_levels_empty_generator( + self, + service_levels_handler: ServiceLevelsHandler, + ) -> None: + """Test list_service_levels method with an empty generator.""" + + # Create mock async generator with no items + async def mock_paginated_request( + *args: Any, **kwargs: Any + ) -> AsyncGenerator[AsyncMock, None]: + # Empty generator + return + yield + + # Patch the paginated request method + with patch( + "newrelic_integration.core.service_levels.send_paginated_graph_api_request", + new=mock_paginated_request, + ): + # Collect results + results = [] + async for batch in service_levels_handler.list_service_levels(): + results.extend(batch) + + # Assertions + assert len(results) == 0 + + async def test_list_service_levels_multiple_batches( + self, + service_levels_handler: ServiceLevelsHandler, + ) -> None: + test_batches = [ + {"id": "1"}, + {"id": "2"}, + {"id": "3"}, + {"id": "4"}, + {"id": "5"}, + {"id": "6"}, + ] + + async def mock_paginated_request( + *args: Any, **kwargs: Any + ) -> AsyncGenerator[dict[str, Any], None]: + # Yield individual items instead of batches + for item in test_batches: + yield item + + with patch( + "newrelic_integration.core.service_levels.send_paginated_graph_api_request", + new=mock_paginated_request, + ): + results = [] + async for batch in service_levels_handler.list_service_levels(): + results.extend(batch) + + assert len(results) == 6 + assert [item["id"] for item in results] == ["1", "2", "3", "4", "5", "6"]