Repository navigation
feat: added Get/Set Environment on Scope - #5772
Conversation
|
📲 Install BuildsAndroid
|
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 6b019b7 | 319.84 ms | 333.15 ms | 13.31 ms |
| c3ee041 | 310.64 ms | 361.90 ms | 51.26 ms |
| d501a7e | 348.06 ms | 431.42 ms | 83.36 ms |
| 3998a95 | 415.94 ms | 478.54 ms | 62.60 ms |
| ed33deb | 343.30 ms | 362.41 ms | 19.10 ms |
| 5b1a06b | 310.56 ms | 362.79 ms | 52.22 ms |
| 91bb874 | 310.68 ms | 359.24 ms | 48.56 ms |
| 0ee65e9 | 317.37 ms | 366.50 ms | 49.13 ms |
| 6edfca2 | 305.52 ms | 432.78 ms | 127.26 ms |
| 5e269de | 292.83 ms | 379.12 ms | 86.29 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 6b019b7 | 0 B | 0 B | 0 B |
| c3ee041 | 0 B | 0 B | 0 B |
| d501a7e | 0 B | 0 B | 0 B |
| 3998a95 | 1.58 MiB | 2.10 MiB | 532.96 KiB |
| ed33deb | 1.58 MiB | 2.13 MiB | 559.52 KiB |
| 5b1a06b | 0 B | 0 B | 0 B |
| 91bb874 | 1.58 MiB | 2.13 MiB | 559.07 KiB |
| 0ee65e9 | 0 B | 0 B | 0 B |
| 6edfca2 | 1.58 MiB | 2.13 MiB | 559.07 KiB |
| 5e269de | 0 B | 0 B | 0 B |
Previous results on branch: feat/scope-set-environment
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 9f4de8c | 324.54 ms | 361.39 ms | 36.85 ms |
| 978ba7b | 315.02 ms | 355.28 ms | 40.26 ms |
| 62d5092 | 312.19 ms | 327.22 ms | 15.03 ms |
| 284a8f9 | 319.11 ms | 381.14 ms | 62.03 ms |
| e9c4258 | 339.22 ms | 436.33 ms | 97.10 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 9f4de8c | 0 B | 0 B | 0 B |
| 978ba7b | 0 B | 0 B | 0 B |
| 62d5092 | 0 B | 0 B | 0 B |
| 284a8f9 | 0 B | 0 B | 0 B |
| e9c4258 | 0 B | 0 B | 0 B |
…nment # Conflicts: # sentry-android-core/src/main/java/io/sentry/android/core/ApplicationExitInfoEventProcessor.java # sentry-android-core/src/test/java/io/sentry/android/core/ApplicationExitInfoEventProcessorTest.kt
romtsn
left a comment
There was a problem hiding this comment.
overall looks good to me, nice job!
There's one place where I think we need to update the usage of environment too, and that is Baggage/DSC (Baggage.java:523, :560). That's the envelope header relay uses for dynamic-sampling rules and trace-level environment in the UI, so we should get it aligned with the rest of telemetry I believe?
Co-authored-by: Roman Zavarnitsyn <rom4ek93@gmail.com>
… the isolation scope
|
(ai summary) Thanks for the review! Merged Baggage/DSC ( Note that while testing this I found the first version was still wrong: Also fixed the same gap in logs, metrics and profiles:
Two things left deliberately unchanged, happy to go either way:
|
Co-Authored-By: Markus Hintersteiner <markus.hintersteiner@sentry.io>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 857f45e. Configure here.
…nment # Conflicts: # CHANGELOG.md
The outproc module shares NdkScopeObserver with the inproc module, which gained setEnvironment on main (#5772).
* feat(android): Add service-managed NDK crash daemon Run the NDK crash daemon from an isolated Android service process so it can release the crashing app, merge the resulting tombstone, and send the enriched envelope before shutdown. * Update CHANGELOG.md * fix(android): Narrow exception handling in the NDK crash service Replace the broad `Throwable` catches in `SentryNdkCrashService` with the exceptions the guarded code can throw, and restore the interrupt flag when the service bind is interrupted. * fix(android): Stop the NDK crash service when its daemon exits The service was started with `startService` but never stopped, so its process kept running after `Sentry.close()` or after the app process died. Call `stopSelf` with the start ID that was current when the worker started, so a newer start from a relaunched app is not stopped. * fix(android): Match the NDK crash service stop to its own start A new service process receives `onBind` before `onStartCommand`, so a worker started from the binder configuration could capture a stale start ID and never stop the service. Tag each start with its shared memory path and stop only when the latest start belongs to the exiting worker. * fix(android): Write the NDK tombstone marker after a successful send The marker was written as soon as the merged envelope existed. If the send then failed with a network error, the daemon discarded the envelope and the next launch skipped the tombstone as already reported, so the crash was lost. Write the marker only once the send succeeds. * fix(android): Bind the NDK crash service for the app's lifetime `startService` throws when the app starts in the background, and the started service stopped about a minute after the app left the foreground, so a later crash found no daemon. Native init then fell back to fork/exec, which cannot exec `libsentry-crash.so` from inside the APK with default packaging, and stalled startup for 10 s. Bind the service with `BIND_AUTO_CREATE` and keep the binding until `SentryNdk.close()`, so the service process keeps the app's importance and is not frozen. Let crash processing continue after `onDestroy`, which runs as soon as a crashed app's binding goes away. Fail NDK init instead of falling back to fork/exec when the service is unavailable: below API 30, without a context, or when binding or configuring fails. Move the inproc `SentryNdkTest` to inproc-only test sources, since outproc init now requires a context. * fix(android): Match NDK tombstones by the app's pid The crash service picked the native crash exit info closest to the event's timestamp and ignored its pid. In a crash loop, the first poll can run before the current crash has an exit info, and the previous crash's tombstone was merged instead. Skip exit infos of other pids. The worker captures the pid when it starts, so a later configure call cannot change it. * fix(android): Start an NDK crash daemon for each relaunch When the app relaunched while the previous daemon was still sending its crash, or before it noticed that its process was gone, the new config was stored but no worker started, and the old worker's cleanup then cleared it. The relaunched app's next crash waited 10 s in the handler and was lost, and its descriptors leaked. Keep the per-app daemon state in a config owned by its worker, and start a worker for each config right away. The old daemon is past crash processing or only waiting for its process to go away, so the two share only process-wide native state such as the logger. * ci(temp): Publish sentry-native-ndk-outproc to Maven Local `sentry-android-ndk-outproc` depends on `io.sentry:sentry-native-ndk-outproc`, which is not on Maven Central yet. Build it from the sentry-native feature branch and publish it to Maven Local so the Build job can resolve it. TODO: Revert. This is temporary until sentry-native publishes the artifact. * fix(android): Wait for the NDK tombstone outside the send budget The tombstone wait and the send shared `shutdownTimeoutMillis`, so a slow tombstone left the send no time at all. Wait for the app's exit info instead. The app exits right after debuggerd writes its tombstone, so the wait ends when the tombstone is there, capped at the 30 s match tolerance. Stop early when the app did not crash natively or the tombstone does not parse. Give the send the full `shutdownTimeoutMillis`, and write the marker for a merged envelope that was sent or kept for the next launch. * fix(android): Remove stale NDK crash IPC files Name the shared-memory file after the app pid and delete leftovers whose process no longer runs when the crash service starts. A setup that fails before the daemon attaches would otherwise leave the file behind, while other processes of a multi-process app keep theirs. * fix(android): Fall back to the 1-argument NDK init With a context, NdkIntegration only looked up `SentryNdk.init(Context, SentryAndroidOptions)`. An older sentry-android-ndk without it disabled NDK. Fall back to `SentryNdk.init(SentryAndroidOptions)` and log a warning. * ref(android): Stop passing the tombstone flag to NdkOptions sentry-native-ndk-outproc no longer reads it, because it always sends native crash envelopes without a minidump. The crash service still gets the tombstone flag through its configure call. * fix(android): Skip SDK init in the NDK crash service process The crash service runs in its own `.sentry_ndk_crash` process, which runs the app's `Application.onCreate`. Apps that call `SentryAndroid.init` there started a second SDK on every launch, with the tombstone, ANR and outbox senders working on the same cache directory as the app process. Return early from `SentryAndroid.init` in that process. * ref(android): Remove the NDK crash service process check SentryAndroid.init now returns early in the `.sentry_ndk_crash` process, so SentryNdk.init is no longer called there. * fix(android): Keep a newer NDK tombstone marker Crash service workers for processes that crash close together can finish out of order. A worker that wrote an older tombstone timestamp let `TombstoneIntegration` report the newer crash again on the next launch. Write the marker only when it is newer than the current one, and serialize the workers. * ci: Stop the sentry-native Gradle daemon after publishing The publish step left its 4 GB Gradle daemon running alongside the 8 GB daemon used by `make preMerge`. The job was killed about 12 minutes into the tests, most likely because the runner ran out of memory. * fix(android): Bind the NDK crash service as important Without `BIND_IMPORTANT`, a foreground app raises its bound crash service process only to the visible oom adj (100), so lmkd would kill the service before the app under memory pressure. The flag gives the service the app's own adj (0) while the app is on top, without lowering the app's priority the way `BIND_ABOVE_CLIENT` would. * Apply batched suggestions from code review Co-authored-by: Roman Zavarnitsyn <rom4ek93@gmail.com> * chore(android): Update the outproc API dump The outproc module shares NdkScopeObserver with the inproc module, which gained setEnvironment on main (#5772). * fix(android): Reattach the NDK crash daemon after a service restart When the crash service process died while the app was bound, Android restarted it and called `onServiceConnected` again, but the connection had already closed the descriptors it configured with, so the app ran without a crash daemon. Configure the restarted service with new pipes and the app's shared memory file, then reattach the native crash IPC. Bind again when the binding dies, and log a lost connection at ERROR. * ref(android): Build the NDK outproc package on top of sentry-android-ndk sentry-android-ndk-outproc compiled sentry-android-ndk's sources through extra source folders, shared its namespace and borrowed its ProGuard rules, so the same files belonged to both modules, and the deprecated `BaseExtension` wired it up. Depend on sentry-android-ndk instead, without its `sentry-native-ndk`, and keep only the crash service and the `SentryNdkOutproc` entry point in the `io.sentry.android.ndk.outproc` namespace. `NdkIntegration` now loads that entry point first and falls back to `SentryNdk`. * fix(android): Write the NDK tombstone marker before the send The native side keeps the envelope for the next launch until it is sent, so the crash is delivered whatever the send result. Write the marker before the send, so that a send that a frozen service finishes late no longer races TombstoneIntegration on the next launch. * fix(android): Close rejected NDK crash service fds `configureDaemon` left the file descriptors of a binder transaction open until garbage collection when it rejected the configuration. Close them before returning. * fix(android): Time the NDK tombstone poll with a monotonic clock The crash service polled for a matching tombstone until a wall clock deadline, so a clock step backward delayed the crash send by the size of the step. Use `SystemClock.uptimeMillis()`, which also drives `Thread.sleep()`. * fix(android): Narrow the NDK outproc library load catch The library loader thread swallowed every `Throwable`, including fatal errors. Catch only the `UnsatisfiedLinkError` of a missing or broken library, which `init()` reports later. `ExceptionUtils.rethrowIfFatal` does not fit, because it rethrows `LinkageError` and would turn a missing library into an app crash. * fix(android): Load the NDK outproc libraries before binding the service A missing or broken native library surfaced only in `initCrashDaemon`, after the crash service was bound and the IPC fds were detached. The `UnsatisfiedLinkError` left the service bound with `BIND_IMPORTANT` and the fds open for the app's lifetime. Load the libraries first, so the failure leaves nothing behind. * fix(android): Write the NDK tombstone marker when the merge fails When the crash service matched the app's native crash exit but could not merge its tombstone, because it stayed unavailable until the deadline or the merge threw, it sent the native event without writing the marker. On the next launch, `TombstoneIntegration` then reported the same crash again as a standalone tombstone event. Write the marker for a matched tombstone, merged or not. * docs(android): Remove the Gradle plugin from the NDK outproc README The `ndkBackend` option of the Gradle plugin is parked for now, so the outproc package is selected through its dependency only. * fix(android): Hand the NDK crash memfd to the crash service The NDK now backs the crash IPC with a memfd instead of a file, because the sparse file could raise `SIGBUS` on a full disk. Create the memfd before configuring the service and send it over Binder, which reattaching already does. This removes the file path from the protocol, and the cleanup of stale files. * docs(changelog): Move the outproc entry back to Unreleased The merge of `origin/main` placed it under `8.63.0`, which was released without out-of-process crash reporting. * build(deps): Bump sentry-native-ndk-outproc to 1.0.0-beta.1 sentry-native now publishes `sentry-native-ndk-outproc` to Maven Central, so CI no longer builds it from the sentry-native branch into Maven Local. * meta(android): Register the NDK outproc module for release Craft's `registry` target fails the release when a published module has no entry. Also list the module in the README packages table and the Android issue template. * ref(android): Pass the thread id as the NDK crash daemon's appTid `appTid` received `System.nanoTime()`, but the daemon expects the parent thread id. On Android it only names the shared memory file that the daemon unlinks, and none exists because the memory is a memfd. * ref(android): Time the NDK tombstone poll with a Deadline New code reads clocks through `io.sentry.time` instead of `SystemClock`. The poll deadline now comes from the options' monotonic ticker, which also counts time the device spends suspended. * ref(android): Document the NDK crash service's broad catches The exception rules in `AGENTS.md` allow a broad catch only at a boundary, with a comment on why and what happens on failure. The two `Exception` catches also call `ExceptionUtils.rethrowIfFatal`, which restores the interrupt flag of a swallowed `InterruptedException`. * fix(android): Unbind the NDK crash service on any native init failure `SentryNdkOutproc.init` only unbound the crash service when native init threw `IllegalStateException`. Any other throwable, such as an `UnsatisfiedLinkError`, left the service bound with `BIND_IMPORTANT`, and the disabled NDK integration never closed it. * docs(changelog): Explain how to enable Android outproc crash reporting --------- Co-authored-by: Roman Zavarnitsyn <rom4ek93@gmail.com>

📜 Description
Adds a getter and setter to the scope that allows users to configure the
Environmentat runtime.💡 Motivation and Context
See #5769
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps