Remove default methods from CoreSpan; implement them in test doubles#11782
Remove default methods from CoreSpan; implement them in test doubles#11782dougqh wants to merge 2 commits into
Conversation
…y DDSpan The four defaults (getServiceNameSource, isKind, unsafeGetTag x2) are unreachable in practice; DDSpan overrides each with a different implementation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The four defaults (getServiceNameSource, isKind, unsafeGetTag x2) were unreachable in practice because DDSpan overrides each one. Making them abstract forces test implementations to be explicit, and coverage is satisfied by the concrete implementations rather than unreachable defaults. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
🟢 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. |
dougqh
changed the title
dd-octo-sts
Bot
added
the
tag: ai generated
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1117c7b82
ℹ️ 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".
| default CharSequence getServiceNameSource() { | ||
| return null; | ||
| } | ||
| CharSequence getServiceNameSource(); |
There was a problem hiding this comment.
Making getServiceNameSource() abstract leaves another in-repo CoreSpan implementation unupdated: dd-trace-core/src/traceAgentTest/groovy/TraceGenerator.groovy:121 declares PojoSpan implements CoreSpan<PojoSpan> but does not implement getServiceNameSource (nor the newly abstract unsafeGetTag/isKind). That source set is registered by dd-trace-core/build.gradle via addTestSuite('traceAgentTest'), so any traceAgentTest compile/run now fails before exercising those tests.
Useful? React with 👍 / 👎.
1 participant
What This Does
Removes the default methods from
CoreSpanMotivation
The default methods on
CoreSpanwere effectively test-only methods, since they are overwritten byDDSpanMy preference is to not have test-only code in production classes. Given that these methods were causing coverage checks to fail, I thought it best to simply remove them.
Test plan
./gradlew :dd-trace-core:jacocoTestCoverageVerification -PcheckCoverageno longer flagsCoreSpan🤖 Generated with Claude Code