Skip to content

Do not automatically start tracing transactions in TaskManager #130607

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

mosche
Copy link
Contributor

@mosche mosche commented Jul 4, 2025

This changes TaskManager to only create new tracing spans if a trace parent (and hence a tracing transaction) already exists.

Tracing transactions are automatically started in the RestController for external requests. To trace detached, internal transport actions, a transaction has to be explicitly started using Tracer.startTrace.

This helps to prevent everlasting tracing transaction for internal self-rescheduling work that is using transport actions. Such transactions have caused memory issues with the APM java agent in the past.

Relates to ES-10969

This changes TaskManager to only create new tracing spans if a trace
parent (and hence a tracing transaction) already exists.

Tracing transactions are automatically started in the RestController for
external requests. To trace detached, internal transport actions, a
transaction has to be explicitly started using Tracer.startTrace.

This helps to prevent everlasting tracing transaction for internal
self-rescheduling work that is using transport actions. Such
transactions have caused memory issues with the APM java agent in the
past.

Relates to ES-10969
@mosche mosche requested a review from a team as a code owner July 4, 2025 07:17
@mosche mosche added >non-issue :Core/Infra/Metrics Metrics and metering infrastructure labels Jul 4, 2025
@mosche mosche requested review from DaveCTurner and a team and removed request for a team July 4, 2025 07:17
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine added Team:Core/Infra Meta label for core/infra team v9.2.0 serverless-linked Added by automation, don't add manually labels Jul 4, 2025
Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

All makes sense to me - I have left comments, some of which suggest maybe breaking this up into separate PRs because there's a lot of stuff going on here.

@mosche
Copy link
Contributor Author

mosche commented Jul 10, 2025

@DaveCTurner this PR contains only the remaining key changes and should be ready for another review

Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

Great stuff LGTM now

@mosche mosche added auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) and removed serverless-linked Added by automation, don't add manually labels Jul 10, 2025
@elasticsearchmachine elasticsearchmachine merged commit 13355fa into elastic:main Jul 10, 2025
33 checks passed
@mosche mosche deleted the tracing/trace_tasks_if_parent_trace_context branch July 10, 2025 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Core/Infra/Metrics Metrics and metering infrastructure >non-issue Team:Core/Infra Meta label for core/infra team v9.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants