Respect maxSpans for nested child spans

CHANGELOG.md

@@ -47,6 +47,7 @@ Similarly, if you have a Sentry SDK (e.g. `sentry-android-core`) dependency on o
 - Set ip_address to {{auto}} by default, even if sendDefaultPII is disabled ([#2860](https://github.com/getsentry/sentry-java/pull/2860))
     - Instead use the "Prevent Storing of IP Addresses" option in the "Security & Privacy" project settings on sentry.io
 - Raw logback message and parameters are now guarded by `sendDefaultPii` if an `encoder` has been configured ([#2976](https://github.com/getsentry/sentry-java/pull/2976))
+- The `maxSpans` setting (defaults to 1000) is enforced for nested child spans which means a single transaction can have `maxSpans` number of children (nested or not) at most ([#3065](https://github.com/getsentry/sentry-java/pull/3065))
 
 ## Deprecations
 

sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt

@@ -45,6 +45,7 @@ import org.mockito.kotlin.clearInvocations
 import org.mockito.kotlin.eq
 import org.mockito.kotlin.mock
 import org.mockito.kotlin.never
+import org.mockito.kotlin.times
 import org.mockito.kotlin.verify
 import org.mockito.kotlin.whenever
 import org.robolectric.Shadows.shadowOf

sentry/src/main/java/io/sentry/SentryTracer.java

@@ -421,40 +421,51 @@ private ISpan createChild(
       return NoOpSpan.getInstance();
     }
 
-    Objects.requireNonNull(parentSpanId, "parentSpanId is required");
-    Objects.requireNonNull(operation, "operation is required");
-    cancelIdleTimer();
-    final Span span =
-        new Span(
-            root.getTraceId(),
-            parentSpanId,
-            this,
-            operation,
-            this.hub,
-            timestamp,
-            spanOptions,
-            __ -> {
-              final FinishStatus finishStatus = this.finishStatus;
-              if (transactionOptions.getIdleTimeout() != null) {
-                // if it's an idle transaction, no matter the status, we'll reset the timeout here
-                // so the transaction will either idle and finish itself, or a new child will be
-                // added and we'll wait for it again
-                if (!transactionOptions.isWaitForChildren() || hasAllChildrenFinished()) {
-                  scheduleFinish();
+    if (children.size() < hub.getOptions().getMaxSpans()) {
+      Objects.requireNonNull(parentSpanId, "parentSpanId is required");
+      Objects.requireNonNull(operation, "operation is required");
+      cancelIdleTimer();
+      final Span span =
+          new Span(
+              root.getTraceId(),
+              parentSpanId,
+              this,
+              operation,
+              this.hub,
+              timestamp,
+              spanOptions,
+              __ -> {
+                final FinishStatus finishStatus = this.finishStatus;
+                if (transactionOptions.getIdleTimeout() != null) {
+                  // if it's an idle transaction, no matter the status, we'll reset the timeout here
+                  // so the transaction will either idle and finish itself, or a new child will be
+                  // added and we'll wait for it again
+                  if (!transactionOptions.isWaitForChildren() || hasAllChildrenFinished()) {
+                    scheduleFinish();
+                  }
+                } else if (finishStatus.isFinishing) {
+                  finish(finishStatus.spanStatus);
                 }
-              } else if (finishStatus.isFinishing) {
-                finish(finishStatus.spanStatus);
-              }
-            });
-    span.setDescription(description);
-    span.setData(SpanDataConvention.THREAD_ID, String.valueOf(Thread.currentThread().getId()));
-    span.setData(
-        SpanDataConvention.THREAD_NAME,
-        hub.getOptions().getMainThreadChecker().isMainThread()
-            ? "main"
-            : Thread.currentThread().getName());
-    this.children.add(span);
-    return span;
+              });
+      span.setDescription(description);
+      span.setData(SpanDataConvention.THREAD_ID, String.valueOf(Thread.currentThread().getId()));
+      span.setData(
+          SpanDataConvention.THREAD_NAME,
+          hub.getOptions().getMainThreadChecker().isMainThread()
+              ? "main"
+              : Thread.currentThread().getName());
+      this.children.add(span);
+      return span;
+    } else {
+      hub.getOptions()
+          .getLogger()
+          .log(
+              SentryLevel.WARNING,
+              "Span operation: %s, description: %s dropped due to limit reached. Returning NoOpSpan.",
+              operation,
+              description);
+      return NoOpSpan.getInstance();
+    }
   }
 
   @Override

sentry/src/test/java/io/sentry/SentryTracerTest.kt

@@ -1332,4 +1332,19 @@ class SentryTracerTest {
         assertNotNull(span.getData(SpanDataConvention.THREAD_ID))
         assertNotEquals("main", span.getData(SpanDataConvention.THREAD_NAME))
     }
+
+    @Test
+    fun `maxSpans is respected by nested child spans`() {
+        val tracer = fixture.getSut(optionsConfiguration = { it.maxSpans = 5 })
+
+        val nested = tracer.startChild("task", "parent span")
+        repeat(10) {
+            val child = nested.startChild("task", "span number $it")
+            child.finish()
+        }
+        nested.finish()
+        tracer.finish()
+
+        assertEquals(5, tracer.children.size)
+    }
 }