From 76e4c64d66a7dc81544b52fc0ea626e80ed28946 Mon Sep 17 00:00:00 2001 From: Andrei Andreev Date: Mon, 27 May 2024 13:13:00 +0200 Subject: [PATCH] Add breadcrumb ordering --- .../core/sentry_reporter/sentry_reporter.py | 8 +++++- .../core/sentry_reporter/sentry_tools.py | 18 +++++++++++- .../tests/test_sentry_reporter.py | 22 +++++++++++---- .../tests/test_sentry_tools.py | 28 ++++++++++++++++++- 4 files changed, 67 insertions(+), 9 deletions(-) diff --git a/src/tribler/core/sentry_reporter/sentry_reporter.py b/src/tribler/core/sentry_reporter/sentry_reporter.py index d17230de90e..da63176f635 100644 --- a/src/tribler/core/sentry_reporter/sentry_reporter.py +++ b/src/tribler/core/sentry_reporter/sentry_reporter.py @@ -17,7 +17,7 @@ from tribler.core import version from tribler.core.sentry_reporter.sentry_tools import ( get_first_item, - get_value + get_value, order_by_utc_time ) @@ -386,6 +386,12 @@ def _before_send(self, event: Optional[Dict], hint: Optional[Dict]) -> Optional[ if self.scrubber: event = self.scrubber.scrub_event(event) + # order breadcrumbs by timestamp in ascending order + if breadcrumbs := event.get(BREADCRUMBS): + try: + event[BREADCRUMBS][VALUES] = order_by_utc_time(breadcrumbs[VALUES]) + except Exception as e: + self._logger.exception(e) return event # pylint: disable=unused-argument diff --git a/src/tribler/core/sentry_reporter/sentry_tools.py b/src/tribler/core/sentry_reporter/sentry_tools.py index f0f2281600c..d3346f41b03 100644 --- a/src/tribler/core/sentry_reporter/sentry_tools.py +++ b/src/tribler/core/sentry_reporter/sentry_tools.py @@ -3,7 +3,8 @@ """ import re from dataclasses import dataclass -from typing import Optional +from datetime import datetime +from typing import Dict, List, Optional from faker import Faker @@ -121,3 +122,18 @@ def obfuscate_string(s: str, part_of_speech: str = 'noun') -> str: faker = Faker(locale='en_US') faker.seed_instance(s) return faker.word(part_of_speech=part_of_speech) + + +def order_by_utc_time(breadcrumbs: Optional[List[Dict]]): + """ Order breadcrumbs by timestamp in ascending order. + + Args: + breadcrumbs: List of breadcrumbs + + Returns: + Ordered list of breadcrumbs + """ + if not breadcrumbs: + return breadcrumbs + + return list(sorted(breadcrumbs, key=lambda breadcrumb: breadcrumb["timestamp"])) diff --git a/src/tribler/core/sentry_reporter/tests/test_sentry_reporter.py b/src/tribler/core/sentry_reporter/tests/test_sentry_reporter.py index 8b6cd088d2a..322320df185 100644 --- a/src/tribler/core/sentry_reporter/tests/test_sentry_reporter.py +++ b/src/tribler/core/sentry_reporter/tests/test_sentry_reporter.py @@ -1,4 +1,3 @@ -from collections import defaultdict from copy import deepcopy from unittest.mock import MagicMock, Mock, patch @@ -30,7 +29,10 @@ def sentry_reporter(): return SentryReporter() -@patch('tribler.core.sentry_reporter.sentry_reporter.sentry_sdk.init') +TARGET = 'tribler.core.sentry_reporter.sentry_reporter' + + +@patch(f'{TARGET}.sentry_sdk.init') def test_init(mocked_init: Mock, sentry_reporter: SentryReporter): # test that `init` method set all necessary variables and calls `sentry_sdk.init()` sentry_reporter.init(sentry_url='url', release_version='release', scrubber=SentryScrubber(), @@ -40,14 +42,14 @@ def test_init(mocked_init: Mock, sentry_reporter: SentryReporter): mocked_init.assert_called_once() -@patch('tribler.core.sentry_reporter.sentry_reporter.ignore_logger') +@patch(f'{TARGET}.ignore_logger') def test_ignore_logger(mocked_ignore_logger: Mock, sentry_reporter: SentryReporter): # test that `ignore_logger` calls `ignore_logger` from sentry_sdk sentry_reporter.ignore_logger('logger name') mocked_ignore_logger.assert_called_with('logger name') -@patch('tribler.core.sentry_reporter.sentry_reporter.sentry_sdk.add_breadcrumb') +@patch(f'{TARGET}.sentry_sdk.add_breadcrumb') def test_add_breadcrumb(mocked_add_breadcrumb: Mock, sentry_reporter: SentryReporter): # test that `add_breadcrumb` passes all necessary arguments to `sentry_sdk` assert sentry_reporter.add_breadcrumb('message', 'category', 'level', named_arg='some') @@ -71,7 +73,7 @@ def test_get_confirmation_no_qt(sentry_reporter: SentryReporter): assert not sentry_reporter.get_confirmation(Exception('test')) -@patch('tribler.core.sentry_reporter.sentry_reporter.sentry_sdk.capture_exception') +@patch(f'{TARGET}.sentry_sdk.capture_exception') def test_capture_exception(mocked_capture_exception: Mock, sentry_reporter: SentryReporter): # test that `capture_exception` passes an exception to `sentry_sdk` exception = Exception('test') @@ -79,7 +81,7 @@ def test_capture_exception(mocked_capture_exception: Mock, sentry_reporter: Sent mocked_capture_exception.assert_called_with(exception) -@patch('tribler.core.sentry_reporter.sentry_reporter.sentry_sdk.capture_exception') +@patch(f'{TARGET}.sentry_sdk.capture_exception') def test_event_from_exception(mocked_capture_exception: Mock, sentry_reporter: SentryReporter): # test that `event_from_exception` returns '{}' in case of an empty exception assert sentry_reporter.event_from_exception(None) == {} @@ -204,6 +206,14 @@ def test_before_send_scrubber_doesnt_exists(sentry_reporter: SentryReporter): assert sentry_reporter._before_send({'some': 'event'}, None) +@patch(f'{TARGET}.order_by_utc_time', Mock(side_effect=ValueError)) +def test_before_send_exception_in_order_by_utc_time(sentry_reporter: SentryReporter): + # test that in case of an exception in `order_by_utc_time`, the event will be sent + sentry_reporter.global_strategy = SentryStrategy.SEND_ALLOWED + event = {'some': 'event'} + assert sentry_reporter._before_send(event, None) == event + + def test_send_defaults(sentry_reporter): assert sentry_reporter.send_event(event={}) == DEFAULT_EVENT diff --git a/src/tribler/core/sentry_reporter/tests/test_sentry_tools.py b/src/tribler/core/sentry_reporter/tests/test_sentry_tools.py index 80bc547bf7d..2a13ed363bb 100644 --- a/src/tribler/core/sentry_reporter/tests/test_sentry_tools.py +++ b/src/tribler/core/sentry_reporter/tests/test_sentry_tools.py @@ -9,7 +9,7 @@ get_last_item, get_value, modify_value, - obfuscate_string, ) + obfuscate_string, order_by_utc_time, ) def test_first(): @@ -111,3 +111,29 @@ def test_extract_dict(): @pytest.mark.parametrize('given, expected', OBFUSCATED_STRINGS) def test_obfuscate_string(given, expected): assert obfuscate_string(given) == expected + + +def test_order_by_utc_time(): + # Test order by timestamp + breadcrumbs = [ + { + "timestamp": "2016-04-20T20:55:53.887Z", + "message": "3", + }, + { + "timestamp": "2016-04-20T20:55:53.845Z", + "message": "1", + }, + { + "timestamp": "2016-04-20T20:55:53.847Z", + "message": "2", + }, + ] + ordered_breadcrumbs = order_by_utc_time(breadcrumbs) + messages = [d['message'] for d in ordered_breadcrumbs] + assert messages == ['1', '2', '3'] + + +def test_order_by_utc_time_empty_breadcrumbs(): + # Test empty breadcrumbsw + assert not order_by_utc_time(None)