diff --git a/CHANGELOG.md b/CHANGELOG.md index 8428f033b7..3af6f8886d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,12 +2,24 @@ ## Unreleased +### Behavioral Changes + +- Collections returned by scope (e.g. `getBreadcrumbs`, `getTags`, `getAttachments`) are shared state and should not be mutated. ([#5541](https://github.com/getsentry/sentry-java/pull/5541)) + - Previously, when going through `CombinedScopeView`, we were returning a copy where mutations didn't show up in the underlying scopes. + - This has now changed in order to reduce SDK overhead. + ### Features - Add experimental `SentrySQLiteDriver` to `sentry-android-sqlite` for instrumenting `androidx.sqlite.SQLiteDriver` ([#5563](https://github.com/getsentry/sentry-java/pull/5563)) - To use it, pass `SQLiteDriver` to `SentrySQLiteDriver.create(...)` - Requires `androidx.sqlite:sqlite` (2.5.0+) on runtime classpath (typically provided by Room or SQLDelight) +### Internal + +- Reduce writer buffer size from 8192 to 512 ([#5544](https://github.com/getsentry/sentry-java/pull/5544)) +- Remove redundant event map copies ([#5536](https://github.com/getsentry/sentry-java/pull/5536)) +- Optimize combined scope by adding an early return if only one scope has data ([#5541](https://github.com/getsentry/sentry-java/pull/5541)) + ## 8.44.0 ### Features diff --git a/sentry/src/main/java/io/sentry/CombinedScopeView.java b/sentry/src/main/java/io/sentry/CombinedScopeView.java index f21f8697fa..ea2d752d44 100644 --- a/sentry/src/main/java/io/sentry/CombinedScopeView.java +++ b/sentry/src/main/java/io/sentry/CombinedScopeView.java @@ -171,10 +171,31 @@ public void setFingerprint(@NotNull List fingerprint) { @Override public @NotNull Queue getBreadcrumbs() { + final @NotNull Queue globalBreadcrumbs = globalScope.getBreadcrumbs(); + final @NotNull Queue isolationBreadcrumbs = isolationScope.getBreadcrumbs(); + final @NotNull Queue currentBreadcrumbs = scope.getBreadcrumbs(); + + final boolean hasGlobalBreadcrumbs = !globalBreadcrumbs.isEmpty(); + final boolean hasIsolationBreadcrumbs = !isolationBreadcrumbs.isEmpty(); + final boolean hasCurrentBreadcrumbs = !currentBreadcrumbs.isEmpty(); + + if (!hasGlobalBreadcrumbs && !hasIsolationBreadcrumbs && !hasCurrentBreadcrumbs) { + return getDefaultScopeValue(globalBreadcrumbs, isolationBreadcrumbs, currentBreadcrumbs); + } + if (!hasIsolationBreadcrumbs && !hasCurrentBreadcrumbs) { + return globalBreadcrumbs; + } + if (!hasGlobalBreadcrumbs && !hasCurrentBreadcrumbs) { + return isolationBreadcrumbs; + } + if (!hasGlobalBreadcrumbs && !hasIsolationBreadcrumbs) { + return currentBreadcrumbs; + } + final @NotNull List allBreadcrumbs = new ArrayList<>(); - allBreadcrumbs.addAll(globalScope.getBreadcrumbs()); - allBreadcrumbs.addAll(isolationScope.getBreadcrumbs()); - allBreadcrumbs.addAll(scope.getBreadcrumbs()); + allBreadcrumbs.addAll(globalBreadcrumbs); + allBreadcrumbs.addAll(isolationBreadcrumbs); + allBreadcrumbs.addAll(currentBreadcrumbs); Collections.sort(allBreadcrumbs); final @NotNull Queue breadcrumbs = @@ -224,10 +245,31 @@ public void clear() { @Override public @NotNull Map getTags() { + final @NotNull Map globalTags = globalScope.getTags(); + final @NotNull Map isolationTags = isolationScope.getTags(); + final @NotNull Map currentTags = scope.getTags(); + + final boolean hasGlobalTags = !globalTags.isEmpty(); + final boolean hasIsolationTags = !isolationTags.isEmpty(); + final boolean hasCurrentTags = !currentTags.isEmpty(); + + if (!hasGlobalTags && !hasIsolationTags && !hasCurrentTags) { + return getDefaultScopeValue(globalTags, isolationTags, currentTags); + } + if (!hasIsolationTags && !hasCurrentTags) { + return globalTags; + } + if (!hasGlobalTags && !hasCurrentTags) { + return isolationTags; + } + if (!hasGlobalTags && !hasIsolationTags) { + return currentTags; + } + final @NotNull Map allTags = new ConcurrentHashMap<>(); - allTags.putAll(globalScope.getTags()); - allTags.putAll(isolationScope.getTags()); - allTags.putAll(scope.getTags()); + allTags.putAll(globalTags); + allTags.putAll(isolationTags); + allTags.putAll(currentTags); return allTags; } @@ -243,10 +285,32 @@ public void removeTag(@Nullable String key) { @Override public @NotNull Map getAttributes() { + final @NotNull Map globalAttributes = globalScope.getAttributes(); + final @NotNull Map isolationAttributes = + isolationScope.getAttributes(); + final @NotNull Map currentAttributes = scope.getAttributes(); + + final boolean hasGlobalAttributes = !globalAttributes.isEmpty(); + final boolean hasIsolationAttributes = !isolationAttributes.isEmpty(); + final boolean hasCurrentAttributes = !currentAttributes.isEmpty(); + + if (!hasGlobalAttributes && !hasIsolationAttributes && !hasCurrentAttributes) { + return getDefaultScopeValue(globalAttributes, isolationAttributes, currentAttributes); + } + if (!hasIsolationAttributes && !hasCurrentAttributes) { + return globalAttributes; + } + if (!hasGlobalAttributes && !hasCurrentAttributes) { + return isolationAttributes; + } + if (!hasGlobalAttributes && !hasIsolationAttributes) { + return currentAttributes; + } + final @NotNull Map allAttributes = new ConcurrentHashMap<>(); - allAttributes.putAll(globalScope.getAttributes()); - allAttributes.putAll(isolationScope.getAttributes()); - allAttributes.putAll(scope.getAttributes()); + allAttributes.putAll(globalAttributes); + allAttributes.putAll(isolationAttributes); + allAttributes.putAll(currentAttributes); return allAttributes; } @@ -272,11 +336,32 @@ public void removeAttribute(@Nullable String key) { @Override public @NotNull Map getExtras() { - final @NotNull Map allTags = new ConcurrentHashMap<>(); - allTags.putAll(globalScope.getExtras()); - allTags.putAll(isolationScope.getExtras()); - allTags.putAll(scope.getExtras()); - return allTags; + final @NotNull Map globalExtras = globalScope.getExtras(); + final @NotNull Map isolationExtras = isolationScope.getExtras(); + final @NotNull Map currentExtras = scope.getExtras(); + + final boolean hasGlobalExtras = !globalExtras.isEmpty(); + final boolean hasIsolationExtras = !isolationExtras.isEmpty(); + final boolean hasCurrentExtras = !currentExtras.isEmpty(); + + if (!hasGlobalExtras && !hasIsolationExtras && !hasCurrentExtras) { + return getDefaultScopeValue(globalExtras, isolationExtras, currentExtras); + } + if (!hasIsolationExtras && !hasCurrentExtras) { + return globalExtras; + } + if (!hasGlobalExtras && !hasCurrentExtras) { + return isolationExtras; + } + if (!hasGlobalExtras && !hasIsolationExtras) { + return currentExtras; + } + + final @NotNull Map allExtras = new ConcurrentHashMap<>(); + allExtras.putAll(globalExtras); + allExtras.putAll(isolationExtras); + allExtras.putAll(currentExtras); + return allExtras; } @Override @@ -342,6 +427,23 @@ public void removeContexts(@Nullable String key) { return getSpecificScope(null); } + private @NotNull T getDefaultScopeValue( + final @NotNull T globalValue, + final @NotNull T isolationValue, + final @NotNull T currentValue) { + switch (getOptions().getDefaultScopeType()) { + case CURRENT: + return currentValue; + case ISOLATION: + return isolationValue; + case GLOBAL: + return globalValue; + default: + // calm the compiler + return currentValue; + } + } + IScope getSpecificScope(final @Nullable ScopeType scopeType) { if (scopeType != null) { switch (scopeType) { @@ -373,10 +475,31 @@ IScope getSpecificScope(final @Nullable ScopeType scopeType) { @Override public @NotNull List getAttachments() { + final @NotNull List globalAttachments = globalScope.getAttachments(); + final @NotNull List isolationAttachments = isolationScope.getAttachments(); + final @NotNull List currentAttachments = scope.getAttachments(); + + final boolean hasGlobalAttachments = !globalAttachments.isEmpty(); + final boolean hasIsolationAttachments = !isolationAttachments.isEmpty(); + final boolean hasCurrentAttachments = !currentAttachments.isEmpty(); + + if (!hasGlobalAttachments && !hasIsolationAttachments && !hasCurrentAttachments) { + return getDefaultScopeValue(globalAttachments, isolationAttachments, currentAttachments); + } + if (!hasIsolationAttachments && !hasCurrentAttachments) { + return globalAttachments; + } + if (!hasGlobalAttachments && !hasCurrentAttachments) { + return isolationAttachments; + } + if (!hasGlobalAttachments && !hasIsolationAttachments) { + return currentAttachments; + } + final @NotNull List allAttachments = new CopyOnWriteArrayList<>(); - allAttachments.addAll(globalScope.getAttachments()); - allAttachments.addAll(isolationScope.getAttachments()); - allAttachments.addAll(scope.getAttachments()); + allAttachments.addAll(globalAttachments); + allAttachments.addAll(isolationAttachments); + allAttachments.addAll(currentAttachments); return allAttachments; } diff --git a/sentry/src/main/java/io/sentry/JsonSerializer.java b/sentry/src/main/java/io/sentry/JsonSerializer.java index 2b24090d0c..79a1c72bef 100644 --- a/sentry/src/main/java/io/sentry/JsonSerializer.java +++ b/sentry/src/main/java/io/sentry/JsonSerializer.java @@ -64,6 +64,8 @@ public final class JsonSerializer implements ISerializer { @SuppressWarnings("CharsetObjectCanBeUsed") private static final Charset UTF_8 = Charset.forName("UTF-8"); + private static final int WRITER_BUFFER_SIZE = 512; + /** the SentryOptions */ private final @NotNull SentryOptions options; @@ -233,7 +235,8 @@ public void serialize(@NotNull SentryEnvelope envelope, @NotNull OutputStream ou // we do not want to close these as we would also close the stream that was passed in final BufferedOutputStream bufferedOutputStream = new BufferedOutputStream(outputStream); - final Writer writer = new BufferedWriter(new OutputStreamWriter(bufferedOutputStream, UTF_8)); + final Writer writer = + new BufferedWriter(new OutputStreamWriter(bufferedOutputStream, UTF_8), WRITER_BUFFER_SIZE); try { envelope diff --git a/sentry/src/main/java/io/sentry/MainEventProcessor.java b/sentry/src/main/java/io/sentry/MainEventProcessor.java index 8c684bfb65..d84c9e47be 100644 --- a/sentry/src/main/java/io/sentry/MainEventProcessor.java +++ b/sentry/src/main/java/io/sentry/MainEventProcessor.java @@ -11,7 +11,6 @@ import java.io.Closeable; import java.io.IOException; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Map; import org.jetbrains.annotations.ApiStatus; @@ -191,7 +190,7 @@ private void setSdk(final @NotNull SentryBaseEvent event) { private void setTags(final @NotNull SentryBaseEvent event) { if (event.getTags() == null) { - event.setTags(new HashMap<>(options.getTags())); + event.setTags(options.getTags()); } else { for (Map.Entry item : options.getTags().entrySet()) { if (!event.getTags().containsKey(item.getKey())) { diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 78225f05d1..a739eddd9d 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -27,7 +27,6 @@ import java.util.Collection; import java.util.Collections; import java.util.Comparator; -import java.util.HashMap; import java.util.List; import java.util.Map; import org.jetbrains.annotations.ApiStatus; @@ -1425,7 +1424,7 @@ public void captureBatchedMetricsEvents(final @NotNull SentryMetricsEvents metri event.setUser(scope.getUser()); } if (event.getTags() == null) { - event.setTags(new HashMap<>(scope.getTags())); + event.setTags(scope.getTags()); } else { for (Map.Entry item : scope.getTags().entrySet()) { if (!event.getTags().containsKey(item.getKey())) { @@ -1483,7 +1482,7 @@ public void captureBatchedMetricsEvents(final @NotNull SentryMetricsEvents metri replayEvent.setUser(scope.getUser()); } if (replayEvent.getTags() == null) { - replayEvent.setTags(new HashMap<>(scope.getTags())); + replayEvent.setTags(scope.getTags()); } else { for (Map.Entry item : scope.getTags().entrySet()) { if (!replayEvent.getTags().containsKey(item.getKey())) { @@ -1523,7 +1522,7 @@ public void captureBatchedMetricsEvents(final @NotNull SentryMetricsEvents metri sentryBaseEvent.setUser(scope.getUser()); } if (sentryBaseEvent.getTags() == null) { - sentryBaseEvent.setTags(new HashMap<>(scope.getTags())); + sentryBaseEvent.setTags(scope.getTags()); } else { for (Map.Entry item : scope.getTags().entrySet()) { if (!sentryBaseEvent.getTags().containsKey(item.getKey())) { @@ -1537,7 +1536,7 @@ public void captureBatchedMetricsEvents(final @NotNull SentryMetricsEvents metri sortBreadcrumbsByDate(sentryBaseEvent, scope.getBreadcrumbs()); } if (sentryBaseEvent.getExtras() == null) { - sentryBaseEvent.setExtras(new HashMap<>(scope.getExtras())); + sentryBaseEvent.setExtras(scope.getExtras()); } else { for (Map.Entry item : scope.getExtras().entrySet()) { if (!sentryBaseEvent.getExtras().containsKey(item.getKey())) { diff --git a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java index dbbc36524d..728478f590 100644 --- a/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java +++ b/sentry/src/main/java/io/sentry/SentryEnvelopeItem.java @@ -41,6 +41,8 @@ public final class SentryEnvelopeItem { @SuppressWarnings("CharsetObjectCanBeUsed") private static final Charset UTF_8 = Charset.forName("UTF-8"); + private static final int WRITER_BUFFER_SIZE = 512; + private final SentryEnvelopeItemHeader header; // Either dataFactory is set or data needs to be set. private final @Nullable Callable dataFactory; @@ -85,7 +87,9 @@ public final class SentryEnvelopeItem { new CachedItem( () -> { try (final ByteArrayOutputStream stream = new ByteArrayOutputStream(); - final Writer writer = new BufferedWriter(new OutputStreamWriter(stream, UTF_8))) { + final Writer writer = + new BufferedWriter( + new OutputStreamWriter(stream, UTF_8), WRITER_BUFFER_SIZE)) { serializer.serialize(session, writer); return stream.toByteArray(); } @@ -119,7 +123,9 @@ public final class SentryEnvelopeItem { new CachedItem( () -> { try (final ByteArrayOutputStream stream = new ByteArrayOutputStream(); - final Writer writer = new BufferedWriter(new OutputStreamWriter(stream, UTF_8))) { + final Writer writer = + new BufferedWriter( + new OutputStreamWriter(stream, UTF_8), WRITER_BUFFER_SIZE)) { serializer.serialize(event, writer); return stream.toByteArray(); } @@ -179,7 +185,9 @@ public static SentryEnvelopeItem fromUserFeedback( new CachedItem( () -> { try (final ByteArrayOutputStream stream = new ByteArrayOutputStream(); - final Writer writer = new BufferedWriter(new OutputStreamWriter(stream, UTF_8))) { + final Writer writer = + new BufferedWriter( + new OutputStreamWriter(stream, UTF_8), WRITER_BUFFER_SIZE)) { serializer.serialize(userFeedback, writer); return stream.toByteArray(); } @@ -206,7 +214,9 @@ public static SentryEnvelopeItem fromCheckIn( new CachedItem( () -> { try (final ByteArrayOutputStream stream = new ByteArrayOutputStream(); - final Writer writer = new BufferedWriter(new OutputStreamWriter(stream, UTF_8))) { + final Writer writer = + new BufferedWriter( + new OutputStreamWriter(stream, UTF_8), WRITER_BUFFER_SIZE)) { serializer.serialize(checkIn, writer); return stream.toByteArray(); } @@ -344,7 +354,9 @@ private static void ensureAttachmentSizeLimit( } try (final ByteArrayOutputStream stream = new ByteArrayOutputStream(); - final Writer writer = new BufferedWriter(new OutputStreamWriter(stream, UTF_8))) { + final Writer writer = + new BufferedWriter( + new OutputStreamWriter(stream, UTF_8), WRITER_BUFFER_SIZE)) { serializer.serialize(profileChunk, writer); return stream.toByteArray(); } catch (IOException e) { @@ -403,7 +415,9 @@ private static void ensureAttachmentSizeLimit( profilingTraceData.readDeviceCpuFrequencies(); try (final ByteArrayOutputStream stream = new ByteArrayOutputStream(); - final Writer writer = new BufferedWriter(new OutputStreamWriter(stream, UTF_8))) { + final Writer writer = + new BufferedWriter( + new OutputStreamWriter(stream, UTF_8), WRITER_BUFFER_SIZE)) { serializer.serialize(profilingTraceData, writer); return stream.toByteArray(); } catch (IOException e) { @@ -437,7 +451,9 @@ private static void ensureAttachmentSizeLimit( new CachedItem( () -> { try (final ByteArrayOutputStream stream = new ByteArrayOutputStream(); - final Writer writer = new BufferedWriter(new OutputStreamWriter(stream, UTF_8))) { + final Writer writer = + new BufferedWriter( + new OutputStreamWriter(stream, UTF_8), WRITER_BUFFER_SIZE)) { serializer.serialize(clientReport, writer); return stream.toByteArray(); } @@ -481,7 +497,8 @@ public static SentryEnvelopeItem fromReplay( try { try (final ByteArrayOutputStream stream = new ByteArrayOutputStream(); final Writer writer = - new BufferedWriter(new OutputStreamWriter(stream, UTF_8))) { + new BufferedWriter( + new OutputStreamWriter(stream, UTF_8), WRITER_BUFFER_SIZE)) { // relay expects the payload to be in this exact order: [event,rrweb,video] final Map replayPayload = new LinkedHashMap<>(); // first serialize replay event json bytes @@ -541,7 +558,9 @@ public static SentryEnvelopeItem fromLogs( new CachedItem( () -> { try (final ByteArrayOutputStream stream = new ByteArrayOutputStream(); - final Writer writer = new BufferedWriter(new OutputStreamWriter(stream, UTF_8))) { + final Writer writer = + new BufferedWriter( + new OutputStreamWriter(stream, UTF_8), WRITER_BUFFER_SIZE)) { serializer.serialize(logEvents, writer); return stream.toByteArray(); } @@ -571,7 +590,9 @@ public static SentryEnvelopeItem fromMetrics( new CachedItem( () -> { try (final ByteArrayOutputStream stream = new ByteArrayOutputStream(); - final Writer writer = new BufferedWriter(new OutputStreamWriter(stream, UTF_8))) { + final Writer writer = + new BufferedWriter( + new OutputStreamWriter(stream, UTF_8), WRITER_BUFFER_SIZE)) { serializer.serialize(metricsEvents, writer); return stream.toByteArray(); } diff --git a/sentry/src/test/java/io/sentry/CombinedScopeViewTest.kt b/sentry/src/test/java/io/sentry/CombinedScopeViewTest.kt index d768d6d32d..fd187235a9 100644 --- a/sentry/src/test/java/io/sentry/CombinedScopeViewTest.kt +++ b/sentry/src/test/java/io/sentry/CombinedScopeViewTest.kt @@ -11,6 +11,7 @@ import junit.framework.TestCase.assertTrue import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNotNull +import kotlin.test.assertNotSame import kotlin.test.assertNull import kotlin.test.assertSame import org.junit.Assert.assertNotEquals @@ -72,6 +73,74 @@ class CombinedScopeViewTest { assertEquals("current 2", breadcrumbs.poll().message) } + @Test + fun `returns single non-empty breadcrumb queue directly`() { + var combined = fixture.getSut() + fixture.globalScope.addBreadcrumb(Breadcrumb.info("global")) + assertSame(fixture.globalScope.breadcrumbs, combined.breadcrumbs) + + combined = fixture.getSut() + fixture.isolationScope.addBreadcrumb(Breadcrumb.info("isolation")) + assertSame(fixture.isolationScope.breadcrumbs, combined.breadcrumbs) + + combined = fixture.getSut() + fixture.scope.addBreadcrumb(Breadcrumb.info("current")) + assertSame(fixture.scope.breadcrumbs, combined.breadcrumbs) + } + + @Test + fun `returns default write scope breadcrumbs when all scopes are empty`() { + val combined = fixture.getSut(SentryOptions().also { it.defaultScopeType = ScopeType.CURRENT }) + + assertSame(fixture.scope.breadcrumbs, combined.breadcrumbs) + } + + @Test + fun `returns merged breadcrumb copy when multiple scopes have breadcrumbs`() { + val combined = fixture.getSut() + + fixture.globalScope.addBreadcrumb(Breadcrumb.info("global")) + fixture.isolationScope.addBreadcrumb(Breadcrumb.info("isolation")) + + val breadcrumbs = combined.breadcrumbs + + assertNotSame(fixture.globalScope.breadcrumbs, breadcrumbs) + assertNotSame(fixture.isolationScope.breadcrumbs, breadcrumbs) + assertEquals(2, breadcrumbs.size) + } + + @Test + fun `returns single non-empty combined collections directly`() { + val globalScope = mock() + val isolationScope = mock() + val scope = mock() + val combined = CombinedScopeView(globalScope, isolationScope, scope) + + val tags = mapOf("tag" to "value") + whenever(globalScope.tags).thenReturn(emptyMap()) + whenever(isolationScope.tags).thenReturn(emptyMap()) + whenever(scope.tags).thenReturn(tags) + assertSame(tags, combined.tags) + + val attributes = mapOf("attribute" to SentryAttribute.named("attribute", "value")) + whenever(globalScope.attributes).thenReturn(emptyMap()) + whenever(isolationScope.attributes).thenReturn(emptyMap()) + whenever(scope.attributes).thenReturn(attributes) + assertSame(attributes, combined.attributes) + + val extras = mapOf("extra" to "value") + whenever(globalScope.extras).thenReturn(emptyMap()) + whenever(isolationScope.extras).thenReturn(emptyMap()) + whenever(scope.extras).thenReturn(extras) + assertSame(extras, combined.extras) + + val attachments = listOf(createAttachment("attachment.png")) + whenever(globalScope.attachments).thenReturn(emptyList()) + whenever(isolationScope.attachments).thenReturn(emptyList()) + whenever(scope.attachments).thenReturn(attachments) + assertSame(attachments, combined.attachments) + } + @Test fun `oldest breadcrumbs are dropped first`() { val options = SentryOptions().also { it.maxBreadcrumbs = 5 } diff --git a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt index 229fd57187..fe5c835c90 100644 --- a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt +++ b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt @@ -358,6 +358,19 @@ class MainEventProcessorTest { } } + @Test + fun `options tags are copied when applied to event`() { + val sut = fixture.getSut(tags = mapOf("tag1" to "value1")) + val event = SentryEvent() + + sut.process(event, Hint()) + val eventTags = event.tags!! + + fixture.sentryOptions.setTag("tag2", "value2") + + assertFalse(eventTags.containsKey("tag2")) + } + @Test fun `when event has a tag set with the same name as SentryOptions tags, the tag value from the event is retained`() { val sut = fixture.getSut(tags = mapOf("tag1" to "value1", "tag2" to "value2")) diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index ab6fd2075a..f51345957e 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -534,6 +534,24 @@ class SentryClientTest { assertNotNull(event.request) { assertEquals("post", it.method) } } + @Test + fun `when captureEvent applies scope tags and extras, event map containers are copied`() { + val event = SentryEvent() + val scope = createScope() + + val sut = fixture.getSut() + + sut.captureEvent(event, scope) + val eventTags = event.tags!! + val eventExtras = event.extras!! + + scope.setTag("newTag", "newValue") + scope.setExtra("newExtra", "newValue") + + assertFalse(eventTags.containsKey("newTag")) + assertFalse(eventExtras.containsKey("newExtra")) + } + @Test fun `when breadcrumbs are not empty, sort them out by date`() { val b1 = Breadcrumb(DateUtils.getDateTime("2020-03-27T08:52:58.001Z")) diff --git a/sentry/src/test/java/io/sentry/protocol/SentryBaseEventSerializationTest.kt b/sentry/src/test/java/io/sentry/protocol/SentryBaseEventSerializationTest.kt index 4cafb1ed8a..35322d2659 100644 --- a/sentry/src/test/java/io/sentry/protocol/SentryBaseEventSerializationTest.kt +++ b/sentry/src/test/java/io/sentry/protocol/SentryBaseEventSerializationTest.kt @@ -9,6 +9,7 @@ import io.sentry.SentryBaseEvent import io.sentry.SentryIntegrationPackageStorage import io.sentry.vendor.gson.stream.JsonToken import kotlin.test.assertEquals +import kotlin.test.assertFalse import org.junit.After import org.junit.Before import org.junit.Test @@ -102,4 +103,26 @@ class SentryBaseEventSerializationTest { assertEquals(expectedJson, actualJson) } + + @Test + fun `setTags copies source map`() { + val source = mutableMapOf("a" to "1") + val sut = Sut() + + sut.tags = source + source["b"] = "2" + + assertFalse(sut.tags!!.containsKey("b")) + } + + @Test + fun `setExtras copies source map`() { + val source = mutableMapOf("a" to "1") + val sut = Sut() + + sut.setExtras(source) + source["b"] = "2" + + assertFalse(sut.extras!!.containsKey("b")) + } }