Merge pull request #7434 from drew2a/feature/7422

Eliminate superfluous conversions from string and back

src/tribler/core/sentry_reporter/sentry_reporter.py

@@ -6,7 +6,7 @@
 from contextvars import ContextVar
 from enum import Enum, auto
 from hashlib import md5
-from typing import Any, Dict, List, Optional
+from typing import Any, Dict, Optional
 
 import sentry_sdk
 from faker import Faker
@@ -18,8 +18,7 @@
  delete_item,
  get_first_item,
  get_last_item, get_value,
- parse_last_core_output, parse_os_environ,
- parse_stacktrace,
+ parse_last_core_output
 )
 
 VALUE = 'value'
@@ -162,9 +161,8 @@ def add_breadcrumb(self, message='', category='', level='info', **kwargs):
 
  return sentry_sdk.add_breadcrumb(crumb, **kwargs)
 
- def send_event(self, event: Dict = None, post_data: Dict = None, sys_info: Dict = None,
- additional_tags: Dict[str, Any] = None, last_core_output: Optional[str] = None,
- last_processes: List[str] = None):
+ def send_event(self, event: Dict, tags: Optional[Dict[str, Any]] = None, info: Optional[Dict[str, Any]] = None,
+ last_core_output: Optional[str] = None, tribler_version='<not set>'):
  """Send the event to the Sentry server
 
  This method
@@ -179,71 +177,33 @@ def send_event(self, event: Dict = None, post_data: Dict = None, sys_info: Dict
  will be raised, will be sent to Sentry automatically.
 
  Args:
- event: event to send. It should be taken from SentryReporter at
- post_data: dictionary made by the feedbackdialog.py
- previous stages of executing.
- sys_info: dictionary made by the feedbackdialog.py
- additional_tags: tags that will be added to the event
+ event: event to send. It should be taken from SentryReporter
+ tags: tags that will be added to the event
+ info: additional information that will be added to the event
  last_core_output: string that represents last core output
- last_processes: list of strings describing last Tribler GUI/Core processes
+ tribler_version: Tribler version
 
  Returns:
  Event that was sent to Sentry server
  """
- self._logger.info(f"Send: {post_data}, {event}")
+ self._logger.info(f"Send: {tags}, {info}, {event}")
 
- if event is None:
- return event
-
- post_data = post_data or dict()
- sys_info = sys_info or dict()
- additional_tags = additional_tags or dict()
+ tags = tags or {}
+ info = info or {}
 
  if CONTEXTS not in event:
  event[CONTEXTS] = {}
 
  if TAGS not in event:
  event[TAGS] = {}
 
- event[CONTEXTS][REPORTER] = {}
-
  # tags
- tags = event[TAGS]
- tags[VERSION] = get_value(post_data, VERSION)
- tags[MACHINE] = get_value(post_data, MACHINE)
- tags[OS] = get_value(post_data, OS)
- tags[PLATFORM] = get_first_item(get_value(sys_info, PLATFORM))
- tags[PLATFORM_DETAILS] = get_first_item(get_value(sys_info, PLATFORM_DETAILS))
- tags.update(additional_tags)
-
- # context
- context = event[CONTEXTS]
- reporter = context[REPORTER]
- tribler_version = get_value(post_data, VERSION)
-
- context[BROWSER] = {VERSION: tribler_version, NAME: TRIBLER}
-
- stacktrace_parts = parse_stacktrace(get_value(post_data, 'stack'))
- reporter[STACKTRACE] = next(stacktrace_parts, [])
- stacktrace_extra = next(stacktrace_parts, [])
- reporter[STACKTRACE_EXTRA] = stacktrace_extra
- reporter[STACKTRACE_CONTEXT] = next(stacktrace_parts, [])
-
- reporter[COMMENTS] = get_value(post_data, COMMENTS)
-
- reporter[OS_ENVIRON] = parse_os_environ(get_value(sys_info, OS_ENVIRON))
- delete_item(sys_info, OS_ENVIRON)
-
- reporter[SYSINFO] = sys_info
- if last_processes:
- reporter[LAST_PROCESSES] = last_processes
-
- reporter[ADDITIONAL_INFORMATION] = self.additional_information
+ event[TAGS].update(tags)
 
  # try to retrieve an error from the last_core_output
  if last_core_output:
  # split for better representation in the web view
- reporter[LAST_CORE_OUTPUT] = last_core_output.split('\n')
+ info[LAST_CORE_OUTPUT] = last_core_output.split('\n')
  if last_core_exception := parse_last_core_output(last_core_output):
  exceptions = event.get(EXCEPTION, {})
  gui_exception = get_last_item(exceptions.get(VALUES, []), {})
@@ -255,6 +215,10 @@ def send_event(self, event: Dict = None, post_data: Dict = None, sys_info: Dict
  delete_item(gui_exception, 'stacktrace')
 
  exceptions[VALUES] = [gui_exception, core_exception]
+
+ event[CONTEXTS][REPORTER] = info
+ event[CONTEXTS][BROWSER] = {VERSION: tribler_version, NAME: TRIBLER}
+
  with this_sentry_strategy(self, SentryStrategy.SEND_ALLOWED):
  sentry_sdk.capture_event(event)
  return event

src/tribler/core/sentry_reporter/sentry_tools.py

@@ -7,73 +7,13 @@
 
 from faker import Faker
 
-LONG_TEXT_DELIMITER = '--LONG TEXT--'
-CONTEXT_DELIMITER = '--CONTEXT--'
-
 # Find an exception in the string like: "OverflowError: bind(): port must be 0-65535"
 _re_search_exception = re.compile(r'^(\S+)\s*:\s*(.+)')
 
 # Remove the substring like "Sentry is attempting to send 1 pending error messages"
 _re_remove_sentry = re.compile(r'Sentry is attempting.*')
 
 
-def parse_os_environ(array):
- """Parse os.environ field.
-
- Args:
- array: strings that represents tuples delimited by `:`
- Example: ["KEY:VALUE", "PATH:~/"]
-
- Returns:
- Dictionary that made from given array.
- Example: {"KEY": "VALUE", "PATH": "~/"}
-
- """
- result = {}
-
- if not array:
- return result
-
- for line in array:
- items = line.split(':', 1)
- if len(items) < 2:
- continue
- result[items[0]] = items[1]
-
- return result
-
-
-def parse_stacktrace(stacktrace, delimiters=None):
- """Parse stacktrace field.
-
- Example of stacktrace:
- Traceback (most recent call last):,
- File "src\run_tribler.py", line 179, in <module>,
- RuntimeError: ('\'utf-8\' codec can\'t decode byte 0xcd in position 0: invalid continuation byte,
- --LONG TEXT--,
- Traceback (most recent call last):,
- File "<user>\\asyncio\\events.py", line 81, in _run,
- UnicodeDecodeError: \'utf-8\' codec can\'t decode byte 0xcd in position 0: invalid continuation byte,
- --CONTEXT--,
- {\'message\': "Exception in callback'
- Args:
- stacktrace: the string that represents stacktrace.
-
- delimiters: hi-level delimiters of the stacktrace.
- ['--LONG TEXT--', '--CONTEXT--'] by default.
-
- Returns:
- The generator of stacktrace parts.
- """
- if not stacktrace:
- return
-
- delimiters = delimiters or [LONG_TEXT_DELIMITER, CONTEXT_DELIMITER]
-
- for part in re.split('|'.join(delimiters), stacktrace):
- yield [line for line in re.split(r'\\n|\n', part) if line]
-
-
 @dataclass
 class LastCoreException:
  type: str

src/tribler/core/sentry_reporter/tests/test_sentry_reporter.py

@@ -4,9 +4,8 @@
 import pytest
 
 from tribler.core.sentry_reporter.sentry_reporter import (
- ADDITIONAL_INFORMATION, BROWSER, COMMENTS, CONTEXTS, LAST_CORE_OUTPUT, MACHINE, NAME, OS, OS_ENVIRON,
- PLATFORM, PLATFORM_DETAILS,
- REPORTER, STACKTRACE, STACKTRACE_CONTEXT, STACKTRACE_EXTRA, SYSINFO, SentryReporter,
+ BROWSER, CONTEXTS, LAST_CORE_OUTPUT, NAME, OS_ENVIRON,
+ REPORTER, STACKTRACE, SentryReporter,
  SentryStrategy,
  TAGS, TRIBLER, TYPE, VALUE, VERSION, this_sentry_strategy,
 )
@@ -15,18 +14,10 @@
 
 DEFAULT_EVENT = {
  CONTEXTS: {
- BROWSER: {NAME: TRIBLER, VERSION: None},
- REPORTER: {
- STACKTRACE: [],
- STACKTRACE_CONTEXT: [],
- STACKTRACE_EXTRA: [],
- COMMENTS: None,
- OS_ENVIRON: {},
- SYSINFO: {},
- ADDITIONAL_INFORMATION: {},
- },
+ BROWSER: {NAME: TRIBLER, VERSION: '<not set>'},
+ REPORTER: {},
  },
- TAGS: {MACHINE: None, OS: None, PLATFORM: None, PLATFORM_DETAILS: None, VERSION: None},
+ TAGS: {},
 }
 
 
@@ -213,62 +204,13 @@ def test_before_send_scrubber_doesnt_exists(sentry_reporter: SentryReporter):
 
 
 def test_send_defaults(sentry_reporter):
- assert sentry_reporter.send_event(None, None, None) is None
  assert sentry_reporter.send_event(event={}) == DEFAULT_EVENT
 
 
-def test_send_additional_information(sentry_reporter):
- # test that additional information is added to the event
- sentry_reporter.additional_information = {'some': 'information'}
-
- actual = sentry_reporter.send_event(event={})
- expected = deepcopy(DEFAULT_EVENT)
- expected[CONTEXTS][REPORTER][ADDITIONAL_INFORMATION] = {'some': 'information'}
- assert actual == expected
-
-
-def test_send_post_data(sentry_reporter):
- # test that post data is added to the event
- event = {'a': 'b'}
- post_data = {
- "version": '0.0.0', "machine": 'x86_64', "os": 'posix',
- "timestamp": 42, "sysinfo": '', "comments": 'comment',
- "stack": 'l1\nl2--LONG TEXT--l3\nl4',
- }
- actual = sentry_reporter.send_event(event=event, post_data=post_data)
- expected = deepcopy(DEFAULT_EVENT)
- expected['a'] = 'b'
- expected[CONTEXTS][BROWSER][VERSION] = '0.0.0'
- expected[CONTEXTS][REPORTER][STACKTRACE] = ['l1', 'l2']
- expected[CONTEXTS][REPORTER][STACKTRACE_EXTRA] = ['l3', 'l4']
- expected[CONTEXTS][REPORTER][COMMENTS] = 'comment'
- expected[TAGS] = {MACHINE: 'x86_64', OS: 'posix', PLATFORM: None, PLATFORM_DETAILS: None,
- VERSION: '0.0.0'}
-
- assert actual == expected
-
-
-def test_send_sys_info(sentry_reporter):
- # test that sys_info is added to the event
- sys_info = {
- PLATFORM: ['darwin'],
- PLATFORM_DETAILS: ['details'],
- OS_ENVIRON: ['KEY:VALUE', 'KEY1:VALUE1'],
- }
- actual = sentry_reporter.send_event(event={}, sys_info=sys_info)
- expected = deepcopy(DEFAULT_EVENT)
- expected[CONTEXTS][REPORTER][OS_ENVIRON] = {'KEY': 'VALUE', 'KEY1': 'VALUE1'}
- expected[CONTEXTS][REPORTER][SYSINFO] = {PLATFORM: ['darwin'], PLATFORM_DETAILS: ['details']}
- expected[TAGS][PLATFORM] = 'darwin'
- expected[TAGS]['platform.details'] = 'details'
-
- assert actual == expected
-
-
 def test_send_additional_tags(sentry_reporter):
  # test that additional tags are added to the event
  tags = {'tag_key': 'tag_value', 'numeric_tag_key': 1}
- actual = sentry_reporter.send_event(event={}, additional_tags=tags)
+ actual = sentry_reporter.send_event(event={}, tags=tags)
  expected = deepcopy(DEFAULT_EVENT)
  expected[TAGS].update(tags)
  assert actual == expected

src/tribler/core/sentry_reporter/tests/test_sentry_tools.py

@@ -9,8 +9,7 @@
  get_last_item,
  get_value,
  modify_value,
- obfuscate_string, parse_last_core_output, parse_os_environ,
- parse_stacktrace,
+ obfuscate_string, parse_last_core_output,
 )
 
 
@@ -46,46 +45,6 @@ def test_delete():
  assert delete_item({'key': 'value'}, 'key') == {}
 
 
-def test_parse_os_environ():
- # simple tests
- assert parse_os_environ(None) == {}
- assert parse_os_environ([]) == {}
- assert parse_os_environ(['KEY:value']) == {'KEY': 'value'}
-
- assert parse_os_environ(['KEY:value', 'KEY1:value1', 'KEY2:value2']) == {
- 'KEY': 'value',
- 'KEY1': 'value1',
- 'KEY2': 'value2',
- }
-
- # test multiply `:`
- assert parse_os_environ(['KEY:value:and:some']) == {'KEY': 'value:and:some'}
-
- # test no `:`
- assert parse_os_environ(['KEY:value', 'key']) == {'KEY': 'value'}
-
-
-def test_parse_stacktrace():
- assert list(parse_stacktrace(None)) == []
- assert list(parse_stacktrace('')) == []
- assert list(parse_stacktrace('\n')) == [[]]
- assert list(parse_stacktrace('\n\n')) == [[]]
- assert list(parse_stacktrace('line1\n\nline2\\nline3')) == [['line1', 'line2', 'line3']]
-
- # split --LONG TEXT-- and --CONTEXT-- parts
- assert list(parse_stacktrace('l1\nl2--LONG TEXT--l3\nl4--CONTEXT--l5\nl6')) == [
- ['l1', 'l2'],
- ['l3', 'l4'],
- ['l5', 'l6'],
- ]
-
- # split custom parts
- assert list(parse_stacktrace('l1\nl2customl3\nl4', delimiters=['custom'])) == [
- ['l1', 'l2'],
- ['l3', 'l4'],
- ]
-
-
 def test_modify():
  assert modify_value(None, None, None) is None
  assert modify_value({}, None, None) == {}