Skip to content

Commit cde1b63

Browse files
authored
Merge branch 'main' into exceptions-over-log-signal
2 parents c22e3fa + e8169cb commit cde1b63

978 files changed

Lines changed: 14468 additions & 6688 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.

.fossa.yml

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -367,9 +367,6 @@ targets:
367367
- type: gradle
368368
path: ./
369369
target: ':instrumentation:aws-lambda:aws-lambda-events-2.2:javaagent'
370-
- type: gradle
371-
path: ./
372-
target: ':instrumentation:aws-lambda:aws-lambda-events-2.2:library'
373370
- type: gradle
374371
path: ./
375372
target: ':instrumentation:aws-lambda:aws-lambda-events-3.11:library'
@@ -420,7 +417,7 @@ targets:
420417
target: ':instrumentation:cassandra:cassandra-4.4:library'
421418
- type: gradle
422419
path: ./
423-
target: ':instrumentation:clickhouse:clickhouse-client-common:javaagent'
420+
target: ':instrumentation:clickhouse:clickhouse-client-common-0.5:javaagent'
424421
- type: gradle
425422
path: ./
426423
target: ':instrumentation:clickhouse:clickhouse-client-v1-0.5:javaagent'
@@ -570,7 +567,7 @@ targets:
570567
target: ':instrumentation:jaxws:jaxws-2.0-metro-2.2:javaagent'
571568
- type: gradle
572569
path: ./
573-
target: ':instrumentation:jaxws:jaxws-common:javaagent'
570+
target: ':instrumentation:jaxws:jaxws-common-2.0:javaagent'
574571
- type: gradle
575572
path: ./
576573
target: ':instrumentation:jaxws:jaxws-jws-api-1.1:javaagent'
@@ -1053,7 +1050,7 @@ targets:
10531050
target: ':instrumentation:xxl-job:xxl-job-2.3.0:javaagent'
10541051
- type: gradle
10551052
path: ./
1056-
target: ':instrumentation:xxl-job:xxl-job-common:javaagent'
1053+
target: ':instrumentation:xxl-job:xxl-job-common-1.9.2:javaagent'
10571054
- type: gradle
10581055
path: ./
10591056
target: ':instrumentation:zio:zio-2.0:javaagent'
@@ -1131,7 +1128,7 @@ targets:
11311128
target: ':instrumentation:play:play-ws:play-ws-2.1:javaagent'
11321129
- type: gradle
11331130
path: ./
1134-
target: ':instrumentation:play:play-ws:play-ws-common:javaagent'
1131+
target: ':instrumentation:play:play-ws:play-ws-common-1.0:javaagent'
11351132
- type: gradle
11361133
path: ./
11371134
target: ':instrumentation:reactor:reactor-netty:reactor-netty-0.9:javaagent'

.gitattributes

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,5 @@
44
*.cmd text eol=crlf
55

66
licenses/** linguist-generated
7+
8+
.github/workflows/*.lock.yml linguist-generated=true merge=ours

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

Lines changed: 0 additions & 156 deletions
This file was deleted.

.github/agents/knowledge/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ Load only files relevant to the current scope to reduce noise and avoid over-con
1515
| `gradle-conventions.md` | `build.gradle.kts` or `settings.gradle.kts` changes, custom test task registration or wiring |
1616
| `javaagent-advice-patterns.md` | ByteBuddy `@Advice` class or advice-method changes |
1717
| `javaagent-module-patterns.md` | `InstrumentationModule`, `TypeInstrumentation`, `VirtualField`, `CallDepth` |
18-
| `javaagent-singletons-patterns.md` | `*Singletons` holder classes, singleton accessors, callers of singleton accessors/fields |
18+
| `javaagent-singletons-patterns.md` | `*Singletons`, `*SpanNaming`, and similar holder classes; singleton accessors; callers of singleton accessors/fields |
1919
| `library-patterns.md` | Library instrumentation telemetry, builder, getter, or setter pattern changes |
2020
| `module-naming.md` | New or renamed modules or packages; settings includes |
2121
| `testing-general-patterns.md` | Test files in scope — assertion style, test method signatures and throws clauses, resource cleanup patterns, attribute assertion patterns, `satisfies()` lambda usage |

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

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ When a "Knowledge File" is listed, load it from `knowledge/` before reviewing th
2121
| Naming | Module/package naming | New or renamed modules/packages | `module-naming.md` |
2222
| Javaagent | Advice patterns | `@Advice` classes | `javaagent-advice-patterns.md` |
2323
| Javaagent | Module structure patterns | `InstrumentationModule`, `TypeInstrumentation` | `javaagent-module-patterns.md` |
24-
| Javaagent | Singletons patterns | `*Singletons` holder classes, singleton accessors, callers of singleton accessors/fields | `javaagent-singletons-patterns.md` |
24+
| Javaagent | Singletons patterns | `*Singletons`, `*SpanNaming`, and similar holder classes; singleton accessors; callers of singleton accessors/fields | `javaagent-singletons-patterns.md` |
2525
| Javaagent | Incorrect `classLoaderMatcher()` | `classLoaderMatcher()` override that is redundant (muzzle already handles it) or missing when needed (muzzle cannot distinguish version range) | `javaagent-module-patterns.md` |
2626
| Semconv | Library vs javaagent semconv constant usage | Semconv constants/assertions ||
2727
| Semconv | Dual semconv testing | `SemconvStability`, `maybeStable`, semconv Gradle tasks | `testing-semconv-stability.md` |
@@ -34,8 +34,9 @@ When a "Knowledge File" is listed, load it from `knowledge/` before reviewing th
3434
| Config | metadata.yaml format and declarative_name conversion — Mandatory for every instrumentation module | Any instrumentation module in scope (javaagent, library, or testing) | `metadata-yaml-format.md` |
3535
| Build | Gradle conventions, muzzle, test tasks, plugins | `build.gradle.kts`, `settings.gradle.kts` | `gradle-conventions.md` |
3636
| Build | `testcontainersBuildService` declaration | Testcontainers dependency without `usesService` | `gradle-conventions.md` |
37-
| Style | Prefer instance creation over singletons for stateless interface impls (except on hot paths or Kotlin `object` declarations) | `TextMapGetter`, `TextMapSetter`, `*AttributesGetter`, `AttributesExtractor`, `SpanNameExtractor`, `HttpServerResponseMutator`, enum/static singletons ||
37+
| Style | Prefer instance creation over singletons for stateless interface impls (except on hot paths or Kotlin `object` declarations) | `TextMapGetter`, `TextMapSetter`, `*AttributesGetter`, `AttributesExtractor`, `SpanNameExtractor`, enum/static singletons ||
3838
| Style | Prefer `value == null` / `value != null` over `null == value` / `null != value` | Null comparisons ||
39+
| Style | Do not rewrite `value.equals(CONSTANT)` to `CONSTANT.equals(value)` solely for defensive null-safety; first verify that `value` can be null and that treating null as non-equal is the intended behavior | `.equals(...)` comparisons ||
3940
| Style | No unnecessary explicit type witnesses on generic method calls (`Collections.<String>emptyList()`) | Java generic method calls with explicit type parameters ||
4041
| Style | Remove redundant null guards on attribute puts | `AttributesBuilder.put`, `onStart`, `onEnd`, attribute extraction methods ||
4142
| General | No redundant `ByteBuffer.duplicate()` on `Value.getValue()` | `Value.getValue()` with `BYTES` type, `ByteBuffer` handling ||
@@ -105,7 +106,8 @@ Reason about visibility from "what does the advice method directly reference?".
105106
## [Style] `@SuppressWarnings` Usage
106107

107108
- Place `@SuppressWarnings` on the single member that needs it, or on the class when two
108-
or more members in the class need the same suppression.
109+
or more members in the class need the same suppression. Do not move an existing
110+
suppression from a member to the class unless multiple members need it.
109111
- **Do not add `@SuppressWarnings("deprecation")` unless the build fails without it.**
110112
The project disables javac's `-Xlint:deprecation` globally and uses a custom Error Prone
111113
check (`OtelDeprecatedApiUsage`) instead. Only add the annotation when it is actually
@@ -124,6 +126,15 @@ Reason about visibility from "what does the advice method directly reference?".
124126

125127
Public API getters should use `get*` (or `is*` for booleans).
126128

129+
## [Style] Equals Comparisons
130+
131+
Prefer the natural receiver form, such as `value.equals(CONSTANT)`, when `value` is expected to be
132+
non-null. Do not flip operands to `CONSTANT.equals(value)` as a blanket null-safety cleanup.
133+
134+
If `value` can actually be null, first decide whether null should be tolerated or should fail fast.
135+
Use an explicit null check or `Objects.equals(...)` when null is part of the valid contract; otherwise
136+
keep the dereference so an unexpected null is visible.
137+
127138
## [Style] Catch Exception Variable Naming
128139

129140
Prefer `e` as the exception variable name in catch clauses when the exception is
@@ -151,9 +162,8 @@ recognizes `expected` and `ok`.
151162
## [Style] Prefer Instance Creation Over Singletons
152163

153164
Stateless implementations of telemetry interfaces — `TextMapGetter`, `TextMapSetter`,
154-
`*AttributesGetter`, `AttributesExtractor`, `SpanNameExtractor`,
155-
`HttpServerResponseMutator` — should use instance creation (`new MyGetter()`) instead of
156-
singleton patterns.
165+
`*AttributesGetter`, `AttributesExtractor`, `SpanNameExtractor` — should use instance creation
166+
(`new MyGetter()`) instead of singleton patterns.
157167

158168
Replace every singleton reference (`MyGetter.INSTANCE`, `MyGetter.getInstance()`) with
159169
`new MyGetter()`. Do not restructure surrounding code — if the original used a local

.github/agents/knowledge/gradle-conventions.md

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,10 @@ usesService(gradle.sharedServices.registrations["testcontainersBuildService"].se
199199
```
200200

201201
Place the declaration in `withType<Test>().configureEach` when **all** test tasks in the
202-
module use Testcontainers. Otherwise place it on **only** the specific task(s) that do.
202+
module use Testcontainers **and** the module has multiple test tasks. When the module has
203+
only a single test task, put the declaration directly inside `tasks.test { ... }` — do not
204+
introduce a `withType<Test>().configureEach` block just for `usesService`. Otherwise place
205+
it on **only** the specific task(s) that use Testcontainers.
203206

204207
**Do not over-apply.** Adding `usesService` to a task that does not use Testcontainers
205208
unnecessarily throttles it against the 2-slot concurrency limit. A module may have some
@@ -215,9 +218,10 @@ because the dependency may only be used by some suites in the module.
215218

216219
## Prefer `withType<Test>().configureEach` (ONLY when multiple test tasks exist)
217220

218-
When a module has custom test tasks (e.g., `testStableSemconv`), system properties and JVM
219-
args that apply to **all** test tasks should be set once in a `withType<Test>().configureEach`
220-
block, not repeated on each individual task.
221+
When a module has custom test tasks (e.g., `testStableSemconv`), shared configuration
222+
(system properties, JVM args, `usesService` declarations, etc.) that applies to **all**
223+
test tasks should be set once in a `withType<Test>().configureEach` block, not repeated
224+
on each individual task.
221225

222226
If a property or JVM arg is moved into `withType<Test>().configureEach`, remove any now-redundant
223227
copies from individual tasks unless a task intentionally overrides the shared value.
@@ -262,20 +266,27 @@ These system properties support the metadata collection pipeline. They are not r
262266
test correctness and are being added as a separate migration — **do not add them during
263267
review**. Only verify correctness when they are already present.
264268

269+
Do not add `collectMetadata` or `metadataConfig` to `javaagent-unit-tests` projects. These are
270+
unit tests, and metadata collection should not run there.
271+
265272
| Property | Type | Value |
266273
| --- | --- | --- |
267274
| `collectMetadata` | System property | Pass-through of `otelProps.collectMetadata`; defaults to `false` |
268275
| `metadataConfig` | System property | A single `key=value` string describing the non-default configuration active during this test run |
269276

270277
When already present, verify:
271278

279+
- `metadataConfig` is only used in files that also configure `collectMetadata`. A lone
280+
`metadataConfig` does not enable collection and should be removed, not added as a partial
281+
metadata migration.
272282
- `collectMetadata` is in `tasks.test` for single-test-task modules, or in
273283
`withType<Test>().configureEach` for modules that explicitly register additional `Test`
274284
tasks via `by registering(Test::class)` (`latestDepTest` does not count) — never on
275285
individual tasks. Do not use
276286
`withType<Test>().configureEach { ... }` in single-test-task modules.
277-
- `metadataConfig` is on each non-default task. It may also appear on the default `test`
278-
task when that task itself runs with non-default `jvmArgs` (e.g., an experimental flag
279-
enabled module-wide via `withType<Test>().configureEach { jvmArgs(...) }`); in that case
280-
the `metadataConfig` value should describe those non-default jvmArgs.
287+
- `metadataConfig` is on each non-default task that participates in metadata collection. It may
288+
also appear on the default `test` task when that task participates in metadata collection and
289+
itself runs with non-default `jvmArgs` (e.g., an experimental flag enabled module-wide via
290+
`withType<Test>().configureEach { jvmArgs(...) }`); in that case the `metadataConfig` value
291+
should describe those non-default jvmArgs.
281292
- The `metadataConfig` value matches at least one of the jvmArgs configured in the task

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,9 @@ the IDE's perspective, and per-method placement is inconsistent with the rest of
4242
## Advice Methods Must Be Static
4343

4444
All `@Advice.OnMethodEnter` and `@Advice.OnMethodExit` methods **must be `static`**. ByteBuddy
45-
inlines advice code directly into the instrumented method — there is no advice object instance.
45+
inlines advice code directly into the instrumented method by default — there is no advice object
46+
instance. Do not add or remove `inline = false` as cleanup; it is a semantic signal used by
47+
muzzle/indy tooling.
4648

4749
```java
4850
// ✅ Correct

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@
22

33
## Quick Reference
44

5-
- Use when: reviewing `*Singletons` holder classes and their callers
5+
- Use when: reviewing `*Singletons`, `*SpanNaming`, and similar holder classes and their callers
66
- Review focus: field/accessor naming, eager initialization, singleton accessor call sites
77

88
Javaagent modules keep shared `Instrumenter` instances and related collaborators in a dedicated
9-
`Singletons` holder class such as `MyLibrarySingletons`.
9+
`Singletons` holder class such as `MyLibrarySingletons`. Some modules also use focused helper
10+
holders such as `*ServerSpanNaming` for shared span-name or route-name collaborators; apply the
11+
same accessor and call-site rules when these classes expose stored singleton fields.
1012

1113
## Rules
1214

@@ -18,8 +20,10 @@ Javaagent modules keep shared `Instrumenter` instances and related collaborators
1820
- For exported collaborators, keep the field `private`, use a lower camel case field name, and
1921
give the accessor method the exact same name as the field. Do not prefix these accessors with
2022
`get`. This rule applies only to zero-arg methods that directly return a stored singleton
21-
field; methods that take arguments or compute a value are not singleton accessors and keep
22-
their normal names (including `get*` when appropriate).
23+
field. It applies regardless of whether the holder class is named `*Singletons`,
24+
`*ServerSpanNaming`, or another focused holder name. Methods that take arguments or compute a
25+
value are not singleton accessors and keep their normal names (including `get*` when
26+
appropriate).
2327
- `instrumenter` -> `instrumenter()`
2428
- `helper` -> `helper()`
2529
- `setter` -> `setter()`
@@ -32,7 +36,9 @@ Javaagent modules keep shared `Instrumenter` instances and related collaborators
3236
- `RESPONSE_STATUS` stays `RESPONSE_STATUS`
3337
- Callers should static import only exported singleton accessors and uppercase constant-like
3438
fields, and use those members unqualified: accessors for lower camel collaborators, fields for
35-
uppercase constant-like members.
39+
uppercase constant-like members. This includes route/span naming accessors such as
40+
`serverSpanName()` when they simply return a stored `HttpServerRouteGetter` or similar
41+
collaborator.
3642
- Keep verb-named helper methods as verbs when they perform work instead of returning a stored
3743
field. These methods are not singleton accessors and should not be static imported under this
3844
rule.

0 commit comments

Comments
 (0)