diff --git a/CHANGELOG.md b/CHANGELOG.md index c1df693fa8..bd8cb0d46d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +### Features + +- Add new User Feedback API ([#4286](https://github.com/getsentry/sentry-java/pull/4286)) + - We now introduced Sentry.captureFeedback, which supersedes Sentry.captureUserFeedback + ### Fixes - Hook User Interaction integration into running Activity in case of deferred SDK init ([#4337](https://github.com/getsentry/sentry-java/pull/4337)) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SessionTrackingIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SessionTrackingIntegrationTest.kt index b67f8aa288..9e79e79b47 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SessionTrackingIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SessionTrackingIntegrationTest.kt @@ -21,6 +21,7 @@ import io.sentry.SentryReplayEvent import io.sentry.Session import io.sentry.TraceContext import io.sentry.UserFeedback +import io.sentry.protocol.Feedback import io.sentry.protocol.SentryId import io.sentry.protocol.SentryTransaction import io.sentry.transport.RateLimiter @@ -147,6 +148,10 @@ class SessionTrackingIntegrationTest { TODO("Not yet implemented") } + override fun captureFeedback(feedback: Feedback, hint: Hint?, scope: IScope): SentryId { + TODO("Not yet implemented") + } + override fun captureReplayEvent( event: SentryReplayEvent, scope: IScope?, diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MainActivity.java b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MainActivity.java index 802e765a9e..f98d718ab9 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MainActivity.java +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MainActivity.java @@ -8,9 +8,8 @@ import io.sentry.ISpan; import io.sentry.MeasurementUnit; import io.sentry.Sentry; -import io.sentry.UserFeedback; import io.sentry.instrumentation.file.SentryFileOutputStream; -import io.sentry.protocol.SentryId; +import io.sentry.protocol.Feedback; import io.sentry.protocol.User; import io.sentry.samples.android.compose.ComposeActivity; import io.sentry.samples.android.databinding.ActivityMainBinding; @@ -70,13 +69,12 @@ protected void onCreate(Bundle savedInstanceState) { binding.sendUserFeedback.setOnClickListener( view -> { - SentryId sentryId = Sentry.captureException(new Exception("I have feedback")); + Feedback feedback = + new Feedback("It broke on Android. I don't know why, but this happens."); + feedback.setContactEmail("john@me.com"); + feedback.setName("John Me"); - UserFeedback userFeedback = new UserFeedback(sentryId); - userFeedback.setComments("It broke on Android. I don't know why, but this happens."); - userFeedback.setEmail("john@me.com"); - userFeedback.setName("John Me"); - Sentry.captureUserFeedback(userFeedback); + Sentry.captureFeedback(feedback); }); binding.addAttachment.setOnClickListener( diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 78b5b0be2b..5c0d0e85b8 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -38,7 +38,7 @@ public final class io/sentry/Baggage { public fun (Ljava/util/concurrent/ConcurrentHashMap;Ljava/lang/Double;Ljava/lang/Double;Ljava/lang/String;ZZLio/sentry/ILogger;)V public fun forceSetSampleRate (Ljava/lang/Double;)V public fun freeze ()V - public static fun fromEvent (Lio/sentry/SentryEvent;Lio/sentry/SentryOptions;)Lio/sentry/Baggage; + public static fun fromEvent (Lio/sentry/SentryBaseEvent;Ljava/lang/String;Lio/sentry/SentryOptions;)Lio/sentry/Baggage; public static fun fromHeader (Ljava/lang/String;)Lio/sentry/Baggage; public static fun fromHeader (Ljava/lang/String;Lio/sentry/ILogger;)Lio/sentry/Baggage; public static fun fromHeader (Ljava/lang/String;ZLio/sentry/ILogger;)Lio/sentry/Baggage; @@ -352,6 +352,7 @@ public final class io/sentry/DataCategory : java/lang/Enum { public static final field Attachment Lio/sentry/DataCategory; public static final field Default Lio/sentry/DataCategory; public static final field Error Lio/sentry/DataCategory; + public static final field Feedback Lio/sentry/DataCategory; public static final field Monitor Lio/sentry/DataCategory; public static final field Profile Lio/sentry/DataCategory; public static final field ProfileChunkUi Lio/sentry/DataCategory; @@ -602,6 +603,9 @@ public final class io/sentry/HubAdapter : io/sentry/IHub { public fun captureEvent (Lio/sentry/SentryEvent;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public fun captureException (Ljava/lang/Throwable;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public fun captureException (Ljava/lang/Throwable;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; + public fun captureFeedback (Lio/sentry/protocol/Feedback;)Lio/sentry/protocol/SentryId; + public fun captureFeedback (Lio/sentry/protocol/Feedback;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; + public fun captureFeedback (Lio/sentry/protocol/Feedback;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;)Lio/sentry/protocol/SentryId; public fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public fun captureProfileChunk (Lio/sentry/ProfileChunk;)Lio/sentry/protocol/SentryId; @@ -670,6 +674,7 @@ public final class io/sentry/HubScopesWrapper : io/sentry/IHub { public fun captureEvent (Lio/sentry/SentryEvent;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public fun captureException (Ljava/lang/Throwable;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public fun captureException (Ljava/lang/Throwable;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; + public fun captureFeedback (Lio/sentry/protocol/Feedback;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;)Lio/sentry/protocol/SentryId; public fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public fun captureProfileChunk (Lio/sentry/ProfileChunk;)Lio/sentry/protocol/SentryId; @@ -903,6 +908,9 @@ public abstract interface class io/sentry/IScopes { public abstract fun captureException (Ljava/lang/Throwable;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public abstract fun captureException (Ljava/lang/Throwable;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public fun captureException (Ljava/lang/Throwable;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; + public fun captureFeedback (Lio/sentry/protocol/Feedback;)Lio/sentry/protocol/SentryId; + public fun captureFeedback (Lio/sentry/protocol/Feedback;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; + public abstract fun captureFeedback (Lio/sentry/protocol/Feedback;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public fun captureMessage (Ljava/lang/String;)Lio/sentry/protocol/SentryId; public fun captureMessage (Ljava/lang/String;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public abstract fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;)Lio/sentry/protocol/SentryId; @@ -987,6 +995,7 @@ public abstract interface class io/sentry/ISentryClient { public fun captureException (Ljava/lang/Throwable;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public fun captureException (Ljava/lang/Throwable;Lio/sentry/IScope;)Lio/sentry/protocol/SentryId; public fun captureException (Ljava/lang/Throwable;Lio/sentry/IScope;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; + public abstract fun captureFeedback (Lio/sentry/protocol/Feedback;Lio/sentry/Hint;Lio/sentry/IScope;)Lio/sentry/protocol/SentryId; public fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;)Lio/sentry/protocol/SentryId; public fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;Lio/sentry/IScope;)Lio/sentry/protocol/SentryId; public abstract fun captureProfileChunk (Lio/sentry/ProfileChunk;Lio/sentry/IScope;)Lio/sentry/protocol/SentryId; @@ -1469,6 +1478,7 @@ public final class io/sentry/NoOpHub : io/sentry/IHub { public fun captureEvent (Lio/sentry/SentryEvent;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public fun captureException (Ljava/lang/Throwable;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public fun captureException (Ljava/lang/Throwable;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; + public fun captureFeedback (Lio/sentry/protocol/Feedback;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;)Lio/sentry/protocol/SentryId; public fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public fun captureProfileChunk (Lio/sentry/ProfileChunk;)Lio/sentry/protocol/SentryId; @@ -1631,6 +1641,7 @@ public final class io/sentry/NoOpScopes : io/sentry/IScopes { public fun captureEvent (Lio/sentry/SentryEvent;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public fun captureException (Ljava/lang/Throwable;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public fun captureException (Ljava/lang/Throwable;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; + public fun captureFeedback (Lio/sentry/protocol/Feedback;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;)Lio/sentry/protocol/SentryId; public fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public fun captureProfileChunk (Lio/sentry/ProfileChunk;)Lio/sentry/protocol/SentryId; @@ -2328,6 +2339,7 @@ public final class io/sentry/Scopes : io/sentry/IScopes { public fun captureEvent (Lio/sentry/SentryEvent;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public fun captureException (Ljava/lang/Throwable;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public fun captureException (Ljava/lang/Throwable;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; + public fun captureFeedback (Lio/sentry/protocol/Feedback;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;)Lio/sentry/protocol/SentryId; public fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public fun captureProfileChunk (Lio/sentry/ProfileChunk;)Lio/sentry/protocol/SentryId; @@ -2395,6 +2407,9 @@ public final class io/sentry/ScopesAdapter : io/sentry/IScopes { public fun captureEvent (Lio/sentry/SentryEvent;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public fun captureException (Ljava/lang/Throwable;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public fun captureException (Ljava/lang/Throwable;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; + public fun captureFeedback (Lio/sentry/protocol/Feedback;)Lio/sentry/protocol/SentryId; + public fun captureFeedback (Lio/sentry/protocol/Feedback;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; + public fun captureFeedback (Lio/sentry/protocol/Feedback;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;)Lio/sentry/protocol/SentryId; public fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public fun captureProfileChunk (Lio/sentry/ProfileChunk;)Lio/sentry/protocol/SentryId; @@ -2504,6 +2519,9 @@ public final class io/sentry/Sentry { public static fun captureException (Ljava/lang/Throwable;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public static fun captureException (Ljava/lang/Throwable;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public static fun captureException (Ljava/lang/Throwable;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; + public static fun captureFeedback (Lio/sentry/protocol/Feedback;)Lio/sentry/protocol/SentryId; + public static fun captureFeedback (Lio/sentry/protocol/Feedback;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; + public static fun captureFeedback (Lio/sentry/protocol/Feedback;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public static fun captureMessage (Ljava/lang/String;)Lio/sentry/protocol/SentryId; public static fun captureMessage (Ljava/lang/String;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public static fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;)Lio/sentry/protocol/SentryId; @@ -2702,6 +2720,7 @@ public final class io/sentry/SentryClient : io/sentry/ISentryClient { public fun captureCheckIn (Lio/sentry/CheckIn;Lio/sentry/IScope;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public fun captureEnvelope (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public fun captureEvent (Lio/sentry/SentryEvent;Lio/sentry/IScope;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; + public fun captureFeedback (Lio/sentry/protocol/Feedback;Lio/sentry/Hint;Lio/sentry/IScope;)Lio/sentry/protocol/SentryId; public fun captureProfileChunk (Lio/sentry/ProfileChunk;Lio/sentry/IScope;)Lio/sentry/protocol/SentryId; public fun captureReplayEvent (Lio/sentry/SentryReplayEvent;Lio/sentry/IScope;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public fun captureSession (Lio/sentry/Session;Lio/sentry/Hint;)V @@ -3050,6 +3069,7 @@ public class io/sentry/SentryOptions { public fun getBeforeBreadcrumb ()Lio/sentry/SentryOptions$BeforeBreadcrumbCallback; public fun getBeforeEnvelopeCallback ()Lio/sentry/SentryOptions$BeforeEnvelopeCallback; public fun getBeforeSend ()Lio/sentry/SentryOptions$BeforeSendCallback; + public fun getBeforeSendFeedback ()Lio/sentry/SentryOptions$BeforeSendCallback; public fun getBeforeSendReplay ()Lio/sentry/SentryOptions$BeforeSendReplayCallback; public fun getBeforeSendTransaction ()Lio/sentry/SentryOptions$BeforeSendTransactionCallback; public fun getBundleIds ()Ljava/util/Set; @@ -3179,6 +3199,7 @@ public class io/sentry/SentryOptions { public fun setBeforeBreadcrumb (Lio/sentry/SentryOptions$BeforeBreadcrumbCallback;)V public fun setBeforeEnvelopeCallback (Lio/sentry/SentryOptions$BeforeEnvelopeCallback;)V public fun setBeforeSend (Lio/sentry/SentryOptions$BeforeSendCallback;)V + public fun setBeforeSendFeedback (Lio/sentry/SentryOptions$BeforeSendCallback;)V public fun setBeforeSendReplay (Lio/sentry/SentryOptions$BeforeSendReplayCallback;)V public fun setBeforeSendTransaction (Lio/sentry/SentryOptions$BeforeSendTransactionCallback;)V public fun setCacheDirPath (Ljava/lang/String;)V @@ -4717,6 +4738,7 @@ public class io/sentry/protocol/Contexts : io/sentry/JsonSerializable { public fun getApp ()Lio/sentry/protocol/App; public fun getBrowser ()Lio/sentry/protocol/Browser; public fun getDevice ()Lio/sentry/protocol/Device; + public fun getFeedback ()Lio/sentry/protocol/Feedback; public fun getGpu ()Lio/sentry/protocol/Gpu; public fun getOperatingSystem ()Lio/sentry/protocol/OperatingSystem; public fun getProfile ()Lio/sentry/ProfileContext; @@ -4737,6 +4759,7 @@ public class io/sentry/protocol/Contexts : io/sentry/JsonSerializable { public fun setApp (Lio/sentry/protocol/App;)V public fun setBrowser (Lio/sentry/protocol/Browser;)V public fun setDevice (Lio/sentry/protocol/Device;)V + public fun setFeedback (Lio/sentry/protocol/Feedback;)V public fun setGpu (Lio/sentry/protocol/Gpu;)V public fun setOperatingSystem (Lio/sentry/protocol/OperatingSystem;)V public fun setProfile (Lio/sentry/ProfileContext;)V @@ -4958,6 +4981,45 @@ public final class io/sentry/protocol/Device$JsonKeys { public fun ()V } +public final class io/sentry/protocol/Feedback : io/sentry/JsonSerializable, io/sentry/JsonUnknown { + public static final field TYPE Ljava/lang/String; + public fun (Lio/sentry/protocol/Feedback;)V + public fun (Ljava/lang/String;)V + public fun equals (Ljava/lang/Object;)Z + public fun getAssociatedEventId ()Lio/sentry/protocol/SentryId; + public fun getContactEmail ()Ljava/lang/String; + public fun getMessage ()Ljava/lang/String; + public fun getName ()Ljava/lang/String; + public fun getReplayId ()Lio/sentry/protocol/SentryId; + public fun getUnknown ()Ljava/util/Map; + public fun getUrl ()Ljava/lang/String; + public fun hashCode ()I + public fun serialize (Lio/sentry/ObjectWriter;Lio/sentry/ILogger;)V + public fun setAssociatedEventId (Lio/sentry/protocol/SentryId;)V + public fun setContactEmail (Ljava/lang/String;)V + public fun setMessage (Ljava/lang/String;)V + public fun setName (Ljava/lang/String;)V + public fun setReplayId (Lio/sentry/protocol/SentryId;)V + public fun setUnknown (Ljava/util/Map;)V + public fun setUrl (Ljava/lang/String;)V +} + +public final class io/sentry/protocol/Feedback$Deserializer : io/sentry/JsonDeserializer { + public fun ()V + public fun deserialize (Lio/sentry/ObjectReader;Lio/sentry/ILogger;)Lio/sentry/protocol/Feedback; + public synthetic fun deserialize (Lio/sentry/ObjectReader;Lio/sentry/ILogger;)Ljava/lang/Object; +} + +public final class io/sentry/protocol/Feedback$JsonKeys { + public static final field ASSOCIATED_EVENT_ID Ljava/lang/String; + public static final field CONTACT_EMAIL Ljava/lang/String; + public static final field MESSAGE Ljava/lang/String; + public static final field NAME Ljava/lang/String; + public static final field REPLAY_ID Ljava/lang/String; + public static final field URL Ljava/lang/String; + public fun ()V +} + public final class io/sentry/protocol/Geo : io/sentry/JsonSerializable, io/sentry/JsonUnknown { public fun ()V public fun (Lio/sentry/protocol/Geo;)V diff --git a/sentry/src/main/java/io/sentry/Baggage.java b/sentry/src/main/java/io/sentry/Baggage.java index 52b7016e6d..5f610a0291 100644 --- a/sentry/src/main/java/io/sentry/Baggage.java +++ b/sentry/src/main/java/io/sentry/Baggage.java @@ -177,14 +177,16 @@ public static Baggage fromHeader( @ApiStatus.Internal @NotNull public static Baggage fromEvent( - final @NotNull SentryEvent event, final @NotNull SentryOptions options) { + final @NotNull SentryBaseEvent event, + final @Nullable String transaction, + final @NotNull SentryOptions options) { final Baggage baggage = new Baggage(options.getLogger()); final SpanContext trace = event.getContexts().getTrace(); baggage.setTraceId(trace != null ? trace.getTraceId().toString() : null); baggage.setPublicKey(options.retrieveParsedDsn().getPublicKey()); baggage.setRelease(event.getRelease()); baggage.setEnvironment(event.getEnvironment()); - baggage.setTransaction(event.getTransaction()); + baggage.setTransaction(transaction); // we don't persist sample rate baggage.setSampleRate(null); baggage.setSampled(null); diff --git a/sentry/src/main/java/io/sentry/DataCategory.java b/sentry/src/main/java/io/sentry/DataCategory.java index 234ba25d64..da10996059 100644 --- a/sentry/src/main/java/io/sentry/DataCategory.java +++ b/sentry/src/main/java/io/sentry/DataCategory.java @@ -8,6 +8,7 @@ public enum DataCategory { All("__all__"), Default("default"), // same as Error Error("error"), + Feedback("feedback"), Session("session"), Attachment("attachment"), Monitor("monitor"), diff --git a/sentry/src/main/java/io/sentry/HubAdapter.java b/sentry/src/main/java/io/sentry/HubAdapter.java index 4e16628ee3..c5eef832c6 100644 --- a/sentry/src/main/java/io/sentry/HubAdapter.java +++ b/sentry/src/main/java/io/sentry/HubAdapter.java @@ -1,5 +1,6 @@ package io.sentry; +import io.sentry.protocol.Feedback; import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; @@ -50,6 +51,22 @@ public boolean isEnabled() { return Sentry.captureMessage(message, level, callback); } + @Override + public @NotNull SentryId captureFeedback(@NotNull Feedback feedback) { + return Sentry.captureFeedback(feedback); + } + + @Override + public @NotNull SentryId captureFeedback(@NotNull Feedback feedback, @Nullable Hint hint) { + return Sentry.captureFeedback(feedback, hint); + } + + @Override + public @NotNull SentryId captureFeedback( + @NotNull Feedback feedback, @Nullable Hint hint, @Nullable ScopeCallback callback) { + return Sentry.captureFeedback(feedback, hint, callback); + } + @ApiStatus.Internal @Override public @NotNull SentryId captureEnvelope(@NotNull SentryEnvelope envelope, @Nullable Hint hint) { diff --git a/sentry/src/main/java/io/sentry/HubScopesWrapper.java b/sentry/src/main/java/io/sentry/HubScopesWrapper.java index 3c623731a4..3555df9004 100644 --- a/sentry/src/main/java/io/sentry/HubScopesWrapper.java +++ b/sentry/src/main/java/io/sentry/HubScopesWrapper.java @@ -1,5 +1,6 @@ package io.sentry; +import io.sentry.protocol.Feedback; import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; @@ -46,6 +47,12 @@ public boolean isEnabled() { return scopes.captureMessage(message, level, callback); } + @Override + public @NotNull SentryId captureFeedback( + @NotNull Feedback feedback, @Nullable Hint hint, @Nullable ScopeCallback callback) { + return scopes.captureFeedback(feedback, hint, callback); + } + @Override public @NotNull SentryId captureEnvelope(@NotNull SentryEnvelope envelope, @Nullable Hint hint) { return scopes.captureEnvelope(envelope, hint); diff --git a/sentry/src/main/java/io/sentry/IScopes.java b/sentry/src/main/java/io/sentry/IScopes.java index 40884f6394..230c1058a8 100644 --- a/sentry/src/main/java/io/sentry/IScopes.java +++ b/sentry/src/main/java/io/sentry/IScopes.java @@ -1,5 +1,6 @@ package io.sentry; +import io.sentry.protocol.Feedback; import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; @@ -108,6 +109,42 @@ SentryId captureMessage( return captureMessage(message, SentryLevel.INFO, callback); } + /** + * Captures the feedback. + * + * @param feedback The feedback to send. + * @return The Id (SentryId object) of the event + */ + default @NotNull SentryId captureFeedback(final @NotNull Feedback feedback) { + return captureFeedback(feedback, null); + } + + /** + * Captures the feedback. + * + * @param feedback The feedback to send. + * @param hint An optional hint to be applied to the event. + * @return The Id (SentryId object) of the event + */ + default @NotNull SentryId captureFeedback( + final @NotNull Feedback feedback, final @Nullable Hint hint) { + return captureFeedback(feedback, hint, null); + } + + /** + * Captures the feedback. + * + * @param feedback The feedback to send. + * @param hint An optional hint to be applied to the event. + * @param callback The callback to configure the scope for a single invocation. + * @return The Id (SentryId object) of the event + */ + @NotNull + SentryId captureFeedback( + final @NotNull Feedback feedback, + final @Nullable Hint hint, + final @Nullable ScopeCallback callback); + /** * Captures an envelope. * diff --git a/sentry/src/main/java/io/sentry/ISentryClient.java b/sentry/src/main/java/io/sentry/ISentryClient.java index 198f77f2f0..7ec3aebfbf 100644 --- a/sentry/src/main/java/io/sentry/ISentryClient.java +++ b/sentry/src/main/java/io/sentry/ISentryClient.java @@ -1,5 +1,6 @@ package io.sentry; +import io.sentry.protocol.Feedback; import io.sentry.protocol.Message; import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; @@ -78,6 +79,17 @@ public interface ISentryClient { return captureEvent(event, null, hint); } + /** + * Captures the feedback. + * + * @param feedback The feedback to send. + * @param hint An optional hint to be applied to the event. + * @param scope An optional scope to be applied to the event. + * @return The Id (SentryId object) of the event + */ + @NotNull + SentryId captureFeedback(@NotNull Feedback feedback, @Nullable Hint hint, @NotNull IScope scope); + /** * Captures the message. * diff --git a/sentry/src/main/java/io/sentry/JsonSerializer.java b/sentry/src/main/java/io/sentry/JsonSerializer.java index bb2d356a1d..61bd366cf7 100644 --- a/sentry/src/main/java/io/sentry/JsonSerializer.java +++ b/sentry/src/main/java/io/sentry/JsonSerializer.java @@ -9,6 +9,7 @@ import io.sentry.protocol.DebugImage; import io.sentry.protocol.DebugMeta; import io.sentry.protocol.Device; +import io.sentry.protocol.Feedback; import io.sentry.protocol.Geo; import io.sentry.protocol.Gpu; import io.sentry.protocol.MeasurementValue; @@ -84,6 +85,7 @@ public JsonSerializer(@NotNull SentryOptions options) { deserializersByClass.put(Device.class, new Device.Deserializer()); deserializersByClass.put( Device.DeviceOrientation.class, new Device.DeviceOrientation.Deserializer()); + deserializersByClass.put(Feedback.class, new Feedback.Deserializer()); deserializersByClass.put(Gpu.class, new Gpu.Deserializer()); deserializersByClass.put(MeasurementValue.class, new MeasurementValue.Deserializer()); deserializersByClass.put(Mechanism.class, new Mechanism.Deserializer()); diff --git a/sentry/src/main/java/io/sentry/NoOpHub.java b/sentry/src/main/java/io/sentry/NoOpHub.java index 8ff8dd7675..c1cd2661b2 100644 --- a/sentry/src/main/java/io/sentry/NoOpHub.java +++ b/sentry/src/main/java/io/sentry/NoOpHub.java @@ -1,5 +1,6 @@ package io.sentry; +import io.sentry.protocol.Feedback; import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; @@ -53,6 +54,12 @@ public boolean isEnabled() { return SentryId.EMPTY_ID; } + @Override + public @NotNull SentryId captureFeedback( + @NotNull Feedback feedback, @Nullable Hint hint, @Nullable ScopeCallback callback) { + return SentryId.EMPTY_ID; + } + @Override public @NotNull SentryId captureEnvelope(@NotNull SentryEnvelope envelope, @Nullable Hint hint) { return SentryId.EMPTY_ID; diff --git a/sentry/src/main/java/io/sentry/NoOpScopes.java b/sentry/src/main/java/io/sentry/NoOpScopes.java index bd3d959de3..c4326117af 100644 --- a/sentry/src/main/java/io/sentry/NoOpScopes.java +++ b/sentry/src/main/java/io/sentry/NoOpScopes.java @@ -1,5 +1,6 @@ package io.sentry; +import io.sentry.protocol.Feedback; import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; @@ -48,6 +49,12 @@ public boolean isEnabled() { return SentryId.EMPTY_ID; } + @Override + public @NotNull SentryId captureFeedback( + @NotNull Feedback feedback, @Nullable Hint hint, @Nullable ScopeCallback callback) { + return SentryId.EMPTY_ID; + } + @Override public @NotNull SentryId captureEnvelope(@NotNull SentryEnvelope envelope, @Nullable Hint hint) { return SentryId.EMPTY_ID; diff --git a/sentry/src/main/java/io/sentry/NoOpSentryClient.java b/sentry/src/main/java/io/sentry/NoOpSentryClient.java index 4bee000805..7050a524d1 100644 --- a/sentry/src/main/java/io/sentry/NoOpSentryClient.java +++ b/sentry/src/main/java/io/sentry/NoOpSentryClient.java @@ -1,5 +1,6 @@ package io.sentry; +import io.sentry.protocol.Feedback; import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; import io.sentry.transport.RateLimiter; @@ -37,6 +38,12 @@ public void close() {} @Override public void flush(long timeoutMillis) {} + @Override + public @NotNull SentryId captureFeedback( + @NotNull Feedback feedback, @Nullable Hint hint, @NotNull IScope scope) { + return SentryId.EMPTY_ID; + } + @Override public void captureUserFeedback(@NotNull UserFeedback userFeedback) {} diff --git a/sentry/src/main/java/io/sentry/Scopes.java b/sentry/src/main/java/io/sentry/Scopes.java index 002043c3df..2031d27be1 100644 --- a/sentry/src/main/java/io/sentry/Scopes.java +++ b/sentry/src/main/java/io/sentry/Scopes.java @@ -3,6 +3,7 @@ import io.sentry.clientreport.DiscardReason; import io.sentry.hints.SessionEndHint; import io.sentry.hints.SessionStartHint; +import io.sentry.protocol.Feedback; import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; @@ -170,7 +171,7 @@ private void assignTraceContext(final @NotNull SentryEvent event) { getCombinedScopeView().assignTraceContext(event); } - private IScope buildLocalScope( + private @NotNull IScope buildLocalScope( final @NotNull IScope parentScope, final @Nullable ScopeCallback callback) { if (callback != null) { try { @@ -230,6 +231,36 @@ private IScope buildLocalScope( return sentryId; } + @Override + public @NotNull SentryId captureFeedback( + final @NotNull Feedback feedback, + final @Nullable Hint hint, + final @Nullable ScopeCallback scopeCallback) { + SentryId sentryId = SentryId.EMPTY_ID; + if (!isEnabled()) { + getOptions() + .getLogger() + .log( + SentryLevel.WARNING, + "Instance is disabled and this 'captureFeedback' call is a no-op."); + } else if (feedback.getMessage().isEmpty()) { + getOptions() + .getLogger() + .log(SentryLevel.WARNING, "captureFeedback called with empty message."); + } else { + try { + final @NotNull IScope localScope = buildLocalScope(getCombinedScopeView(), scopeCallback); + + sentryId = getClient().captureFeedback(feedback, hint, localScope); + } catch (Throwable e) { + getOptions() + .getLogger() + .log(SentryLevel.ERROR, "Error while capturing feedback: " + feedback.getMessage(), e); + } + } + return sentryId; + } + @ApiStatus.Internal @Override public @NotNull SentryId captureEnvelope( diff --git a/sentry/src/main/java/io/sentry/ScopesAdapter.java b/sentry/src/main/java/io/sentry/ScopesAdapter.java index 7b50a0d3d3..b3eaeb2b38 100644 --- a/sentry/src/main/java/io/sentry/ScopesAdapter.java +++ b/sentry/src/main/java/io/sentry/ScopesAdapter.java @@ -1,5 +1,6 @@ package io.sentry; +import io.sentry.protocol.Feedback; import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; @@ -46,6 +47,22 @@ public boolean isEnabled() { return Sentry.captureMessage(message, level, callback); } + @Override + public @NotNull SentryId captureFeedback(@NotNull Feedback feedback) { + return Sentry.captureFeedback(feedback); + } + + @Override + public @NotNull SentryId captureFeedback(@NotNull Feedback feedback, @Nullable Hint hint) { + return Sentry.captureFeedback(feedback, hint); + } + + @Override + public @NotNull SentryId captureFeedback( + @NotNull Feedback feedback, @Nullable Hint hint, @Nullable ScopeCallback callback) { + return Sentry.captureFeedback(feedback, hint, callback); + } + @ApiStatus.Internal @Override public @NotNull SentryId captureEnvelope(@NotNull SentryEnvelope envelope, @Nullable Hint hint) { diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 70d4c7b380..f961d466da 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -14,6 +14,7 @@ import io.sentry.internal.modules.NoOpModulesLoader; import io.sentry.internal.modules.ResourcesModulesLoader; import io.sentry.opentelemetry.OpenTelemetryUtil; +import io.sentry.protocol.Feedback; import io.sentry.protocol.SentryId; import io.sentry.protocol.User; import io.sentry.transport.NoOpEnvelopeCache; @@ -759,6 +760,43 @@ public static void close() { return getCurrentScopes().captureMessage(message, level, callback); } + /** + * Captures the feedback. + * + * @param feedback The feedback to send. + * @return The Id (SentryId object) of the event + */ + public static @NotNull SentryId captureFeedback(final @NotNull Feedback feedback) { + return getCurrentScopes().captureFeedback(feedback); + } + + /** + * Captures the feedback. + * + * @param feedback The feedback to send. + * @param hint An optional hint to be applied to the event. + * @return The Id (SentryId object) of the event + */ + public static @NotNull SentryId captureFeedback( + final @NotNull Feedback feedback, final @Nullable Hint hint) { + return getCurrentScopes().captureFeedback(feedback, hint); + } + + /** + * Captures the feedback. + * + * @param feedback The feedback to send. + * @param hint An optional hint to be applied to the event. + * @param callback The callback to configure the scope for a single invocation. + * @return The Id (SentryId object) of the event + */ + public static @NotNull SentryId captureFeedback( + final @NotNull Feedback feedback, + final @Nullable Hint hint, + final @Nullable ScopeCallback callback) { + return getCurrentScopes().captureFeedback(feedback, hint, callback); + } + /** * Captures the exception. * diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index bb8c2b4605..77438e943d 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -8,6 +8,7 @@ import io.sentry.hints.TransactionEnd; import io.sentry.protocol.Contexts; import io.sentry.protocol.DebugMeta; +import io.sentry.protocol.Feedback; import io.sentry.protocol.SentryId; import io.sentry.protocol.SentryTransaction; import io.sentry.transport.ITransport; @@ -207,24 +208,7 @@ private boolean shouldApplyScopeData(final @NotNull CheckIn event, final @NotNul } try { - @Nullable TraceContext traceContext = null; - if (isBackfillable) { - // for backfillable hint we synthesize Baggage from event values - if (event != null) { - final Baggage baggage = Baggage.fromEvent(event, options); - traceContext = baggage.toTraceContext(); - } - } else if (scope != null) { - final @Nullable ITransaction transaction = scope.getTransaction(); - if (transaction != null) { - traceContext = transaction.traceContext(); - } else { - final @NotNull PropagationContext propagationContext = - TracingUtils.maybeUpdateBaggage(scope, options); - traceContext = propagationContext.traceContext(); - } - } - + final @Nullable TraceContext traceContext = getTraceContext(scope, hint, event); final boolean shouldSendAttachments = event != null; List attachments = shouldSendAttachments ? getAttachments(hint) : null; final @Nullable SentryEnvelope envelope = @@ -303,19 +287,7 @@ private void finalizeTransaction(final @NotNull IScope scope, final @NotNull Hin } try { - // TODO: check if event is Backfillable and backfill traceContext from the event values - @Nullable TraceContext traceContext = null; - if (scope != null) { - final @Nullable ITransaction transaction = scope.getTransaction(); - if (transaction != null) { - traceContext = transaction.traceContext(); - } else { - final @NotNull PropagationContext propagationContext = - TracingUtils.maybeUpdateBaggage(scope, options); - traceContext = propagationContext.traceContext(); - } - } - + final @Nullable TraceContext traceContext = getTraceContext(scope, hint, event, null); final boolean cleanupReplayFolder = HintUtils.hasType(hint, Backfillable.class); final SentryEnvelope envelope = buildEnvelope(event, hint.getReplayRecording(), traceContext, cleanupReplayFolder); @@ -571,6 +543,40 @@ private SentryReplayEvent processReplayEvent( return replayEvent; } + @Nullable + private SentryEvent processFeedbackEvent( + @NotNull SentryEvent feedbackEvent, + final @NotNull Hint hint, + final @NotNull List eventProcessors) { + for (final EventProcessor processor : eventProcessors) { + try { + feedbackEvent = processor.process(feedbackEvent, hint); + } catch (Throwable e) { + options + .getLogger() + .log( + SentryLevel.ERROR, + e, + "An exception occurred while processing feedback event by processor: %s", + processor.getClass().getName()); + } + + if (feedbackEvent == null) { + options + .getLogger() + .log( + SentryLevel.DEBUG, + "Feedback event was dropped by a processor: %s", + processor.getClass().getName()); + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.EVENT_PROCESSOR, DataCategory.Feedback); + break; + } + } + return feedbackEvent; + } + @Override public void captureUserFeedback(final @NotNull UserFeedback userFeedback) { Objects.requireNonNull(userFeedback, "SentryEvent is required."); @@ -970,18 +976,7 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint SentryId sentryId = checkIn.getCheckInId(); try { - @Nullable TraceContext traceContext = null; - if (scope != null) { - final @Nullable ITransaction transaction = scope.getTransaction(); - if (transaction != null) { - traceContext = transaction.traceContext(); - } else { - final @NotNull PropagationContext propagationContext = - TracingUtils.maybeUpdateBaggage(scope, options); - traceContext = propagationContext.traceContext(); - } - } - + final @Nullable TraceContext traceContext = getTraceContext(scope, hint, null); final @NotNull SentryEnvelope envelope = buildEnvelope(checkIn, traceContext); hint.clear(); @@ -995,6 +990,124 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint return sentryId; } + /** + * Captures the feedback. + * + * @param feedback The feedback to send. + * @param hint An optional hint to be applied to the event. + * @param scope An optional scope to be applied to the event. + * @return The Id (SentryId object) of the event + */ + @Override + public @NotNull SentryId captureFeedback( + final @NotNull Feedback feedback, @Nullable Hint hint, final @NotNull IScope scope) { + SentryEvent event = new SentryEvent(); + event.getContexts().setFeedback(feedback); + + if (hint == null) { + hint = new Hint(); + } + + if (feedback.getUrl() == null) { + feedback.setUrl(scope.getScreen()); + } + + options.getLogger().log(SentryLevel.DEBUG, "Capturing feedback: %s", event.getEventId()); + + if (shouldApplyScopeData(event, hint)) { + // Event has already passed through here before it was cached + // Going through again could be reading data that is no longer relevant + // i.e proguard id, app version, threads + event = applyFeedbackScope(event, scope, hint); + + if (event == null) { + options.getLogger().log(SentryLevel.DEBUG, "Feedback was dropped by applyScope"); + return SentryId.EMPTY_ID; + } + } + + event = processFeedbackEvent(event, hint, options.getEventProcessors()); + + if (event != null) { + event = executeBeforeSendFeedback(event, hint); + + if (event == null) { + options.getLogger().log(SentryLevel.DEBUG, "Event was dropped by beforeSend"); + options + .getClientReportRecorder() + .recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.Feedback); + } + } + + if (event == null) { + return SentryId.EMPTY_ID; + } + + SentryId sentryId = SentryId.EMPTY_ID; + if (event.getEventId() != null) { + sentryId = event.getEventId(); + } + + // If feedback already has a replayId, we don't want to overwrite it. + if (feedback.getReplayId() == null) { + options.getReplayController().captureReplay(false); + final @NotNull SentryId replayId = scope.getReplayId(); + if (!replayId.equals(SentryId.EMPTY_ID)) { + feedback.setReplayId(replayId); + } + } + + try { + final @Nullable TraceContext traceContext = getTraceContext(scope, hint, event); + final List attachments = getAttachments(hint); + final @Nullable SentryEnvelope envelope = + buildEnvelope(event, attachments, null, traceContext, null); + + hint.clear(); + if (envelope != null) { + sentryId = sendEnvelope(envelope, hint); + } + } catch (IOException | SentryEnvelopeException e) { + options.getLogger().log(SentryLevel.WARNING, e, "Capturing feedback %s failed.", sentryId); + + // if there was an error capturing the event, we return an emptyId + sentryId = SentryId.EMPTY_ID; + } + + return sentryId; + } + + private @Nullable TraceContext getTraceContext( + final @Nullable IScope scope, final @NotNull Hint hint, final @Nullable SentryEvent event) { + return getTraceContext(scope, hint, event, event != null ? event.getTransaction() : null); + } + + private @Nullable TraceContext getTraceContext( + final @Nullable IScope scope, + final @NotNull Hint hint, + final @Nullable SentryBaseEvent event, + final @Nullable String txn) { + @Nullable TraceContext traceContext = null; + final boolean isBackfillable = HintUtils.hasType(hint, Backfillable.class); + if (isBackfillable) { + // for backfillable hint we synthesize Baggage from event values + if (event != null) { + final Baggage baggage = Baggage.fromEvent(event, txn, options); + traceContext = baggage.toTraceContext(); + } + } else if (scope != null) { + final @Nullable ITransaction transaction = scope.getTransaction(); + if (transaction != null) { + traceContext = transaction.traceContext(); + } else { + final @NotNull PropagationContext propagationContext = + TracingUtils.maybeUpdateBaggage(scope, options); + traceContext = propagationContext.traceContext(); + } + } + return traceContext; + } + private @Nullable List filterForTransaction(@Nullable List attachments) { if (attachments == null) { return null; @@ -1042,6 +1155,43 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint return event; } + private @Nullable SentryEvent applyFeedbackScope( + @NotNull SentryEvent event, final @NotNull IScope scope, final @NotNull Hint hint) { + + if (event.getUser() == null) { + event.setUser(scope.getUser()); + } + if (event.getTags() == null) { + event.setTags(new HashMap<>(scope.getTags())); + } else { + for (Map.Entry item : scope.getTags().entrySet()) { + if (!event.getTags().containsKey(item.getKey())) { + event.getTags().put(item.getKey(), item.getValue()); + } + } + } + final Contexts contexts = event.getContexts(); + for (Map.Entry entry : new Contexts(scope.getContexts()).entrySet()) { + if (!contexts.containsKey(entry.getKey())) { + contexts.put(entry.getKey(), entry.getValue()); + } + } + // Set trace data from active span to connect events with transactions + final ISpan span = scope.getSpan(); + if (event.getContexts().getTrace() == null) { + if (span == null) { + event + .getContexts() + .setTrace(TransactionContext.fromPropagationContext(scope.getPropagationContext())); + } else { + event.getContexts().setTrace(span.getSpanContext()); + } + } + + event = processFeedbackEvent(event, hint, scope.getEventProcessors()); + return event; + } + private @NotNull CheckIn applyScope(@NotNull CheckIn checkIn, final @Nullable IScope scope) { if (scope != null) { // Set trace data from active span to connect events with transactions @@ -1195,6 +1345,24 @@ private void sortBreadcrumbsByDate( return transaction; } + private @Nullable SentryEvent executeBeforeSendFeedback( + @NotNull SentryEvent event, final @NotNull Hint hint) { + final SentryOptions.BeforeSendCallback beforeSendFeedback = options.getBeforeSendFeedback(); + if (beforeSendFeedback != null) { + try { + event = beforeSendFeedback.execute(event, hint); + } catch (Throwable e) { + options + .getLogger() + .log(SentryLevel.ERROR, "The BeforeSendFeedback callback threw an exception.", e); + + // drop feedback in case of an error in beforeSend due to PII concerns + event = null; + } + } + return event; + } + private @Nullable SentryReplayEvent executeBeforeSendReplay( @NotNull SentryReplayEvent event, final @NotNull Hint hint) { final SentryOptions.BeforeSendReplayCallback beforeSendReplay = options.getBeforeSendReplay(); diff --git a/sentry/src/main/java/io/sentry/SentryItemType.java b/sentry/src/main/java/io/sentry/SentryItemType.java index 068c37ab1f..f65e54eed1 100644 --- a/sentry/src/main/java/io/sentry/SentryItemType.java +++ b/sentry/src/main/java/io/sentry/SentryItemType.java @@ -28,7 +28,7 @@ public enum SentryItemType implements JsonSerializable { public static SentryItemType resolve(Object item) { if (item instanceof SentryEvent) { - return Event; + return ((SentryEvent) item).getContexts().getFeedback() == null ? Event : Feedback; } else if (item instanceof SentryTransaction) { return Transaction; } else if (item instanceof Session) { diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 54e37a405c..801f46c8c1 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -155,6 +155,12 @@ public class SentryOptions { */ private @Nullable BeforeSendCallback beforeSend; + /** + * This function is called with an SDK specific feedback object and can return a modified feedback + * object or nothing to skip reporting the feedback + */ + private @Nullable BeforeSendCallback beforeSendFeedback; + /** * This function is called with an SDK specific transaction object and can return a modified * transaction object or nothing to skip reporting the transaction @@ -830,6 +836,24 @@ public void setBeforeSendTransaction( this.beforeSendTransaction = beforeSendTransaction; } + /** + * Returns the BeforeSendFeedback callback + * + * @return the beforeSendFeedback callback or null if not set + */ + public @Nullable BeforeSendCallback getBeforeSendFeedback() { + return beforeSendFeedback; + } + + /** + * Sets the beforeSendFeedback callback + * + * @param beforeSendFeedback the beforeSendFeedback callback + */ + public void setBeforeSendFeedback(@Nullable BeforeSendCallback beforeSendFeedback) { + this.beforeSendFeedback = beforeSendFeedback; + } + /** * Returns the BeforeSendReplay callback * diff --git a/sentry/src/main/java/io/sentry/clientreport/ClientReportRecorder.java b/sentry/src/main/java/io/sentry/clientreport/ClientReportRecorder.java index 452e2837b9..5e188be9a0 100644 --- a/sentry/src/main/java/io/sentry/clientreport/ClientReportRecorder.java +++ b/sentry/src/main/java/io/sentry/clientreport/ClientReportRecorder.java @@ -162,6 +162,9 @@ private DataCategory categoryFromItemType(SentryItemType itemType) { if (SentryItemType.UserFeedback.equals(itemType)) { return DataCategory.UserReport; } + if (SentryItemType.Feedback.equals(itemType)) { + return DataCategory.Feedback; + } if (SentryItemType.Profile.equals(itemType)) { return DataCategory.Profile; } diff --git a/sentry/src/main/java/io/sentry/protocol/Contexts.java b/sentry/src/main/java/io/sentry/protocol/Contexts.java index 375ad0f05b..e97431db4d 100644 --- a/sentry/src/main/java/io/sentry/protocol/Contexts.java +++ b/sentry/src/main/java/io/sentry/protocol/Contexts.java @@ -52,6 +52,8 @@ public Contexts(final @NotNull Contexts contexts) { this.setOperatingSystem(new OperatingSystem((OperatingSystem) value)); } else if (SentryRuntime.TYPE.equals(entry.getKey()) && value instanceof SentryRuntime) { this.setRuntime(new SentryRuntime((SentryRuntime) value)); + } else if (Feedback.TYPE.equals(entry.getKey()) && value instanceof Feedback) { + this.setFeedback(new Feedback((Feedback) value)); } else if (Gpu.TYPE.equals(entry.getKey()) && value instanceof Gpu) { this.setGpu(new Gpu((Gpu) value)); } else if (SpanContext.TYPE.equals(entry.getKey()) && value instanceof SpanContext) { @@ -132,6 +134,14 @@ public void setRuntime(final @NotNull SentryRuntime runtime) { this.put(SentryRuntime.TYPE, runtime); } + public @Nullable Feedback getFeedback() { + return toContextType(Feedback.TYPE, Feedback.class); + } + + public void setFeedback(final @NotNull Feedback feedback) { + this.put(Feedback.TYPE, feedback); + } + public @Nullable Gpu getGpu() { return toContextType(Gpu.TYPE, Gpu.class); } @@ -314,6 +324,9 @@ public static final class Deserializer implements JsonDeserializer { case SentryRuntime.TYPE: contexts.setRuntime(new SentryRuntime.Deserializer().deserialize(reader, logger)); break; + case Feedback.TYPE: + contexts.setFeedback(new Feedback.Deserializer().deserialize(reader, logger)); + break; case SpanContext.TYPE: contexts.setTrace(new SpanContext.Deserializer().deserialize(reader, logger)); break; diff --git a/sentry/src/main/java/io/sentry/protocol/Feedback.java b/sentry/src/main/java/io/sentry/protocol/Feedback.java new file mode 100644 index 0000000000..5cf6097f1e --- /dev/null +++ b/sentry/src/main/java/io/sentry/protocol/Feedback.java @@ -0,0 +1,238 @@ +package io.sentry.protocol; + +import io.sentry.ILogger; +import io.sentry.JsonDeserializer; +import io.sentry.JsonSerializable; +import io.sentry.JsonUnknown; +import io.sentry.ObjectReader; +import io.sentry.ObjectWriter; +import io.sentry.SentryLevel; +import io.sentry.util.CollectionUtils; +import io.sentry.util.Objects; +import io.sentry.vendor.gson.stream.JsonToken; +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +// Specs can be found at https://develop.sentry.dev/sdk/data-model/envelope-items/#user-feedback + +public final class Feedback implements JsonUnknown, JsonSerializable { + public static final String TYPE = "feedback"; + + private @NotNull String message; + private @Nullable String contactEmail; + private @Nullable String name; + private @Nullable SentryId associatedEventId; + private @Nullable SentryId replayId; + private @Nullable String url; + + private @Nullable Map unknown; + + public Feedback(final @NotNull String message) { + setMessage(message); + } + + public Feedback(final @NotNull Feedback feedback) { + this.message = feedback.message; + this.contactEmail = feedback.contactEmail; + this.name = feedback.name; + this.associatedEventId = feedback.associatedEventId; + this.replayId = feedback.replayId; + this.url = feedback.url; + this.unknown = CollectionUtils.newConcurrentHashMap(feedback.unknown); + } + + public @Nullable String getContactEmail() { + return contactEmail; + } + + public void setContactEmail(final @Nullable String contactEmail) { + this.contactEmail = contactEmail; + } + + public @Nullable String getName() { + return name; + } + + public void setName(final @Nullable String name) { + this.name = name; + } + + public @Nullable SentryId getAssociatedEventId() { + return associatedEventId; + } + + public void setAssociatedEventId(final @NotNull SentryId associatedEventId) { + this.associatedEventId = associatedEventId; + } + + public @Nullable SentryId getReplayId() { + return replayId; + } + + public void setReplayId(final @NotNull SentryId replayId) { + this.replayId = replayId; + } + + public @Nullable String getUrl() { + return url; + } + + public void setUrl(final @Nullable String url) { + this.url = url; + } + + public @NotNull String getMessage() { + return message; + } + + public void setMessage(final @NotNull String message) { + // Sentry limits the message to 4096 characters + if (message.length() > 4096) { + this.message = message.substring(0, 4096); + } else { + this.message = message; + } + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof Feedback)) return false; + Feedback feedback = (Feedback) o; + return Objects.equals(message, feedback.message) + && Objects.equals(contactEmail, feedback.contactEmail) + && Objects.equals(name, feedback.name) + && Objects.equals(associatedEventId, feedback.associatedEventId) + && Objects.equals(replayId, feedback.replayId) + && Objects.equals(url, feedback.url) + && Objects.equals(unknown, feedback.unknown); + } + + @Override + public int hashCode() { + return Objects.hash(message, contactEmail, name, associatedEventId, replayId, url, unknown); + } + + // JsonKeys + + public static final class JsonKeys { + public static final String MESSAGE = "message"; + public static final String CONTACT_EMAIL = "contact_email"; + public static final String NAME = "name"; + public static final String ASSOCIATED_EVENT_ID = "associated_event_id"; + public static final String REPLAY_ID = "replay_id"; + public static final String URL = "url"; + } + + // JsonUnknown + + @Override + public @Nullable Map getUnknown() { + return unknown; + } + + @Override + public void setUnknown(@Nullable Map unknown) { + this.unknown = unknown; + } + + // JsonSerializable + + @Override + public void serialize(final @NotNull ObjectWriter writer, final @NotNull ILogger logger) + throws IOException { + writer.beginObject(); + writer.name(JsonKeys.MESSAGE).value(message); + if (contactEmail != null) { + writer.name(JsonKeys.CONTACT_EMAIL).value(contactEmail); + } + if (name != null) { + writer.name(JsonKeys.NAME).value(name); + } + if (associatedEventId != null) { + writer.name(JsonKeys.ASSOCIATED_EVENT_ID); + associatedEventId.serialize(writer, logger); + } + if (replayId != null) { + writer.name(JsonKeys.REPLAY_ID); + replayId.serialize(writer, logger); + } + if (url != null) { + writer.name(JsonKeys.URL).value(url); + } + if (unknown != null) { + for (String key : unknown.keySet()) { + Object value = unknown.get(key); + writer.name(key).value(logger, value); + } + } + writer.endObject(); + } + + // JsonDeserializer + + public static final class Deserializer implements JsonDeserializer { + @Override + public @NotNull Feedback deserialize(@NotNull ObjectReader reader, @NotNull ILogger logger) + throws Exception { + @Nullable String message = null; + @Nullable String contactEmail = null; + @Nullable String name = null; + @Nullable SentryId associatedEventId = null; + @Nullable SentryId replayId = null; + @Nullable String url = null; + @Nullable Map unknown = null; + + reader.beginObject(); + while (reader.peek() == JsonToken.NAME) { + final String nextName = reader.nextName(); + switch (nextName) { + case JsonKeys.MESSAGE: + message = reader.nextStringOrNull(); + break; + case JsonKeys.CONTACT_EMAIL: + contactEmail = reader.nextStringOrNull(); + break; + case JsonKeys.NAME: + name = reader.nextStringOrNull(); + break; + case JsonKeys.ASSOCIATED_EVENT_ID: + associatedEventId = new SentryId.Deserializer().deserialize(reader, logger); + break; + case JsonKeys.REPLAY_ID: + replayId = new SentryId.Deserializer().deserialize(reader, logger); + break; + case JsonKeys.URL: + url = reader.nextStringOrNull(); + break; + default: + if (unknown == null) { + unknown = new HashMap<>(); + } + reader.nextUnknown(logger, unknown, nextName); + break; + } + } + reader.endObject(); + + if (message == null) { + String errorMessage = "Missing required field \"" + JsonKeys.MESSAGE + "\""; + Exception exception = new IllegalStateException(errorMessage); + logger.log(SentryLevel.ERROR, errorMessage, exception); + throw exception; + } + + Feedback feedback = new Feedback(message); + feedback.contactEmail = contactEmail; + feedback.name = name; + feedback.associatedEventId = associatedEventId; + feedback.replayId = replayId; + feedback.url = url; + feedback.unknown = unknown; + return feedback; + } + } +} diff --git a/sentry/src/main/java/io/sentry/transport/RateLimiter.java b/sentry/src/main/java/io/sentry/transport/RateLimiter.java index 11a0ea02f4..438bc28904 100644 --- a/sentry/src/main/java/io/sentry/transport/RateLimiter.java +++ b/sentry/src/main/java/io/sentry/transport/RateLimiter.java @@ -202,6 +202,8 @@ private boolean isRetryAfter(final @NotNull String itemType) { return DataCategory.Monitor; case "replay_video": return DataCategory.Replay; + case "feedback": + return DataCategory.Feedback; default: return DataCategory.Unknown; } diff --git a/sentry/src/test/java/io/sentry/HubAdapterTest.kt b/sentry/src/test/java/io/sentry/HubAdapterTest.kt index 371620ed79..3aee7df3f4 100644 --- a/sentry/src/test/java/io/sentry/HubAdapterTest.kt +++ b/sentry/src/test/java/io/sentry/HubAdapterTest.kt @@ -1,5 +1,6 @@ package io.sentry +import io.sentry.protocol.Feedback import io.sentry.protocol.SentryTransaction import io.sentry.protocol.User import io.sentry.test.createSentryClientMock @@ -57,6 +58,20 @@ class HubAdapterTest { verify(scopes).captureMessage(eq("message"), eq(sentryLevel), eq(scopeCallback)) } + @Test fun `captureFeedback calls Hub`() { + val hint = Hint() + val scopeCallback = mock() + val feedback = Feedback("message") + HubAdapter.getInstance().captureFeedback(feedback) + verify(scopes).captureFeedback(eq(feedback)) + + HubAdapter.getInstance().captureFeedback(feedback, hint) + verify(scopes).captureFeedback(eq(feedback), eq(hint)) + + HubAdapter.getInstance().captureFeedback(feedback, hint, scopeCallback) + verify(scopes).captureFeedback(eq(feedback), eq(hint), eq(scopeCallback)) + } + @Test fun `captureEnvelope calls Hub`() { val envelope = mock() val hint = mock() diff --git a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt index 86f5c70bbc..8a11e2f525 100644 --- a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt +++ b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt @@ -3,6 +3,7 @@ package io.sentry import io.sentry.profilemeasurements.ProfileMeasurement import io.sentry.profilemeasurements.ProfileMeasurementValue import io.sentry.protocol.Device +import io.sentry.protocol.Feedback import io.sentry.protocol.ReplayRecordingSerializationTest import io.sentry.protocol.Request import io.sentry.protocol.SdkVersion @@ -1482,6 +1483,42 @@ class JsonSerializerTest { ) } + @Test + fun `serializes feedback`() { + val replayId = SentryId("00000000-0000-0000-0000-000000000001") + val eventId = SentryId("00000000-0000-0000-0000-000000000002") + val feedback = Feedback("message") + feedback.name = "name" + feedback.contactEmail = "email" + feedback.url = "url" + feedback.setReplayId(replayId) + feedback.setAssociatedEventId(eventId) + val actual = serializeToString(feedback) + val expected = "{\"message\":\"message\",\"contact_email\":\"email\",\"name\":\"name\",\"associated_event_id\":\"00000000000000000000000000000002\",\"replay_id\":\"00000000000000000000000000000001\",\"url\":\"url\"}" + assertEquals(expected, actual) + } + + @Test + fun `deserializes feedback`() { + val json = """{ + "message":"message", + "contact_email":"email", + "name":"name", + "associated_event_id":"00000000000000000000000000000002", + "replay_id":"00000000000000000000000000000001", + "url":"url" + }""" + val feedback = fixture.serializer.deserialize(StringReader(json), Feedback::class.java) + val expected = Feedback("message").apply { + name = "name" + contactEmail = "email" + url = "url" + setReplayId(SentryId("00000000-0000-0000-0000-000000000001")) + setAssociatedEventId(SentryId("00000000-0000-0000-0000-000000000002")) + } + assertEquals(expected, feedback) + } + @Test fun `ser deser replay data`() { val replayEvent = SentryReplayEventSerializationTest.Fixture().getSut() diff --git a/sentry/src/test/java/io/sentry/NoOpHubTest.kt b/sentry/src/test/java/io/sentry/NoOpHubTest.kt index e0eb08ded0..d32009ea07 100644 --- a/sentry/src/test/java/io/sentry/NoOpHubTest.kt +++ b/sentry/src/test/java/io/sentry/NoOpHubTest.kt @@ -43,6 +43,10 @@ class NoOpHubTest { fun `captureMessage returns empty SentryId`() = assertEquals(SentryId.EMPTY_ID, sut.captureMessage("message")) + @Test + fun `captureFeedback returns empty SentryId`() = + assertEquals(SentryId.EMPTY_ID, sut.captureFeedback(mock())) + @Test fun `close does not affect captureEvent`() { sut.close() diff --git a/sentry/src/test/java/io/sentry/NoOpSentryClientTest.kt b/sentry/src/test/java/io/sentry/NoOpSentryClientTest.kt index 8f8d76eba2..d39dba4c70 100644 --- a/sentry/src/test/java/io/sentry/NoOpSentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/NoOpSentryClientTest.kt @@ -29,6 +29,10 @@ class NoOpSentryClientTest { fun `captureEnvelope returns empty SentryId`() = assertEquals(SentryId.EMPTY_ID, sut.captureEnvelope(mock())) + @Test + fun `captureFeedback returns empty SentryId`() = + assertEquals(SentryId.EMPTY_ID, sut.captureFeedback(mock(), mock(), mock())) + @Test fun `close does not affect captureEvent`() { sut.close() diff --git a/sentry/src/test/java/io/sentry/ScopesAdapterTest.kt b/sentry/src/test/java/io/sentry/ScopesAdapterTest.kt index 518bf2317b..3c0afc17d4 100644 --- a/sentry/src/test/java/io/sentry/ScopesAdapterTest.kt +++ b/sentry/src/test/java/io/sentry/ScopesAdapterTest.kt @@ -1,5 +1,6 @@ package io.sentry +import io.sentry.protocol.Feedback import io.sentry.protocol.SentryTransaction import io.sentry.protocol.User import io.sentry.test.createSentryClientMock @@ -57,6 +58,20 @@ class ScopesAdapterTest { verify(scopes).captureMessage(eq("message"), eq(sentryLevel), eq(scopeCallback)) } + @Test fun `captureFeedback calls Scopes`() { + val scopeCallback = mock() + val hint = mock() + val feedback = Feedback("message") + ScopesAdapter.getInstance().captureFeedback(feedback) + verify(scopes).captureFeedback(eq(feedback)) + + ScopesAdapter.getInstance().captureFeedback(feedback, hint) + verify(scopes).captureFeedback(eq(feedback), eq(hint)) + + ScopesAdapter.getInstance().captureFeedback(feedback, hint, scopeCallback) + verify(scopes).captureFeedback(eq(feedback), eq(hint), eq(scopeCallback)) + } + @Test fun `captureEnvelope calls Scopes`() { val envelope = mock() val hint = mock() diff --git a/sentry/src/test/java/io/sentry/ScopesTest.kt b/sentry/src/test/java/io/sentry/ScopesTest.kt index ebb8416224..86cd5949f4 100644 --- a/sentry/src/test/java/io/sentry/ScopesTest.kt +++ b/sentry/src/test/java/io/sentry/ScopesTest.kt @@ -7,6 +7,7 @@ import io.sentry.clientreport.DiscardReason import io.sentry.clientreport.DiscardedEvent import io.sentry.hints.SessionEndHint import io.sentry.hints.SessionStartHint +import io.sentry.protocol.Feedback import io.sentry.protocol.SentryId import io.sentry.protocol.SentryTransaction import io.sentry.protocol.User @@ -607,6 +608,108 @@ class ScopesTest { //endregion + //region captureFeedback tests + @Test + fun `when captureFeedback is called and message is empty, client is never called`() { + val (sut, mockClient) = getEnabledScopes() + sut.captureFeedback(Feedback("")) + assertEquals(SentryId.EMPTY_ID, sut.lastEventId) + verify(mockClient, never()).captureFeedback(any(), anyOrNull(), anyOrNull()) + } + + @Test + fun `when captureFeedback is called, lastEventId is not updated`() { + val (sut, mockClient) = getEnabledScopes() + sut.captureFeedback(Feedback("message")) + assertEquals(SentryId.EMPTY_ID, sut.lastEventId) + verify(mockClient).captureFeedback(any(), anyOrNull(), anyOrNull()) + } + + @Test + fun `when captureFeedback is called on disabled client, do nothing`() { + val (sut, mockClient) = getEnabledScopes() + sut.close() + + sut.captureFeedback(mock()) + verify(mockClient, never()).captureFeedback(any(), anyOrNull(), anyOrNull()) + } + + @Test + fun `when captureFeedback is called with a valid message, captureFeedback on the client should be called`() { + val (sut, mockClient) = getEnabledScopes() + + sut.captureFeedback(Feedback("test")) + verify(mockClient).captureFeedback(any(), anyOrNull(), anyOrNull()) + } + + @Test + fun `when captureFeedback is called with a ScopeCallback then the modified scope is sent to the client`() { + val (sut, mockClient) = getEnabledScopes() + + sut.captureFeedback(Feedback("test"), null) { + it.setTag("test", "testValue") + } + + verify(mockClient).captureFeedback( + any(), + eq(null), + check { + assertEquals("testValue", it.tags["test"]) + } + ) + } + + @Test + fun `when captureFeedback is called with a ScopeCallback then subsequent calls to captureFeedback send the unmodified Scope to the client`() { + val (sut, mockClient) = getEnabledScopes() + val argumentCaptor = argumentCaptor() + + sut.captureFeedback(Feedback("testMessage"), null) { + it.setTag("test", "testValue") + } + + sut.captureFeedback(Feedback("test")) + + verify(mockClient, times(2)).captureFeedback( + any(), + anyOrNull(), + argumentCaptor.capture() + ) + + assertEquals("testValue", argumentCaptor.allValues[0].tags["test"]) + assertNull(argumentCaptor.allValues[1].tags["test"]) + } + + @Test + fun `when captureFeedback is called with a ScopeCallback that crashes then the feedback should still be captured`() { + val (sut, mockClient, logger) = getEnabledScopes() + + val exception = Exception("scope callback exception") + sut.captureFeedback(Feedback("Hello World"), null) { + throw exception + } + + verify(mockClient).captureFeedback( + any(), + anyOrNull(), + anyOrNull() + ) + + verify(logger).log(eq(SentryLevel.ERROR), any(), eq(exception)) + } + + @Test + fun `when captureFeedback is called with a Hint, it is passed to the client`() { + val (sut, mockClient) = getEnabledScopes() + val hint = Hint() + + sut.captureFeedback(Feedback("Hello World"), hint) + + verify(mockClient).captureFeedback(any(), eq(hint), anyOrNull()) + } + + //endregion + //region captureException tests @Test fun `when captureException is called and exception is null, lastEventId is empty`() { diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index 4f50d2eeda..d9b6f53e6d 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -14,6 +14,7 @@ import io.sentry.hints.Cached import io.sentry.hints.DiskFlushNotification import io.sentry.hints.TransactionEnd import io.sentry.protocol.Contexts +import io.sentry.protocol.Feedback import io.sentry.protocol.Mechanism import io.sentry.protocol.Message import io.sentry.protocol.Request @@ -2750,6 +2751,8 @@ class SentryClientTest { verify(fixture.transport).send(anyOrNull(), anyOrNull()) } + //region Replay + @Test fun `when captureReplayEvent, envelope is sent`() { val sut = fixture.getSut() @@ -2990,6 +2993,181 @@ class SentryClientTest { ) } + //endregion + + //region Feedback + + @Test + fun `when captureFeedback is called, sentry event contains feedback in contexts and header type`() { + var sentEvent: SentryEvent? = null + fixture.sentryOptions.setBeforeSendFeedback { e, _ -> sentEvent = e; e } + val sut = fixture.getSut() + val scope = createScope() + sut.captureFeedback(Feedback("message"), null, scope) + + val sentFeedback = sentEvent!!.contexts.feedback + assertNotNull(sentFeedback) + assertEquals("message", sentFeedback.message) + assertNull(sentFeedback.replayId) + assertNull(sentFeedback.url) + + verify(fixture.transport).send( + check { + assertEquals(SentryItemType.Feedback, it.items.first().header.type) + }, + anyOrNull() + ) + } + + @Test + fun `when captureFeedback, scope data is attached to feedback`() { + var sentEvent: SentryEvent? = null + fixture.sentryOptions.setBeforeSendFeedback { e, _ -> sentEvent = e; e } + val sut = fixture.getSut() + val scope = createScope() + val scopeReplayId = SentryId() + scope.contexts.setTrace(SpanContext("test")) + scope.setContexts("context-key", "context-value") + scope.screen = "screen" + scope.replayId = scopeReplayId + sut.captureFeedback(Feedback("message"), null, scope) + + val sentFeedback = sentEvent!!.contexts.feedback + assertNotNull(sentFeedback) + // User, tags and contexts are applied to the feedback + assertEquals(scope.user, sentEvent!!.user) + assertEquals("tags", sentEvent!!.tags!!["tags"]) + assertEquals( + scope.contexts.trace!!.traceId.toString(), + sentEvent!!.contexts.trace!!.traceId.toString() + ) + assertEquals(mapOf("value" to "context-value"), sentEvent!!.contexts["context-key"]) + // currently running replay id set in scope is applied to feedback + assertEquals(scopeReplayId, sentFeedback.replayId) + // screen set to scope is applied as url + assertEquals("screen", sentFeedback.url) + // extras and breadcrumbs are not applied to feedback + assertNull(sentEvent!!.extras) + assertNull(sentEvent!!.breadcrumbs) + } + + @Test + fun `when captureFeedback, replay controller is stopped if no replay id is provided`() { + var sentEvent: SentryEvent? = null + fixture.sentryOptions.setBeforeSendFeedback { e, _ -> sentEvent = e; e } + val replayController = mock() + val replayId = SentryId() + val scope = createScope() + whenever(replayController.captureReplay(any())).thenAnswer { + run { scope.replayId = replayId } + } + val sut = fixture.getSut { it.setReplayController(replayController) } + // When there is no replay id in the feedback + sut.captureFeedback(Feedback("message"), null, scope) + + // Then the replay controller captures the replay + verify(replayController).captureReplay(eq(false)) + + val sentFeedback = sentEvent!!.contexts.feedback + assertNotNull(sentFeedback) + // And the replay id is set to the one from the scope (coming from the replay controller) + assertEquals(replayId, sentFeedback.replayId) + } + + @Test + fun `when captureFeedback, replay controller is not stopped if replay id is provided`() { + var sentEvent: SentryEvent? = null + fixture.sentryOptions.setBeforeSendFeedback { e, _ -> sentEvent = e; e } + val replayController = mock() + val replayId = SentryId() + val scope = createScope() + whenever(replayController.captureReplay(any())).thenAnswer { + run { scope.replayId = replayId } + } + val sut = fixture.getSut { it.setReplayController(replayController) } + // When there is replay id in the feedback + val feedback = Feedback("message") + feedback.setReplayId(SentryId()) + sut.captureFeedback(feedback, null, scope) + + // Then the replay controller doesn't capture the replay + verify(replayController, never()).captureReplay(any()) + + val sentFeedback = sentEvent!!.contexts.feedback + assertNotNull(sentFeedback) + // And the replay id is set to the one from the scope (coming from the replay controller) + assertNotNull(sentFeedback.replayId) + assertNotEquals(replayId, sentFeedback.replayId) + } + + @Test + fun `when beforeSendFeedback is set, callback is invoked`() { + var invoked = false + fixture.sentryOptions.setBeforeSendFeedback { event: SentryEvent, _: Hint -> invoked = true; event } + fixture.getSut().captureFeedback(Feedback("message"), null, createScope()) + assertTrue(invoked) + } + + @Test + fun `when beforeSendFeedback returns null, feedback is dropped`() { + fixture.sentryOptions.setBeforeSendFeedback { event: SentryEvent, _: Hint -> null } + fixture.getSut().captureFeedback(Feedback("message"), null, createScope()) + verify(fixture.transport, never()).send(any(), anyOrNull()) + + assertClientReport( + fixture.sentryOptions.clientReportRecorder, + listOf( + DiscardedEvent(DiscardReason.BEFORE_SEND.reason, DataCategory.Feedback.category, 1) + ) + ) + } + + @Test + fun `when beforeSendFeedback returns new instance, new instance is sent`() { + val expected = SentryEvent().apply { contexts.setFeedback(Feedback("expected")) } + fixture.sentryOptions.setBeforeSendFeedback { _, _ -> expected } + + fixture.getSut().captureFeedback(Feedback("sent"), null, Scope(fixture.sentryOptions)) + + verify(fixture.transport).send( + check { + val event = getEventFromData(it.items.first().data) + assertEquals("expected", event.contexts.feedback!!.message) + }, + anyOrNull() + ) + verifyNoMoreInteractions(fixture.transport) + } + + @Test + fun `when beforeSendFeedback throws an exception, feedback is dropped`() { + val exception = Exception("test") + fixture.sentryOptions.setBeforeSendFeedback { _, _ -> throw exception } + val id = fixture.getSut().captureFeedback(Feedback("message"), null, Scope(fixture.sentryOptions)) + assertEquals(SentryId.EMPTY_ID, id) + + assertClientReport( + fixture.sentryOptions.clientReportRecorder, + listOf( + DiscardedEvent(DiscardReason.BEFORE_SEND.reason, DataCategory.Feedback.category, 1) + ) + ) + } + + @Test + fun `when feedback is dropped, captures client report with datacategory feedback`() { + fixture.sentryOptions.addEventProcessor(DropEverythingEventProcessor()) + val sut = fixture.getSut() + sut.captureFeedback(Feedback("message"), null, createScope()) + + assertClientReport( + fixture.sentryOptions.clientReportRecorder, + listOf(DiscardedEvent(DiscardReason.EVENT_PROCESSOR.reason, DataCategory.Feedback.category, 1)) + ) + } + + //endregion + private fun givenScopeWithStartedSession(errored: Boolean = false, crashed: Boolean = false): IScope { val scope = createScope(fixture.sentryOptions) scope.startSession() diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt index 5d510d20e4..ffcacf20b6 100644 --- a/sentry/src/test/java/io/sentry/SentryTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTest.kt @@ -11,6 +11,7 @@ import io.sentry.internal.debugmeta.ResourcesDebugMetaLoader import io.sentry.internal.modules.CompositeModulesLoader import io.sentry.internal.modules.IModulesLoader import io.sentry.internal.modules.NoOpModulesLoader +import io.sentry.protocol.Feedback import io.sentry.protocol.SdkVersion import io.sentry.protocol.SentryId import io.sentry.protocol.SentryThread @@ -959,6 +960,33 @@ class SentryTest { assertFalse(previousSessionFile.exists()) } + @Test + fun `captureFeedback gets forwarded to client`() { + initForTest { it.dsn = dsn } + + val client = createSentryClientMock() + Sentry.getCurrentScopes().bindClient(client) + + val feedback = Feedback("message") + val hint = Hint() + + Sentry.captureFeedback(feedback) + Sentry.captureFeedback(feedback, hint) + Sentry.captureFeedback(feedback, hint) { it.setTag("testKey", "testValue") } + + verify(client).captureFeedback(eq(feedback), eq(null), anyOrNull()) + verify(client).captureFeedback( + eq(feedback), + eq(hint), + check { assertFalse(it.tags.containsKey("testKey")) } + ) + verify(client).captureFeedback( + eq(feedback), + eq(hint), + check { assertEquals("testValue", it.tags["testKey"]) } + ) + } + @Test fun `captureCheckIn gets forwarded to client`() { initForTest { it.dsn = dsn } diff --git a/sentry/src/test/java/io/sentry/clientreport/ClientReportTest.kt b/sentry/src/test/java/io/sentry/clientreport/ClientReportTest.kt index 23f086dd04..3a09d2258f 100644 --- a/sentry/src/test/java/io/sentry/clientreport/ClientReportTest.kt +++ b/sentry/src/test/java/io/sentry/clientreport/ClientReportTest.kt @@ -26,6 +26,7 @@ import io.sentry.UncaughtExceptionHandlerIntegration.UncaughtExceptionHint import io.sentry.UserFeedback import io.sentry.dsnString import io.sentry.hints.Retryable +import io.sentry.protocol.Feedback import io.sentry.protocol.SentryId import io.sentry.protocol.SentryTransaction import io.sentry.protocol.User @@ -54,6 +55,7 @@ class ClientReportTest { val scopes = mock() whenever(scopes.options).thenReturn(opts) val transaction = SentryTracer(TransactionContext("name", "op"), scopes) + val feedbackEvent = SentryEvent().apply { contexts.setFeedback(Feedback("message")) } val lostClientReport = ClientReport( DateUtils.getCurrentDateTime(), @@ -73,13 +75,14 @@ class ClientReportTest { SentryEnvelopeItem.fromAttachment(opts.serializer, NoOpLogger.getInstance(), Attachment("{ \"number\": 10 }".toByteArray(), "log.json"), 1000), SentryEnvelopeItem.fromProfilingTrace(ProfilingTraceData(File(""), transaction), 1000, opts.serializer), SentryEnvelopeItem.fromCheckIn(opts.serializer, CheckIn("monitor-slug-1", CheckInStatus.ERROR)), - SentryEnvelopeItem.fromReplay(opts.serializer, opts.logger, SentryReplayEvent(), ReplayRecording(), false) + SentryEnvelopeItem.fromReplay(opts.serializer, opts.logger, SentryReplayEvent(), ReplayRecording(), false), + SentryEnvelopeItem.fromEvent(opts.serializer, feedbackEvent) ) clientReportRecorder.recordLostEnvelope(DiscardReason.NETWORK_ERROR, envelope) val clientReportAtEnd = clientReportRecorder.resetCountsAndGenerateClientReport() - testHelper.assertTotalCount(15, clientReportAtEnd) + testHelper.assertTotalCount(16, clientReportAtEnd) testHelper.assertCountFor(DiscardReason.SAMPLE_RATE, DataCategory.Error, 3, clientReportAtEnd) testHelper.assertCountFor(DiscardReason.BEFORE_SEND, DataCategory.Error, 2, clientReportAtEnd) testHelper.assertCountFor(DiscardReason.QUEUE_OVERFLOW, DataCategory.Transaction, 1, clientReportAtEnd) @@ -92,6 +95,7 @@ class ClientReportTest { testHelper.assertCountFor(DiscardReason.NETWORK_ERROR, DataCategory.Profile, 1, clientReportAtEnd) testHelper.assertCountFor(DiscardReason.NETWORK_ERROR, DataCategory.Monitor, 1, clientReportAtEnd) testHelper.assertCountFor(DiscardReason.NETWORK_ERROR, DataCategory.Replay, 1, clientReportAtEnd) + testHelper.assertCountFor(DiscardReason.NETWORK_ERROR, DataCategory.Feedback, 1, clientReportAtEnd) } @Test @@ -136,7 +140,7 @@ class ClientReportTest { DateUtils.getCurrentDateTime(), listOf( DiscardedEvent(DiscardReason.SAMPLE_RATE.reason, DataCategory.Error.category, 3), - DiscardedEvent(DiscardReason.BEFORE_SEND.reason, DataCategory.Error.category, 2), + DiscardedEvent(DiscardReason.BEFORE_SEND.reason, DataCategory.Feedback.category, 2), DiscardedEvent(DiscardReason.QUEUE_OVERFLOW.reason, DataCategory.Transaction.category, 1), DiscardedEvent(DiscardReason.SAMPLE_RATE.reason, DataCategory.Profile.category, 2) ) @@ -149,7 +153,7 @@ class ClientReportTest { val clientReportAtEnd = clientReportRecorder.resetCountsAndGenerateClientReport() testHelper.assertTotalCount(8, clientReportAtEnd) testHelper.assertCountFor(DiscardReason.SAMPLE_RATE, DataCategory.Error, 3, clientReportAtEnd) - testHelper.assertCountFor(DiscardReason.BEFORE_SEND, DataCategory.Error, 2, clientReportAtEnd) + testHelper.assertCountFor(DiscardReason.BEFORE_SEND, DataCategory.Feedback, 2, clientReportAtEnd) testHelper.assertCountFor(DiscardReason.QUEUE_OVERFLOW, DataCategory.Transaction, 1, clientReportAtEnd) testHelper.assertCountFor(DiscardReason.SAMPLE_RATE, DataCategory.Profile, 2, clientReportAtEnd) } diff --git a/sentry/src/test/java/io/sentry/protocol/CombinedContextsViewSerializationTest.kt b/sentry/src/test/java/io/sentry/protocol/CombinedContextsViewSerializationTest.kt index f5e92713eb..80c9db6ce9 100644 --- a/sentry/src/test/java/io/sentry/protocol/CombinedContextsViewSerializationTest.kt +++ b/sentry/src/test/java/io/sentry/protocol/CombinedContextsViewSerializationTest.kt @@ -24,6 +24,7 @@ class CombinedContextsViewSerializationTest { current.setApp(AppSerializationTest.Fixture().getSut()) current.setBrowser(BrowserSerializationTest.Fixture().getSut()) + current.setFeedback(FeedbackTest.Fixture().getSut()) current.setTrace(SpanContextSerializationTest.Fixture().getSut()) isolation.setDevice(DeviceSerializationTest.Fixture().getSut()) diff --git a/sentry/src/test/java/io/sentry/protocol/ContextsSerializationTest.kt b/sentry/src/test/java/io/sentry/protocol/ContextsSerializationTest.kt index 5ea43e377c..1ab6623d90 100644 --- a/sentry/src/test/java/io/sentry/protocol/ContextsSerializationTest.kt +++ b/sentry/src/test/java/io/sentry/protocol/ContextsSerializationTest.kt @@ -21,6 +21,7 @@ class ContextsSerializationTest { setOperatingSystem(OperatingSystemSerializationTest.Fixture().getSut()) setRuntime(SentryRuntimeSerializationTest.Fixture().getSut()) setGpu(GpuSerializationTest.Fixture().getSut()) + setFeedback(FeedbackTest.Fixture().getSut()) setResponse(ResponseSerializationTest.Fixture().getSut()) setTrace(SpanContextSerializationTest.Fixture().getSut()) setSpring(SpringSerializationTest.Fixture().getSut()) diff --git a/sentry/src/test/java/io/sentry/protocol/ContextsTest.kt b/sentry/src/test/java/io/sentry/protocol/ContextsTest.kt index e06ac18332..4769b9a70e 100644 --- a/sentry/src/test/java/io/sentry/protocol/ContextsTest.kt +++ b/sentry/src/test/java/io/sentry/protocol/ContextsTest.kt @@ -23,6 +23,7 @@ class ContextsTest { contexts.setTrace(SpanContext("op")) contexts.profile = ProfileContext(SentryId()) contexts.setSpring(Spring()) + contexts.setFeedback(Feedback("message")) val clone = Contexts(contexts) @@ -38,6 +39,7 @@ class ContextsTest { assertNotSame(contexts.profile, clone.profile) assertNotSame(contexts.response, clone.response) assertNotSame(contexts.spring, clone.spring) + assertNotSame(contexts.feedback, clone.feedback) } @Test diff --git a/sentry/src/test/java/io/sentry/protocol/FeedbackTest.kt b/sentry/src/test/java/io/sentry/protocol/FeedbackTest.kt new file mode 100644 index 0000000000..fa32600d48 --- /dev/null +++ b/sentry/src/test/java/io/sentry/protocol/FeedbackTest.kt @@ -0,0 +1,61 @@ +package io.sentry.protocol + +import io.sentry.ILogger +import org.mockito.kotlin.mock +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNotSame + +class FeedbackTest { + + class Fixture { + val logger = mock() + + fun getSut() = Feedback("message").apply { + name = "name" + contactEmail = "contact@email.com" + url = "url" + setReplayId(SentryId("00000000-0000-0000-0000-000000000001")) + setAssociatedEventId(SentryId("00000000-0000-0000-0000-000000000002")) + unknown = mapOf(Pair("unknown", "unknown")) + } + } + private val fixture = Fixture() + + @Test + fun `copying feedback wont have the same references`() { + val feedback = fixture.getSut() + + val clone = Feedback(feedback) + + assertNotNull(clone) + assertNotSame(feedback, clone) + assertNotSame(feedback.unknown, clone.unknown) + } + + @Test + fun `copying feedback will have the same values`() { + val feedback = fixture.getSut() + + val clone = Feedback(feedback) + assertEquals("message", clone.message) + assertEquals("name", clone.name) + assertEquals("contact@email.com", clone.contactEmail) + assertEquals("url", clone.url) + assertEquals("00000000000000000000000000000001", clone.replayId.toString()) + assertEquals("00000000000000000000000000000002", clone.associatedEventId.toString()) + assertNotNull(clone.unknown) { + assertEquals("unknown", it["unknown"]) + } + } + + @Test + fun `setting a message longer than 4096 characters truncates the message`() { + val feedback = fixture.getSut() + feedback.message = "X".repeat(4095) + "Y" + "Z" + val expectedMessage = "X".repeat(4095) + "Y" + assertEquals(expectedMessage, feedback.message) + assertEquals(4096, feedback.message.length) + } +} diff --git a/sentry/src/test/java/io/sentry/transport/RateLimiterTest.kt b/sentry/src/test/java/io/sentry/transport/RateLimiterTest.kt index b89a37856f..16cd803435 100644 --- a/sentry/src/test/java/io/sentry/transport/RateLimiterTest.kt +++ b/sentry/src/test/java/io/sentry/transport/RateLimiterTest.kt @@ -26,6 +26,7 @@ import io.sentry.UserFeedback import io.sentry.clientreport.DiscardReason import io.sentry.clientreport.IClientReportRecorder import io.sentry.hints.DiskFlushNotification +import io.sentry.protocol.Feedback import io.sentry.protocol.SentryId import io.sentry.protocol.SentryTransaction import io.sentry.protocol.User @@ -206,14 +207,16 @@ class RateLimiterTest { val scopes = mock() whenever(scopes.options).thenReturn(SentryOptions()) val transaction = SentryTracer(TransactionContext("name", "op"), scopes) + val feedbackEvent = SentryEvent().apply { contexts.setFeedback(Feedback("message")) } val sessionItem = SentryEnvelopeItem.fromSession(fixture.serializer, Session("123", User(), "env", "release")) val attachmentItem = SentryEnvelopeItem.fromAttachment(fixture.serializer, NoOpLogger.getInstance(), Attachment("{ \"number\": 10 }".toByteArray(), "log.json"), 1000) val profileItem = SentryEnvelopeItem.fromProfilingTrace(ProfilingTraceData(File(""), transaction), 1000, fixture.serializer) val checkInItem = SentryEnvelopeItem.fromCheckIn(fixture.serializer, CheckIn("monitor-slug-1", CheckInStatus.ERROR)) val profileChunkItem = SentryEnvelopeItem.fromProfileChunk(ProfileChunk(), fixture.serializer) + val feedbackEventItem = SentryEnvelopeItem.fromEvent(fixture.serializer, feedbackEvent) - val envelope = SentryEnvelope(SentryEnvelopeHeader(), arrayListOf(eventItem, userFeedbackItem, sessionItem, attachmentItem, profileItem, checkInItem, profileChunkItem)) + val envelope = SentryEnvelope(SentryEnvelopeHeader(), arrayListOf(eventItem, userFeedbackItem, sessionItem, attachmentItem, profileItem, checkInItem, profileChunkItem, feedbackEventItem)) rateLimiter.updateRetryAfterLimits(null, null, 429) val result = rateLimiter.filter(envelope, Hint()) @@ -227,6 +230,7 @@ class RateLimiterTest { verify(fixture.clientReportRecorder, times(1)).recordLostEnvelopeItem(eq(DiscardReason.RATELIMIT_BACKOFF), same(profileItem)) verify(fixture.clientReportRecorder, times(1)).recordLostEnvelopeItem(eq(DiscardReason.RATELIMIT_BACKOFF), same(checkInItem)) verify(fixture.clientReportRecorder, times(1)).recordLostEnvelopeItem(eq(DiscardReason.RATELIMIT_BACKOFF), same(profileChunkItem)) + verify(fixture.clientReportRecorder, times(1)).recordLostEnvelopeItem(eq(DiscardReason.RATELIMIT_BACKOFF), same(feedbackEventItem)) verifyNoMoreInteractions(fixture.clientReportRecorder) } @@ -354,6 +358,25 @@ class RateLimiterTest { verifyNoMoreInteractions(fixture.clientReportRecorder) } + @Test + fun `drop feedback items as lost`() { + val rateLimiter = fixture.getSUT() + + val feedbackEvent = SentryEvent().apply { contexts.setFeedback(Feedback("message")) } + val feedbackEventItem = SentryEnvelopeItem.fromEvent(fixture.serializer, feedbackEvent) + val attachmentItem = SentryEnvelopeItem.fromAttachment(fixture.serializer, NoOpLogger.getInstance(), Attachment("{ \"number\": 10 }".toByteArray(), "log.json"), 1000) + val envelope = SentryEnvelope(SentryEnvelopeHeader(), arrayListOf(feedbackEventItem, attachmentItem)) + + rateLimiter.updateRetryAfterLimits("60:feedback:key", null, 1) + val result = rateLimiter.filter(envelope, Hint()) + + assertNotNull(result) + assertEquals(1, result.items.toList().size) + + verify(fixture.clientReportRecorder, times(1)).recordLostEnvelopeItem(eq(DiscardReason.RATELIMIT_BACKOFF), same(feedbackEventItem)) + verifyNoMoreInteractions(fixture.clientReportRecorder) + } + @Test fun `apply rate limits notifies observers`() { val rateLimiter = fixture.getSUT() diff --git a/sentry/src/test/resources/json/contexts.json b/sentry/src/test/resources/json/contexts.json index 8eb4000fc6..6f58243127 100644 --- a/sentry/src/test/resources/json/contexts.json +++ b/sentry/src/test/resources/json/contexts.json @@ -65,6 +65,16 @@ "processor_frequency": 800.0, "cpu_description": "cpu0" }, + "feedback": + { + "message": "message", + "contact_email": "contact@email.com", + "name": "name", + "associated_event_id": "00000000000000000000000000000002", + "replay_id": "00000000000000000000000000000001", + "url": "url", + "unknown": "unknown" + }, "gpu": { "name": "d623a6b5-e1ab-4402-931b-c06f5a43a5ae",