feat: Pass the enableProfiling configuration through to the native SDK #851

indragiek · 2022-04-28T03:34:41Z

📜 Description

The most recent versions of the Sentry SDK's for iOS and Android support profiling. This PR adds an argument to the Flutter configuration options that gets passed through to the native SDK.

💡 Motivation and Context

To be able to profile native code in an app that uses the Sentry Flutter SDK. Note that this does not add support for actually profiling within the Dart VM, only native code.

💚 How did you test it?

Updated the existing tests.

📝 Checklist

🔮 Next steps

github-actions · 2022-04-28T03:35:09Z

	Messages
📖	Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 9f0b06c

brustolin

LGTM

marandaneto · 2022-04-28T06:55:36Z

flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt

@@ -132,6 +132,7 @@ class SentryFlutterPlugin : FlutterPlugin, MethodCallHandler, ActivityAware {
      args.getIfNotNull<Boolean>("anrEnabled") { options.isAnrEnabled = it }
      args.getIfNotNull<Boolean>("sendDefaultPii") { options.isSendDefaultPii = it }
      args.getIfNotNull<Boolean>("enableNdkScopeSync") { options.isEnableScopeSync = it }
+      args.getIfNotNull<Boolean>("enableProfiling") { options.isProfilingEnabled = it }


That alone isn't enough, since we don't enable performance by default on the Native layer.
My suggestion is: if enableProfiling is given, set the tracesSampleRate to the options as well.

Ps: This will only work if people startTransaction on the Native layer or using auto instrumentation for Native screens, transactions started on the Dart layer won't contain any profiling data, this is something we can improve later.

marandaneto · 2022-04-28T06:55:49Z

flutter/ios/Classes/SentryFlutterPluginApple.swift

+        if let enableProfiling = arguments["enableProfiling"] as? Bool {
+            options.enableProfiling = enableProfiling
+        }


Similar to https://github.com/getsentry/sentry-dart/pull/851/files#r860545679

marandaneto · 2022-04-28T06:57:06Z

flutter/lib/src/sentry_flutter_options.dart

+  /// Enable capturing CPU profiles alongside transactions.
+  /// Only available on iOS and Android. This is currently a beta feature that is
+  /// not available to all Sentry customers.


I'd also write that transactions started on the Dart layer won't have profile data, but only transactions started on the Native layer (Android/iOS).

marandaneto · 2022-04-28T07:18:03Z

flutter/example/lib/main.dart

@@ -376,12 +376,24 @@ class AndroidExample extends StatelessWidget {
        },
        child: const Text('Platform exception'),
      ),
