From 018379b769493e6b405fc7e2e29fd3f2b400c82a Mon Sep 17 00:00:00 2001 From: Bruno Garcia Date: Mon, 28 Oct 2019 18:58:40 -0400 Subject: [PATCH 1/2] feat: sample --- .../java/io/sentry/core/SentryClient.java | 23 ++++++++++++ .../java/io/sentry/core/SentryOptions.java | 16 +++++++++ .../java/io/sentry/core/SentryClientTest.kt | 21 +++++++++++ .../java/io/sentry/core/SentryOptionsTest.kt | 35 +++++++++++++++++++ 4 files changed, 95 insertions(+) diff --git a/sentry-core/src/main/java/io/sentry/core/SentryClient.java b/sentry-core/src/main/java/io/sentry/core/SentryClient.java index 96748586b..fab918d00 100644 --- a/sentry-core/src/main/java/io/sentry/core/SentryClient.java +++ b/sentry-core/src/main/java/io/sentry/core/SentryClient.java @@ -9,6 +9,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.Map; +import java.util.Random; public class SentryClient implements ISentryClient { static final String SENTRY_PROTOCOL_VERSION = "7"; @@ -17,6 +18,7 @@ public class SentryClient implements ISentryClient { private final SentryOptions options; private final AsyncConnection connection; + private final Random random; public boolean isEnabled() { return isEnabled; @@ -33,9 +35,19 @@ public SentryClient(SentryOptions options, @Nullable AsyncConnection connection) connection = AsyncConnectionFactory.create(options); } this.connection = connection; + random = options.getSampling() == null ? null : new Random(); } public SentryId captureEvent(SentryEvent event, @Nullable Scope scope) { + if (!sample()) { + log( + options.getLogger(), + SentryLevel.DEBUG, + "Event %s was dropped due to sampling decision.", + event.getEventId()); + return SentryId.EMPTY_ID; + } + log(options.getLogger(), SentryLevel.DEBUG, "Capturing event: %s", event.getEventId()); if (scope != null) { @@ -128,4 +140,15 @@ public void close() { public void flush(long timeoutMills) { // TODO: Flush transport } + + private boolean sample() { + // https://docs.sentry.io/development/sdk-dev/features/#event-sampling + if (options.getSampling() != null && random != null) { + double sampling = options.getSampling(); + if (sampling < random.nextFloat()) { + return false; // bad luck + } + } + return true; + } } diff --git a/sentry-core/src/main/java/io/sentry/core/SentryOptions.java b/sentry-core/src/main/java/io/sentry/core/SentryOptions.java index 1d1fa2861..d84925a02 100644 --- a/sentry-core/src/main/java/io/sentry/core/SentryOptions.java +++ b/sentry-core/src/main/java/io/sentry/core/SentryOptions.java @@ -27,6 +27,7 @@ public class SentryOptions { private String release; private String environment; private Proxy proxy; + private Double sampling; public void addEventProcessor(EventProcessor eventProcessor) { eventProcessors.add(eventProcessor); @@ -171,6 +172,21 @@ public void setProxy(Proxy proxy) { this.proxy = proxy; } + public Double getSampling() { + return sampling; + } + + // Can be anything between 0.01 (1%) and 1.0 (99.9%) or null (default), to disable it. + public void setSampling(Double sampling) { + if (sampling != null && (sampling > 1.0 || sampling <= 0.0)) { + throw new IllegalArgumentException( + "The value " + + sampling + + " is not valid. Use null to disable or values between 0.01 (inclusive) and 1.0 (exclusive)."); + } + this.sampling = sampling; + } + public interface BeforeSendCallback { SentryEvent execute(SentryEvent event); } diff --git a/sentry-core/src/test/java/io/sentry/core/SentryClientTest.kt b/sentry-core/src/test/java/io/sentry/core/SentryClientTest.kt index 5b1a4c67f..cb9dccf21 100644 --- a/sentry-core/src/test/java/io/sentry/core/SentryClientTest.kt +++ b/sentry-core/src/test/java/io/sentry/core/SentryClientTest.kt @@ -11,6 +11,7 @@ import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertTrue +import org.mockito.Mockito class SentryClientTest { @@ -239,6 +240,26 @@ class SentryClientTest { assertEquals(SentryLevel.FATAL, event.level) } + @Test + fun `when captureEvent with sampling, some events not captured`() { + fixture.sentryOptions.sampling = 0.000000001 + val sut = fixture.getSut() + + val allEvents = 10 + (0..allEvents).forEach { _ -> sut.captureEvent(SentryEvent()) } + assertTrue(allEvents > Mockito.mockingDetails(fixture.connection).invocations.size) + } + + @Test + fun `when captureEvent without sampling, all events are captured`() { + fixture.sentryOptions.sampling = null + val sut = fixture.getSut() + + val allEvents = 10 + (0..allEvents).forEach { _ -> sut.captureEvent(SentryEvent()) } + assertEquals(allEvents, Mockito.mockingDetails(fixture.connection).invocations.size - 1) // 1 extra invocation outside .send() + } + private fun createScope(): Scope { return Scope(fixture.sentryOptions.maxBreadcrumbs).apply { addBreadcrumb(Breadcrumb().apply { diff --git a/sentry-core/src/test/java/io/sentry/core/SentryOptionsTest.kt b/sentry-core/src/test/java/io/sentry/core/SentryOptionsTest.kt index beb46195c..2e80406f3 100644 --- a/sentry-core/src/test/java/io/sentry/core/SentryOptionsTest.kt +++ b/sentry-core/src/test/java/io/sentry/core/SentryOptionsTest.kt @@ -2,8 +2,10 @@ package io.sentry.core import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFailsWith import kotlin.test.assertFalse import kotlin.test.assertNotNull +import kotlin.test.assertNull import kotlin.test.assertTrue class SentryOptionsTest { @@ -51,4 +53,37 @@ class SentryOptionsTest { options.maxBreadcrumbs = 1 assertEquals(1, options.maxBreadcrumbs) } + + @Test + fun `when options is initialized, default sampling is null`() = + assertNull(SentryOptions().sampling) + + @Test + fun `when setSampling is called, overrides default`() { + val options = SentryOptions() + options.sampling = 0.5 + assertEquals(0.5, options.sampling) + } + + @Test + fun `when setSampling is called with null, disables it`() { + val options = SentryOptions() + options.sampling = null + assertNull(options.sampling) + } + + @Test + fun `when setSampling is set to higher than 1_0, setter throws`() { + assertFailsWith { SentryOptions().sampling = 1.0000000000001 } + } + + @Test + fun `when setSampling is set to lower than 0, setter throws`() { + assertFailsWith { SentryOptions().sampling = -0.0000000000001 } + } + + @Test + fun `when setSampling is set to exactly 0, setter throws`() { + assertFailsWith { SentryOptions().sampling = 0.0 } + } } From 0bc1560c9a36ce9975220d6ca3fd96c91152a397 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Tue, 29 Oct 2019 09:12:33 +0100 Subject: [PATCH 2/2] code review --- sentry-core/src/main/java/io/sentry/core/SentryClient.java | 2 +- .../src/test/java/io/sentry/core/SentryClientTest.kt | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sentry-core/src/main/java/io/sentry/core/SentryClient.java b/sentry-core/src/main/java/io/sentry/core/SentryClient.java index fab918d00..b169f2bc3 100644 --- a/sentry-core/src/main/java/io/sentry/core/SentryClient.java +++ b/sentry-core/src/main/java/io/sentry/core/SentryClient.java @@ -145,7 +145,7 @@ private boolean sample() { // https://docs.sentry.io/development/sdk-dev/features/#event-sampling if (options.getSampling() != null && random != null) { double sampling = options.getSampling(); - if (sampling < random.nextFloat()) { + if (sampling < random.nextDouble()) { return false; // bad luck } } diff --git a/sentry-core/src/test/java/io/sentry/core/SentryClientTest.kt b/sentry-core/src/test/java/io/sentry/core/SentryClientTest.kt index cb9dccf21..eb8b4ca6d 100644 --- a/sentry-core/src/test/java/io/sentry/core/SentryClientTest.kt +++ b/sentry-core/src/test/java/io/sentry/core/SentryClientTest.kt @@ -1,6 +1,7 @@ package io.sentry.core import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.mockingDetails import com.nhaarman.mockitokotlin2.never import com.nhaarman.mockitokotlin2.times import com.nhaarman.mockitokotlin2.verify @@ -11,7 +12,6 @@ import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertTrue -import org.mockito.Mockito class SentryClientTest { @@ -247,7 +247,7 @@ class SentryClientTest { val allEvents = 10 (0..allEvents).forEach { _ -> sut.captureEvent(SentryEvent()) } - assertTrue(allEvents > Mockito.mockingDetails(fixture.connection).invocations.size) + assertTrue(allEvents > mockingDetails(fixture.connection).invocations.size) } @Test @@ -257,7 +257,7 @@ class SentryClientTest { val allEvents = 10 (0..allEvents).forEach { _ -> sut.captureEvent(SentryEvent()) } - assertEquals(allEvents, Mockito.mockingDetails(fixture.connection).invocations.size - 1) // 1 extra invocation outside .send() + assertEquals(allEvents, mockingDetails(fixture.connection).invocations.size - 1) // 1 extra invocation outside .send() } private fun createScope(): Scope {