Skip to content

Commit 9eb3e25

Browse files
committed
Update package cleanup plan after merged PRs
1 parent f827053 commit 9eb3e25

2 files changed

Lines changed: 211 additions & 5 deletions

File tree

.github/scripts/check-package-names.sh

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,19 +71,14 @@ check_source_set() {
7171
instrumentation/akka/akka-actor-fork-join-2.5/javaagent/*) continue ;;
7272
instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/main/java/io/opentelemetry/instrumentation/awssdk/v1_11) continue ;;
7373
instrumentation/aws-sdk/aws-sdk-2.2/javaagent/src/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/internal) continue ;;
74-
instrumentation/internal/internal-application-logger/javaagent/*) continue ;;
7574
instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-annotations/javaagent/*) continue ;;
7675
instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/javaagent/*) continue ;;
7776
instrumentation/jaxrs/jaxrs-3.0/jaxrs-3.0-annotations/javaagent/*) continue ;;
7877
instrumentation/jaxrs/jaxrs-3.0/jaxrs-3.0-common/javaagent/*) continue ;;
79-
instrumentation/jaxrs/jaxrs-common/javaagent/*) continue ;;
8078
instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/*) continue ;;
8179
instrumentation/opentelemetry-extension-annotations-1.0/javaagent/*) continue ;;
8280
instrumentation/opentelemetry-instrumentation-annotations-1.16/javaagent/*) continue ;;
8381
instrumentation/opentelemetry-instrumentation-api/javaagent/*) continue ;;
84-
instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet) continue ;;
85-
instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/snippet) continue ;;
86-
instrumentation/spring/spring-boot-resources/javaagent/*) continue ;;
8782
esac
8883
fi
8984

package-name-exceptions-plan.md

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,211 @@
1+
# Package Name Exceptions Cleanup Plan
2+
3+
Generated on 2026-05-11 from the remaining exceptions in `.github/scripts/check-package-names.sh`.
4+
Updated on 2026-05-13 after PRs 5-8 merged and their historical javaagent exceptions were removed from the checker.
5+
Updated again on 2026-05-13 after PRs 9-12 were implemented and opened as draft PRs.
6+
Updated on 2026-05-14 after PRs 9-12 merged and their historical javaagent exceptions were removed from the checker.
7+
Updated again on 2026-05-14 to plan faster batches: up to 20 changed files per PR, still four PRs per batch.
8+
Updated again on 2026-05-14 to bump the target to up to 40 changed files per PR and plan all remaining historical package renames.
9+
Updated on 2026-05-17 after PRs 15-16 merged, and after deciding to handle Akka/Scala forkjoin as a module-name cleanup instead of a package-only cleanup.
10+
Updated on 2026-05-18 after PRs 13, 18, 20, and 21 merged and their historical package exceptions were removed from the checker.
11+
12+
## Goal
13+
14+
Continue reducing package-name checker exceptions in reviewable PRs. Prefer changes where the current package name is only a historical abbreviation and the rename has a small blast radius. Use up to about 40 changed files per package-cleanup PR, while still submitting only four PRs at a time.
15+
16+
## Quick Audit Summary
17+
18+
The remaining exceptions fall into four buckets:
19+
20+
| Bucket | Current signal | Recommendation |
21+
| --- | --- | --- |
22+
| Module-wide skips | `java-*` has 39 files, `jmx-metrics` has 43 files | Do not start here; these need broader naming decisions. |
23+
| Library packages under third-party namespaces | `grpc`, `lettuce`, `nats`, `rxjava` each have 1 file | Leave for later; these are likely compatibility/shim packages. |
24+
| Javaagent advice under instrumented library namespaces | mostly 1 file each | Leave for later; the script already says these must live in the instrumented library namespace. |
25+
| Historical javaagent packages | many one-dir modules remain, usually with one source directory | Best place to keep chipping away. |
26+
27+
## Completed Cleanups
28+
29+
These PRs have merged, and their historical javaagent package-name exceptions have been removed from `.github/scripts/check-package-names.sh`:
30+
31+
- PR 1: `internal-eclipse-osgi-3.6` and `opentelemetry-extension-kotlin-1.0`.
32+
- PR 2: `spark-2.3`.
33+
- PR 3: `external-annotations`.
34+
- PR 4: `hibernate-procedure-call-4.3`.
35+
- PR 5: `elasticsearch-rest-common-5.0`.
36+
- PR 6: `kotlinx-coroutines-flow-1.3`, `liberty-dispatcher-20.0`, and `scala-fork-join-2.8`.
37+
- PR 7: `jaxws-jws-api-1.1`.
38+
- PR 8: `jsf-mojarra-1.2`, `jsf-mojarra-3.0`, `jsf-myfaces-1.2`, and `jsf-myfaces-3.0`.
39+
- PR 9: `elasticsearch-api-client-7.16`.
40+
- PR 10: `elasticsearch-transport-common`.
41+
- PR 11: `opensearch-rest-common`.
42+
- PR 12: `spring-boot-actuator-autoconfigure-2.0`.
43+
- PR 13: `internal-application-logger` and `spring-boot-resources`.
44+
- PR 18: `jaxrs-common`.
45+
- PR 20: `servlet-common` snippet package.
46+
- PR 21: `servlet-common` root helper package.
47+
48+
PRs 5-8 merged upstream as:
49+
50+
- #18720: `elasticsearch-rest-common-5.0`.
51+
- #18721: `kotlinx-coroutines-flow-1.3`, `liberty-dispatcher-20.0`, and `scala-fork-join-2.8`.
52+
- #18722: `jaxws-jws-api-1.1`.
53+
- #18723: `jsf-mojarra-1.2`, `jsf-mojarra-3.0`, `jsf-myfaces-1.2`, and `jsf-myfaces-3.0`.
54+
55+
PRs 9-12 merged upstream as:
56+
57+
- #18730: `elasticsearch-api-client-7.16`.
58+
- #18731: `elasticsearch-transport-common`.
59+
- #18732: `opensearch-rest-common`.
60+
- #18733: `spring-boot-actuator-autoconfigure-2.0`.
61+
62+
PRs 15-16 merged upstream as:
63+
64+
- #18748: `jaxrs-2.0-annotations` and `jaxrs-2.0-common`.
65+
- #18749: `jaxrs-3.0-annotations` and `jaxrs-3.0-common`.
66+
67+
PRs 13, 18, 20, and 21 merged upstream as:
68+
69+
- #18746: `internal-application-logger` and `spring-boot-resources`.
70+
- #18776: `jaxrs-common`.
71+
- #18777: `servlet-common` snippet package.
72+
- #18778: `servlet-common` root helper package.
73+
74+
`external-annotations` still remains in the unversioned-module allowlist as `javaagent:external-annotations`; that is a separate module-name exception, not a historical package exception.
75+
76+
## PR Creation Notes
77+
78+
Use draft PRs against upstream `main` for the package-only cleanup batches. These PR branches must be based directly on `upstream/main`, not on `next`, so the PR diffs contain only package moves and dependent import updates. Do not include `.github/scripts/check-package-names.sh` changes in those PRs; checker exception removals stay on the tracking/checker branch and should be updated separately after the package cleanup PRs merge.
79+
80+
Batch sizing: keep submitting four PRs at a time, but each PR can include up to about 40 changed files when the modules are closely related or the reference spread is easy to audit. Count dependent import updates and test package moves in that limit. Avoid combining package moves that require broad cross-module rewrites just to fill the file budget. The known outlier is the root `servlet-common` helper package move, which likely exceeds 40 changed files because many servlet modules import those helpers; treat that as a dedicated final PR unless a clean split emerges during implementation.
81+
82+
Preferred PR description for future package cleanup PRs:
83+
84+
```text
85+
Part of
86+
- https://github.com/open-telemetry/opentelemetry-java-instrumentation/pull/18428
87+
```
88+
89+
When splitting several already-implemented package moves into separate PRs:
90+
91+
1. Keep a local WIP branch or commit that contains the full tested implementation.
92+
2. Recreate each PR branch from `upstream/main`, not `next`.
93+
3. Apply only the module paths for that PR, for example:
94+
95+
```bash
96+
git switch -C <branch-name> upstream/main
97+
git diff --binary <base-before-package-moves> <wip-branch> -- <module-paths> | git apply --index
98+
git commit -m "<PR title>"
99+
git push -u origin <branch-name> --force-with-lease
100+
gh pr create --repo open-telemetry/opentelemetry-java-instrumentation --base main --head trask:<branch-name> --draft --title "<PR title>" --body "Part of
101+
- https://github.com/open-telemetry/opentelemetry-java-instrumentation/pull/18428"
102+
```
103+
104+
After pushing or force-pushing, verify that each PR file list contains only the intended module paths and does not include `.github/scripts/check-package-names.sh`:
105+
106+
```bash
107+
gh pr diff <pr-number> --repo open-telemetry/opentelemetry-java-instrumentation --name-only
108+
```
109+
110+
For common-module package moves, search for downstream versioned modules importing the moved helper package. Compile those dependent modules too; the common module's own test can pass while the full smoke-test build fails on stale imports.
111+
112+
## Open Cleanup PRs
113+
114+
PR 14 is open as #18747 and PR 17 is open as #18772. Keep `.github/scripts/check-package-names.sh` and checker exception removals on `next` until cleanup PRs merge.
115+
116+
### PR 14: OpenTelemetry annotation and instrumentation API modules (open #18747)
117+
118+
Modules:
119+
120+
- `opentelemetry-extension-annotations-1.0`
121+
- `opentelemetry-instrumentation-api`
122+
- `opentelemetry-instrumentation-annotations-1.16`
123+
124+
Expected package changes:
125+
126+
- `io.opentelemetry.javaagent.instrumentation.extensionannotations.v1_0` -> `io.opentelemetry.javaagent.instrumentation.opentelemetry.extension.annotations.v1_0`
127+
- `io.opentelemetry.javaagent.instrumentation.instrumentationapi` -> `io.opentelemetry.javaagent.instrumentation.opentelemetry.instrumentation.api`
128+
- `io.opentelemetry.javaagent.instrumentation.instrumentationannotations.v1_16` -> `io.opentelemetry.javaagent.instrumentation.opentelemetry.instrumentation.annotations.v1_16`
129+
130+
Notes:
131+
132+
- Around 30 changed Java files after the dependent Kotlin coroutines import is included.
133+
- Reference audit found only local package declarations/imports for `opentelemetry-extension-annotations-1.0`.
134+
- `opentelemetry-instrumentation-api` has local tests in `src/test` and `src/testOldServerSpan` that must move with the main package.
135+
- Update dependent import(s) in `kotlinx-coroutines-1.0` for instrumentation annotations.
136+
- `opentelemetry-instrumentation-api` still remains in the unversioned-module allowlist unless the module name changes; the PR removes only the broad historical package skip after merge.
137+
138+
Suggested verification:
139+
140+
```bash
141+
.github/scripts/check-package-names.sh
142+
./gradlew :instrumentation:opentelemetry-extension-annotations-1.0:javaagent:test :instrumentation:opentelemetry-instrumentation-api:javaagent:test :instrumentation:opentelemetry-instrumentation-annotations-1.16:javaagent:test :instrumentation:kotlinx-coroutines:kotlinx-coroutines-1.0:javaagent:compileJava
143+
```
144+
145+
### PR 17: Akka and Scala forkjoin module/package names (open #18772)
146+
147+
Modules:
148+
149+
- `akka-actor-fork-join-2.5` -> `akka-actor-forkjoin-2.5`
150+
- `scala-fork-join-2.8` -> `scala-forkjoin-2.8`
151+
152+
Expected package changes:
153+
154+
- `io.opentelemetry.javaagent.instrumentation.akkaforkjoin` -> `io.opentelemetry.javaagent.instrumentation.akka.actor.forkjoin.v2_5`
155+
- `io.opentelemetry.javaagent.instrumentation.scala.fork.join.v2_8` -> `io.opentelemetry.javaagent.instrumentation.scala.forkjoin.v2_8`
156+
157+
Notes:
158+
159+
- This is intentionally a module-name cleanup, not just a package cleanup: `forkjoin` is a single upstream package/concept in both Akka and Scala.
160+
- Update `settings.gradle.kts`, documentation inventory, instrumentation module names, and dependent Gradle project references.
161+
- Keep `akka-actor` as a compatibility instrumentation alias for the Akka module.
162+
163+
Suggested verification:
164+
165+
```bash
166+
.github/scripts/check-package-names.sh
167+
./gradlew :instrumentation:akka:akka-actor-forkjoin-2.5:javaagent:test :instrumentation:scala-forkjoin-2.8:javaagent:test :instrumentation:akka:akka-http-10.0:javaagent:compileTestJava
168+
```
169+
170+
### PR 19: OpenTelemetry API package
171+
172+
Module:
173+
174+
- `opentelemetry-api-1.0`
175+
176+
Expected package change:
177+
178+
- `io.opentelemetry.javaagent.instrumentation.opentelemetryapi` -> `io.opentelemetry.javaagent.instrumentation.opentelemetry.api.v1_0`
179+
180+
Notes:
181+
182+
- Around 25 changed Java files.
183+
- Package names may be more compatibility-sensitive or user-facing than pure internal helper modules; keep this as its own PR.
184+
185+
Suggested verification:
186+
187+
```bash
188+
.github/scripts/check-package-names.sh
189+
./gradlew :instrumentation:opentelemetry-api:opentelemetry-api-1.0:javaagent:test
190+
```
191+
192+
## Do Later
193+
194+
These are probably not the next easiest wins:
195+
196+
- `java-*` module skip: the current packages use names like `javahttpclient`, while the checker would expect `http.client` after eliding `java`. This touches 39 files and needs a naming decision.
197+
- `jmx-metrics`: current packages are under `jmx`, while the module says `jmx-metrics`. This touches 43 files and may be user-facing enough to deserve a dedicated PR.
198+
- Library-specific third-party packages: `io.grpc.override`, `io.lettuce.core.protocol`, `io.nats.client.impl`, and `rx` are likely intentional shims or package-private access points.
199+
- Advice-native package exceptions: packages under `com.clickhouse`, `com.twitter`, `io.netty`, `reactor.netty`, `org.springframework`, and `io.vertx` should stay until each one is proven not to need native package placement.
200+
- OpenTelemetry API and Akka/Scala forkjoin package/module renames are planned above; AWS SDK remains deferred.
201+
- Other unversioned module allowlist entries: these need package/module naming decisions beyond a package-only cleanup.
202+
203+
## Re-Audit Command
204+
205+
After each PR, run:
206+
207+
```bash
208+
.github/scripts/check-package-names.sh
209+
```
210+
211+
Then look for remaining broad exceptions in `.github/scripts/check-package-names.sh` and prefer candidates where `rg --fixed-strings <current.package>` only reports package declarations and local imports.

0 commit comments

Comments
 (0)