feat(rust/gui-client): add sentry.io error reporting by ReactorScram · Pull Request #6782 · firezone/firezone

ReactorScram · 2024-09-19T16:29:42Z

Sentry is always enabled for now. In the near future we'll make it opt-out per device and opt-in per org (see #6138 for details)

Replaces the crash_handling module
Catches panics in GUI process, tunnel daemon, and Headless Client
Added a couple "breadcrumbs" to play with that feature
User ID is not set yet
Environment is set to the API URL, e.g. wss://api.firezone.dev
Reports panics from the connlib async task
Release should be automatically pulled from the Cargo version which we automatically set in the version Makefile

Example screenshot of sentry.io with a caught panic:

vercel · 2024-09-19T16:29:46Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
firezone	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 27, 2024 4:22pm

Also add `firezone-logging` which slipped through This is factored out from #6782

thomaseizinger

LGTM

Just one ask around instrumenting caught panics in Session.

thomaseizinger · 2024-09-27T06:31:08Z

rust/gui-client/src-tauri/src/client/gui.rs

+                        telemetry::add_breadcrumb(telemetry::Breadcrumb {
+                            ty: "fail_on_purpose".into(),
+                            message: Some("Will crash / error / panic on purpose to test error handling.".into()),
+                            ..Default::default()
+                        });


I guess we are not leaving this in?

I'd like to, it's handy for testing error handling. The flags for it were already present since they were left in after testing the existing crash handling and logging.

rust/gui-client/src-tauri/src/client/gui.rs

rust/headless-client/src/ipc_service.rs

thomaseizinger · 2024-09-27T06:36:45Z

rust/headless-client/src/ipc_service.rs

            .make_tun()
            .map_err(|e| Error::TunnelDevice(e.to_string()))?;
        connlib.set_tun(Box::new(tun));
+        tun_span.finish();


I think this might also be somewhat misleading because all we do as part of set_tun is writing it to a channel.

Doesn't make_tun run some of the platform-specific code like parts of WinTun?

Ah yeah, I didn't see that it is covering the make_tun part too. It would be nice if we could just get this data from tracing without having to depend on our telemetry module everywhere.

rust/headless-client/src/ipc_service.rs

Extracted from #6782 This moves more of `main` inside the async block, which makes it easier to set up telemetry in the future. We also log errors for the DNS notifier, which was overlooked before.

#6826) This makes it easier to add more fields to the settings without writing them twice This is factored out from #6782

This makes it easy to add more fields to `Controller` without making them all public. This is factored out from #6782 --------- Signed-off-by: Reactor Scram <ReactorScram@users.noreply.github.com>

ReactorScram · 2024-09-19T18:17:31Z

.github/workflows/_rust.yml

        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
        with:
-          tool: dump_syms,minidump-stackwalk


Don't need this since Sentry reads crash dumps for us

ReactorScram · 2024-09-19T18:18:01Z

rust/Cargo.lock

Something in here was causing a bug in sentry, so I deleted the file and let Cargo recreate it.

ReactorScram · 2024-09-19T18:18:09Z

rust/gui-client/src-common/Cargo.toml

 firezone-logging = { workspace = true }
 futures = { version = "0.3", default-features = false }
 hex = "0.4.3"
-minidumper = "0.8.3"


Don't need any of our old crash handling stuff

ReactorScram · 2024-09-19T18:18:47Z

rust/gui-client/src-tauri/Cargo.toml

 tokio = { workspace = true, features = ["signal", "time", "macros", "rt", "rt-multi-thread"] }
 tokio-util = { version = "0.7.11", features = ["codec"] }
 tracing = { workspace = true }
-tracing-panic = "0.1.2"


Can't use this since there can only be one panic hook. (I guess we could wrap both of them but I didn't try that)

ReactorScram · 2024-09-19T18:19:10Z

rust/gui-client/src-tauri/src/client.rs

+    sentry::add_breadcrumb(Breadcrumb {
+        ty: "logging_start".into(),
+        category: None,
+        data: {
+            let mut map = BTreeMap::default();
+            map.insert("directives".into(), directives.into());
+            map.insert("git_version".into(), git_version.into());
+            map.insert("system_uptime_seconds".into(), system_uptime_seconds.into());
+            map
+        },
+        ..Default::default()
+    });


This breadcrumb shows up in Sentry's web UI and I like the idea of breadcrumbs a lot

ReactorScram · 2024-09-26T15:16:59Z

rust/headless-client/src/main.rs

        callbacks,
    };
    let _guard = rt.enter(); // Constructing `PhoenixChannel` requires a runtime context.