+      ElevatedButton(


Move under the Main widget instead of AndroidExample only

codecov-commenter · 2022-04-28T07:18:51Z

Codecov Report

Merging #851 (9f0b06c) into main (dfe8ff4) will decrease coverage by 0.39%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #851      +/-   ##
==========================================
- Coverage   90.33%   89.94%   -0.40%     
==========================================
  Files         119        9     -110     
  Lines        3776      169    -3607     
==========================================
- Hits         3411      152    -3259     
+ Misses        365       17     -348

Impacted Files	Coverage Δ
flutter/lib/src/sentry_flutter_options.dart
dart/lib/src/protocol/sentry_transaction.dart
...art/lib/src/http_client/failed_request_client.dart
flutter/lib/src/sentry_native_channel.dart
dart/lib/src/protocol/sentry_gpu.dart
dart/lib/src/version.dart
dart/lib/src/client_reports/client_report.dart
dart/lib/src/noop_client.dart
dart/lib/src/sentry_sampling_context.dart
...ib/src/environment/_web_environment_variables.dart
... and 100 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dfe8ff4...9f0b06c. Read the comment docs.

marandaneto · 2022-04-28T07:19:24Z

flutter/example/android/app/src/main/kotlin/io/sentry/samples/flutter/MainActivity.kt

@@ -39,6 +43,12 @@ class MainActivity : FlutterActivity() {
        "platform_exception" -> {
          result.success(Thread.currentThread().getStackTrace())
        }
+        "startProfiling" -> {


Should also exist under AppDelegate#handleMessage or let the buttons live only in the Android widget as it is now.

marandaneto · 2022-04-28T07:25:14Z

flutter/example/android/app/src/main/kotlin/io/sentry/samples/flutter/MainActivity.kt

@@ -39,6 +43,12 @@ class MainActivity : FlutterActivity() {
        "platform_exception" -> {
          result.success(Thread.currentThread().getStackTrace())
        }
+        "startProfiling" -> {
+          transaction = Sentry.startTransaction("myTransactionWithProfiling", "task")


I can't test this because of getsentry/sentry-java#2009

marandaneto · 2022-04-28T13:16:35Z

flutter/example/android/app/src/main/AndroidManifest.xml

@@ -42,5 +42,7 @@
        <meta-data
            android:name="flutterEmbedding"
            android:value="2" />
+
+        <meta-data android:name="io.sentry.traces.profiling.enable" android:value="true" />


Workaround for https://github.com/getsentry/sentry-dart/pull/851/files#r860567670

getsantry · 2023-10-18T07:00:09Z

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you remove the label Waiting for: Community, I will leave it alone ... forever!

"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

feat: Pass the enableProfiling configuration through to the native SDK

99983b2

indragiek requested review from marandaneto and brustolin as code owners April 28, 2022 03:34

indragiek added 3 commits April 27, 2022 20:36

Merge branch 'main' into indragiek/enable-profiling

6f67276

Update CHANGELOG

66c80ee

Add a note about it being a beta feature

ca04e80

brustolin approved these changes Apr 28, 2022

View reviewed changes

marandaneto added 2 commits April 28, 2022 08:46

fix conflict

45797fd

fix

32af409

marandaneto reviewed Apr 28, 2022

View reviewed changes

profiling button

f1af6d6

marandaneto reviewed Apr 28, 2022

View reviewed changes

fix

9f0b06c

marandaneto reviewed Apr 28, 2022

View reviewed changes

marandaneto marked this pull request as draft May 4, 2022 17:53

getsantry bot added the Stale label Oct 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Pass the enableProfiling configuration through to the native SDK #851

feat: Pass the enableProfiling configuration through to the native SDK #851

indragiek commented Apr 28, 2022

github-actions bot commented Apr 28, 2022 •

edited

Loading

brustolin left a comment

marandaneto Apr 28, 2022

marandaneto Apr 28, 2022

marandaneto Apr 28, 2022

marandaneto Apr 28, 2022

codecov-commenter commented Apr 28, 2022 •

edited

Loading

marandaneto Apr 28, 2022 •

edited

Loading

marandaneto Apr 28, 2022

marandaneto Apr 28, 2022

getsantry bot commented Oct 18, 2023

feat: Pass the enableProfiling configuration through to the native SDK #851

Are you sure you want to change the base?

feat: Pass the enableProfiling configuration through to the native SDK #851

Conversation

indragiek commented Apr 28, 2022

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

github-actions bot commented Apr 28, 2022 • edited Loading

brustolin left a comment

Choose a reason for hiding this comment

marandaneto Apr 28, 2022

Choose a reason for hiding this comment

marandaneto Apr 28, 2022

Choose a reason for hiding this comment

marandaneto Apr 28, 2022

Choose a reason for hiding this comment

marandaneto Apr 28, 2022

Choose a reason for hiding this comment

codecov-commenter commented Apr 28, 2022 • edited Loading

Codecov Report

marandaneto Apr 28, 2022 • edited Loading

Choose a reason for hiding this comment

marandaneto Apr 28, 2022

Choose a reason for hiding this comment

marandaneto Apr 28, 2022

Choose a reason for hiding this comment

getsantry bot commented Oct 18, 2023

github-actions bot commented Apr 28, 2022 •

edited

Loading

codecov-commenter commented Apr 28, 2022 •

edited

Loading

marandaneto Apr 28, 2022 •

edited

Loading