Remove AgentTracer.noopScope() sentinel from advice call sites#12010
Conversation
Replace noopScope() with null where the returned scope has no observable side effects to preserve. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
dd-octo-sts
Bot
added
the
tag: ai generated
| setRoute(routingContext); | ||
| } | ||
| try (final ContextScope scope = span != null ? activateSpan(span) : noopScope()) { | ||
| try (final ContextScope scope = span != null ? activateSpan(span) : null) { |
There was a problem hiding this comment.
Note this is safe - try-with-resources allows use of null without causing an NPE
![datadog-datadog-prod-us1-2[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
All five changed files are safe mechanical refactors: replacing noopScope() (which returns the NoopScope.INSTANCE singleton whose close() is empty {}) with null at call sites that only ever called close(). Java's try-with-resources explicitly handles null resources without NPE per JLS 14.20.3, and the identical … : null pattern is already used in tibco-businessworks and grizzly-client. The noopScope() method itself is preserved for identity-comparison callers (ContinuableScopeManager, TypeConverter, RedisFutureSendAdvice) that are not touched by this PR.
📊 Validated against 3 scenarios · Open Bits AI session
🤖 Datadog Autotest · Commit ec2820c · What is Autotest? · Any feedback? Reach out in #autotest
This comment has been minimized.
This comment has been minimized.
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
| if (channelSpan == null || channelSpan == activeSpan()) { | ||
| // don't modify the scope | ||
| return noopScope(); | ||
| return null; |
There was a problem hiding this comment.
Note returning noopScope() had no effect on the active scope (unlike activating noopSpan) - so returning null instead has the same behaviour, and matches the convention in the rest of the codebase.
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
4a5a1e2
into
master
2 participants
What Does This Do
Replace
noopScope()with null where the returned scope has no observable side effects to preserve.Motivation
Further step towards being able to completely remove the
noopScope()sentinelContributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]