Skip to content

Commit dac1228

Browse files
authored
Merge branch 'main' into capture-request-headers
2 parents 35ce572 + 035f3d2 commit dac1228

533 files changed

Lines changed: 4583 additions & 2088 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/agents/code-review-and-fix.agent.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ Auto-fix boundaries:
126126
same parent and apply the step-by-step procedure in `gradle-conventions.md`.
127127
After adding, verify by running the module's tests.
128128
- missing version comments on `hasClassesNamed()` landmark classes in existing
129-
`classLoaderMatcher()` overrides (multi-class checks or `.and(not(...))` chains only)
129+
`classLoaderMatcher()` overrides, including single-class lower-bound checks
130130
determine each class's **role** (floor vs ceiling) and add the matching comment.
131131
First check: does a **newer** sibling instrumentation module exist for this library
132132
(e.g., `mongo-4.0` next to `mongo-3.7`)? If so, look at what the newer module checks
@@ -142,6 +142,8 @@ Auto-fix boundaries:
142142
A ceiling class might have been *introduced* much earlier than the module's target version.
143143
Do not use `// added in` for a ceiling class — that is misleading. The relevant fact is
144144
when it was **removed**.
145+
Validate the version in the comment before adding or requesting it. Do not guess the
146+
version from the module name alone; confirm it with repository or upstream evidence.
145147
Sources: muzzle `versions.set(...)` ranges, sibling module `classLoaderMatcher()` checks,
146148
module directory names, existing code comments, Javadoc/release notes.
147149
Do NOT add a `classLoaderMatcher()` override where one does not already exist —
@@ -151,6 +153,9 @@ Auto-fix boundaries:
151153
names a specific, non-empty method (e.g., `isMethod().and(named("execute"))`
152154
`named("execute")`). Do not remove `isMethod()` when the name could be empty —
153155
`named("")` matches constructors and static initializers.
156+
- redundant `this.` qualifier on advice class references inside `transform()` — prefer
157+
`getClass().getName() + "$InnerClassName"`, not `this.getClass().getName() +
158+
"$InnerClassName"`
154159
- singleton-to-instance-creation conversion for stateless telemetry interface
155160
implementations (`TextMapGetter`, `TextMapSetter`, `*AttributesGetter`,
156161
`AttributesExtractor`, `SpanNameExtractor`, `HttpServerResponseMutator`) — replace

.github/agents/code-review.agent.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,11 @@ For each file in scope:
9494
5. Do not flag a non-capturing lambda or method reference as an unnecessary allocation,
9595
because on modern JDKs these are typically cached at the call site rather than
9696
allocated on every invocation.
97-
6. Prevent duplicates:
97+
6. Flag a missing `// added in X.Y` comment on a single-class lower-bound
98+
`hasClassesNamed(...)` check in `classLoaderMatcher()` only after validating that the
99+
stated version is factually correct from repository or upstream evidence; the comment
100+
documents why the matcher exists and which version boundary it enforces.
101+
7. Prevent duplicates:
98102
- If equivalent `REVIEW:` already exists above the same line, do not add another.
99103

100104
Comment formatting rules:

.github/agents/knowledge/general-rules.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ When a "Knowledge File" is listed, load it from `knowledge/` before reviewing th
1515
| --- | --- | --- | --- |
1616
| General | Logic, correctness, reliability, safety, copy/paste mistakes, incorrect comments | Always ||
1717
| Style | Style guide | Always ||
18+
| Style | Uppercase field names should reflect semantic constants, not simply `static final` | Always ||
1819
| Naming | Getter naming (`get` / `is`) | Always ||
1920
| Naming | Module/package naming | New or renamed modules/packages | `module-naming.md` |
2021
| Javaagent | Advice patterns | `@Advice` classes | `javaagent-advice-patterns.md` |

.github/agents/knowledge/javaagent-advice-patterns.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,5 +191,5 @@ the instrumentation automatically rather than letting it fail at runtime.
191191
- **`@Advice.OnMethodEnter` or `@Advice.OnMethodExit` missing `suppress = Throwable.class`** when the method has a non-trivial body (library calls, collection iteration, reflection). Exceptions: `instrumentation/internal/` infrastructure code, test sources, and methods whose bodies provably cannot throw (e.g., `return true;`, returning a literal or a single constant). Do not add or flag `suppress` on provably throw-free bodies.
192192
- **Exception thrown in advice code or a helper called from advice** — javaagent code must never throw; use `suppress = Throwable.class` as the safety net.
193193
- **`@Advice.OnMethodExit` method named `onEnter`** (or vice versa) — the method name should match the annotation. A mismatch is a copy-paste bug that compiles but confuses readers and may mask intent errors.
194-
- **`.class.getName()` used in `transform()` to reference advice** — see `javaagent-module-patterns.md` for the correct `getClass().getName()` pattern. Flag both `InnerAdvice.class.getName()` and `OuterInstrumentation.class.getName() + "$InnerAdvice"` any `.class` literal in a `transform()` method triggers unwanted class loading.
194+
- **Advice referenced in `transform()` using anything other than `getClass().getName() + "$InnerAdvice"`** — see `javaagent-module-patterns.md` for the canonical pattern. Flag `this.getClass().getName() + "$InnerAdvice"` as a redundant qualifier, and flag both `InnerAdvice.class.getName()` and `OuterInstrumentation.class.getName() + "$InnerAdvice"` because any `.class` literal in a `transform()` method triggers unwanted class loading.
195195
- **`onThrowable = Throwable.class` on return-only exit advice** — if the exit method only processes `@Advice.Return` and has no `@Advice.Enter` state to clean up, `onThrowable` should be omitted. The return value is `null`/zero on the exceptional path, and dereferencing it causes a suppressed exception for no benefit. Keep `suppress = Throwable.class` but remove `onThrowable`.

.github/agents/knowledge/javaagent-module-patterns.md

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -187,10 +187,13 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
187187
which is irrelevant and misleading; the purpose is the upper bound.
188188
To identify ceiling classes, check if a newer sibling module's `classLoaderMatcher()`
189189
checks a different variant or replacement class.
190-
- **Do NOT add version comments on trivial single-class checks.** A single-class
191-
`hasClassesNamed(...)` check whose landmark corresponds to the module's base
192-
version does not need a comment — the version is obvious from the module name. Do not
193-
suggest adding one.
190+
- **Single-class lower-bound comments are required.** When a single-class
191+
`hasClassesNamed(...)` check exists solely to establish the module's lower bound, add an
192+
inline `// added in X.Y` comment. The comment explains why the matcher exists and which
193+
version boundary it enforces. First validate that `X.Y` is factually correct from
194+
repository or upstream evidence; do not infer it from the module name alone. Flag a
195+
missing comment in this case, and do not flag an existing one for removal unless the
196+
comment is demonstrably wrong.
194197
- Prefer **one landmark class** per version boundary — choose the most stable/specific class.
195198
- Pair with **muzzle config** (`assertInverse.set(true)`) for full coverage.
196199
- Use `hasClassesNamed(...)` (from `AgentElementMatchers`) — not raw ByteBuddy matchers.
@@ -254,9 +257,7 @@ sufficient for optimization.
254257

255258
### Rules
256259

257-
- Do not flag or change the visibility or `final` modifier on `TypeInstrumentation`,
258-
`InstrumentationModule`, or advice classes. Both `public class` and package-private `class`
259-
(with or without `final`) are acceptable — this is not a style issue in javaagent code.
260+
- Do not flag or change the visibility or `final` modifier on advice classes.
260261
- `typeMatcher()` should match only the types the instrumentation genuinely needs. Prefer
261262
`named("fully.qualified.ClassName")` or `namedOneOf(...)` for single classes.
262263
`extendsClass(...)` and `implementsInterface(...)` are appropriate when the instrumentation
@@ -267,12 +268,14 @@ sufficient for optimization.
267268
Keep `isMethod()` when the name could be empty, since `named("")` matches constructors and
268269
class initializers.
269270
- Reference the advice class using `getClass().getName() + "$InnerClassName"` — not
270-
`InnerClassName.class.getName()`, `OuterClass.class.getName()`, or a string literal.
271+
`this.getClass().getName() + "$InnerClassName"`, `InnerClassName.class.getName()`,
272+
`OuterClass.class.getName()`, or a string literal.
271273
Any `.class.getName()` reference — whether to the inner advice class or the outer
272274
instrumentation class — causes class loading in the agent's class loader, where library
273275
types used by the advice are unavailable (causing `NoClassDefFoundError`).
274276
`getClass().getName()` avoids this because it is a virtual call on the already-loaded
275-
`this` instance, not a class literal.
277+
instance, not a class literal. Omit the redundant `this.` qualifier and use the shorter
278+
repository convention.
276279

277280
## Singletons Pattern
278281

.github/labeler.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,5 @@ test native:
77
- instrumentation/spring/**
88
- smoke-tests-otel-starter/**
99
- dependencyManagement/build.gradle.kts
10+
- settings.gradle.kts
1011
- all-globs-to-all-files: '!instrumentation/spring/**/javaagent/**'

.github/renovate.json5

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
'patch',
1919
],
2020
schedule: [
21-
'before 8am on Tuesday',
21+
'* 0-7 * * 2', // weekly, before 8am on Tuesday
2222
],
2323
},
2424
{
@@ -29,8 +29,8 @@
2929
'dockerfile',
3030
'custom.regex',
3131
],
32-
extends: [
33-
'schedule:weekly',
32+
schedule: [
33+
'* 0-7 * * 2', // weekly, before 8am on Tuesday
3434
],
3535
groupName: 'weekly update',
3636
},

.github/workflows/codeql.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ jobs:
3232
include:
3333
- language: actions
3434
- language: java
35-
runs-on: oracle-vm-8cpu-32gb-x86-64
35+
runs-on: ${{ github.repository == 'open-telemetry/opentelemetry-java-instrumentation' && 'oracle-vm-8cpu-32gb-x86-64' || 'ubuntu-latest' }}
3636
steps:
3737
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
3838

.github/workflows/label.yml

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,14 @@ permissions:
77
jobs:
88
label:
99
runs-on: ubuntu-latest
10-
permissions:
11-
contents: read
12-
pull-requests: write
1310
steps:
11+
- uses: actions/create-github-app-token@f8d387b68d61c58ab83c6c016672934102569859 # v3.0.0
12+
id: otelbot-token
13+
with:
14+
app-id: ${{ vars.OTELBOT_JAVA_INSTRUMENTATION_APP_ID }}
15+
private-key: ${{ secrets.OTELBOT_JAVA_INSTRUMENTATION_PRIVATE_KEY }}
16+
1417
- uses: actions/labeler@634933edcd8ababfe52f92936142cc22ac488b1b # v6.0.1
1518
with:
16-
repo-token: "${{ secrets.GITHUB_TOKEN }}"
19+
# Not using GITHUB_TOKEN since labels from that token do not trigger workflows.
20+
repo-token: "${{ steps.otelbot-token.outputs.token }}"

conventions/src/main/kotlin/otel.java-conventions.gradle.kts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,7 @@ afterEvaluate {
418418
checkstyle {
419419
configFile = rootProject.file("buildscripts/checkstyle.xml")
420420
// this version should match the version of google_checks.xml used as basis for above configuration
421-
toolVersion = "13.3.0"
421+
toolVersion = "13.4.0"
422422
maxWarnings = 0
423423
}
424424

0 commit comments

Comments
 (0)