Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CURA-11364 Add Sentry Support #1985

Merged
merged 22 commits into from
Dec 5, 2023
Merged

CURA-11364 Add Sentry Support #1985

merged 22 commits into from
Dec 5, 2023

Conversation

nallath
Copy link
Member

@nallath nallath commented Nov 21, 2023

Description

This adds support for sentry in the engine, which will give us much better insights regarding slicing crashes.

CURA-11364

Goes with: Ultimaker/Cura#17394

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

src/main.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented Nov 21, 2023

Unit Test Results

26 tests  ±0   26 ✔️ ±0   6s ⏱️ ±0s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ±0 

Results for commit 686df6a. ± Comparison against base commit 537f1c2.

♻️ This comment has been updated with latest results.

jellespijker and others added 8 commits November 21, 2023 12:17
The update adds an option to enable or disable Sentry integration in the code. Sentry is configured to capture and report crashes only if the appropriate option is enabled and a valid DSN (data source name) is provided. The suitable changes have been made in the Conan, CMake configurations and source code files.

To compile locally with Sentry support run the conan install command with:
```
-c user.curaengine:sentry_url=<url> -o curaengine:enable_sentry=True
```

For the runners the conf `user.curaengine:sentry_url` has been added to
the relevant runner profiles. Once the new workflows have been merged
that url will move out of the profiles and passed along as a repository
variable in the GH runner.

Contributes to CURA-11364
The revision consists of refactoring of the sentry options in the main.cpp
 file, including setting the database path and release version.
 The filesystem library and fmt library have been used to improve the
 code readability and simplicity. Also, some changes have been made in
 the CMakeLists.txt file, specifically for enabling sentry and its
 related configurations.

 Contributes to CURA-11364
jellespijker and others added 4 commits November 21, 2023 14:56
The sentry config path creation is improved by leveraging the fmt
library to format the respective path strings. This change streamlines
the code and boosts its overall comprehensibility. Also, the sentry
options set database path now makes use of the native function for
string conversion.

Contributes to CURA-11364
Made sure that user identifiable paths aren't logged with a custom
formatter.

See example output on Linux
```
[2023-11-21 14:57:13.764] [info] Sentry config path: /home/*******/.local/share/cura/.sentry-native
```
Untested on Windows

Contributes to CURA-11364
The issue was windows defaults to wide-strings. Sadly sentry doesn't seem to take wide-string paths, so that users with non-ASCII names might be left out again :-/

part of CURA-11364
Copy link
Member

@rburema rburema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plz also test with different locales and non-ASCII user names (specifically on Win64)!

The file 'subrange.hpp' of the ranges library wouldn't compile properly on Windows, work around this strange limitation.

part of CURA-11364
@jellespijker jellespijker merged commit faece8f into main Dec 5, 2023
21 checks passed
@jellespijker jellespijker deleted the CURA-11364_add_sentry branch December 5, 2023 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants