Skip to content

Commit dda6eed

Browse files
traskCopilot
andcommitted
Recognize bounded JAX-RS roles and unversioned common modules
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 9b379e6 commit dda6eed

4 files changed

Lines changed: 99 additions & 24 deletions

File tree

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

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -64,23 +64,24 @@ check_source_set() {
6464
fi
6565

6666
if [[ "$source_set" == "javaagent" ]]; then
67-
# advice and injected helper packages that must live under the instrumented library's own
68-
# namespace, so that they can reach package-private library types and members
67+
# advice and injected helper packages that must share a namespace with package-private
68+
# library types and members, including library instrumentation helpers
6969
case "$dir" in
7070
instrumentation/apache-dbcp-2.0/javaagent/src/main/java/org/apache/commons/dbcp2) continue ;;
7171
instrumentation/clickhouse/clickhouse-client-v1-0.5/javaagent/src/main/java/com/clickhouse/client | instrumentation/clickhouse/clickhouse-client-v1-0.5/javaagent/src/main/java/com/clickhouse/client/*) continue ;;
7272
instrumentation/finagle-http-23.11/javaagent/src/main/java/com/twitter/finagle | instrumentation/finagle-http-23.11/javaagent/src/main/java/com/twitter/finagle/*) continue ;;
7373
instrumentation/finagle-http-23.11/javaagent/src/main/java/io/netty/channel | instrumentation/finagle-http-23.11/javaagent/src/main/java/io/netty/channel/*) continue ;;
7474
instrumentation/hbase/hbase-client-common-1.0/javaagent/src/main/java/org/apache/hadoop/hbase/ipc) continue ;;
75+
instrumentation/jedis/jedis-3.0/javaagent/src/main/java/redis/clients/jedis) continue ;;
7576
instrumentation/jedis/jedis-4.0/javaagent/src/main/java/redis/clients/jedis) continue ;;
7677
instrumentation/reactor/reactor-netty/reactor-netty-1.0/javaagent/src/main/java/reactor/netty/http/client | instrumentation/reactor/reactor-netty/reactor-netty-1.0/javaagent/src/main/java/reactor/netty/http/client/*) continue ;;
7778
instrumentation/redisson/redisson-3.0/javaagent/src/main/java/org/redisson/config) continue ;;
7879
instrumentation/redisson/redisson-3.17/javaagent/src/main/java/org/redisson/config) continue ;;
79-
instrumentation/spring/spring-boot-resources/javaagent/src/main/java/io/opentelemetry/instrumentation/spring/resources) continue ;;
80+
instrumentation/rocketmq/rocketmq-client-4.8/javaagent/src/main/java/io/opentelemetry/instrumentation/rocketmqclient/v4_8) continue ;;
8081
instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/org/springframework/web/servlet/v3_1 | instrumentation/spring/spring-webmvc/spring-webmvc-3.1/javaagent/src/main/java/org/springframework/web/servlet/v3_1/*) continue ;;
8182
instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/org/springframework/web/servlet/v6_0 | instrumentation/spring/spring-webmvc/spring-webmvc-6.0/javaagent/src/main/java/org/springframework/web/servlet/v6_0/*) continue ;;
8283
instrumentation/tomcat/tomcat-dbcp-8.0/javaagent/src/main/java/org/apache/tomcat/dbcp/dbcp2) continue ;;
83-
instrumentation/vertx/vertx-redis-client-4.0/javaagent/src/main/java/io/vertx/redis/client/impl | instrumentation/vertx/vertx-redis-client-4.0/javaagent/src/main/java/io/vertx/redis/client/impl/*) continue ;;
84+
instrumentation/vertx/vertx-redis-client/vertx-redis-client-4.0/javaagent/src/main/java/io/vertx/redis/client/impl | instrumentation/vertx/vertx-redis-client/vertx-redis-client-4.0/javaagent/src/main/java/io/vertx/redis/client/impl/*) continue ;;
8485
instrumentation/vertx/vertx-sql-client/vertx-sql-client-common-4.0/javaagent/src/main/java/io/vertx/sqlclient/impl | instrumentation/vertx/vertx-sql-client/vertx-sql-client-common-4.0/javaagent/src/main/java/io/vertx/sqlclient/impl/*) continue ;;
8586
esac
8687

@@ -95,28 +96,27 @@ check_source_set() {
9596
instrumentation/opentelemetry-instrumentation-api/opentelemetry-instrumentation-api-1.14/javaagent/*) continue ;;
9697
esac
9798

98-
# historical javaagent modules that do not follow the module-name <-> package-name convention
99+
# javaagent modules with package layouts that differ from their module names
99100
case "$dir" in
100101
instrumentation/hbase/hbase-client-common-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hbase/client/common) continue ;;
101102
instrumentation/java-http-client/javaagent/*) continue ;;
102103
instrumentation/java-http-server/javaagent/*) continue ;;
103104
instrumentation/java-util-logging/javaagent/*) continue ;;
105+
instrumentation/jaxrs/jaxrs-1.0-annotations/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0) continue ;;
104106
esac
105107
fi
106108

107-
# modules whose name does not end with a version digit, either because they have no base
108-
# version (e.g. jdbc, netty-common) or because the version is embedded mid-name (e.g.
109-
# jaxrs-2.0-annotations); package layout is still derived from the module name below
110-
if [[ ! "$module_name" =~ [0-9]$ ]]; then
109+
# JAX-RS roles may follow a major.minor version. Unversioned common modules may
110+
# have a javax/jakarta variant, but no numeric version token in their prefix.
111+
# These rules allow module names; package layout is still checked below.
112+
if [[ ! "$module_name" =~ [0-9]$
113+
&& ! "$module_name" =~ ^jaxrs-[0-9]+\.[0-9]+-(annotations|common)$
114+
&& ! "$module_name" =~ ^[a-z][a-z0-9]*(-[a-z][a-z0-9]*)*-common(-(javax|jakarta))?$ ]]; then
111115
case "$source_set:$module_name" in
112116
# library:
113117
library:jdbc) ;;
114-
library:lettuce-common) ;;
115-
library:netty-common) ;;
116118
library:resources) ;;
117119
library:runtime-telemetry) ;;
118-
library:servlet-common) ;;
119-
library:servlet-common-javax) ;;
120120
# javaagent:
121121
javaagent:executors) ;;
122122
javaagent:external-annotations) ;;
@@ -126,20 +126,11 @@ check_source_set() {
126126
javaagent:internal-lambda) ;;
127127
javaagent:internal-reflection) ;;
128128
javaagent:internal-url-class-loader) ;;
129-
javaagent:jaxrs-2.0-annotations) ;;
130-
javaagent:jaxrs-2.0-common) ;;
131-
javaagent:jaxrs-3.0-annotations) ;;
132-
javaagent:jaxrs-3.0-common) ;;
133-
javaagent:jaxrs-common) ;;
134129
javaagent:jdbc) ;;
135-
javaagent:jsf-common-jakarta) ;;
136-
javaagent:jsf-common-javax) ;;
137130
javaagent:methods) ;;
138131
javaagent:rmi) ;;
139132
javaagent:runtime-telemetry) ;;
140-
javaagent:servlet-common) ;;
141133
javaagent:spring-boot-resources) ;;
142-
javaagent:spring-cloud-gateway-common) ;;
143134
*)
144135
echo "module name doesn't have a base version: $dir"
145136
exit 1
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
#!/bin/bash -e
2+
3+
set -o pipefail
4+
5+
checker_script="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/check-package-names.sh"
6+
fixture_dir=$(mktemp -d)
7+
trap 'rm -rf "$fixture_dir"' EXIT
8+
9+
check_case() {
10+
local module_name="$1"
11+
local package_name="$2"
12+
local expected_error="$3"
13+
local output
14+
15+
rm -rf "$fixture_dir/instrumentation"
16+
local source_dir="$fixture_dir/instrumentation/$module_name/$source_set/src/main/java/$expected_prefix/$package_name"
17+
mkdir -p "$source_dir"
18+
touch "$source_dir/Test.java"
19+
20+
if output=$(cd "$fixture_dir" && bash -e "$checker_script" 2>&1); then
21+
if [[ -n "$expected_error" ]]; then
22+
echo "ERROR: unexpectedly accepted $source_set:$module_name/$package_name"
23+
exit 1
24+
fi
25+
elif [[ -z "$expected_error" || "$output" != "$expected_error"* ]]; then
26+
echo "ERROR: unexpected failure for $source_set:$module_name/$package_name"
27+
echo "$output"
28+
exit 1
29+
fi
30+
}
31+
32+
for source_set in library javaagent; do
33+
if [[ "$source_set" == "library" ]]; then
34+
expected_prefix=io/opentelemetry/instrumentation
35+
else
36+
expected_prefix=io/opentelemetry/javaagent/instrumentation
37+
fi
38+
39+
while read -r module_name package_name; do
40+
check_case "$module_name" "$package_name" ""
41+
check_case "$module_name" wrong/package "ERROR:"
42+
done <<'EOF'
43+
jaxrs-1.0-annotations jaxrs/v1_0/annotations
44+
jaxrs-2.0-annotations jaxrs/v2_0/annotations
45+
jaxrs-3.0-annotations jaxrs/v3_0/annotations
46+
jaxrs-2.0-common jaxrs/v2_0/common
47+
jaxrs-3.0-common jaxrs/v3_0/common
48+
jaxrs-common jaxrs/common
49+
jedis-common jedis/common
50+
lettuce-common lettuce/common
51+
netty-common netty/common
52+
servlet-common servlet/common
53+
spring-cloud-gateway-common spring/cloud/gateway/common
54+
servlet-common-javax servlet/common/javax
55+
jsf-common-javax jsf/common/javax
56+
jsf-common-jakarta jsf/common/jakarta
57+
EOF
58+
59+
for module_name in \
60+
jaxrs-2-annotations jaxrs-2.0.1-annotations jaxrs-2.0-other \
61+
jaxrs-2-common jaxrs-2.0.1-common jaxrs-2.0-common-javax \
62+
other-2.0-annotations other-2.0-common other-2-common \
63+
other-common-variant other-common-javax-extra other-common-jakarta-extra; do
64+
check_case "$module_name" wrong/package "module name doesn't have a base version:"
65+
done
66+
67+
check_case jaxrs-2.0-annotations jaxrs/v2_0 "ERROR:"
68+
check_case jaxrs-2.0-annotations jaxrs/v2_0/annotationsExtra "ERROR:"
69+
check_case jaxrs-2.0-common jaxrs/v2_0/commonExtra "ERROR:"
70+
check_case servlet-common-javax servlet/common/jakarta "ERROR:"
71+
check_case jsf-common-jakarta jsf/common/javax "ERROR:"
72+
done
73+
74+
echo "Package-name rule tests passed"

‎.github/workflows/build-common.yml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,9 @@ jobs:
192192

193193
- name: Check package names
194194
if: ${{ !cancelled() && steps.checkout.outcome == 'success' }}
195-
run: .github/scripts/check-package-names.sh
195+
run: |
196+
.github/scripts/check-package-names.sh
197+
bash -e .github/scripts/test-check-package-names.sh
196198
197199
# kept on ubuntu-latest: `gh aw validate` checks container images with `docker pull` and silently
198200
# skips that when no Docker daemon is available, as on ubuntu-slim

‎package-name-exceptions-plan.md‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,14 @@ Updated on 2026-09-19 after #20070 added Redisson package-private configuration
2626

2727
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.
2828

29+
## Module-name rules
30+
31+
The checker accepts `jaxrs-<major.minor>-annotations` and `jaxrs-<major.minor>-common` without individual allowlist entries. Other roles, other libraries with a version before the role, and patch-level versions before these roles are not covered by this rule.
32+
33+
Unversioned shared modules may end in `-common`, `-common-javax`, or `-common-jakarta`. The prefix consists of dash-separated tokens starting with a lowercase letter and containing lowercase letters or digits, so this rule does not allow embedded numeric version tokens. Other variant suffixes still require an explicit decision.
34+
35+
Both rules only allow the module name. Expected package names are still derived and checked, subject to the existing package-specific exceptions. Whether a common module needs a minimum dependency version remains a separate module-naming review.
36+
2937
## Quick Audit Summary
3038

3139
The remaining exceptions fall into four buckets:
@@ -109,7 +117,7 @@ The `servlet-common` library internal package was moved upstream as:
109117
- #18855: `io.opentelemetry.instrumentation.servlet.internal` -> `io.opentelemetry.instrumentation.servlet.common.internal`.
110118

111119
`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.
112-
`spring-boot-resources` keeps a narrow deprecated compatibility-package exception for `io.opentelemetry.instrumentation.spring.resources`; the replacement javaagent package is already present under `io.opentelemetry.javaagent.instrumentation.spring.boot.resources`.
120+
The deprecated `spring-boot-resources` compatibility wrappers were removed in #20806, so their package exception is no longer needed.
113121

114122
## PR Creation Notes
115123

0 commit comments

Comments
 (0)