From b38ed260d8cf721fa4b8d555703a121e5e8e9517 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 21 Apr 2021 10:42:18 +0200 Subject: [PATCH 01/14] draft --- .../main/java/io/sentry/EventProcessor.java | 6 ++++ .../java/io/sentry/MainEventProcessor.java | 27 +++++++++------ .../main/java/io/sentry/SentryBaseEvent.java | 34 +++++++++++++++++++ .../src/main/java/io/sentry/SentryEvent.java | 32 ----------------- 4 files changed, 57 insertions(+), 42 deletions(-) diff --git a/sentry/src/main/java/io/sentry/EventProcessor.java b/sentry/src/main/java/io/sentry/EventProcessor.java index bcb487d11b..3346305c62 100644 --- a/sentry/src/main/java/io/sentry/EventProcessor.java +++ b/sentry/src/main/java/io/sentry/EventProcessor.java @@ -1,9 +1,15 @@ package io.sentry; +import io.sentry.protocol.SentryTransaction; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; public interface EventProcessor { @Nullable SentryEvent process(@NotNull SentryEvent event, @Nullable Object hint); + + @Nullable + default SentryTransaction process(@NotNull SentryTransaction transaction, @Nullable Object hint) { + return transaction; + } } diff --git a/sentry/src/main/java/io/sentry/MainEventProcessor.java b/sentry/src/main/java/io/sentry/MainEventProcessor.java index 7e4b6601bd..087598a569 100644 --- a/sentry/src/main/java/io/sentry/MainEventProcessor.java +++ b/sentry/src/main/java/io/sentry/MainEventProcessor.java @@ -1,6 +1,7 @@ package io.sentry; import io.sentry.protocol.SentryException; +import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; import io.sentry.util.ApplyScopeUtils; import io.sentry.util.Objects; @@ -14,12 +15,6 @@ @ApiStatus.Internal public final class MainEventProcessor implements EventProcessor { - /** - * Default value for {@link User#getIpAddress()} set when event does not have user and ip address - * set and when {@link SentryOptions#isSendDefaultPii()} is set to true. - */ - public static final String DEFAULT_IP_ADDRESS = "{{auto}}"; - /** * Default value for {@link SentryEvent#getEnvironment()} set when both event and {@link * SentryOptions} do not have the environment field set. @@ -64,10 +59,7 @@ public final class MainEventProcessor implements EventProcessor { @Override public @NotNull SentryEvent process( final @NotNull SentryEvent event, final @Nullable Object hint) { - if (event.getPlatform() == null) { - // this actually means JVM related. - event.setPlatform(SentryBaseEvent.DEFAULT_PLATFORM); - } + setPlatform(event); final Throwable throwable = event.getThrowable(); if (throwable != null) { @@ -112,6 +104,7 @@ private void processNonCachedEvent(final @NotNull SentryEvent event) { } } + // special if (event.getThreads() == null) { // collecting threadIds that came from the exception mechanism, so we can mark threads as // crashed properly @@ -152,4 +145,18 @@ private void processNonCachedEvent(final @NotNull SentryEvent event) { event.setServerName(hostnameCache.getHostname()); } } + + @Override + public @Nullable SentryTransaction process(@NotNull SentryTransaction transaction, @Nullable Object hint) { + setPlatform(transaction); + + return transaction; + } + + private void setPlatform(final @NotNull SentryBaseEvent event) { + if (event.getPlatform() == null) { + // this actually means JVM related. + event.setPlatform(SentryBaseEvent.DEFAULT_PLATFORM); + } + } } diff --git a/sentry/src/main/java/io/sentry/SentryBaseEvent.java b/sentry/src/main/java/io/sentry/SentryBaseEvent.java index e7e9ecae1f..927fdc06f1 100644 --- a/sentry/src/main/java/io/sentry/SentryBaseEvent.java +++ b/sentry/src/main/java/io/sentry/SentryBaseEvent.java @@ -77,6 +77,24 @@ public abstract class SentryBaseEvent { /** The captured Throwable */ protected transient @Nullable Throwable throwable; + /** + * Server or device name the event was generated on. + * + *

This is supposed to be a hostname. + */ + private String serverName; + + /** + * Program's distribution identifier. + * + *

The distribution of the application. + * + *

Distributions are used to disambiguate build or deployment variants of the same release of + * an application. For example, the dist can be the build number of an XCode build or the version + * code of an Android build. + */ + private String dist; + protected SentryBaseEvent(final @NotNull SentryId eventId) { this.eventId = eventId; } @@ -198,4 +216,20 @@ public void setEnvironment(String environment) { public void setPlatform(final @Nullable String platform) { this.platform = platform; } + + public String getServerName() { + return serverName; + } + + public void setServerName(String serverName) { + this.serverName = serverName; + } + + public String getDist() { + return dist; + } + + public void setDist(String dist) { + this.dist = dist; + } } diff --git a/sentry/src/main/java/io/sentry/SentryEvent.java b/sentry/src/main/java/io/sentry/SentryEvent.java index 766064e4d6..cc0aa9e720 100644 --- a/sentry/src/main/java/io/sentry/SentryEvent.java +++ b/sentry/src/main/java/io/sentry/SentryEvent.java @@ -31,23 +31,7 @@ public final class SentryEvent extends SentryBaseEvent implements IUnknownProper private final Date timestamp; private Message message; - /** - * Server or device name the event was generated on. - * - *

This is supposed to be a hostname. - */ - private String serverName; - /** - * Program's distribution identifier. - * - *

The distribution of the application. - * - *

Distributions are used to disambiguate build or deployment variants of the same release of - * an application. For example, the dist can be the build number of an XCode build or the version - * code of an Android build. - */ - private String dist; /** Logger that created the event. */ private String logger; /** Threads that were active when the event occurred. */ @@ -146,22 +130,6 @@ public void setMessage(Message message) { this.message = message; } - public String getServerName() { - return serverName; - } - - public void setServerName(String serverName) { - this.serverName = serverName; - } - - public String getDist() { - return dist; - } - - public void setDist(String dist) { - this.dist = dist; - } - public String getLogger() { return logger; } From b5b4b6fae70c6a326682e0fb0cee4d8508a6766b Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 21 Apr 2021 12:41:18 +0200 Subject: [PATCH 02/14] Fix: Run event processors and enrich transactions --- .../core/DefaultAndroidEventProcessor.java | 163 ++++++++++++------ .../spring/boot/CustomEventProcessor.java | 3 +- .../SentryUserProviderEventProcessor.java | 1 + .../java/io/sentry/MainEventProcessor.java | 41 +++-- .../src/main/java/io/sentry/SentryClient.java | 102 +++++++---- 5 files changed, 205 insertions(+), 105 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java index 7f1a1a6334..3d8be816dd 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java @@ -22,6 +22,7 @@ import io.sentry.DateUtils; import io.sentry.EventProcessor; import io.sentry.ILogger; +import io.sentry.SentryBaseEvent; import io.sentry.SentryEvent; import io.sentry.SentryLevel; import io.sentry.android.core.util.ConnectivityChecker; @@ -34,6 +35,7 @@ import io.sentry.protocol.Device; import io.sentry.protocol.OperatingSystem; import io.sentry.protocol.SentryThread; +import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; import io.sentry.util.ApplyScopeUtils; import io.sentry.util.Objects; @@ -139,15 +141,38 @@ public DefaultAndroidEventProcessor( @Override public @NotNull SentryEvent process( final @NotNull SentryEvent event, final @Nullable Object hint) { - if (ApplyScopeUtils.shouldApplyScopeData(hint)) { + if (shouldApplyScopeData(event, hint)) { processNonCachedEvent(event); + mergeDebugImages(event); + setThreads(event); + } + + setCommons(event, true); + + return event; + } + + private void setCommons(final @NotNull SentryBaseEvent event, final boolean doIO) { + mergeUser(event); + setDevice(event, doIO); + mergeOS(event); + setSideLoadedInfo(event); + } + + private boolean shouldApplyScopeData( + final @NotNull SentryBaseEvent event, final @Nullable Object hint) { + if (ApplyScopeUtils.shouldApplyScopeData(hint)) { + return true; } else { logger.log( SentryLevel.DEBUG, "Event was cached so not applying data relevant to the current app execution/version: %s", event.getEventId()); + return false; } + } + private void mergeUser(final @NotNull SentryBaseEvent event) { // userId should be set even if event is Cached as the userId is static and won't change anyway. final User user = event.getUser(); if (user == null) { @@ -155,19 +180,15 @@ public DefaultAndroidEventProcessor( } else if (user.getId() == null) { user.setId(getDeviceId()); } + } + private void setDevice(final @NotNull SentryBaseEvent event, final boolean doIO) { if (event.getContexts().getDevice() == null) { - event.getContexts().setDevice(getDevice()); + event.getContexts().setDevice(getDevice(doIO)); } - - mergeOS(event); - - setSideLoadedInfo(event); - - return event; } - private void mergeOS(final @NotNull SentryEvent event) { + private void mergeOS(final @NotNull SentryBaseEvent event) { final OperatingSystem currentOS = event.getContexts().getOperatingSystem(); final OperatingSystem androidOS = getOperatingSystem(); @@ -187,27 +208,19 @@ private void mergeOS(final @NotNull SentryEvent event) { } // Data to be applied to events that was created in the running process - private void processNonCachedEvent(final @NotNull SentryEvent event) { + private void processNonCachedEvent(final @NotNull SentryBaseEvent event) { App app = event.getContexts().getApp(); if (app == null) { app = new App(); } setAppExtras(app); - mergeDebugImages(event); - - PackageInfo packageInfo = ContextUtils.getPackageInfo(context, logger); - if (packageInfo != null) { - String versionCode = ContextUtils.getVersionCode(packageInfo); - - if (event.getDist() == null) { - event.setDist(versionCode); - } - setAppPackageInfo(app, packageInfo); - } + setPackageInfo(event, app); event.getContexts().setApp(app); + } + private void setThreads(final @NotNull SentryEvent event) { if (event.getThreads() != null) { for (SentryThread thread : event.getThreads()) { thread.setCurrent(MainThreadChecker.isMainThread(thread)); @@ -215,6 +228,22 @@ private void processNonCachedEvent(final @NotNull SentryEvent event) { } } + private void setPackageInfo(final @NotNull SentryBaseEvent event, final @NotNull App app) { + final PackageInfo packageInfo = ContextUtils.getPackageInfo(context, logger); + if (packageInfo != null) { + String versionCode = ContextUtils.getVersionCode(packageInfo); + + setDist(event, versionCode); + setAppPackageInfo(app, packageInfo); + } + } + + private void setDist(final @NotNull SentryBaseEvent event, final @NotNull String versionCode) { + if (event.getDist() == null) { + event.setDist(versionCode); + } + } + private void mergeDebugImages(final @NotNull SentryEvent event) { final List debugImages = getDebugImages(); if (debugImages == null) { @@ -302,7 +331,7 @@ private void setArchitectures(final @NotNull Device device) { // we can get some inspiration here // https://github.com/flutter/plugins/blob/master/packages/device_info/android/src/main/java/io/flutter/plugins/deviceinfo/DeviceInfoPlugin.java - private @NotNull Device getDevice() { + private @NotNull Device getDevice(final boolean doIO) { // TODO: missing usable memory Device device = new Device(); @@ -314,12 +343,51 @@ private void setArchitectures(final @NotNull Device device) { device.setModelId(Build.ID); setArchitectures(device); - Intent batteryIntent = getBatteryIntent(); + // setting such values require IO hence we don't run for transactions + if (doIO) { + setDeviceIO(device); + } + + device.setOrientation(getOrientation()); + + try { + Object emulator = contextData.get().get(EMULATOR); + if (emulator != null) { + device.setSimulator((Boolean) emulator); + } + } catch (Exception e) { + logger.log(SentryLevel.ERROR, "Error getting emulator.", e); + } + + DisplayMetrics displayMetrics = getDisplayMetrics(); + if (displayMetrics != null) { + device.setScreenWidthPixels(displayMetrics.widthPixels); + device.setScreenHeightPixels(displayMetrics.heightPixels); + device.setScreenDensity(displayMetrics.density); + device.setScreenDpi(displayMetrics.densityDpi); + } + + device.setBootTime(getBootTime()); + device.setTimezone(getTimeZone()); + + if (device.getId() == null) { + device.setId(getDeviceId()); + } + if (device.getLanguage() == null) { + device.setLanguage(Locale.getDefault().toString()); // eg en_US + } + + return device; + } + + private void setDeviceIO(final @NotNull Device device) { + final Intent batteryIntent = getBatteryIntent(); if (batteryIntent != null) { device.setBatteryLevel(getBatteryLevel(batteryIntent)); device.setCharging(isCharging(batteryIntent)); device.setBatteryTemperature(getBatteryTemperature(batteryIntent)); } + Boolean connected; switch (ConnectivityChecker.getConnectionStatus(context, logger)) { case NOT_CONNECTED: @@ -332,18 +400,8 @@ private void setArchitectures(final @NotNull Device device) { connected = null; } device.setOnline(connected); - device.setOrientation(getOrientation()); - - try { - Object emulator = contextData.get().get(EMULATOR); - if (emulator != null) { - device.setSimulator((Boolean) emulator); - } - } catch (Exception e) { - logger.log(SentryLevel.ERROR, "Error getting emulator.", e); - } - ActivityManager.MemoryInfo memInfo = getMemInfo(); + final ActivityManager.MemoryInfo memInfo = getMemInfo(); if (memInfo != null) { // in bytes device.setMemorySize(getMemorySize(memInfo)); @@ -355,43 +413,24 @@ private void setArchitectures(final @NotNull Device device) { // this way of getting the size of storage might be problematic for storages bigger than 2GB // check the use of https://developer.android.com/reference/java/io/File.html#getFreeSpace%28%29 - File internalStorageFile = context.getExternalFilesDir(null); + final File internalStorageFile = context.getExternalFilesDir(null); if (internalStorageFile != null) { StatFs internalStorageStat = new StatFs(internalStorageFile.getPath()); device.setStorageSize(getTotalInternalStorage(internalStorageStat)); device.setFreeStorage(getUnusedInternalStorage(internalStorageStat)); } - StatFs externalStorageStat = getExternalStorageStat(internalStorageFile); + final StatFs externalStorageStat = getExternalStorageStat(internalStorageFile); if (externalStorageStat != null) { device.setExternalStorageSize(getTotalExternalStorage(externalStorageStat)); device.setExternalFreeStorage(getUnusedExternalStorage(externalStorageStat)); } - DisplayMetrics displayMetrics = getDisplayMetrics(); - if (displayMetrics != null) { - device.setScreenWidthPixels(displayMetrics.widthPixels); - device.setScreenHeightPixels(displayMetrics.heightPixels); - device.setScreenDensity(displayMetrics.density); - device.setScreenDpi(displayMetrics.densityDpi); - } - - device.setBootTime(getBootTime()); - device.setTimezone(getTimeZone()); - - if (device.getId() == null) { - device.setId(getDeviceId()); - } - if (device.getLanguage() == null) { - device.setLanguage(Locale.getDefault().toString()); // eg en_US - } if (device.getConnectionType() == null) { // wifi, ethernet or cellular, null if none device.setConnectionType( ConnectivityChecker.getConnectionType(context, logger, buildInfoProvider)); } - - return device; } @SuppressWarnings("ObsoleteSdkInt") @@ -949,7 +988,7 @@ private void setAppPackageInfo(final @NotNull App app, final @NotNull PackageInf } @SuppressWarnings("unchecked") - private void setSideLoadedInfo(final @NotNull SentryEvent event) { + private void setSideLoadedInfo(final @NotNull SentryBaseEvent event) { try { final Object sideLoadedInfo = contextData.get().get(SIDE_LOADED); @@ -963,4 +1002,16 @@ private void setSideLoadedInfo(final @NotNull SentryEvent event) { logger.log(SentryLevel.ERROR, "Error getting side loaded info.", e); } } + + @Override + public @Nullable SentryTransaction process( + final @NotNull SentryTransaction transaction, final @Nullable Object hint) { + if (shouldApplyScopeData(transaction, hint)) { + processNonCachedEvent(transaction); + } + + setCommons(transaction, false); + + return transaction; + } } diff --git a/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/boot/CustomEventProcessor.java b/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/boot/CustomEventProcessor.java index 1eccfb1a55..a86384442f 100644 --- a/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/boot/CustomEventProcessor.java +++ b/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/boot/CustomEventProcessor.java @@ -3,6 +3,7 @@ import io.sentry.EventProcessor; import io.sentry.SentryEvent; import io.sentry.protocol.SentryRuntime; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.springframework.stereotype.Component; @@ -25,7 +26,7 @@ public CustomEventProcessor() { } @Override - public SentryEvent process(SentryEvent event, @Nullable Object hint) { + public SentryEvent process(@NotNull SentryEvent event, @Nullable Object hint) { final SentryRuntime runtime = new SentryRuntime(); runtime.setVersion(javaVersion); runtime.setName(javaVendor); diff --git a/sentry-spring/src/main/java/io/sentry/spring/SentryUserProviderEventProcessor.java b/sentry-spring/src/main/java/io/sentry/spring/SentryUserProviderEventProcessor.java index 300fc2d3b2..3dcf917bc7 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/SentryUserProviderEventProcessor.java +++ b/sentry-spring/src/main/java/io/sentry/spring/SentryUserProviderEventProcessor.java @@ -13,6 +13,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +// TODO: check if I need to support process(SentryTransaction) here public final class SentryUserProviderEventProcessor implements EventProcessor { private final @NotNull SentryOptions options; private final @NotNull SentryUserProvider sentryUserProvider; diff --git a/sentry/src/main/java/io/sentry/MainEventProcessor.java b/sentry/src/main/java/io/sentry/MainEventProcessor.java index 16cb3e4b2b..0af56f253e 100644 --- a/sentry/src/main/java/io/sentry/MainEventProcessor.java +++ b/sentry/src/main/java/io/sentry/MainEventProcessor.java @@ -6,6 +6,7 @@ import io.sentry.util.ApplyScopeUtils; import io.sentry.util.Objects; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import java.util.Map; import org.jetbrains.annotations.ApiStatus; @@ -59,11 +60,21 @@ public final class MainEventProcessor implements EventProcessor { @Override public @NotNull SentryEvent process( final @NotNull SentryEvent event, final @Nullable Object hint) { - setPlatform(event); + setCommons(event); setExceptions(event); - if (ApplyScopeUtils.shouldApplyScopeData(hint)) { + if (shouldApplyScopeData(event, hint)) { processNonCachedEvent(event); + setThreads(event); + } + + return event; + } + + private boolean shouldApplyScopeData( + final @NotNull SentryBaseEvent event, final @Nullable Object hint) { + if (ApplyScopeUtils.shouldApplyScopeData(hint)) { + return true; } else { options .getLogger() @@ -71,30 +82,36 @@ public final class MainEventProcessor implements EventProcessor { SentryLevel.DEBUG, "Event was cached so not applying data relevant to the current app execution/version: %s", event.getEventId()); + return false; } - - return event; } - private void processNonCachedEvent(final @NotNull SentryEvent event) { + private void processNonCachedEvent(final @NotNull SentryBaseEvent event) { setRelease(event); setEnvironment(event); setServerName(event); setDist(event); setSdk(event); setTags(event); - setThreads(event); mergeUser(event); } @Override public @Nullable SentryTransaction process( final @NotNull SentryTransaction transaction, final @Nullable Object hint) { - setPlatform(transaction); + setCommons(transaction); + + if (shouldApplyScopeData(transaction, hint)) { + processNonCachedEvent(transaction); + } return transaction; } + private void setCommons(final @NotNull SentryBaseEvent event) { + setPlatform(event); + } + private void setPlatform(final @NotNull SentryBaseEvent event) { if (event.getPlatform() == null) { // this actually means JVM related. @@ -138,9 +155,13 @@ private void setSdk(final @NotNull SentryBaseEvent event) { } private void setTags(final @NotNull SentryBaseEvent event) { - for (final Map.Entry tag : options.getTags().entrySet()) { - if (event.getTag(tag.getKey()) == null) { - event.setTag(tag.getKey(), tag.getValue()); + if (event.getTags() == null) { + event.setTags(new HashMap<>(options.getTags())); + } else { + for (Map.Entry item : options.getTags().entrySet()) { + if (!event.getTags().containsKey(item.getKey())) { + event.setTag(item.getKey(), item.getValue()); + } } } } diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index a32be6e40e..ec3c11020d 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -54,6 +54,18 @@ public boolean isEnabled() { this.random = options.getSampleRate() == null ? null : new Random(); } + private boolean shouldApplyScopeData( + final @NotNull SentryBaseEvent event, final @Nullable Object hint) { + if (ApplyScopeUtils.shouldApplyScopeData(hint)) { + return true; + } else { + options + .getLogger() + .log(SentryLevel.DEBUG, "Event was cached so not applying scope: %s", event.getEventId()); + return false; + } + } + @Override public @NotNull SentryId captureEvent( @NotNull SentryEvent event, final @Nullable Scope scope, final @Nullable Object hint) { @@ -61,7 +73,7 @@ public boolean isEnabled() { options.getLogger().log(SentryLevel.DEBUG, "Capturing event: %s", event.getEventId()); - if (ApplyScopeUtils.shouldApplyScopeData(hint)) { + 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 @@ -70,10 +82,6 @@ public boolean isEnabled() { if (event == null) { options.getLogger().log(SentryLevel.DEBUG, "Event was dropped by applyScope"); } - } else { - options - .getLogger() - .log(SentryLevel.DEBUG, "Event was cached so not applying scope: %s", event.getEventId()); } event = processEvent(event, hint, options.getEventProcessors()); @@ -193,7 +201,7 @@ private SentryEvent processEvent( @NotNull SentryEvent event, final @Nullable Object hint, final @NotNull List eventProcessors) { - for (EventProcessor processor : eventProcessors) { + for (final EventProcessor processor : eventProcessors) { try { event = processor.process(event, hint); } catch (Exception e) { @@ -219,6 +227,37 @@ private SentryEvent processEvent( return event; } + @Nullable + private SentryTransaction processTransaction( + @NotNull SentryTransaction transaction, + final @Nullable Object hint, + final @NotNull List eventProcessors) { + for (final EventProcessor processor : eventProcessors) { + try { + transaction = processor.process(transaction, hint); + } catch (Exception e) { + options + .getLogger() + .log( + SentryLevel.ERROR, + e, + "An exception occurred while processing transaction by processor: %s", + processor.getClass().getName()); + } + + if (transaction == null) { + options + .getLogger() + .log( + SentryLevel.DEBUG, + "Transaction was dropped by a processor: %s", + processor.getClass().getName()); + break; + } + } + return transaction; + } + @Override public void captureUserFeedback(final @NotNull UserFeedback userFeedback) { Objects.requireNonNull(userFeedback, "SentryEvent is required."); @@ -365,15 +404,25 @@ public void captureSession(final @NotNull Session session, final @Nullable Objec SentryId sentryId = transaction.getEventId(); - if (ApplyScopeUtils.shouldApplyScopeData(hint)) { + if (shouldApplyScopeData(transaction, hint)) { transaction = applyScope(transaction, scope); - } else { - options - .getLogger() - .log( - SentryLevel.DEBUG, - "Transaction was cached so not applying scope: %s", - transaction.getEventId()); + + if (transaction != null) { + transaction = processTransaction(transaction, hint, scope.getEventProcessors()); + } + + if (transaction == null) { + options.getLogger().log(SentryLevel.DEBUG, "Transaction was dropped by applyScope"); + } + } + + if (transaction != null) { + transaction = processTransaction(transaction, hint, options.getEventProcessors()); + } + + if (transaction == null) { + options.getLogger().log(SentryLevel.DEBUG, "Transaction was dropped by Event processors."); + return SentryId.EMPTY_ID; } processTransaction(transaction); @@ -412,27 +461,6 @@ public void captureSession(final @NotNull Session session, final @Nullable Objec private @NotNull SentryTransaction processTransaction( final @NotNull SentryTransaction transaction) { - if (transaction.getPlatform() == null) { - transaction.setPlatform(SentryBaseEvent.DEFAULT_PLATFORM); - } - if (transaction.getRelease() == null) { - transaction.setRelease(options.getRelease()); - } - if (transaction.getEnvironment() == null) { - transaction.setEnvironment(options.getEnvironment()); - } - if (transaction.getSdk() == null) { - transaction.setSdk(options.getSdkVersion()); - } - if (transaction.getTags() == null) { - transaction.setTags(new HashMap<>(options.getTags())); - } else { - for (Map.Entry item : options.getTags().entrySet()) { - if (!transaction.getTags().containsKey(item.getKey())) { - transaction.setTag(item.getKey(), item.getValue()); - } - } - } final List unfinishedSpans = new ArrayList<>(); for (SentrySpan span : transaction.getSpans()) { if (!span.isFinished()) { @@ -452,12 +480,10 @@ public void captureSession(final @NotNull Session session, final @Nullable Objec @NotNull SentryEvent event, final @Nullable Scope scope, final @Nullable Object hint) { if (scope != null) { applyScope(event, scope); + if (event.getTransaction() == null) { event.setTransaction(scope.getTransactionName()); } - if (event.getUser() == null) { - event.setUser(scope.getUser()); - } if (event.getFingerprints() == null) { event.setFingerprints(scope.getFingerprint()); } From 2293314805feeb135168fe176b7b5c97f2a640f9 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 21 Apr 2021 12:46:26 +0200 Subject: [PATCH 03/14] add comments --- sentry/src/main/java/io/sentry/SentryEvent.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sentry/src/main/java/io/sentry/SentryEvent.java b/sentry/src/main/java/io/sentry/SentryEvent.java index 9d9f161258..221ad8dc09 100644 --- a/sentry/src/main/java/io/sentry/SentryEvent.java +++ b/sentry/src/main/java/io/sentry/SentryEvent.java @@ -64,9 +64,12 @@ public final class SentryEvent extends SentryBaseEvent implements IUnknownProper *

```json { "fingerprint": ["myrpc", "POST", "/foo.bar"] } */ private List fingerprint; + + // TODO: should breadcrumbs be part of a transaction? /** List of breadcrumbs recorded before this event. */ private List breadcrumbs; + // TODO: should extras be part of a transaction? /** * Arbitrary extra information set by the user. * From e2db18af08d26265e407891c602bdd7c77732ace Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 22 Apr 2021 13:57:04 +0200 Subject: [PATCH 04/14] fix tests --- sentry-android-core/build.gradle.kts | 1 + .../core/DefaultAndroidEventProcessor.java | 18 +- .../core/DefaultAndroidEventProcessorTest.kt | 265 ++++++++++++------ sentry-spring/api/sentry-spring.api | 1 + .../SentryUserProviderEventProcessor.java | 29 +- .../api/sentry-test-support.api | 4 + .../main/kotlin/io/sentry/test/reflection.kt | 7 + sentry/api/sentry.api | 27 +- ...DuplicateEventDetectionEventProcessor.java | 3 +- .../java/io/sentry/MainEventProcessor.java | 2 +- .../main/java/io/sentry/SentryBaseEvent.java | 59 ++++ .../src/main/java/io/sentry/SentryClient.java | 34 +-- .../src/main/java/io/sentry/SentryEvent.java | 60 ---- .../java/io/sentry/MainEventProcessorTest.kt | 5 +- .../test/java/io/sentry/SentryClientTest.kt | 7 +- 15 files changed, 323 insertions(+), 199 deletions(-) diff --git a/sentry-android-core/build.gradle.kts b/sentry-android-core/build.gradle.kts index 77d2dcd330..667c612fe1 100644 --- a/sentry-android-core/build.gradle.kts +++ b/sentry-android-core/build.gradle.kts @@ -94,4 +94,5 @@ dependencies { testImplementation(Config.TestLibs.mockitoKotlin) testImplementation(Config.TestLibs.mockitoInline) testImplementation(Config.TestLibs.awaitility) + testImplementation(project(":sentry-test-support")) } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java index da0217de52..67c01f4d5e 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java @@ -157,9 +157,11 @@ public DefaultAndroidEventProcessor( } private void setCommons( - final @NotNull SentryBaseEvent event, final boolean doIO, final boolean applyScopeData) { + final @NotNull SentryBaseEvent event, + final boolean errorEvent, + final boolean applyScopeData) { mergeUser(event); - setDevice(event, doIO, applyScopeData); + setDevice(event, errorEvent, applyScopeData); mergeOS(event); setSideLoadedInfo(event); } @@ -188,9 +190,11 @@ private void mergeUser(final @NotNull SentryBaseEvent event) { } private void setDevice( - final @NotNull SentryBaseEvent event, final boolean doIO, final boolean applyScopeData) { + final @NotNull SentryBaseEvent event, + final boolean errorEvent, + final boolean applyScopeData) { if (event.getContexts().getDevice() == null) { - event.getContexts().setDevice(getDevice(doIO, applyScopeData)); + event.getContexts().setDevice(getDevice(errorEvent, applyScopeData)); } } @@ -337,7 +341,7 @@ private void setArchitectures(final @NotNull Device device) { // we can get some inspiration here // https://github.com/flutter/plugins/blob/master/packages/device_info/android/src/main/java/io/flutter/plugins/deviceinfo/DeviceInfoPlugin.java - private @NotNull Device getDevice(final boolean doIO, final boolean applyScopeData) { + private @NotNull Device getDevice(final boolean errorEvent, final boolean applyScopeData) { // TODO: missing usable memory Device device = new Device(); @@ -350,7 +354,7 @@ private void setArchitectures(final @NotNull Device device) { setArchitectures(device); // setting such values require IO hence we don't run for transactions - if (doIO) { + if (errorEvent) { setDeviceIO(device, applyScopeData); } @@ -1012,7 +1016,7 @@ private void setSideLoadedInfo(final @NotNull SentryBaseEvent event) { } @Override - public @Nullable SentryTransaction process( + public @NotNull SentryTransaction process( final @NotNull SentryTransaction transaction, final @Nullable Object hint) { final boolean applyScopeData = shouldApplyScopeData(transaction, hint); diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt index a88d1452e5..eeed18dfc0 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt @@ -14,6 +14,8 @@ import io.sentry.ILogger import io.sentry.SentryEvent import io.sentry.SentryLevel import io.sentry.SentryOptions +import io.sentry.SentryTracer +import io.sentry.TransactionContext import io.sentry.android.core.DefaultAndroidEventProcessor.ANDROID_ID import io.sentry.android.core.DefaultAndroidEventProcessor.EMULATOR import io.sentry.android.core.DefaultAndroidEventProcessor.KERNEL_VERSION @@ -25,7 +27,9 @@ import io.sentry.protocol.DebugMeta import io.sentry.protocol.OperatingSystem import io.sentry.protocol.SdkVersion import io.sentry.protocol.SentryThread +import io.sentry.protocol.SentryTransaction import io.sentry.protocol.User +import io.sentry.test.getCtor import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals @@ -41,16 +45,17 @@ import org.junit.runner.RunWith class DefaultAndroidEventProcessorTest { private lateinit var context: Context + private val className = "io.sentry.android.core.DefaultAndroidEventProcessor" + private val ctorTypes = arrayOf(Context::class.java, ILogger::class.java, IBuildInfoProvider::class.java) + private class Fixture { val buildInfo = mock() val options = SentryOptions().apply { setDebug(true) setLogger(mock()) - sdkVersion = SdkVersion().apply { - name = "test" - version = "1.2.3" - } + sdkVersion = SdkVersion("test", "1.2.3") } + val sentryTracer = SentryTracer(TransactionContext("", ""), mock()) fun getSut(context: Context): DefaultAndroidEventProcessor { return DefaultAndroidEventProcessor(context, options.logger, buildInfo) @@ -73,49 +78,59 @@ class DefaultAndroidEventProcessorTest { @Test fun `when null context is provided, invalid argument is thrown`() { - val clazz = Class.forName("io.sentry.android.core.DefaultAndroidEventProcessor") - val ctor = clazz.getConstructor(Context::class.java, ILogger::class.java, IBuildInfoProvider::class.java) + val ctor = className.getCtor(ctorTypes) + val params = arrayOf(null, mock(), null) assertFailsWith { ctor.newInstance(params) } } @Test fun `when null options is provided, invalid argument is thrown`() { - val clazz = Class.forName("io.sentry.android.core.DefaultAndroidEventProcessor") - val ctor = clazz.getConstructor(Context::class.java, ILogger::class.java, IBuildInfoProvider::class.java) + val ctor = className.getCtor(ctorTypes) + val params = arrayOf(mock(), null, null) assertFailsWith { ctor.newInstance(params) } } @Test fun `when null buildInfo is provided, invalid argument is thrown`() { - val clazz = Class.forName("io.sentry.android.core.DefaultAndroidEventProcessor") - val ctor = clazz.getConstructor(Context::class.java, ILogger::class.java, IBuildInfoProvider::class.java) + val ctor = className.getCtor(ctorTypes) + val params = arrayOf(null, null, mock()) assertFailsWith { ctor.newInstance(params) } } @Test - fun `When hint is not Cached, data should be applied`() { + fun `When Event and hint is not Cached, data should be applied`() { + val sut = fixture.getSut(context) + + assertNotNull(sut.process(SentryEvent(), null)) { + assertNotNull(it.contexts.app) + assertEquals("test", it.debugMeta.images[0].uuid) + assertNotNull(it.dist) + } + } + + @Test + fun `When Transaction and hint is not Cached, data should be applied`() { val sut = fixture.getSut(context) - var event = SentryEvent() - // refactor and mock data later on - event = sut.process(event, null) - assertNotNull(event.contexts.app) - assertEquals("test", event.debugMeta.images[0].uuid) - assertNotNull(event.dist) + + assertNotNull(sut.process(SentryTransaction(fixture.sentryTracer), null)) { + assertNotNull(it.contexts.app) + assertNotNull(it.dist) + } } @Test fun `When debug meta is not null, set the image list`() { val sut = fixture.getSut(context) - var event = SentryEvent().apply { + val event = SentryEvent().apply { debugMeta = DebugMeta() } - event = sut.process(event, null) - - assertEquals("test", event.debugMeta.images[0].uuid) + assertNotNull(sut.process(event, null)) { + assertEquals("test", it.debugMeta.images[0].uuid) + } } @Test @@ -126,16 +141,16 @@ class DefaultAndroidEventProcessorTest { uuid = "abc" type = "proguard" } - var event = SentryEvent().apply { + val event = SentryEvent().apply { debugMeta = DebugMeta().apply { images = mutableListOf(image) } } - event = sut.process(event, null) - - assertEquals("abc", event.debugMeta.images.first().uuid) - assertEquals("test", event.debugMeta.images.last().uuid) + assertNotNull(sut.process(event, null)) { + assertEquals("abc", it.debugMeta.images.first().uuid) + assertEquals("test", it.debugMeta.images.last().uuid) + } } @Test @@ -145,76 +160,97 @@ class DefaultAndroidEventProcessorTest { val sentryThread = SentryThread().apply { id = Looper.getMainLooper().thread.id } - var event = SentryEvent().apply { + val event = SentryEvent().apply { threads = mutableListOf(sentryThread) } - // refactor and mock data later on - event = sut.process(event, null) - assertTrue(event.threads.first().isCurrent) + + assertNotNull(sut.process(event, null)) { + assertTrue(it.threads.first().isCurrent) + } } @Test fun `Current should be false if it its not the main thread`() { val sut = fixture.getSut(context) - val sentryThread = SentryThread().apply { - id = 10L + val event = SentryEvent().apply { + threads = mutableListOf(SentryThread().apply { + id = 10L + }) } - var event = SentryEvent().apply { - threads = mutableListOf(sentryThread) + + assertNotNull(sut.process(event, null)) { + assertFalse(it.threads.first().isCurrent) + } + } + + @Test + fun `When Event and hint is Cached, data should not be applied`() { + val sut = fixture.getSut(context) + + assertNotNull(sut.process(SentryEvent(), CachedEvent())) { + assertNull(it.contexts.app) + assertNull(it.debugMeta) + assertNull(it.dist) } - // refactor and mock data later on - event = sut.process(event, null) - assertFalse(event.threads.first().isCurrent) } @Test - fun `When hint is Cached, data should not be applied`() { + fun `When Transaction and hint is Cached, data should not be applied`() { val sut = fixture.getSut(context) - var event = SentryEvent() - // refactor and mock data later on - event = sut.process(event, CachedEvent()) - assertNull(event.contexts.app) - assertNull(event.debugMeta) - assertNull(event.release) - assertNull(event.dist) + assertNotNull(sut.process(SentryTransaction(fixture.sentryTracer), CachedEvent())) { + assertNull(it.contexts.app) + assertNull(it.dist) + } } @Test - fun `When hint is Cached, userId is applied anyway`() { + fun `When Event and hint is Cached, userId is applied anyway`() { val sut = fixture.getSut(context) - var event = SentryEvent() - event = sut.process(event, CachedEvent()) - assertNotNull(event.user) + assertNotNull(sut.process(SentryEvent(), CachedEvent())) { + assertNotNull(it.user) + } + } + + @Test + fun `When Transaction and hint is Cached, userId is applied anyway`() { + val sut = fixture.getSut(context) + + assertNotNull(sut.process(SentryTransaction(fixture.sentryTracer), CachedEvent())) { + assertNotNull(it.user) + } } @Test fun `When user with id is already set, do not overwrite it`() { val sut = fixture.getSut(context) - val user = User() - user.id = "user-id" - var event = SentryEvent().apply { + val user = User().apply { + id = "user-id" + } + val event = SentryEvent().apply { setUser(user) } - event = sut.process(event, null) - assertNotNull(event.user) - assertSame(user, event.user) + + assertNotNull(sut.process(event, null)) { + assertNotNull(it.user) + assertSame(user, it.user) + } } @Test fun `When user without id is set, user id is applied`() { val sut = fixture.getSut(context) - val user = User() - var event = SentryEvent().apply { - setUser(user) + val event = SentryEvent().apply { + user = User() } - event = sut.process(event, null) - assertNotNull(event.user) { - assertNotNull(it.id) + + assertNotNull(sut.process(event, null)) { + assertNotNull(it.user) + assertNotNull(it.user!!.id) } } @@ -223,6 +259,7 @@ class DefaultAndroidEventProcessorTest { val sut = fixture.getSut(context) val contextData = sut.contextData.get() + assertNotNull(contextData) assertEquals("test", (contextData[PROGUARD_UUID] as Array<*>)[0]) assertNotNull(contextData[ROOTED]) @@ -237,13 +274,16 @@ class DefaultAndroidEventProcessorTest { val sut = fixture.getSut(context) sut.process(SentryEvent(), null) + verify((fixture.options.logger as DiagnosticLogger).logger, never())!!.log(eq(SentryLevel.ERROR), any(), any()) } @Test fun `Processor won't throw exception when theres a hint`() { val processor = DefaultAndroidEventProcessor(context, fixture.options.logger, fixture.buildInfo, mock()) + processor.process(SentryEvent(), CachedEvent()) + verify((fixture.options.logger as DiagnosticLogger).logger, never())!!.log(eq(SentryLevel.ERROR), any(), any()) } @@ -251,10 +291,9 @@ class DefaultAndroidEventProcessorTest { fun `When event is processed, sideLoaded info should be set`() { val sut = fixture.getSut(context) - var event = SentryEvent() - event = sut.process(event, null) - - assertNotNull(event.getTag("isSideLoaded")) + assertNotNull(sut.process(SentryEvent(), null)) { + assertNotNull(it.getTag("isSideLoaded")) + } } @Test @@ -264,14 +303,14 @@ class DefaultAndroidEventProcessorTest { val osLinux = OperatingSystem().apply { name = " Linux " } - - var event = SentryEvent().apply { + val event = SentryEvent().apply { contexts.setOperatingSystem(osLinux) } - event = sut.process(event, null) - assertSame(osLinux, (event.contexts["os_linux"] as OperatingSystem)) - assertEquals("Android", event.contexts.operatingSystem!!.name) + assertNotNull(sut.process(event, null)) { + assertSame(osLinux, (it.contexts["os_linux"] as OperatingSystem)) + assertEquals("Android", it.contexts.operatingSystem!!.name) + } } @Test @@ -281,35 +320,93 @@ class DefaultAndroidEventProcessorTest { val osNoName = OperatingSystem().apply { version = "1.0" } - - var event = SentryEvent().apply { + val event = SentryEvent().apply { contexts.setOperatingSystem(osNoName) } - event = sut.process(event, null) - assertSame(osNoName, (event.contexts["os_1"] as OperatingSystem)) - assertEquals("Android", event.contexts.operatingSystem!!.name) + assertNotNull(sut.process(event, null)) { + assertSame(osNoName, (it.contexts["os_1"] as OperatingSystem)) + assertEquals("Android", it.contexts.operatingSystem!!.name) + } } @Test fun `When hint is Cached, memory data should not be applied`() { val sut = fixture.getSut(context) - var event = SentryEvent() - event = sut.process(event, CachedEvent()) - - assertNull(event.contexts.device!!.freeMemory) - assertNull(event.contexts.device!!.isLowMemory) + assertNotNull(sut.process(SentryEvent(), CachedEvent())) { + assertNull(it.contexts.device!!.freeMemory) + assertNull(it.contexts.device!!.isLowMemory) + } } @Test fun `When hint is not Cached, memory data should be applied`() { val sut = fixture.getSut(context) - var event = SentryEvent() - event = sut.process(event, null) + assertNotNull(sut.process(SentryEvent(), null)) { + assertNotNull(it.contexts.device!!.freeMemory) + assertNotNull(it.contexts.device!!.isLowMemory) + } + } + + @Test + fun `Transaction sets device's context`() { + val sut = fixture.getSut(context) + + assertNotNull(sut.process(SentryTransaction(fixture.sentryTracer), null)) { + assertNotNull(it.contexts.device) + } + } + + @Test + fun `Transaction sets device's OS`() { + val sut = fixture.getSut(context) + + assertNotNull(sut.process(SentryTransaction(fixture.sentryTracer), null)) { + assertNotNull(it.contexts.operatingSystem) + } + } - assertNotNull(event.contexts.device!!.freeMemory) - assertNotNull(event.contexts.device!!.isLowMemory) + @Test + fun `Transaction do not set device's context that requires heavy work`() { + val sut = fixture.getSut(context) + + assertNotNull(sut.process(SentryTransaction(fixture.sentryTracer), null)) { + val device = it.contexts.device!! + assertNull(device.batteryLevel) + assertNull(device.isCharging) + assertNull(device.batteryTemperature) + assertNull(device.isOnline) + assertNull(device.freeMemory) + assertNull(device.isLowMemory) + assertNull(device.storageSize) + assertNull(device.freeStorage) + assertNull(device.externalFreeStorage) + assertNull(device.externalStorageSize) + assertNull(device.connectionType) + } + } + + @Test + fun `Event sets device's context that requires heavy work`() { + val sut = fixture.getSut(context) + + assertNotNull(sut.process(SentryEvent(), null)) { + val device = it.contexts.device!! + assertNotNull(device.freeMemory) + assertNotNull(device.isLowMemory) + assertNotNull(device.storageSize) + assertNotNull(device.freeStorage) + +// commented values are not mocked by robolectric +// assertNotNull(device.batteryLevel) +// assertNotNull(device.isCharging) +// assertNotNull(device.batteryTemperature) +// assertNotNull(device.isOnline) +// assertNotNull(device.externalFreeStorage) +// assertNotNull(device.externalStorageSize) +// assertNotNull(device.connectionType) + } } } diff --git a/sentry-spring/api/sentry-spring.api b/sentry-spring/api/sentry-spring.api index 2d8638f980..101eb3a1a1 100644 --- a/sentry-spring/api/sentry-spring.api +++ b/sentry-spring/api/sentry-spring.api @@ -62,6 +62,7 @@ public final class io/sentry/spring/SentryUserProviderEventProcessor : io/sentry public fun (Lio/sentry/SentryOptions;Lio/sentry/spring/SentryUserProvider;)V public fun getSentryUserProvider ()Lio/sentry/spring/SentryUserProvider; public fun process (Lio/sentry/SentryEvent;Ljava/lang/Object;)Lio/sentry/SentryEvent; + public fun process (Lio/sentry/protocol/SentryTransaction;Ljava/lang/Object;)Lio/sentry/protocol/SentryTransaction; } public class io/sentry/spring/SentryWebConfiguration { diff --git a/sentry-spring/src/main/java/io/sentry/spring/SentryUserProviderEventProcessor.java b/sentry-spring/src/main/java/io/sentry/spring/SentryUserProviderEventProcessor.java index 3dcf917bc7..3b20f8c7a4 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/SentryUserProviderEventProcessor.java +++ b/sentry-spring/src/main/java/io/sentry/spring/SentryUserProviderEventProcessor.java @@ -2,8 +2,10 @@ import io.sentry.EventProcessor; import io.sentry.IpAddressUtils; +import io.sentry.SentryBaseEvent; import io.sentry.SentryEvent; import io.sentry.SentryOptions; +import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.User; import io.sentry.util.Objects; import java.util.Map; @@ -13,7 +15,6 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -// TODO: check if I need to support process(SentryTransaction) here public final class SentryUserProviderEventProcessor implements EventProcessor { private final @NotNull SentryOptions options; private final @NotNull SentryUserProvider sentryUserProvider; @@ -26,7 +27,20 @@ public SentryUserProviderEventProcessor( } @Override - public SentryEvent process(final @NotNull SentryEvent event, final @Nullable Object hint) { + public @NotNull SentryEvent process( + final @NotNull SentryEvent event, final @Nullable Object hint) { + mergeUser(event); + + return event; + } + + @NotNull + @ApiStatus.Internal + public SentryUserProvider getSentryUserProvider() { + return sentryUserProvider; + } + + private void mergeUser(final @NotNull SentryBaseEvent event) { final User user = sentryUserProvider.provideUser(); if (user != null) { final User existingUser = Optional.ofNullable(event.getUser()).orElseGet(User::new); @@ -52,12 +66,13 @@ public SentryEvent process(final @NotNull SentryEvent event, final @Nullable Obj existingUser.setIpAddress(null); } } - return event; } - @NotNull - @ApiStatus.Internal - public SentryUserProvider getSentryUserProvider() { - return sentryUserProvider; + @Override + public @NotNull SentryTransaction process( + final @NotNull SentryTransaction transaction, final @Nullable Object hint) { + mergeUser(transaction); + + return transaction; } } diff --git a/sentry-test-support/api/sentry-test-support.api b/sentry-test-support/api/sentry-test-support.api index 45dc6c51f3..b91579c3ca 100644 --- a/sentry-test-support/api/sentry-test-support.api +++ b/sentry-test-support/api/sentry-test-support.api @@ -2,3 +2,7 @@ public final class io/sentry/test/AssertionsKt { public static final fun checkEvent (Lkotlin/jvm/functions/Function1;)Lio/sentry/SentryEnvelope; } +public final class io/sentry/test/ReflectionKt { + public static final fun getCtor (Ljava/lang/String;[Ljava/lang/Class;)Ljava/lang/reflect/Constructor; +} + diff --git a/sentry-test-support/src/main/kotlin/io/sentry/test/reflection.kt b/sentry-test-support/src/main/kotlin/io/sentry/test/reflection.kt index e8e326f6c4..6a77f3753d 100644 --- a/sentry-test-support/src/main/kotlin/io/sentry/test/reflection.kt +++ b/sentry-test-support/src/main/kotlin/io/sentry/test/reflection.kt @@ -1,5 +1,7 @@ package io.sentry.test +import java.lang.reflect.Constructor + inline fun T.injectForField(name: String, value: Any?) { T::class.java.getDeclaredField(name) .apply { isAccessible = true } @@ -19,3 +21,8 @@ inline fun Any.getProperty(name: String): T = }.apply { this.isAccessible = true }.get(this) as T + +fun String.getCtor(ctorTypes: Array>): Constructor<*> { + val clazz = Class.forName(this) + return clazz.getConstructor(*ctorTypes) +} diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 51a9789db7..747e5e5278 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -88,6 +88,7 @@ public final class io/sentry/EnvelopeSender : io/sentry/IEnvelopeSender { public abstract interface class io/sentry/EventProcessor { public abstract fun process (Lio/sentry/SentryEvent;Ljava/lang/Object;)Lio/sentry/SentryEvent; + public fun process (Lio/sentry/protocol/SentryTransaction;Ljava/lang/Object;)Lio/sentry/protocol/SentryTransaction; } public final class io/sentry/GsonSerializer : io/sentry/ISerializer { @@ -331,8 +332,8 @@ public final class io/sentry/IpAddressUtils { } public final class io/sentry/MainEventProcessor : io/sentry/EventProcessor { - public static final field DEFAULT_IP_ADDRESS Ljava/lang/String; public fun process (Lio/sentry/SentryEvent;Ljava/lang/Object;)Lio/sentry/SentryEvent; + public fun process (Lio/sentry/protocol/SentryTransaction;Ljava/lang/Object;)Lio/sentry/protocol/SentryTransaction; } public final class io/sentry/NoOpEnvelopeReader : io/sentry/IEnvelopeReader { @@ -601,25 +602,37 @@ public abstract class io/sentry/SentryBaseEvent { protected field throwable Ljava/lang/Throwable; protected fun ()V protected fun (Lio/sentry/protocol/SentryId;)V + public fun addBreadcrumb (Lio/sentry/Breadcrumb;)V + public fun addBreadcrumb (Ljava/lang/String;)V + public fun getBreadcrumbs ()Ljava/util/List; public fun getContexts ()Lio/sentry/protocol/Contexts; + public fun getDist ()Ljava/lang/String; public fun getEnvironment ()Ljava/lang/String; public fun getEventId ()Lio/sentry/protocol/SentryId; + public fun getExtra (Ljava/lang/String;)Ljava/lang/Object; public fun getOriginThrowable ()Ljava/lang/Throwable; public fun getPlatform ()Ljava/lang/String; public fun getRelease ()Ljava/lang/String; public fun getRequest ()Lio/sentry/protocol/Request; public fun getSdk ()Lio/sentry/protocol/SdkVersion; + public fun getServerName ()Ljava/lang/String; public fun getTag (Ljava/lang/String;)Ljava/lang/String; public fun getTags ()Ljava/util/Map; public fun getThrowable ()Ljava/lang/Throwable; public fun getUser ()Lio/sentry/protocol/User; + public fun removeExtra (Ljava/lang/String;)V public fun removeTag (Ljava/lang/String;)V + public fun setBreadcrumbs (Ljava/util/List;)V + public fun setDist (Ljava/lang/String;)V public fun setEnvironment (Ljava/lang/String;)V public fun setEventId (Lio/sentry/protocol/SentryId;)V + public fun setExtra (Ljava/lang/String;Ljava/lang/Object;)V + public fun setExtras (Ljava/util/Map;)V public fun setPlatform (Ljava/lang/String;)V public fun setRelease (Ljava/lang/String;)V public fun setRequest (Lio/sentry/protocol/Request;)V public fun setSdk (Lio/sentry/protocol/SdkVersion;)V + public fun setServerName (Ljava/lang/String;)V public fun setTag (Ljava/lang/String;Ljava/lang/String;)V public fun setTags (Ljava/util/Map;)V public fun setThrowable (Ljava/lang/Throwable;)V @@ -694,40 +707,28 @@ public final class io/sentry/SentryEvent : io/sentry/SentryBaseEvent, io/sentry/ public fun (Ljava/lang/Throwable;)V public fun (Ljava/util/Date;)V public fun acceptUnknownProperties (Ljava/util/Map;)V - public fun addBreadcrumb (Lio/sentry/Breadcrumb;)V - public fun addBreadcrumb (Ljava/lang/String;)V - public fun getBreadcrumbs ()Ljava/util/List; public fun getDebugMeta ()Lio/sentry/protocol/DebugMeta; - public fun getDist ()Ljava/lang/String; public fun getExceptions ()Ljava/util/List; - public fun getExtra (Ljava/lang/String;)Ljava/lang/Object; public fun getFingerprints ()Ljava/util/List; public fun getLevel ()Lio/sentry/SentryLevel; public fun getLogger ()Ljava/lang/String; public fun getMessage ()Lio/sentry/protocol/Message; public fun getModule (Ljava/lang/String;)Ljava/lang/String; - public fun getServerName ()Ljava/lang/String; public fun getThreads ()Ljava/util/List; public fun getTimestamp ()Ljava/util/Date; public fun getTransaction ()Ljava/lang/String; public fun getUnknown ()Ljava/util/Map; public fun isCrashed ()Z public fun isErrored ()Z - public fun removeExtra (Ljava/lang/String;)V public fun removeModule (Ljava/lang/String;)V - public fun setBreadcrumbs (Ljava/util/List;)V public fun setDebugMeta (Lio/sentry/protocol/DebugMeta;)V - public fun setDist (Ljava/lang/String;)V public fun setExceptions (Ljava/util/List;)V - public fun setExtra (Ljava/lang/String;Ljava/lang/Object;)V - public fun setExtras (Ljava/util/Map;)V public fun setFingerprints (Ljava/util/List;)V public fun setLevel (Lio/sentry/SentryLevel;)V public fun setLogger (Ljava/lang/String;)V public fun setMessage (Lio/sentry/protocol/Message;)V public fun setModule (Ljava/lang/String;Ljava/lang/String;)V public fun setModules (Ljava/util/Map;)V - public fun setServerName (Ljava/lang/String;)V public fun setThreads (Ljava/util/List;)V public fun setTransaction (Ljava/lang/String;)V } diff --git a/sentry/src/main/java/io/sentry/DuplicateEventDetectionEventProcessor.java b/sentry/src/main/java/io/sentry/DuplicateEventDetectionEventProcessor.java index 1d70d2f2b7..c985cb817c 100644 --- a/sentry/src/main/java/io/sentry/DuplicateEventDetectionEventProcessor.java +++ b/sentry/src/main/java/io/sentry/DuplicateEventDetectionEventProcessor.java @@ -20,7 +20,8 @@ public DuplicateEventDetectionEventProcessor(final @NotNull SentryOptions option } @Override - public SentryEvent process(final @NotNull SentryEvent event, final @Nullable Object hint) { + public @Nullable SentryEvent process( + final @NotNull SentryEvent event, final @Nullable Object hint) { if (options.isEnableDeduplication()) { final Throwable throwable = event.getOriginThrowable(); if (throwable != null) { diff --git a/sentry/src/main/java/io/sentry/MainEventProcessor.java b/sentry/src/main/java/io/sentry/MainEventProcessor.java index 0af56f253e..7e08dbc5e7 100644 --- a/sentry/src/main/java/io/sentry/MainEventProcessor.java +++ b/sentry/src/main/java/io/sentry/MainEventProcessor.java @@ -97,7 +97,7 @@ private void processNonCachedEvent(final @NotNull SentryBaseEvent event) { } @Override - public @Nullable SentryTransaction process( + public @NotNull SentryTransaction process( final @NotNull SentryTransaction transaction, final @Nullable Object hint) { setCommons(transaction); diff --git a/sentry/src/main/java/io/sentry/SentryBaseEvent.java b/sentry/src/main/java/io/sentry/SentryBaseEvent.java index 0b7b30e90e..6c79e82e72 100644 --- a/sentry/src/main/java/io/sentry/SentryBaseEvent.java +++ b/sentry/src/main/java/io/sentry/SentryBaseEvent.java @@ -6,7 +6,9 @@ import io.sentry.protocol.SdkVersion; import io.sentry.protocol.SentryId; import io.sentry.protocol.User; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; @@ -99,6 +101,16 @@ public abstract class SentryBaseEvent { */ private String dist; + /** List of breadcrumbs recorded before this event. */ + private List breadcrumbs; + + /** + * Arbitrary extra information set by the user. + * + *

```json { "extra": { "my_key": 1, "some_other_value": "foo bar" } }``` + */ + private Map extra; + protected SentryBaseEvent(final @NotNull SentryId eventId) { this.eventId = eventId; } @@ -244,4 +256,51 @@ public void setDist(String dist) { public void setUser(final @Nullable User user) { this.user = user; } + + public List getBreadcrumbs() { + return breadcrumbs; + } + + public void setBreadcrumbs(List breadcrumbs) { + this.breadcrumbs = breadcrumbs; + } + + public void addBreadcrumb(Breadcrumb breadcrumb) { + if (breadcrumbs == null) { + breadcrumbs = new ArrayList<>(); + } + breadcrumbs.add(breadcrumb); + } + + Map getExtras() { + return extra; + } + + public void setExtras(Map extra) { + this.extra = extra; + } + + public void setExtra(String key, Object value) { + if (extra == null) { + extra = new HashMap<>(); + } + extra.put(key, value); + } + + public void removeExtra(@NotNull String key) { + if (extra != null) { + extra.remove(key); + } + } + + public @Nullable Object getExtra(final @NotNull String key) { + if (extra != null) { + return extra.get(key); + } + return null; + } + + public void addBreadcrumb(final @Nullable String message) { + this.addBreadcrumb(new Breadcrumb(message)); + } } diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index ec3c11020d..002f0e046d 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -394,7 +394,7 @@ public void captureSession(final @NotNull Session session, final @Nullable Objec @Override public @NotNull SentryId captureTransaction( @NotNull SentryTransaction transaction, - final @NotNull Scope scope, + final @Nullable Scope scope, final @Nullable Object hint) { Objects.requireNonNull(transaction, "Transaction is required."); @@ -407,7 +407,7 @@ public void captureSession(final @NotNull Session session, final @Nullable Objec if (shouldApplyScopeData(transaction, hint)) { transaction = applyScope(transaction, scope); - if (transaction != null) { + if (transaction != null && scope != null) { transaction = processTransaction(transaction, hint, scope.getEventProcessors()); } @@ -487,20 +487,6 @@ public void captureSession(final @NotNull Session session, final @Nullable Objec if (event.getFingerprints() == null) { event.setFingerprints(scope.getFingerprint()); } - if (event.getBreadcrumbs() == null) { - event.setBreadcrumbs(new ArrayList<>(scope.getBreadcrumbs())); - } else { - sortBreadcrumbsByDate(event, scope.getBreadcrumbs()); - } - if (event.getExtras() == null) { - event.setExtras(new HashMap<>(scope.getExtras())); - } else { - for (Map.Entry item : scope.getExtras().entrySet()) { - if (!event.getExtras().containsKey(item.getKey())) { - event.getExtras().put(item.getKey(), item.getValue()); - } - } - } // Level from scope exceptionally take precedence over the event if (scope.getLevel() != null) { event.setLevel(scope.getLevel()); @@ -534,6 +520,20 @@ private T applyScope( } } } + if (sentryBaseEvent.getBreadcrumbs() == null) { + sentryBaseEvent.setBreadcrumbs(new ArrayList<>(scope.getBreadcrumbs())); + } else { + sortBreadcrumbsByDate(sentryBaseEvent, scope.getBreadcrumbs()); + } + if (sentryBaseEvent.getExtras() == null) { + sentryBaseEvent.setExtras(new HashMap<>(scope.getExtras())); + } else { + for (Map.Entry item : scope.getExtras().entrySet()) { + if (!sentryBaseEvent.getExtras().containsKey(item.getKey())) { + sentryBaseEvent.getExtras().put(item.getKey(), item.getValue()); + } + } + } final Contexts contexts = sentryBaseEvent.getContexts(); try { for (Map.Entry entry : scope.getContexts().clone().entrySet()) { @@ -551,7 +551,7 @@ private T applyScope( } private void sortBreadcrumbsByDate( - final @NotNull SentryEvent event, final @NotNull Collection breadcrumbs) { + final @NotNull SentryBaseEvent event, final @NotNull Collection breadcrumbs) { final List sortedBreadcrumbs = event.getBreadcrumbs(); if (!breadcrumbs.isEmpty()) { diff --git a/sentry/src/main/java/io/sentry/SentryEvent.java b/sentry/src/main/java/io/sentry/SentryEvent.java index 221ad8dc09..f700dba141 100644 --- a/sentry/src/main/java/io/sentry/SentryEvent.java +++ b/sentry/src/main/java/io/sentry/SentryEvent.java @@ -1,7 +1,6 @@ package io.sentry; import io.sentry.protocol.*; -import java.util.ArrayList; import java.util.Date; import java.util.HashMap; import java.util.List; @@ -65,18 +64,6 @@ public final class SentryEvent extends SentryBaseEvent implements IUnknownProper */ private List fingerprint; - // TODO: should breadcrumbs be part of a transaction? - /** List of breadcrumbs recorded before this event. */ - private List breadcrumbs; - - // TODO: should extras be part of a transaction? - /** - * Arbitrary extra information set by the user. - * - *

```json { "extra": { "my_key": 1, "some_other_value": "foo bar" } }``` - */ - private Map extra; - private Map unknown; /** * Name and versions of all installed modules/packages/dependencies in the current @@ -183,53 +170,6 @@ public void setFingerprints(List fingerprint) { this.fingerprint = fingerprint; } - public List getBreadcrumbs() { - return breadcrumbs; - } - - public void setBreadcrumbs(List breadcrumbs) { - this.breadcrumbs = breadcrumbs; - } - - public void addBreadcrumb(Breadcrumb breadcrumb) { - if (breadcrumbs == null) { - breadcrumbs = new ArrayList<>(); - } - breadcrumbs.add(breadcrumb); - } - - public void addBreadcrumb(final @Nullable String message) { - this.addBreadcrumb(new Breadcrumb(message)); - } - - Map getExtras() { - return extra; - } - - public void setExtras(Map extra) { - this.extra = extra; - } - - public void setExtra(String key, Object value) { - if (extra == null) { - extra = new HashMap<>(); - } - extra.put(key, value); - } - - public void removeExtra(@NotNull String key) { - if (extra != null) { - extra.remove(key); - } - } - - public @Nullable Object getExtra(final @NotNull String key) { - if (extra != null) { - return extra.get(key); - } - return null; - } - @ApiStatus.Internal @Override public void acceptUnknownProperties(Map unknown) { diff --git a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt index b10da81298..c8e737b673 100644 --- a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt +++ b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt @@ -26,10 +26,7 @@ class MainEventProcessorTest { dsn = dsnString release = "release" dist = "dist" - sdkVersion = SdkVersion().apply { - name = "test" - version = "1.2.3" - } + sdkVersion = SdkVersion("test", "1.2.3") } val getLocalhost = mock() diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index 0f363eb270..ec29154a04 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -56,10 +56,7 @@ class SentryClientTest { var sentryOptions: SentryOptions = SentryOptions().apply { dsn = dsnString - sdkVersion = SdkVersion().apply { - name = "test" - version = "1.2.3" - } + sdkVersion = SdkVersion("test", "1.2.3") setDebug(true) setDiagnosticLevel(SentryLevel.DEBUG) setSerializer(GsonSerializer(this)) @@ -544,7 +541,7 @@ class SentryClientTest { val event = SentryEvent() val scope = createScope() val processor = mock() - whenever(processor.process(any(), anyOrNull())).thenReturn(event) + whenever(processor.process(any(), anyOrNull())).thenReturn(event) scope.addEventProcessor(processor) val sut = fixture.getSut() From c5775240ee7821334b8208b0ae92a519c11de8a7 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 22 Apr 2021 13:59:52 +0200 Subject: [PATCH 05/14] fix --- .../src/main/java/io/sentry/samples/console/Main.java | 3 ++- .../io/sentry/samples/spring/boot/CustomEventProcessor.java | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/sentry-samples/sentry-samples-console/src/main/java/io/sentry/samples/console/Main.java b/sentry-samples/sentry-samples-console/src/main/java/io/sentry/samples/console/Main.java index d8be4e6825..b099097c97 100644 --- a/sentry-samples/sentry-samples-console/src/main/java/io/sentry/samples/console/Main.java +++ b/sentry-samples/sentry-samples-console/src/main/java/io/sentry/samples/console/Main.java @@ -1,5 +1,6 @@ package io.sentry.samples.console; +import com.sun.istack.internal.NotNull; import io.sentry.Breadcrumb; import io.sentry.EventProcessor; import io.sentry.ISpan; @@ -166,7 +167,7 @@ public static void main(String[] args) throws InterruptedException { private static class SomeEventProcessor implements EventProcessor { @Override - public SentryEvent process(SentryEvent event, Object hint) { + public @NotNull SentryEvent process(SentryEvent event, Object hint) { // Here you can modify the event as you need if (event.getLevel() != null && event.getLevel().ordinal() > SentryLevel.INFO.ordinal()) { event.addBreadcrumb(new Breadcrumb("Processed by " + SomeEventProcessor.class)); diff --git a/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/boot/CustomEventProcessor.java b/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/boot/CustomEventProcessor.java index a86384442f..7a52dc7d2e 100644 --- a/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/boot/CustomEventProcessor.java +++ b/sentry-samples/sentry-samples-spring-boot/src/main/java/io/sentry/samples/spring/boot/CustomEventProcessor.java @@ -26,7 +26,7 @@ public CustomEventProcessor() { } @Override - public SentryEvent process(@NotNull SentryEvent event, @Nullable Object hint) { + public @NotNull SentryEvent process(@NotNull SentryEvent event, @Nullable Object hint) { final SentryRuntime runtime = new SentryRuntime(); runtime.setVersion(javaVersion); runtime.setName(javaVendor); From 49860a23030bf31d0e0088ccf964380d8cf281f1 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 22 Apr 2021 14:17:07 +0200 Subject: [PATCH 06/14] tests for main event processor --- .../SentryUserProviderEventProcessorTest.kt | 19 +++++++++-- .../java/io/sentry/MainEventProcessorTest.kt | 32 +++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/SentryUserProviderEventProcessorTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/SentryUserProviderEventProcessorTest.kt index bf82da89c3..8163ec200c 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/SentryUserProviderEventProcessorTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/SentryUserProviderEventProcessorTest.kt @@ -1,7 +1,11 @@ package io.sentry.spring +import com.nhaarman.mockitokotlin2.mock import io.sentry.SentryEvent import io.sentry.SentryOptions +import io.sentry.SentryTracer +import io.sentry.TransactionContext +import io.sentry.protocol.SentryTransaction import io.sentry.protocol.User import kotlin.test.Test import kotlin.test.assertEquals @@ -11,10 +15,12 @@ import kotlin.test.assertNull class SentryUserProviderEventProcessorTest { class Fixture { + val sentryTracer = SentryTracer(TransactionContext("", ""), mock()) fun getSut(isSendDefaultPii: Boolean = false, userProvider: () -> User?): SentryUserProviderEventProcessor { - val options = SentryOptions() - options.isSendDefaultPii = isSendDefaultPii + val options = SentryOptions().apply { + setSendDefaultPii(isSendDefaultPii) + } return SentryUserProviderEventProcessor(options, userProvider) } } @@ -200,4 +206,13 @@ class SentryUserProviderEventProcessorTest { assertNull(it.ipAddress) } } + + @Test + fun `Transaction sets the user`() { + val processor = fixture.getSut(isSendDefaultPii = true) { + User() + } + + assertNotNull(processor.process(SentryTransaction(fixture.sentryTracer), null)) + } } diff --git a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt index c8e737b673..7d3560caa6 100644 --- a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt +++ b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt @@ -8,6 +8,7 @@ import com.nhaarman.mockitokotlin2.whenever import io.sentry.hints.ApplyScopeData import io.sentry.hints.Cached import io.sentry.protocol.SdkVersion +import io.sentry.protocol.SentryTransaction import io.sentry.protocol.User import java.lang.RuntimeException import java.net.InetAddress @@ -29,6 +30,7 @@ class MainEventProcessorTest { sdkVersion = SdkVersion("test", "1.2.3") } val getLocalhost = mock() + val sentryTracer = SentryTracer(TransactionContext("", ""), mock()) fun getSut(attachThreads: Boolean = true, attachStackTrace: Boolean = true, environment: String? = "environment", tags: Map = emptyMap(), sendDefaultPii: Boolean? = null, serverName: String? = "server", host: String? = null, resolveHostDelay: Long? = null, hostnameCacheDuration: Long = 10): MainEventProcessor { sentryOptions.isAttachThreads = attachThreads @@ -339,6 +341,36 @@ class MainEventProcessorTest { assertEquals("optionsHost", event.serverName) } + @Test + fun `Transaction sets server name`() { + val processor = fixture.getSut(serverName = "optionsHost") + + var transaction = SentryTransaction(fixture.sentryTracer) + transaction = processor.process(transaction, null) + + assertEquals("optionsHost", transaction.serverName) + } + + @Test + fun `Transaction sets dist`() { + val processor = fixture.getSut() + + var transaction = SentryTransaction(fixture.sentryTracer) + transaction = processor.process(transaction, null) + + assertEquals("dist", transaction.dist) + } + + @Test + fun `Transaction merges user`() { + val processor = fixture.getSut(sendDefaultPii = true) + + var transaction = SentryTransaction(fixture.sentryTracer) + transaction = processor.process(transaction, null) + + assertNotNull(transaction.user) + } + private fun generateCrashedEvent(crashedThread: Thread = Thread.currentThread()) = SentryEvent().apply { val mockThrowable = mock() val actualThrowable = UncaughtExceptionHandlerIntegration.getUnhandledThrowable(crashedThread, mockThrowable) From b5e9914fb44010d7ac513c0a4b65518181211bc8 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 22 Apr 2021 14:37:00 +0200 Subject: [PATCH 07/14] tests --- .../src/main/java/io/sentry/SentryClient.java | 13 +++--- .../test/java/io/sentry/SentryClientTest.kt | 43 ++++++++++++++++--- 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 002f0e046d..66593808ef 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -81,6 +81,7 @@ private boolean shouldApplyScopeData( if (event == null) { options.getLogger().log(SentryLevel.DEBUG, "Event was dropped by applyScope"); + return SentryId.EMPTY_ID; } } @@ -103,8 +104,6 @@ private boolean shouldApplyScopeData( } } - SentryId sentryId = SentryId.EMPTY_ID; - if (event != null) { if (event.getOriginThrowable() != null && options.containsIgnoredExceptionForType(event.getOriginThrowable())) { @@ -114,7 +113,7 @@ private boolean shouldApplyScopeData( SentryLevel.DEBUG, "Event was dropped as the exception %s is ignored", event.getOriginThrowable().getClass()); - return sentryId; + return SentryId.EMPTY_ID; } event = executeBeforeSend(event, hint); @@ -123,7 +122,8 @@ private boolean shouldApplyScopeData( } } - if (event != null) { + SentryId sentryId = SentryId.EMPTY_ID; + if (event != null && event.getEventId() != null) { sentryId = event.getEventId(); } @@ -402,7 +402,10 @@ public void captureSession(final @NotNull Session session, final @Nullable Objec .getLogger() .log(SentryLevel.DEBUG, "Capturing transaction: %s", transaction.getEventId()); - SentryId sentryId = transaction.getEventId(); + SentryId sentryId = SentryId.EMPTY_ID; + if (transaction.getEventId() != null) { + sentryId = transaction.getEventId(); + } if (shouldApplyScopeData(transaction, hint)) { transaction = applyScope(transaction, scope); diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index ec29154a04..46bf1b38c8 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -53,6 +53,7 @@ class SentryClientTest { var transport = mock() var factory = mock() val maxAttachmentSize: Long = 5 * 1024 * 1024 + val sentryTracer = SentryTracer(TransactionContext("a-transaction", "op"), mock()) var sentryOptions: SentryOptions = SentryOptions().apply { dsn = dsnString @@ -550,6 +551,20 @@ class SentryClientTest { verify(processor).process(eq(event), anyOrNull()) } + @Test + fun `when scope has event processors, apply for transactions`() { + val transaction = SentryTransaction(fixture.sentryTracer) + val scope = createScope() + val processor = mock() + whenever(processor.process(any(), anyOrNull())).thenReturn(transaction) + scope.addEventProcessor(processor) + + val sut = fixture.getSut() + + sut.captureTransaction(transaction, scope, null) + verify(processor).process(eq(transaction), anyOrNull()) + } + @Test fun `when options have event processors, they should be applied`() { val processor = mock() @@ -561,6 +576,17 @@ class SentryClientTest { verify(processor).process(eq(event), anyOrNull()) } + @Test + fun `when options have event processors, apply for transactions`() { + val processor = mock() + fixture.sentryOptions.addEventProcessor(processor) + + val transaction = SentryTransaction(fixture.sentryTracer) + + fixture.getSut().captureTransaction(transaction) + verify(processor).process(eq(transaction), anyOrNull()) + } + @Test fun `when captureSession and no release is set, do nothing`() { fixture.getSut().captureSession(createSession("")) @@ -756,7 +782,7 @@ class SentryClientTest { @Test fun `transactions are sent using connection`() { val sut = fixture.getSut() - sut.captureTransaction(SentryTransaction(SentryTracer(TransactionContext("a-transaction", "op"), mock())), Scope(fixture.sentryOptions), null) + sut.captureTransaction(SentryTransaction(fixture.sentryTracer), Scope(fixture.sentryOptions), null) verify(fixture.transport).send(check { val transaction = it.items.first().getTransaction(fixture.sentryOptions.serializer) assertNotNull(transaction) @@ -785,7 +811,7 @@ class SentryClientTest { @Test fun `when captureTransaction with attachments`() { - val transaction = SentryTransaction(SentryTracer(TransactionContext("a-transaction", "op"), mock())) + val transaction = SentryTransaction(fixture.sentryTracer) fixture.getSut().captureTransaction(transaction, createScopeWithAttachments(), null) verifyAttachmentsInEnvelope(transaction.eventId) @@ -793,7 +819,7 @@ class SentryClientTest { @Test fun `when captureTransaction with attachments not added to transaction`() { - val transaction = SentryTransaction(SentryTracer(TransactionContext("a-transaction", "op"), mock())) + val transaction = SentryTransaction(fixture.sentryTracer) val scope = createScopeWithAttachments() scope.addAttachment(Attachment("hello".toByteArray(), "application/octet-stream")) fixture.getSut().captureTransaction(transaction, scope, null) @@ -810,7 +836,10 @@ class SentryClientTest { scope.request = Request().apply { url = "/url" } - sut.captureTransaction(SentryTransaction(SentryTracer(TransactionContext("a-transaction", "op"), mock())), scope, null) + scope.addBreadcrumb(Breadcrumb("message")) + scope.setExtra("a", "b") + + sut.captureTransaction(SentryTransaction(fixture.sentryTracer), scope, null) verify(fixture.transport).send(check { envelope -> val transaction = envelope.items.first().getTransaction(fixture.sentryOptions.serializer) assertNotNull(transaction) { @@ -819,6 +848,8 @@ class SentryClientTest { assertNotNull(it.request) { request -> assertEquals("/url", request.url) } + assertEquals("message", it.breadcrumbs.first().message) + assertEquals("b", it.getExtra("a")) } }, eq(null)) } @@ -841,7 +872,7 @@ class SentryClientTest { val event = SentryEvent() val sut = fixture.getSut() val scope = createScope() - val transaction = SentryTracer(TransactionContext("a-transaction", "op"), mock()) + val transaction = fixture.sentryTracer scope.setTransaction(transaction) transaction.finish() sut.captureEvent(event, scope) @@ -867,7 +898,7 @@ class SentryClientTest { fixture.sentryOptions.release = "optionsRelease" fixture.sentryOptions.environment = "optionsEnvironment" val sut = fixture.getSut() - val transaction = SentryTransaction(SentryTracer(TransactionContext("a-transaction", "op"), mock())) + val transaction = SentryTransaction(fixture.sentryTracer) sut.captureTransaction(transaction) assertEquals("optionsRelease", transaction.release) assertEquals("optionsEnvironment", transaction.environment) From 43c89a72e0715d41732e25ed8ffa3c0872232fb8 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 22 Apr 2021 14:38:39 +0200 Subject: [PATCH 08/14] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 99bcf8e632..ed68244e02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ * Fix: Do not set free memory and is low memory fields when it's a NDK hard crash (#1399) * Fix: Apply user from the scope to transaction (#1424) * Fix: Pass maxBreadcrumbs config. to sentry-native (#1425) +* Fix: Run event processors and enrich transactions with contexts (#1430) # 4.4.0-alpha.2 From 457098eb0fbc3d84d27548fd4180d554642f463e Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 22 Apr 2021 14:53:15 +0200 Subject: [PATCH 09/14] fix --- .../src/main/java/io/sentry/samples/console/Main.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sentry-samples/sentry-samples-console/src/main/java/io/sentry/samples/console/Main.java b/sentry-samples/sentry-samples-console/src/main/java/io/sentry/samples/console/Main.java index b099097c97..d8be4e6825 100644 --- a/sentry-samples/sentry-samples-console/src/main/java/io/sentry/samples/console/Main.java +++ b/sentry-samples/sentry-samples-console/src/main/java/io/sentry/samples/console/Main.java @@ -1,6 +1,5 @@ package io.sentry.samples.console; -import com.sun.istack.internal.NotNull; import io.sentry.Breadcrumb; import io.sentry.EventProcessor; import io.sentry.ISpan; @@ -167,7 +166,7 @@ public static void main(String[] args) throws InterruptedException { private static class SomeEventProcessor implements EventProcessor { @Override - public @NotNull SentryEvent process(SentryEvent event, Object hint) { + public SentryEvent process(SentryEvent event, Object hint) { // Here you can modify the event as you need if (event.getLevel() != null && event.getLevel().ordinal() > SentryLevel.INFO.ordinal()) { event.addBreadcrumb(new Breadcrumb("Processed by " + SomeEventProcessor.class)); From b56c8d897723df1dea5d51b4ffda13f26b1bf57a Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 23 Apr 2021 09:28:01 +0200 Subject: [PATCH 10/14] make event processor for event default --- sentry/src/main/java/io/sentry/EventProcessor.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sentry/src/main/java/io/sentry/EventProcessor.java b/sentry/src/main/java/io/sentry/EventProcessor.java index 3346305c62..824f5dfcca 100644 --- a/sentry/src/main/java/io/sentry/EventProcessor.java +++ b/sentry/src/main/java/io/sentry/EventProcessor.java @@ -6,7 +6,9 @@ public interface EventProcessor { @Nullable - SentryEvent process(@NotNull SentryEvent event, @Nullable Object hint); + default SentryEvent process(@NotNull SentryEvent event, @Nullable Object hint) { + return event; + } @Nullable default SentryTransaction process(@NotNull SentryTransaction transaction, @Nullable Object hint) { From 8af5bfb2f52c3b6468fa1219de2c4ada16a7de28 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 23 Apr 2021 14:25:15 +0200 Subject: [PATCH 11/14] fix --- Makefile | 4 ++++ sentry/api/sentry.api | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index ca482e7718..f386c079c5 100644 --- a/Makefile +++ b/Makefile @@ -41,3 +41,7 @@ checkFormat: # Spotless format's code format: ./gradlew spotlessApply + +# Binary compatibility validator +api: + ./gradlew apiDump diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 747e5e5278..ad039b1ac2 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -87,7 +87,7 @@ public final class io/sentry/EnvelopeSender : io/sentry/IEnvelopeSender { } public abstract interface class io/sentry/EventProcessor { - public abstract fun process (Lio/sentry/SentryEvent;Ljava/lang/Object;)Lio/sentry/SentryEvent; + public fun process (Lio/sentry/SentryEvent;Ljava/lang/Object;)Lio/sentry/SentryEvent; public fun process (Lio/sentry/protocol/SentryTransaction;Ljava/lang/Object;)Lio/sentry/protocol/SentryTransaction; } From 7771ba4f23ac29af66356ccd80a9b869ef449ef5 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 23 Apr 2021 16:27:16 +0200 Subject: [PATCH 12/14] fix --- sentry/src/test/java/io/sentry/ScopeTest.kt | 10 +++++++++- sentry/src/test/java/io/sentry/SentryClientTest.kt | 10 +++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/sentry/src/test/java/io/sentry/ScopeTest.kt b/sentry/src/test/java/io/sentry/ScopeTest.kt index f990dd806b..b6806362c7 100644 --- a/sentry/src/test/java/io/sentry/ScopeTest.kt +++ b/sentry/src/test/java/io/sentry/ScopeTest.kt @@ -221,7 +221,7 @@ class ScopeTest { scope.addBreadcrumb(Breadcrumb()) scope.setTag("some", "tag") scope.setExtra("some", "extra") - scope.addEventProcessor { event, _ -> event } + scope.addEventProcessor(eventProcessor()) scope.addAttachment(Attachment("path")) scope.clear() @@ -751,4 +751,12 @@ class ScopeTest { assertNull(it) } } + + private fun eventProcessor(): EventProcessor { + return object : EventProcessor { + override fun process(event: SentryEvent, hint: Any?): SentryEvent? { + return event + } + } + } } diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index 46bf1b38c8..6cdf6ee051 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -774,7 +774,7 @@ class SentryClientTest { @Test fun `exception thrown by an event processor is handled gracefully`() { - fixture.sentryOptions.addEventProcessor { _, _ -> throw RuntimeException() } + fixture.sentryOptions.addEventProcessor(eventProcessorThrows()) val sut = fixture.getSut() sut.captureEvent(SentryEvent()) } @@ -1101,4 +1101,12 @@ class SentryClientTest { internal class DiskFlushNotificationHint : DiskFlushNotification { override fun markFlushed() {} } + + private fun eventProcessorThrows(): EventProcessor { + return object : EventProcessor { + override fun process(event: SentryEvent, hint: Any?): SentryEvent? { + throw RuntimeException() + } + } + } } From 90462b127c1557c68066dd471cb89af669c33d05 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Mon, 26 Apr 2021 13:48:40 +0200 Subject: [PATCH 13/14] fix --- .../core/DefaultAndroidEventProcessorTest.kt | 4 ++-- sentry-android-ndk/sentry-native | 2 +- .../SentryUserProviderEventProcessorTest.kt | 6 ++++-- .../main/java/io/sentry/EventProcessor.java | 18 ++++++++++++++++++ .../java/io/sentry/MainEventProcessorTest.kt | 6 +++--- 5 files changed, 28 insertions(+), 8 deletions(-) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt index eeed18dfc0..e67a2ff3f3 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/DefaultAndroidEventProcessorTest.kt @@ -351,7 +351,7 @@ class DefaultAndroidEventProcessorTest { } @Test - fun `Transaction sets device's context`() { + fun `Device's context is set on transactions`() { val sut = fixture.getSut(context) assertNotNull(sut.process(SentryTransaction(fixture.sentryTracer), null)) { @@ -360,7 +360,7 @@ class DefaultAndroidEventProcessorTest { } @Test - fun `Transaction sets device's OS`() { + fun `Device's OS is set on transactions`() { val sut = fixture.getSut(context) assertNotNull(sut.process(SentryTransaction(fixture.sentryTracer), null)) { diff --git a/sentry-android-ndk/sentry-native b/sentry-android-ndk/sentry-native index b3f2b6ddac..1c046603d9 160000 --- a/sentry-android-ndk/sentry-native +++ b/sentry-android-ndk/sentry-native @@ -1 +1 @@ -Subproject commit b3f2b6ddac886616b40cdccbb05b81af14b6adc9 +Subproject commit 1c046603d9c61e1790a80ac2f8fb80c255152225 diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/SentryUserProviderEventProcessorTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/SentryUserProviderEventProcessorTest.kt index 8163ec200c..b79db88983 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/SentryUserProviderEventProcessorTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/SentryUserProviderEventProcessorTest.kt @@ -208,11 +208,13 @@ class SentryUserProviderEventProcessorTest { } @Test - fun `Transaction sets the user`() { + fun `User is set on transaction`() { val processor = fixture.getSut(isSendDefaultPii = true) { User() } - assertNotNull(processor.process(SentryTransaction(fixture.sentryTracer), null)) + val result = processor.process(SentryTransaction(fixture.sentryTracer), null) + + assertNotNull(result.user) } } diff --git a/sentry/src/main/java/io/sentry/EventProcessor.java b/sentry/src/main/java/io/sentry/EventProcessor.java index 824f5dfcca..d5f601f430 100644 --- a/sentry/src/main/java/io/sentry/EventProcessor.java +++ b/sentry/src/main/java/io/sentry/EventProcessor.java @@ -4,12 +4,30 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +/** + * An Event Processor that may mutate or drop an event. Runs for SentryEvent or SentryTransaction + */ public interface EventProcessor { + + /** + * May mutate or drop a SentryEvent + * + * @param event the SentryEvent + * @param hint the Hint + * @return the event itself, a mutated SentryEvent or null + */ @Nullable default SentryEvent process(@NotNull SentryEvent event, @Nullable Object hint) { return event; } + /** + * May mutate or drop a SentryTransaction + * + * @param transaction the SentryTransaction + * @param hint the Hint + * @return the event itself, a mutated SentryTransaction or null + */ @Nullable default SentryTransaction process(@NotNull SentryTransaction transaction, @Nullable Object hint) { return transaction; diff --git a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt index 7d3560caa6..935257e889 100644 --- a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt +++ b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt @@ -342,7 +342,7 @@ class MainEventProcessorTest { } @Test - fun `Transaction sets server name`() { + fun `Server name is set on transaction`() { val processor = fixture.getSut(serverName = "optionsHost") var transaction = SentryTransaction(fixture.sentryTracer) @@ -352,7 +352,7 @@ class MainEventProcessorTest { } @Test - fun `Transaction sets dist`() { + fun `Dist is set on transaction`() { val processor = fixture.getSut() var transaction = SentryTransaction(fixture.sentryTracer) @@ -362,7 +362,7 @@ class MainEventProcessorTest { } @Test - fun `Transaction merges user`() { + fun `User is merged on transaction`() { val processor = fixture.getSut(sendDefaultPii = true) var transaction = SentryTransaction(fixture.sentryTracer) From ba88de4868324501cfe3817879a38ca808cc8ecc Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Mon, 26 Apr 2021 13:59:17 +0200 Subject: [PATCH 14/14] rollback --- sentry-android-ndk/sentry-native | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-android-ndk/sentry-native b/sentry-android-ndk/sentry-native index 1c046603d9..b3f2b6ddac 160000 --- a/sentry-android-ndk/sentry-native +++ b/sentry-android-ndk/sentry-native @@ -1 +1 @@ -Subproject commit 1c046603d9c61e1790a80ac2f8fb80c255152225 +Subproject commit b3f2b6ddac886616b40cdccbb05b81af14b6adc9