+    rt.block_on(async {


This refactor was extracted to #6835

ReactorScram · 2024-09-26T15:17:32Z

rust/telemetry/Cargo.toml

Made firezone-telemetry its own crate even though the code is small, because Gateways and Relays may want it in the future.

ReactorScram · 2024-09-27T15:14:21Z

rust/gui-client/src-tauri/src/client/gui.rs

+                        telemetry::add_breadcrumb(telemetry::Breadcrumb {
+                            ty: "fail_on_purpose".into(),
+                            message: Some("Will crash / error / panic on purpose to test error handling.".into()),
+                            ..Default::default()
+                        });


I'd like to, it's handy for testing error handling. The flags for it were already present since they were left in after testing the existing crash handling and logging.

rust/headless-client/src/ipc_service.rs

ReactorScram · 2024-09-27T15:15:51Z

rust/headless-client/src/ipc_service.rs

            .make_tun()
            .map_err(|e| Error::TunnelDevice(e.to_string()))?;
        connlib.set_tun(Box::new(tun));
+        tun_span.finish();


Doesn't make_tun run some of the platform-specific code like parts of WinTun?

feat(rust/gui-client): add sentry.io error reporting

c3ac2b3

ReactorScram self-assigned this Sep 19, 2024

ReactorScram added 2 commits September 19, 2024 11:29

add PR number

c7bf681

fix

6da3ff4

vercel bot deployed to Preview September 19, 2024 16:32 View deployment

vercel bot deployed to Preview September 19, 2024 16:50 View deployment

ReactorScram mentioned this pull request Sep 19, 2024

Integrate with Sentry for crash logging and error reporting #6138

Open

15 tasks

ReactorScram added 2 commits September 19, 2024 13:13

fix

c8cb80c

remove unused crash dump reading stuff

2fe688a

vercel bot deployed to Preview September 19, 2024 18:18 View deployment

document

cd42d8d

vercel bot deployed to Preview September 19, 2024 18:47 View deployment

remove unused code

132d706

vercel bot deployed to Preview September 19, 2024 18:54 View deployment

fix unused dep

5664e19

vercel bot deployed to Preview September 19, 2024 18:58 View deployment

ReactorScram added 2 commits September 19, 2024 14:05

fix lockfile

3bee7af

sort deps

752774f

vercel bot deployed to Preview September 19, 2024 19:07 View deployment

try switching sentry to rustls

937a0b2

vercel bot deployed to Preview September 19, 2024 20:13 View deployment

remove openssl dep from sentry

1f219bb

vercel bot deployed to Preview September 19, 2024 20:33 View deployment

checkpoint

d4495dd

vercel bot deployed to Preview September 19, 2024 21:20 View deployment

fix sentry feature flags

19c1549

vercel bot deployed to Preview September 19, 2024 22:24 View deployment

Merge remote-tracking branch 'origin/main' into feat/sentry

e4aa27e

vercel bot deployed to Preview September 20, 2024 16:12 View deployment

fix

a0382b9

vercel bot deployed to Preview September 25, 2024 21:15 View deployment

This was referenced Sep 25, 2024

chore(ci): alphabetize Windows packages #6824

Merged

refactor(rust/gui-client): use builder pattern for Controller #6825

Merged

refactor(rust/gui-client): change how default settings are initialized #6826

Merged

Merge remote-tracking branch 'origin/main' into feat/sentry

ca1d2fb

vercel bot deployed to Preview September 26, 2024 14:28 View deployment

make telemetry always-on

0f3b87a

vercel bot deployed to Preview September 26, 2024 14:49 View deployment

fix little things

8f44f37

vercel bot deployed to Preview September 26, 2024 15:13 View deployment

ReactorScram mentioned this pull request Sep 26, 2024

refactor(headless-client): re-arrange main #6835

Merged

github-merge-queue bot pushed a commit that referenced this pull request Sep 26, 2024

chore(ci): alphabetize Windows packages (#6824)

8ae1eb7

Also add `firezone-logging` which slipped through This is factored out from #6782

thomaseizinger approved these changes Sep 27, 2024

View reviewed changes

github-merge-queue bot pushed a commit that referenced this pull request Sep 27, 2024

refactor(rust/gui-client): change how default settings are initialized (

e8d23ac

#6826) This makes it easier to add more fields to the settings without writing them twice This is factored out from #6782

Merge remote-tracking branch 'origin/main' into feat/sentry

c49ca19

vercel bot deployed to Preview September 27, 2024 15:25 View deployment

ReactorScram commented Sep 27, 2024

View reviewed changes

ReactorScram added 2 commits September 27, 2024 10:59

simplify how run_controller is called

201444e

fix span desc

51e6a03

vercel bot deployed to Preview September 27, 2024 16:02 View deployment

capture connlib panics

83a61f9

vercel bot deployed to Preview September 27, 2024 16:22 View deployment

ReactorScram enabled auto-merge September 27, 2024 16:33

ReactorScram added this pull request to the merge queue Sep 27, 2024

Merged via the queue into main with commit 05a2b28 Sep 27, 2024

ReactorScram deleted the feat/sentry branch September 27, 2024 16:47

Conversation

ReactorScram commented Sep 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vercel bot commented Sep 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thomaseizinger left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ReactorScram commented Sep 19, 2024 •

edited

Loading

vercel bot commented Sep 19, 2024 •

edited

Loading