Refactor setTag and removeTag to FFI/JNI

Author: buenaflor

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

@@ -63,8 +63,6 @@ class SentryFlutterPlugin :
       "closeNativeSdk" -> closeNativeSdk(result)
       "setExtra" -> setExtra(call.argument("key"), call.argument("value"), result)
       "removeExtra" -> removeExtra(call.argument("key"), result)
-      "setTag" -> setTag(call.argument("key"), call.argument("value"), result)
-      "removeTag" -> removeTag(call.argument("key"), result)
       "setReplayConfig" -> setReplayConfig(call, result)
       "captureReplay" -> captureReplay(result)
       else -> result.notImplemented()
@@ -166,33 +164,6 @@ class SentryFlutterPlugin :
     result.success("")
   }
 
-  private fun setTag(
-    key: String?,
-    value: String?,
-    result: Result,
-  ) {
-    if (key == null || value == null) {
-      result.success("")
-      return
-    }
-    Sentry.setTag(key, value)
-
-    result.success("")
-  }
-
-  private fun removeTag(
-    key: String?,
-    result: Result,
-  ) {
-    if (key == null) {
-      result.success("")
-      return
-    }
-    Sentry.removeTag(key)
-
-    result.success("")
-  }
-
   private fun closeNativeSdk(result: Result) {
     ScopesAdapter.getInstance().close()
 

packages/flutter/example/integration_test/integration_test.dart

@@ -793,6 +793,36 @@ void main() {
     expect(removedValues['key5'], isNull, reason: 'key5 should be removed');
   });
 
+  testWidgets('setTag and removeTag sync to native', (tester) async {
+    await restoreFlutterOnErrorAfter(() async {
+      await setupSentryAndApp(tester);
+    });
+
+    await Sentry.configureScope((scope) async {
+      scope.setTag('key1', 'randomValue');
+      scope.setTag('key2', '12');
+    });
+
+    var contexts = await SentryFlutter.native?.loadContexts();
+    final tags = contexts!['tags'];
+    expect(tags, isNotNull, reason: 'Tags are null');
+
+    expect(tags['key1'], 'randomValue', reason: 'key1 mismatch');
+    expect(tags['key2'], '12', reason: 'key2 mismatch');
+
+    await Sentry.configureScope((scope) async {
+      scope.removeTag('key1');
+      scope.removeTag('key2');
+    });
+
+    contexts = await SentryFlutter.native?.loadContexts();
+    if (Platform.isIOS) {
+      expect(contexts!['tags'], isNull, reason: 'Tags are not null');
+    } else if (Platform.isAndroid) {
+      expect(contexts!['tags'], isEmpty, reason: 'Tags are not empty');
+    }
+  });
+
   group('e2e', () {
     var output = find.byKey(const Key('output'));
     late Fixture fixture;

packages/flutter/ffi-cocoa.yaml

@@ -40,6 +40,8 @@ objc-interfaces:
         - 'clearBreadcrumbs'
         - 'setContextValue:forKey:'
         - 'removeContextForKey:'
+        - 'setTagValue:forKey:'
+        - 'removeTagForKey:'
 preamble: |
   // ignore_for_file: type=lint, unused_element
 

packages/flutter/ios/sentry_flutter/Sources/sentry_flutter/SentryFlutterPlugin.swift

@@ -86,17 +86,6 @@ public class SentryFlutterPlugin: NSObject, FlutterPlugin {
             let key = arguments?["key"] as? String
             removeExtra(key: key, result: result)
 
-        case "setTag":
-            let arguments = call.arguments as? [String: Any?]
-            let key = arguments?["key"] as? String
-            let value = arguments?["value"] as? String
-            setTag(key: key, value: value, result: result)
-
-        case "removeTag":
-            let arguments = call.arguments as? [String: Any?]
-            let key = arguments?["key"] as? String
-            removeTag(key: key, result: result)
-
         #if !os(tvOS) && !os(watchOS)
         case "discardProfiler":
             discardProfiler(call, result)
@@ -278,30 +267,6 @@ public class SentryFlutterPlugin: NSObject, FlutterPlugin {
       }
     }
 
-    private func setTag(key: String?, value: String?, result: @escaping FlutterResult) {
-      guard let key = key, let value = value else {
-        result("")
-        return
-      }
-      SentrySDK.configureScope { scope in
-        scope.setTag(value: value, key: key)
-
-        result("")
-      }
-    }
-
-    private func removeTag(key: String?, result: @escaping FlutterResult) {
-      guard let key = key else {
-        result("")
-        return
-      }
-      SentrySDK.configureScope { scope in
-        scope.removeTag(key: key)
-
-        result("")
-      }
-    }
-
     private func collectProfile(_ call: FlutterMethodCall, _ result: @escaping FlutterResult) {
         guard let arguments = call.arguments as? [String: Any],
               let traceId = arguments["traceId"] as? String else {

packages/flutter/lib/src/native/cocoa/binding.dart

@@ -1148,6 +1148,8 @@ interface class SentrySerializable extends objc.ObjCProtocolBase
       : this._(other, retain: retain, release: release);
 }
 
+late final _sel_setTagValue_forKey_ = objc.registerName("setTagValue:forKey:");
+late final _sel_removeTagForKey_ = objc.registerName("removeTagForKey:");
 late final _sel_clearBreadcrumbs = objc.registerName("clearBreadcrumbs");
 final _objc_msgSend_1pl9qdv = objc.msgSendPointer
     .cast<
@@ -1183,6 +1185,19 @@ class SentryScope extends objc.NSObject implements SentrySerializable {
         obj.ref.pointer, _sel_isKindOfClass_, _class_SentryScope);
   }
 
+  /// Set a global tag. Tags are searchable key/value string pairs attached to
+  /// every event.
+  void setTagValue(objc.NSString value, {required objc.NSString forKey}) {
+    _objc_msgSend_pfv6jd(this.ref.pointer, _sel_setTagValue_forKey_,
+        value.ref.pointer, forKey.ref.pointer);
+  }
+
+  /// Remove the tag for the specified key.
+  void removeTagForKey(objc.NSString key) {
+    _objc_msgSend_xtuoz7(
+        this.ref.pointer, _sel_removeTagForKey_, key.ref.pointer);
+  }
+
   /// Clears all breadcrumbs in the scope
   void clearBreadcrumbs() {
     _objc_msgSend_1pl9qdv(this.ref.pointer, _sel_clearBreadcrumbs);

packages/flutter/lib/src/native/cocoa/sentry_native_cocoa.dart

@@ -266,6 +266,24 @@ class SentryNativeCocoa extends SentryNativeChannel {
           scope.removeContextForKey(key.toNSString());
         }));
       });
+
+  @override
+  void setTag(String key, String value) => tryCatchSync('setTag', () {
+        cocoa.SentrySDK.configureScope(
+            cocoa.ObjCBlock_ffiVoid_SentryScope.fromFunction(
+                (cocoa.SentryScope scope) {
+          scope.setTagValue(value.toNSString(), forKey: key.toNSString());
+        }));
+      });
+
+  @override
+  void removeTag(String key) => tryCatchSync('removeTag', () {
+        cocoa.SentrySDK.configureScope(
+            cocoa.ObjCBlock_ffiVoid_SentryScope.fromFunction(
+                (cocoa.SentryScope scope) {
+          scope.removeTagForKey(key.toNSString());
+        }));
+      });
 }
 
 // The default conversion does not handle bool so we will add it ourselves

packages/flutter/lib/src/native/java/sentry_native_java.dart

@@ -319,6 +319,23 @@ class SentryNativeJava extends SentryNativeChannel {
           });
         })));
       });
+
+  @override
+  void setTag(String key, String value) => tryCatchSync('setTag', () {
+        using((arena) {
+          final jKey = key.toJString()..releasedBy(arena);
+          final jVal = value.toJString()..releasedBy(arena);
+          native.Sentry.setTag(jKey, jVal);
+        });
+      });
+
+  @override
+  void removeTag(String key) => tryCatchSync('removeTag', () {
+        using((arena) {
+          final jKey = key.toJString()..releasedBy(arena);
+          native.Sentry.removeTag(jKey);
+        });
+      });
 }
 
 JObject? _dartToJObject(Object? value, Arena arena) => switch (value) {

packages/flutter/lib/src/native/sentry_native_channel.dart

@@ -158,12 +158,14 @@ class SentryNativeChannel
       channel.invokeMethod('removeExtra', {'key': key});
 
   @override
-  Future<void> setTag(String key, String value) =>
-      channel.invokeMethod('setTag', {'key': key, 'value': value});
+  FutureOr<void> setTag(String key, String value) {
+    assert(false, 'setTag should not be used through method channels.');
+  }
 
   @override
-  Future<void> removeTag(String key) =>
-      channel.invokeMethod('removeTag', {'key': key});
+  FutureOr<void> removeTag(String key) {
+    assert(false, 'removeTag should not be used through method channels.');
+  }
 
   @override
   int? startProfiler(SentryId traceId) =>

packages/flutter/test/sentry_native_channel_test.dart

@@ -142,23 +142,27 @@ void main() {
       });
 
       test('setTag', () async {
-        when(channel.invokeMethod(
-                'setTag', {'key': 'fixture-key', 'value': 'fixture-value'}))
-            .thenAnswer((_) => Future.value());
+        final matcher = _nativeUnavailableMatcher(
+          mockPlatform,
+          includeLookupSymbol: true,
+          includeFailedToLoadClassException: true,
+        );
 
-        await sut.setTag('fixture-key', 'fixture-value');
+        expect(() => sut.setTag('fixture-key', 'fixture-value'), matcher);
 
-        verify(channel.invokeMethod(
-            'setTag', {'key': 'fixture-key', 'value': 'fixture-value'}));
+        verifyZeroInteractions(channel);
       });
 
       test('removeTag', () async {
-        when(channel.invokeMethod('removeTag', {'key': 'fixture-key'}))
-            .thenAnswer((_) => Future.value());
+        final matcher = _nativeUnavailableMatcher(
+          mockPlatform,
+          includeLookupSymbol: true,
+          includeFailedToLoadClassException: true,
+        );
 
-        await sut.removeTag('fixture-key');
+        expect(() => sut.removeTag('fixture-key'), matcher);
 
-        verify(channel.invokeMethod('removeTag', {'key': 'fixture-key'}));
+        verifyZeroInteractions(channel);
       });
 
       test('startProfiler', () {