Skip to content

Cleanup of Spotbugs annotations. Removed Spotbugs from test scope. - #11693

Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 2 commits into
masterfrom
alexeyk/cleanup-fb
Jun 22, 2026
Merged

Cleanup of Spotbugs annotations. Removed Spotbugs from test scope.#11693
gh-worker-dd-mergequeue-cf854d[bot] merged 2 commits into
masterfrom
alexeyk/cleanup-fb

Conversation

@AlexeyKuznetsov-DD

@AlexeyKuznetsov-DDAlexeyKuznetsov-DD commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

What Does This Do

  1. Replaces usages of edu.umd.cs.findbugs.annotations.* annotations with the appropriate javax.annotation.* equivalents.
  2. Removes SpotBugs dependencies from the test scope, where they are unnecessary and were primarily present because of the annotations mentioned above.

Motivation

  • javax.annotation.* annotations are more standard and widely adopted.
  • Running SpotBugs in the test scope provides little to no value while introducing an unnecessary dependency.
  • This change simplifies dependency management and reduces reliance on SpotBugs-specific annotations.

Additional Notes

This is a cleanup/refactoring change only. No functional behavior is affected.

Contributor Checklist

  • Format the title according to the contribution guidelines
  • Assign the type: and (comp: or inst:) labels in addition to any other useful labels
  • Avoid using close, fix, or any linking keywords when referencing an issue
    Use solves instead, and assign the PR milestone to the issue
  • Update the CODEOWNERS file on source file addition, migration, or deletion
  • Update public documentation with any new configuration flags or behaviors
  • Add your completed PR to the merge queue by commenting /merge. You can also:
    • Customize the commit message associated with the merge with /merge --commit-message "..."
    • Remove your PR from the merge queue with /merge -c
    • Skip all merge queue checks with /merge -f --reason "reason"; please use this judiciously, as some checks do not run at the PR-level (note: the PR still needs to be mergeable, this will only skip the pre-merge build)
    • Get more information in this doc

Jira ticket: [PROJ-IDENT]

@AlexeyKuznetsov-DDAlexeyKuznetsov-DD self-assigned this Jun 22, 2026
@AlexeyKuznetsov-DDAlexeyKuznetsov-DD added tag: no release notes Changes to exclude from release notes type: refactoring labels Jun 22, 2026
@datadog-datadog-prod-us1-2

This comment has been minimized.

@CallSite.Around("java.lang.StringBuilder java.lang.StringBuilder.append(java.lang.Object)")
@CallSite.Around("java.lang.StringBuffer java.lang.StringBuffer.append(java.lang.Object)")
@Nonnull
@SuppressFBWarnings("NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE")

@AlexeyKuznetsov-DDAlexeyKuznetsov-DDJun 22, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Removed this annotation, as it was a copy-paste, not checked in test and usage of Spotbugs in test scope make almost no sense.

@AlexeyKuznetsov-DD
AlexeyKuznetsov-DD marked this pull request as ready for review June 22, 2026 13:15
@AlexeyKuznetsov-DD
AlexeyKuznetsov-DD requested review from a team as code ownersJune 22, 2026 13:15
@AlexeyKuznetsov-DD
AlexeyKuznetsov-DD requested review from a team, PerfectSlayer, bric3, dromanol, evanchooly, jandro996, jordan-wong, mcculls and sarahchen6 and removed request for a teamJune 22, 2026 13:15

@chatgpt-codex-connectorchatgpt-codex-connectorBot 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:3b9e51969f

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment threadbuildSrc/call-site-instrumentation-plugin/build.gradle.kts
@pr-commenter

pr-commenterBot commented Jun 22, 2026

Copy link
Copy Markdown

Debugger benchmarks

Parameters

BaselineCandidate
baseline_or_candidatebaselinecandidate
ci_job_date17821359711782136368
end_time2026-06-22T13:47:372026-06-22T13:54:15
git_branchmasteralexeyk/cleanup-fb
git_commit_sha300600490085d9
start_time2026-06-22T13:46:122026-06-22T13:52:50
See matching parameters
BaselineCandidate
ci_job_id17914115241791411524
ci_pipeline_id120207643120207643
cpu_modelIntel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHzIntel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
git_commit_date17821353131782135313

Summary

Found 5 performance improvements and 0 performance regressions! Performance is the same for 5 metrics, 5 unstable metrics.

scenarioΔ mean agg_http_req_duration_minΔ mean agg_http_req_duration_p50Δ mean agg_http_req_duration_p75Δ mean agg_http_req_duration_p99Δ mean throughput
scenario:loopbetter
[-3.444ms; -2.784ms] or [-28.695%; -23.191%]
better
[-3.523ms; -2.850ms] or [-28.989%; -23.450%]
better
[-3.490ms; -2.824ms] or [-28.554%; -23.103%]
better
[-3.544ms; -2.856ms] or [-28.618%; -23.063%]
better
[+21.734op/s; +27.095op/s] or [+25.429%; +31.701%]
See unchanged results
scenarioΔ mean agg_http_req_duration_minΔ mean agg_http_req_duration_p50Δ mean agg_http_req_duration_p75Δ mean agg_http_req_duration_p99Δ mean throughput
scenario:noprobeunstable
[-25.385µs; +15.099µs] or [-8.650%; +5.145%]
unstable
[-40.248µs; +21.200µs] or [-11.855%; +6.245%]
unstable
[-54.164µs; +30.323µs] or [-15.194%; +8.506%]
unstable
[+76.961µs; +317.077µs] or [+6.679%; +27.516%]
same
scenario:basicunsure
[-5.588µs; -0.049µs] or [-2.098%; -0.018%]
samesameunstable
[+205.957µs; +488.823µs] or [+19.714%; +46.790%]
same
Request duration reports for reports
gantt
title reports - request duration [CI 0.99] : candidate=None, baseline=None
dateFormat X
axisFormat %s
section baseline
noprobe (339.492 µs) : 304, 375
. : milestone, 339,
basic (297.515 µs) : 291, 304
. : milestone, 298,
loop (12.154 ms) : 11711, 12596
. : milestone, 12154,
section candidate
noprobe (329.968 µs) : 308, 352
. : milestone, 330,
basic (292.849 µs) : 287, 299
. : milestone, 293,
loop (8.967 ms) : 8962, 8972
. : milestone, 8967,
Loading
  • baseline results
ScenarioRequest median duration [CI 0.99]
noprobe339.492 µs [304.372 µs, 374.613 µs]
basic297.515 µs [290.787 µs, 304.243 µs]
loop12.154 ms [11.711 ms, 12.596 ms]
  • candidate results
ScenarioRequest median duration [CI 0.99]
noprobe329.968 µs [308.229 µs, 351.708 µs]
basic292.849 µs [286.574 µs, 299.124 µs]
loop8.967 ms [8.962 ms, 8.972 ms]

@dd-octo-sts

dd-octo-stsBot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

🟡 Java Benchmark SLOs — Performance SLO warning (near threshold)

SuiteStatus
Startup🟡 warning

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
ScenarioCandidatemasterΔ (95% CI of mean)
startup:insecure-bank:iast:Agent13.95 s13.97 s[-1.1%; +0.7%] (no difference)
startup:insecure-bank:tracing:Agent12.87 s13.01 s[-1.8%; -0.4%] (maybe better)
startup:petclinic:appsec:Agent17.48 s17.29 s[+0.2%; +2.0%] (maybe worse)
startup:petclinic:iast:Agent17.52 s17.54 s[-0.9%; +0.7%] (no difference)
startup:petclinic:profiling:Agent17.44 s17.39 s[-0.6%; +1.2%] (no difference)
startup:petclinic:sca:Agent17.58 s17.49 s[-0.5%; +1.6%] (no difference)
startup:petclinic:tracing:Agent16.60 s16.73 s[-1.9%; +0.3%] (no difference)

Commit:90085d9a · 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.

@PerfectSlayerPerfectSlayer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Overall looking good but I would like to know more about this first:

Running SpotBugs in the test scope provides little to no value while introducing an unnecessary dependency.

Don't you think it could be useful to catch test implementation issue?

Comment threadgradle/spotbugs.gradle
@AlexeyKuznetsov-DD

Copy link
Copy Markdown
ContributorAuthor

Overall looking good but I would like to know more about this first:

Running SpotBugs in the test scope provides little to no value while introducing an unnecessary dependency.

Don't you think it could be useful to catch test implementation issue?

@PerfectSlayer
Good question — but it's actually a no-op for bug detection here. SpotBugs analysis has never run against the test scope: in gradle/spotbugs.gradle (line 10) the configureEach block disables every spotbugs* task whose name doesn't end in Main/Main_java11 (it.enabled = false), so spotbugsTest is and always has been off.
BTW @bric3 do you recall why we have that?

The testImplementation(libs.spotbugs.annotations) line wasn't enabling any test analysis — it only put the edu.umd.cs.findbugs.annotations.* classes on the test compile classpath so test code using those annotations would compile. The only test-scope usages were a handful of annotation imports (@NonNull, @SuppressFBWarnings), which this PR swaps for the standard javax.annotation.* equivalents. To keep those available at compile time I kept testImplementation(libs.jsr305).

So this drops a heavier dependency that was only ever there for a few annotation imports (one of which was a copy-paste leftover), without losing any actual analysis. If we ever want SpotBugs to catch test implementation issues, that'd be a separate change — enabling the spotbugsTest tasks — which isn't wired up today.

@PerfectSlayer

Copy link
Copy Markdown
Collaborator

Thanks @AlexeyKuznetsov-DD for the clarification 🙏 Approval confirmed 👍

@AlexeyKuznetsov-DD

Copy link
Copy Markdown
ContributorAuthor

/merge

@gh-worker-devflow-routing-ef8351

gh-worker-devflow-routing-ef8351Bot commented Jun 22, 2026

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2026-06-22 16:53:34 UTC ℹ️ Start processing command /merge


2026-06-22 16:53:47 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in master is approximately 1h (p90).


2026-06-22 18:28:00 UTC ℹ️ MergeQueue: This merge request was merged

@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854dBot merged commit 319ed20 into masterJun 22, 2026
795 of 805 checks passed
@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854dBot deleted the alexeyk/cleanup-fb branch June 22, 2026 18:27
@github-actionsgithub-actionsBot added this to the 1.64.0 milestone Jun 22, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tag: no release notesChanges to exclude from release notestype: refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@AlexeyKuznetsov-DD@PerfectSlayer@sarahchen6@daniel-mohedano