Skip to content

Conversation

@mosche
Copy link
Contributor

@mosche mosche commented Oct 23, 2025

This fixes a bug in APM tracer that ignored the recording status (isRecording) of spans as decided by the APM Java agent.
This circumvented transaction_max_spans and resulted in a chain of descendent spans in a transaction being held in memory potentially causing OOMs for very long running transactions.

This PR fixes the tracer to properly respect isRecording.

  1. APMTracer receives the parent task's trace context and sets that as the parent of a new span.
            final Context parentContext = getParentContext(traceContext);
                spanBuilder.setParent(parentContext);
  1. APMTracer requests a new span, setting the start time to the same as the transaction start time
            Instant startTime = traceContext.getTransient(Task.TRACE_START_TIME);
                spanBuilder.setStartTimestamp(startTime);
            final Span span = spanBuilder.startSpan();
  1. The span is created but is too many for the transaction because transaction_max_spans is exceeded for this span, so it is set as a non-recording span
  2. APMTracer stores the Context containing the span in its spans map, ie this new span is now the next parent span so will be stored as the task's trace context
  • this is incorrect because the span is a non-recording span, so should be discarded
  • the result of the incorrect approach is that you can have a chain of descendent spans in a transaction which can be unlimited in size, and all with the same start time

(Replaces #136978)

@mosche mosche requested a review from a team October 23, 2025 10:13
@mosche mosche added auto-backport Automatically create backport pull requests when merged :Core/Infra/Metrics Metrics and metering infrastructure v9.2.1 v9.3.0 v8.19.7 v9.1.7 labels Oct 23, 2025
@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Oct 23, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@mosche mosche requested a review from rjernst October 24, 2025 14:02
Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mosche mosche merged commit eb793cf into elastic:main Oct 26, 2025
34 checks passed
mosche added a commit to mosche/elasticsearch that referenced this pull request Oct 26, 2025
mosche added a commit to mosche/elasticsearch that referenced this pull request Oct 26, 2025
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
9.2
8.19
9.1

mosche added a commit to mosche/elasticsearch that referenced this pull request Oct 26, 2025
shmuelhanoch pushed a commit to shmuelhanoch/elasticsearch that referenced this pull request Oct 27, 2025
fzowl pushed a commit to voyage-ai/elasticsearch that referenced this pull request Nov 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged :Core/Infra/Metrics Metrics and metering infrastructure >non-issue Team:Core/Infra Meta label for core/infra team v8.19.7 v9.1.7 v9.2.1 v9.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants