Skip to content

Commit

Permalink
Fix: Allow MaxBreadcrumb 0 / Expose MaxBreadcrumb metadata. (#3836)
Browse files Browse the repository at this point in the history
* expose max-breadcrumbs on meta data and implement disabled queue when maxbreadcrumbs sets to 0
* missing queue class and test
* update changelog

---------

Co-authored-by: Lucas <luca-zimerman1@hotmail.com>
Co-authored-by: Stefano <stefano.siano91@gmail.com>
  • Loading branch information
3 people authored Nov 4, 2024
1 parent 28a11a7 commit 2af8d1a
Show file tree
Hide file tree
Showing 8 changed files with 257 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@

### Features

- Add meta option to set the maximum amount of breadcrumbs to be logged. ([#3836](https://github.com/getsentry/sentry-java/pull/3836))
- Use a separate `Random` instance per thread to improve SDK performance ([#3835](https://github.com/getsentry/sentry-java/pull/3835))

### Fixes

- Using MaxBreadcrumb with value 0 no longer crashes. ([#3836](https://github.com/getsentry/sentry-java/pull/3836))
- Accept manifest integer values when requiring floating values ([#3823](https://github.com/getsentry/sentry-java/pull/3823))

## 7.16.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ final class ManifestMetadataReader {

static final String ENABLE_METRICS = "io.sentry.enable-metrics";

static final String MAX_BREADCRUMBS = "io.sentry.max-breadcrumbs";

static final String REPLAYS_SESSION_SAMPLE_RATE = "io.sentry.session-replay.session-sample-rate";

static final String REPLAYS_ERROR_SAMPLE_RATE = "io.sentry.session-replay.on-error-sample-rate";
Expand Down Expand Up @@ -213,6 +215,9 @@ static void applyMetadata(
SESSION_TRACKING_TIMEOUT_INTERVAL_MILLIS,
options.getSessionTrackingIntervalMillis()));

options.setMaxBreadcrumbs(
(int) readLong(metadata, logger, MAX_BREADCRUMBS, options.getMaxBreadcrumbs()));

options.setEnableActivityLifecycleBreadcrumbs(
readBool(
metadata,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1516,6 +1516,31 @@ class ManifestMetadataReaderTest {
assertTrue(fixture.options.experimental.sessionReplay.maskViewClasses.contains(SentryReplayOptions.TEXT_VIEW_CLASS_NAME))
}

@Test
fun `applyMetadata reads maxBreadcrumbs to options and sets the value if found`() {
// Arrange
val bundle = bundleOf(ManifestMetadataReader.MAX_BREADCRUMBS to 1)
val context = fixture.getContext(metaData = bundle)

// Act
ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider)

// Assert
assertEquals(1, fixture.options.maxBreadcrumbs)
}

@Test
fun `applyMetadata reads maxBreadcrumbs to options and keeps default if not found`() {
// Arrange
val context = fixture.getContext()

// Act
ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider)

// Assert
assertEquals(100, fixture.options.maxBreadcrumbs)
}

@Test
fun `applyMetadata reads integers even when expecting floats`() {
// Arrange
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,9 @@
<!-- how to enable the attach screenshot feature-->
<meta-data android:name="io.sentry.attach-screenshot" android:value="true" />

<!-- how many breadcrumbs will be stored-->
<meta-data android:name="io.sentry.max-breadcrumbs" android:value="100"/>

<!-- how to enable the attach view hierarchy feature-->
<meta-data android:name="io.sentry.attach-view-hierarchy" android:value="true" />

Expand Down
115 changes: 115 additions & 0 deletions sentry/src/main/java/io/sentry/DisabledQueue.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
package io.sentry;

import java.io.Serializable;
import java.util.AbstractCollection;
import java.util.Iterator;
import java.util.NoSuchElementException;
import java.util.Queue;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

final class DisabledQueue<E> extends AbstractCollection<E> implements Queue<E>, Serializable {

/** Serialization version. */
private static final long serialVersionUID = -8423413834657610417L;

/** Constructor that creates a queue that does not accept any element. */
public DisabledQueue() {}

// -----------------------------------------------------------------------
/**
* Returns the number of elements stored in the queue.
*
* @return this queue's size
*/
@Override
public int size() {
return 0;
}

/**
* Returns true if this queue is empty; false otherwise.
*
* @return false
*/
@Override
public boolean isEmpty() {
return false;
}

/** Does nothing. */
@Override
public void clear() {}

/**
* Since the queue is disabled, the element will not be added.
*
* @param element the element to add
* @return false, always
*/
@Override
public boolean add(final @NotNull E element) {
return false;
}

// -----------------------------------------------------------------------

/**
* Receives an element but do nothing with it.
*
* @param element the element to add
* @return false, always
*/
@Override
public boolean offer(@NotNull E element) {
return false;
}

@Override
public @Nullable E poll() {
return null;
}

@Override
public @Nullable E element() {
return null;
}

@Override
public @Nullable E peek() {
return null;
}

@Override
public @NotNull E remove() {
throw new NoSuchElementException("queue is disabled");
}

// -----------------------------------------------------------------------

/**
* Returns an iterator over this queue's elements.
*
* @return an iterator over this queue's elements
*/
@Override
public @NotNull Iterator<E> iterator() {
return new Iterator<E>() {

@Override
public boolean hasNext() {
return false;
}

@Override
public E next() {
throw new NoSuchElementException();
}

@Override
public void remove() {
throw new IllegalStateException();
}
};
}
}
4 changes: 3 additions & 1 deletion sentry/src/main/java/io/sentry/Scope.java
Original file line number Diff line number Diff line change
Expand Up @@ -754,7 +754,9 @@ public void clearAttachments() {
* @return the breadcrumbs queue
*/
private @NotNull Queue<Breadcrumb> createBreadcrumbsList(final int maxBreadcrumb) {
return SynchronizedQueue.synchronizedQueue(new CircularFifoQueue<>(maxBreadcrumb));
return maxBreadcrumb > 0
? SynchronizedQueue.synchronizedQueue(new CircularFifoQueue<>(maxBreadcrumb))
: SynchronizedQueue.synchronizedQueue(new DisabledQueue<>());
}

/**
Expand Down
93 changes: 93 additions & 0 deletions sentry/src/test/java/io/sentry/DisabledQueueTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package io.sentry
import org.junit.Assert.assertThrows
import java.util.NoSuchElementException
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertNull

class DisabledQueueTest {

@Test
fun `size starts empty`() {
val queue = DisabledQueue<Int>()
assertEquals(0, queue.size, "Size should always be zero.")
}

@Test
fun `add does not add elements`() {
val queue = DisabledQueue<Int>()
assertFalse(queue.add(1), "add should always return false.")
assertEquals(0, queue.size, "Size should still be zero after attempting to add an element.")
}

@Test
fun `isEmpty returns false when created`() {
val queue = DisabledQueue<Int>()
assertFalse(queue.isEmpty(), "isEmpty should always return false.")
}

@Test
fun `isEmpty always returns false if add function was called`() {
val queue = DisabledQueue<Int>()
queue.add(1)

assertFalse(queue.isEmpty(), "isEmpty should always return false.")
}

@Test
fun `offer does not add elements`() {
val queue = DisabledQueue<Int>()
assertFalse(queue.offer(1), "offer should always return false.")
assertEquals(0, queue.size, "Size should still be zero after attempting to offer an element.")
}

@Test
fun `poll returns null`() {
val queue = DisabledQueue<Int>()
queue.add(1)
assertNull(queue.poll(), "poll should always return null.")
}

@Test
fun `peek returns null`() {
val queue = DisabledQueue<Int>()
queue.add(1)

assertNull(queue.peek(), "peek should always return null.")
}

@Test
fun `element returns null`() {
val queue = DisabledQueue<Int>()
assertNull(queue.element(), "element should always return null.")
}

@Test
fun `remove throws NoSuchElementException`() {
val queue = DisabledQueue<Int>()
assertThrows(NoSuchElementException::class.java) { queue.remove() }
}

@Test
fun `clear does nothing`() {
val queue = DisabledQueue<Int>()
queue.clear() // Should not throw an exception
assertEquals(0, queue.size, "Size should remain zero after clear.")
}

@Test
fun `iterator has no elements`() {
val queue = DisabledQueue<Int>()
val iterator = queue.iterator()
assertFalse(iterator.hasNext(), "Iterator should have no elements.")
assertThrows(NoSuchElementException::class.java) { iterator.next() }
}

@Test
fun `iterator remove throws IllegalStateException`() {
val queue = DisabledQueue<Int>()
val iterator = queue.iterator()
assertThrows(IllegalStateException::class.java) { iterator.remove() }
}
}
11 changes: 11 additions & 0 deletions sentry/src/test/java/io/sentry/ScopeTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -1029,6 +1029,17 @@ class ScopeTest {
)
}

@Test
fun `creating a new scope won't crash if max breadcrumbs is set to zero`() {
val options = SentryOptions().apply {
maxBreadcrumbs = 0
}
val scope = Scope(options)

// expect no exception to be thrown
// previously was crashing, see https://github.com/getsentry/sentry-java/issues/3313
}

private fun eventProcessor(): EventProcessor {
return object : EventProcessor {
override fun process(event: SentryEvent, hint: Hint): SentryEvent? {
Expand Down

0 comments on commit 2af8d1a

Please sign in to comment.