Work-in-progress extension #3

limbonaut · 2024-10-09T17:25:11Z

Godot SDK initial development. Currently, works on Linux. Windows target has issues with the build process at the moment.

Requirements: Git, C++ compiler, python & SCons build tool, CMake.

Features so far:

Sentry singleton exported to GDScript to be used by game devs
Tags
Breadcrumbs
Contexts
- Device, App, etc.
- Including useful engine-specific stats in custom contexts
- User-supplied contexts
Configuration (project-specific)
Capture message events
Capture crashes on Linux and Windows*
Detecting environments (release export, editor, headless server, etc.)
Log file attachment

Screenshots

Demo project

Sentry extension settings

What's included: * Minimal Godot C++ extension code - buildable and runnable. * .gdextension manifest template (most targets specified except iOS for now) * SCons build file * Automatic cloning of godot-cpp (we may decide to use submodule though) * .gdextension manifest deployment with substitutions * clang format file for consistent style and automatic formatting * .git-ignore with common offenders

Edit sentry/config/dsn to assing your project's DSN. Later, we can add GUI for settings.

Executable is expected to be in the addons/sentrysdk/bin/ folder for the editor, and in the same folder as executable for exported projects. Expected names: - Linux: crashpad_handler - Windows: crashpad_handler.exe

Will be used when hints are needed in project settings.

bruno-garcia · 2024-10-10T01:49:51Z

Do you plan on adding some unit tests (and/or integration tests) on this PR already or is that landing after we get this into main?

limbonaut · 2024-10-10T10:57:34Z

Need to consider, from a technical perspective, what can be tested and how it can be tested.

limbonaut · 2024-10-10T20:18:36Z

Hello, Windows.

- Use the same build steps for sentry-native on Linux as on Windows - Linux sentry-native build script - Don't build sentry-native in parallel - Use `sentry-native/src` source dir as dependency - Remove sentry-native/{install,build} on `scons --clean`

bruno-garcia · 2024-10-11T15:03:06Z

woo awesome.

Should we merge this?

limbonaut · 2024-10-11T15:37:53Z

@bruno-garcia
I'm going to test if I didn't break anything on Windows, update the README with the new instructions, and check the sources if I didn't leave any weird bits. And then I think it can be merged.

Also, we need a license in the repo, so it's clear what the terms are for the source code. Can you add it?

bitsandfoxes

This is coming together so nicely!

README.md

bitsandfoxes · 2024-10-15T11:54:28Z

README.md

+    The build process should produce a GDExtension library file for the ***editor target*** at `project/addons/sentrysdk/bin/libsentrysdk.*.editor.*`.
+
+    To export a project in Godot that uses this extension, you'll also need the libraries for the export templates:


Oh, so you need two sets of libraries: Editor and Release. The Release ones actually make it into the exported game. Makes sense.
I wonder if there is something that we can do with that. This way we have a guaranteed way of knowing whether the SDK is in use in the editor or in production. I'm thinking of debug logging, verbosity levels, error capture. We probably should not mess with default options tho.

There are preprocessor defines for build options, and you can add your own, of course. Useful for conditional code inclusion for different builds, platforms, etc.

SConstruct

bitsandfoxes · 2024-10-15T11:57:35Z

SConstruct

+
+    def build_sentry_native(target, source, env):
+        result = subprocess.run(
+            ["sh", "scripts/build-sentry-native.sh"],


I've not looked at the script (yet) but any way to unify this in one script? I see there's also a scripts/build-sentry-native.ps1.

Both can be used, really. I figured it's better to stick to something that is guaranteed to exist on a target platform. PowerShell is not really a standard on Linux, for instance, but shell is, and vice versa. On my distro, I don't even have a native powershell package - putting it as a requirement would create an unnecessary barrier. I could try rewriting it in Python - not sure how it would interact with Windows, but it's a possibility.

SConstruct

src/sentry_settings.h

bitsandfoxes · 2024-10-15T12:33:20Z

src/sentry_singleton.cpp

+		// double fps_metric = Engine::get_singleton()->get_frames_per_second();
+		// if (fps_metric) {
+		// 	sentry_value_set_by_key(perf_context, "fps",
+		// 			sentry_value_new_double(fps_metric));
+		// }
+
+		// Frames drawn since engine started - age metric.
+		// sentry_value_set_by_key(perf_context, "frames_drawn",
+		// 		sentry_value_new_int32(Engine::get_singleton()->get_frames_drawn()));


That'll be really nice at some point!

This is some additional context. I commented it out, because I'm not sure if those are useful on crash/error.

bitsandfoxes · 2024-10-15T12:36:06Z

src/sentry_singleton.cpp

+	}
+
+	// TODO: Collect more useful metrics: physics, navigation, audio...
+	// TODO: Q: Split into categories and make it optional?


We do have some developer docs regarding existing contexts here.

I consulted with that doc on contexts, but what if we can provide useful info outside established protocols? How do you deal with such? This Performance context is just an idea to provide information that is mostly Godot engine-specific, but nevertheless is quite useful for debugging.

src/sentry_settings.h

src/sentry_singleton.h

In SentryOptions: * `sentry_enabled` => `enabled` * `debug_printing` => `debug`

bruno-garcia · 2024-10-20T03:55:52Z

Hey folks, how about we merge this and continue follow ups on main?

Btw please merge it as opposed to squashing it.

limbonaut · 2024-10-20T22:39:10Z

I agree. I continue the work in other branches for now, waiting for this one to be merged. Only adding small fixes.

bruno-garcia

This is looking great. Please get any open review items in github tickets (or a ticket with checkboxes for smaller stuff), lets get this into main.

Please use branches in this repo instead of a fork to make it easier for collaboration (e.g: branch off of branches etc)

README.md

bruno-garcia · 2024-10-21T00:56:37Z

scripts/build-sentry-native.sh

@@ -0,0 +1,8 @@
+#!/bin/sh
+
+pushd sentry-native


Should we remove this in favor of the ps1? Or have this wrap that with pwsh ./a.ps1?

bruno-garcia · 2024-10-21T01:01:03Z

src/sentry_singleton.cpp

+	// Generates an error on wasm builds.
+	String unique_id = OS::get_singleton()->get_unique_id();
+	if (!unique_id.is_empty()) {
+		sentry_value_set_by_key(device_context, "device_unique_identifier", sentry_value_new_string(unique_id.utf8()));


Is this identifier consistant across installs? If so, we might want to use a different identifier.
We usually create a new uuid that's unique to that installation. On mobile it's easier since if the app is reinstalled that file is deleted in the process and we get a new id.

The key is, we don't want this id to be the same across different 'godot' apps in the user device, because we don't want this to be used in a tracking manner. The goal is only to allow this app developer to know how many users were impacted by crashes

So this is basically like installation ID? Random UUID then?

bruno-garcia · 2024-10-21T01:05:00Z

src/sentry_options.cpp

+	// Read project config since ProjectSettings singleton may not be initialized at this point.
+	Ref<ConfigFile> conf = memnew(ConfigFile);
+	Error err = conf->load("res://project.godot");
+	ERR_FAIL_COND_MSG(err > 0, "Sentry: Fatal error: Failed to open project config.");


Will this cause the app/game to crash? We have a policy of supressing errors while logging it all to console (if option.debug=true) but we absolutely can't cause a crash if the SDK fails to do something

This is a macro that logs an error to godot logger and returns from the current function. In practice, such error is added to "Errors" tab in the editor if debugging, and in production it is printed to stderr and the log file if enabled. The crash macro has CRASH in the name.

limbonaut added 30 commits September 20, 2024 16:19

Add sentry submodule & include header in the build process

9a2fc4f

Add libsentry.so and libsentry.dll as a dependency for extension

d21f179

Update .gitignore

48c308a

Add Settings class

bd10e3a

Add Sentry singleton interface, capture message events

9b0f451

Simple demo scene: capture message events

83d41c5

Expose sentry level enum for events and breadcrumbs

6a6bf17

Use sentry levels in the demo project

04c9db9

Godot demo project config

ba88ded

Edit sentry/config/dsn to assing your project's DSN. Later, we can add GUI for settings.

Add Sentry.add_breadcrumb(...)

380abaa

Update demo: add breadcrumbs

0e6af69

Include user data dictionary in add_breadcrumb

a01741f

Keep sentry cache in user data dir

423c9f7

Initialize crashpad on Linux and Windows

3c833c8

Executable is expected to be in the addons/sentrysdk/bin/ folder for the editor, and in the same folder as executable for exported projects. Expected names: - Linux: crashpad_handler - Windows: crashpad_handler.exe

Add crashpad handler as a dependency for exported projects

72b016f

Fix manifest syntax error

64227f1

Rename: capture_message_event() => capture_message()

5358cf9

Define sentry-specific project settings, add debug_printing setting

61fe00e

Clean up settings class

f926752

Define sentry settings using PropertyInfo

14197e8

Will be used when hints are needed in project settings.

Setting for sample rate

4c70b00

Refactor code and project structure

5eccba1

Enable/disable plugin via project settings

4fc1bca

Customize release string via project settings

bb663aa

Add and remove tags + project example

7ea1f26

Update gitignore

7b4369c

Set SDK name

5be6e83

Add get_last_event_id()

30e420e

New attach_log setting, fix setting defaults

aa748a1

limbonaut added 3 commits October 4, 2024 18:32

Add app context

dfabb74

Add Culture context

8dd9fc2

Refine custom contexts

281297a

bruno-garcia assigned bitsandfoxes Oct 10, 2024

limbonaut added 3 commits October 10, 2024 15:03

Fix build issue with msvc

f97ac76

Build process for Windows and MSVC

46875c9

Improve build process

18c3607

- Use the same build steps for sentry-native on Linux as on Windows - Linux sentry-native build script - Don't build sentry-native in parallel - Use `sentry-native/src` source dir as dependency - Remove sentry-native/{install,build} on `scons --clean`

limbonaut force-pushed the develop branch from 21865ae to 0616c8a Compare October 11, 2024 16:45

limbonaut added 3 commits October 11, 2024 18:45

Update README

0616c8a

Refactor build script and fix Linux dependency

08db81d

Clean up

a161c37

limbonaut marked this pull request as ready for review October 11, 2024 18:36

limbonaut requested a review from bitsandfoxes October 14, 2024 11:59

bitsandfoxes reviewed Oct 15, 2024

View reviewed changes

limbonaut added 5 commits October 15, 2024 17:26

Rename SentrySettings => SentryOptions

321018a

API renames and clarify the commented out line in SConstruct

24cc1ad

In SentryOptions: * `sentry_enabled` => `enabled` * `debug_printing` => `debug`

Fix SentryUtil::sentry_event_set_context

e7354d1

Same add_breadcrumb argument defaults as exposed

b7f671f

chore: config update

c6ac925

bruno-garcia approved these changes Oct 21, 2024

View reviewed changes

Update README.md

5c0de96

bruno-garcia merged commit 83747c3 into getsentry:main Oct 21, 2024
1 check passed

limbonaut deleted the develop branch December 13, 2024 21:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Work-in-progress extension #3

Work-in-progress extension #3

limbonaut commented Oct 9, 2024 •

edited

Loading

bruno-garcia commented Oct 10, 2024

limbonaut commented Oct 10, 2024

limbonaut commented Oct 10, 2024

bruno-garcia commented Oct 11, 2024

limbonaut commented Oct 11, 2024

bitsandfoxes left a comment

bitsandfoxes Oct 15, 2024

limbonaut Oct 15, 2024

bitsandfoxes Oct 15, 2024

limbonaut Oct 15, 2024

bitsandfoxes Oct 15, 2024

limbonaut Oct 15, 2024

bitsandfoxes Oct 15, 2024

limbonaut Oct 15, 2024

bruno-garcia commented Oct 20, 2024

limbonaut commented Oct 20, 2024 •

edited

Loading

bruno-garcia left a comment

bruno-garcia Oct 21, 2024

bruno-garcia Oct 21, 2024

limbonaut Oct 21, 2024

bruno-garcia Oct 21, 2024

limbonaut Oct 21, 2024

		The build process should produce a GDExtension library file for the *editor target* at `project/addons/sentrysdk/bin/libsentrysdk..editor.`.

		To export a project in Godot that uses this extension, you'll also need the libraries for the export templates:

Work-in-progress extension #3

Work-in-progress extension #3

Conversation

limbonaut commented Oct 9, 2024 • edited Loading

Screenshots

Demo project

Sentry extension settings

bruno-garcia commented Oct 10, 2024

limbonaut commented Oct 10, 2024

limbonaut commented Oct 10, 2024

bruno-garcia commented Oct 11, 2024

limbonaut commented Oct 11, 2024

bitsandfoxes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bruno-garcia commented Oct 20, 2024

limbonaut commented Oct 20, 2024 • edited Loading

bruno-garcia left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

limbonaut commented Oct 9, 2024 •

edited

Loading

limbonaut commented Oct 20, 2024 •

edited

Loading