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

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

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

* Feat: Client Reports (#829)
* Fix: Add missing iOS contexts (#761)
* Feat: Pass the enableProfiling configuration through to the native SDK (#851)

### Sentry Self-hosted Compatibility

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,13 @@ 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

if (options.isProfilingEnabled) {
options.tracesSampleRate = 1.0 // read tracesSampleRate from args which is yet not given
}
}

val nativeCrashHandling = (args["enableNativeCrashHandling"] as? Boolean) ?: true
// nativeCrashHandling has priority over anrEnabled
Expand Down
2 changes: 2 additions & 0 deletions flutter/example/android/app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,7 @@
<meta-data
android:name="flutterEmbedding"
android:value="2" />

<meta-data android:name="io.sentry.traces.profiling.enable" android:value="true" />
Copy link
Contributor

Choose a reason for hiding this comment

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

</application>
</manifest>
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,16 @@ package io.sentry.samples.flutter
import io.flutter.embedding.android.FlutterActivity
import io.flutter.embedding.engine.FlutterEngine
import io.flutter.plugin.common.MethodChannel
import io.sentry.ITransaction
import io.sentry.Sentry
import io.sentry.SpanStatus
import kotlin.concurrent.thread

class MainActivity : FlutterActivity() {
private val _channel = "example.flutter.sentry.io"

private var transaction: ITransaction? = null

override fun configureFlutterEngine(flutterEngine: FlutterEngine) {
super.configureFlutterEngine(flutterEngine)
MethodChannel(flutterEngine.dartExecutor.binaryMessenger, _channel).setMethodCallHandler {
Expand Down Expand Up @@ -39,6 +43,12 @@ class MainActivity : FlutterActivity() {
"platform_exception" -> {
throw RuntimeException("Catch this platform exception!")
}
"startProfiling" -> {
Copy link
Contributor

@marandaneto marandaneto Apr 28, 2022

Choose a reason for hiding this comment

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

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

transaction = Sentry.startTransaction("myTransactionWithProfiling", "task")
Copy link
Contributor

Choose a reason for hiding this comment

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

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

}
"stopProfiling" -> {
transaction?.finish(SpanStatus.OK)
}
else -> {
result.notImplemented()
}
Expand Down
13 changes: 13 additions & 0 deletions flutter/example/lib/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ Future<void> main() async {
options.reportPackages = false;
options.addInAppInclude('sentry_flutter_example');
options.considerInAppFramesByDefault = false;
options.enableProfiling = true;
},
// Init your App.
appRunner: () => runApp(
Expand Down Expand Up @@ -375,6 +376,18 @@ class AndroidExample extends StatelessWidget {
},
child: const Text('Platform exception'),
),
ElevatedButton(
Copy link
Contributor

Choose a reason for hiding this comment

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

Move under the Main widget instead of AndroidExample only

onPressed: () async {
await execute('startProfiling');
},
child: const Text('Start native profiling'),
),
ElevatedButton(
onPressed: () async {
await execute('stopProfiling');
},
child: const Text('Stop native profiling'),
),
]);
}
}
Expand Down
4 changes: 4 additions & 0 deletions flutter/ios/Classes/SentryFlutterPluginApple.swift
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,10 @@ public class SentryFlutterPluginApple: NSObject, FlutterPlugin {
options.enableOutOfMemoryTracking = enableOutOfMemoryTracking
}

if let enableProfiling = arguments["enableProfiling"] as? Bool {
options.enableProfiling = enableProfiling
}
Comment on lines +265 to +267
Copy link
Contributor

Choose a reason for hiding this comment

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


if let sendClientReports = arguments["sendClientReports"] as? Bool {
options.sendClientReports = sendClientReports
}
Expand Down
2 changes: 1 addition & 1 deletion flutter/ios/sentry_flutter.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Sentry SDK for Flutter with support to native through sentry-cocoa.
:tag => s.version.to_s }
s.source_files = 'Classes/**/*'
s.public_header_files = 'Classes/**/*.h'
s.dependency 'Sentry', '~> 7.13.0'
s.dependency 'Sentry', '~> 7.14.0'
s.ios.dependency 'Flutter'
s.osx.dependency 'FlutterMacOS'
s.ios.deployment_target = '9.0'
Expand Down
1 change: 1 addition & 0 deletions flutter/lib/src/default_integrations.dart
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,7 @@ class NativeSdkIntegration extends Integration<SentryFlutterOptions> {
'enableOutOfMemoryTracking': options.enableOutOfMemoryTracking,
'enableNdkScopeSync': options.enableNdkScopeSync,
'enableAutoPerformanceTracking': options.enableAutoPerformanceTracking,
'enableProfiling': options.enableProfiling,
'sendClientReports': options.sendClientReports,
});

Expand Down
5 changes: 5 additions & 0 deletions flutter/lib/src/sentry_flutter_options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,11 @@ class SentryFlutterOptions extends SentryOptions {
/// [SentryFlutter.setAppStartEnd].
bool autoAppStart = true;

/// 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.
Comment on lines +174 to +176
Copy link
Contributor

Choose a reason for hiding this comment

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

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).

bool enableProfiling = false;

/// By using this, you are disabling native [Breadcrumb] tracking and instead
/// you are just tracking [Breadcrumb]s which result from events available
/// in the current Flutter environment.
Expand Down
7 changes: 5 additions & 2 deletions flutter/test/native_sdk_integration_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ void main() {
'enableOutOfMemoryTracking': true,
'enableNdkScopeSync': false,
'enableAutoPerformanceTracking': true,
'sendClientReports': true
'enableProfiling': false,
'sendClientReports': true,
});
});

Expand Down Expand Up @@ -85,6 +86,7 @@ void main() {
..enableOutOfMemoryTracking = false
..enableNdkScopeSync = true
..enableAutoPerformanceTracking = false
..enableProfiling = true
..sendClientReports = false;

options.sdk.addIntegration('foo');
Expand Down Expand Up @@ -121,7 +123,8 @@ void main() {
'enableOutOfMemoryTracking': false,
'enableNdkScopeSync': true,
'enableAutoPerformanceTracking': false,
'sendClientReports': false
'enableProfiling': true,
'sendClientReports': false,
});
});

Expand Down