Skip to content

Ensure tracer resources cleaned up in exit advice#11905

Open
mcculls wants to merge 1 commit into
masterfrom
ncculls/add-onThrowable-to-cleanup-onexit-advice
Open

Ensure tracer resources cleaned up in exit advice#11905
mcculls wants to merge 1 commit into
masterfrom
ncculls/add-onThrowable-to-cleanup-onexit-advice

Conversation

@mcculls

@mcculls mcculls commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

What Does This Do

Ensure tracer resources cleaned up in exit advice are still cleaned up even if the patched method throws an exception

Motivation

Discovered while using Claude to check codebase consistency.

Contributor Checklist

Jira ticket: [PROJ-IDENT]

@mcculls mcculls added the type: bug fix Bug fix label Jul 9, 2026
@mcculls mcculls requested review from a team as code owners July 9, 2026 20:59
@mcculls mcculls added the inst: others All other instrumentations label Jul 9, 2026
@mcculls mcculls requested a review from a team as a code owner July 9, 2026 20:59
@mcculls mcculls added tag: no release notes Changes to exclude from release notes inst: lettuce Lettuce instrumentation labels Jul 9, 2026
@mcculls mcculls requested review from jordan-wong and removed request for a team July 9, 2026 20:59
@mcculls mcculls added inst: kafka Kafka instrumentation inst: elasticsearch Elasticsearch instrumentation inst: jax-rs JAX-RS instrumentation labels Jul 9, 2026
@mcculls mcculls requested review from PerfectSlayer and removed request for a team July 9, 2026 20:59
@mcculls mcculls added the tag: ai generated Largely based on code generated by an AI or LLM label Jul 9, 2026
@mcculls mcculls requested a review from amarziali July 9, 2026 20:59

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b7dc8d5123

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@datadog-datadog-prod-us1-2

datadog-datadog-prod-us1-2 Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 68.69% (+11.74%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 01796fc | Docs | Datadog PR Page | Give us feedback!

@dd-octo-sts

dd-octo-sts Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🟢 Java Benchmark SLOs — All performance SLOs passed

Suite Status
Startup 🟢 pass

SLO thresholds are defined here based on automatically generated metrics. A warning is raised when results are within 5% of the threshold.

PR vs. master results
Scenario Candidate master Δ (95% CI of mean)
startup:insecure-bank:iast:Agent 14.01 s 14.01 s [-0.8%; +0.8%] (no difference)
startup:insecure-bank:tracing:Agent 12.87 s 13.04 s [-2.2%; -0.4%] (maybe better)
startup:petclinic:appsec:Agent 16.75 s 16.81 s [-1.3%; +0.7%] (no difference)
startup:petclinic:iast:Agent 16.41 s 16.90 s [-7.2%; +1.4%] (no difference)
startup:petclinic:profiling:Agent 16.75 s 16.67 s [-0.7%; +1.7%] (no difference)
startup:petclinic:sca:Agent 16.85 s 16.80 s [-0.9%; +1.4%] (no difference)
startup:petclinic:tracing:Agent 16.07 s 15.93 s [-0.1%; +1.8%] (no difference)

Commit: 01796fcf · CI Pipeline · Benchmarking Platform UI


Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion.

@mcculls mcculls force-pushed the ncculls/add-onThrowable-to-cleanup-onexit-advice branch 2 times, most recently from fa80293 to 32a9fed Compare July 9, 2026 21:28
@mcculls mcculls requested a review from Copilot July 9, 2026 21:31
@mcculls

mcculls commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR updates multiple ByteBuddy @Advice.OnMethodExit hooks across instrumentations so cleanup/finishing logic still runs when the instrumented method exits by throwing, preventing leaked scopes/spans and stale context-store entries.

Changes:

  • Add onThrowable = Throwable.class to various @Advice.OnMethodExit annotations so exit advice executes on exceptional control flow.
  • Capture @Advice.Thrown in selected advices and record errors on spans (with special-case filtering where appropriate, e.g., Kafka WakeupException).
  • Ensure scope/context cleanup is performed from exit advice paths that previously only ran on normal returns.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
dd-java-agent/instrumentation/servicetalk/servicetalk-0.42.0/.../ContextPreservingInstrumentation.java Run scope-close exit advice even when the target method throws.
dd-java-agent/instrumentation/rs/jax-rs/jax-rs-annotations/jax-rs-annotations-2.0/.../JaxRsAsyncResponseInstrumentation.java Run stop-span logic on exceptional exits; report thrown errors via decorator.
dd-java-agent/instrumentation/rs/jakarta-rs-annotations-3.0/.../JakartaRsAsyncResponseInstrumentation.java Same as JAX-RS 2.x variant for Jakarta RS.
dd-java-agent/instrumentation/opensearch/opensearch-transport-1.0/.../ShadowExistingScopeAdvice.java Ensure shadow scope is closed even if the method throws.
dd-java-agent/instrumentation/lettuce/lettuce-5.0/.../rx/RedisSubscriptionSubscribeAdvice.java Ensure exit advice runs on throwable; (needs follow-up to finish/clear spans on thrown for response-expected commands).
dd-java-agent/instrumentation/lettuce/lettuce-5.0/.../rx/RedisSubscriptionCommandOnCompleteAdvice.java Run completion cleanup on exceptional exit as well.
dd-java-agent/instrumentation/lettuce/lettuce-5.0/.../rx/RedisSubscriptionCommandErrorAdvice.java Ensure error cleanup runs even if the error handler throws.
dd-java-agent/instrumentation/lettuce/lettuce-5.0/.../rx/RedisSubscriptionCommandCompleteAdvice.java Run completion/cancel cleanup even on exceptional exit.
dd-java-agent/instrumentation/lettuce/lettuce-5.0/.../CommandHandlerInstrumentation.java Ensure endTaskScope runs on exceptional exit from decode.
dd-java-agent/instrumentation/kafka/kafka-clients-3.8/.../RecordsAdvice.java Finish/close scope on exceptional exit; record thrown error except WakeupException.
dd-java-agent/instrumentation/kafka/kafka-clients-0.11/.../KafkaConsumerInfoInstrumentation.java Same as above for Kafka 0.11 instrumentation.
dd-java-agent/instrumentation/java/java-concurrent/java-concurrent-1.8/.../AsyncTaskInstrumentation.java Ensure task scopes are ended even if run()/cancel() throw.
dd-java-agent/instrumentation/grizzly/grizzly-http-2.3.20/.../GrizzlyCharBodyInstrumentation.java Ensure context-store cleanup runs even if recycle throws.
dd-java-agent/instrumentation/grizzly/grizzly-http-2.3.20/.../GrizzlyByteBodyInstrumentation.java Same as above for byte-body storage.
dd-java-agent/instrumentation/gax-1.4/.../CallbackChainRetryingFutureInstrumentation.java Run superseded-listener cancellation on exceptional exit.
dd-java-agent/instrumentation/elasticsearch/.../ShadowExistingScopeAdvice.java Ensure shadow scope is closed even if the method throws.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 32a9fed1bb

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

…p even if the patched method throws an exception
@mcculls mcculls force-pushed the ncculls/add-onThrowable-to-cleanup-onexit-advice branch from 32a9fed to 01796fc Compare July 9, 2026 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

inst: elasticsearch Elasticsearch instrumentation inst: jax-rs JAX-RS instrumentation inst: kafka Kafka instrumentation inst: lettuce Lettuce instrumentation inst: others All other instrumentations tag: ai generated Largely based on code generated by an AI or LLM tag: no release notes Changes to exclude from release notes type: bug fix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants