MaxSpans is ignored for nested spans

[bignoncedric]

Integration

sentry

Java Version

17

Version

6.33.1

Steps to Reproduce

The following code generates a high number of spans (1500) nested in a transaction (as direct parent) - testInTransaction(), or nested in a span - testInNestedSpan.

The Hub is configured with MaxSpan = 1000.

private void testInTransaction() {
  ITransaction transaction = Sentry.startTransaction("Main Transaction", "task");
  createManySpans(transaction);
  transaction.finish();
}

private void testInNestedSpan() {
  ITransaction transaction = Sentry.startTransaction("Main Transaction", "task");
  ISpan parent = transaction.startChild("task", "Parent Span");
  createManySpans(parent);
  parent.finish();
  transaction.finish();
}

private static void createManySpans(ISpan parent) {
  for (int numSpan = 1; numSpan <= 1500; numSpan++) {
    ISpan innerSpan = parent.startChild("task", "Span Number = " + numSpan);
    innerSpan.finish();
  }
}

Execute both methods: testInTransaction() and testInNestedSpan()

Expected Result

Given the configuration set to limit MaxSpan to 1000 spans, we should not more than 1000 spans in both transactions.

Actual Result

While the first case (testInTransaction) works as expected. See:

The second case (testInNestedSpan) do not follow the 1000 Max spans constraints:

When looking at the code, the check to getMaxSpans() is done in SentryTrace.createChild:441 but the actual add of child is done in SentryTrace.createChild:346.

Here is the stack trace for calls to SentryTracer.children.add() for the first case:

• createChild(SpanId, String, String, SentryDate, Instrumenter, SpanOptions):393, SentryTracer (io.sentry), SentryTracer.java
• startChild(SpanId, String, String, SentryDate, Instrumenter, SpanOptions):324, SentryTracer (io.sentry), SentryTracer.java
• startChild(String, String, SentryDate, Instrumenter, SpanOptions):121, Span (io.sentry), Span.java
• createChild(String, String, SentryDate, Instrumenter, SpanOptions):456, SentryTracer (io.sentry), SentryTracer.java (<-- check on MaxSpans is done here)
• startChild(String, String, SentryDate, Instrumenter, SpanOptions):418, SentryTracer (io.sentry), SentryTracer.java
• startChild(String, String):432, SentryTracer (io.sentry), SentryTracer.java
• createManySpans(ISpan)
• testInTransaction()

And here is the stack trace for the second case:

• createChild(SpanId, String, String, SentryDate, Instrumenter, SpanOptions):393, SentryTracer (io.sentry), SentryTracer.java
• createChild(SpanId, String, String, SpanOptions):342, SentryTracer (io.sentry), SentryTracer.java
• startChild(SpanId, String, String, SpanOptions):302, SentryTracer (io.sentry), SentryTracer.java
• startChild(SpanId, String, String):284, SentryTracer (io.sentry), SentryTracer.java
• startChild(String, String):132, Span (io.sentry), Span.java
• createManySpans(ISpan)
• testInNestedSpan()

We can see that in the second case, the Span.startChild "bypasses" the check on MaxSpans.

[kahest]

Thank you for the detailed report 🙏 we discussed this and will follow up here

[adinauer]

Thanks for the report @bignoncedric. We do agree this limit should apply to all spans including child spans. While this is technically a breaking change we do think this should be fixed even without a new major in case we can't fix it in time for 7.0.0. @kahest would you agree? The current implementation misses the point of limiting memory consumption. Other SDKs seem to use a concept of a SpanRecorder to enforce the limit.

[romtsn]

Closed by #3065

[bignoncedric]

Thanks!
Is it going to be merged into Sentry 6.x branch? If not, do you know when 7.0.0 will be shipped?

[romtsn]

We're gonna ship it this week, stay tuned!