Merge branch 'main' into dubbo-client-lb

Author: steverao

.fossa.yml

@@ -346,9 +346,6 @@ targets:
     - type: gradle
       path: ./
       target: ':instrumentation:armeria:armeria-grpc-1.14:javaagent'
-    - type: gradle
-      path: ./
-      target: ':instrumentation:async-http-client:async-http-client-1-common:javaagent'
     - type: gradle
       path: ./
       target: ':instrumentation:async-http-client:async-http-client-1.8:javaagent'
@@ -358,6 +355,9 @@ targets:
     - type: gradle
       path: ./
       target: ':instrumentation:async-http-client:async-http-client-2.0:javaagent'
+    - type: gradle
+      path: ./
+      target: ':instrumentation:async-http-client:async-http-client-common-1.8:javaagent'
     - type: gradle
       path: ./
       target: ':instrumentation:aws-lambda:aws-lambda-core-1.0:javaagent'
@@ -427,9 +427,6 @@ targets:
     - type: gradle
       path: ./
       target: ':instrumentation:clickhouse:clickhouse-client-v2-0.8:javaagent'
-    - type: gradle
-      path: ./
-      target: ':instrumentation:couchbase:couchbase-2-common:javaagent'
     - type: gradle
       path: ./
       target: ':instrumentation:couchbase:couchbase-2.0:javaagent'
@@ -448,6 +445,9 @@ targets:
     - type: gradle
       path: ./
       target: ':instrumentation:couchbase:couchbase-3.4:javaagent'
+    - type: gradle
+      path: ./
+      target: ':instrumentation:couchbase:couchbase-common-2.0:javaagent'
     - type: gradle
       path: ./
       target: ':instrumentation:dropwizard:dropwizard-metrics-4.0:javaagent'
@@ -630,10 +630,10 @@ targets:
       target: ':instrumentation:jms:jms-common:javaagent'
     - type: gradle
       path: ./
-      target: ':instrumentation:jsf:jsf-jakarta-common:javaagent'
+      target: ':instrumentation:jsf:jsf-common-javax:javaagent'
     - type: gradle
       path: ./
-      target: ':instrumentation:jsf:jsf-javax-common:javaagent'
+      target: ':instrumentation:jsf:jsf-jakarta-common:javaagent'
     - type: gradle
       path: ./
       target: ':instrumentation:jsf:jsf-mojarra-1.2:javaagent'
@@ -835,9 +835,6 @@ targets:
     - type: gradle
       path: ./
       target: ':instrumentation:opentelemetry-api:opentelemetry-api-1.50:javaagent'
-    - type: gradle
-      path: ./
-      target: ':instrumentation:opentelemetry-api:opentelemetry-api-1.52:javaagent'
     - type: gradle
       path: ./
       target: ':instrumentation:opentelemetry-api:opentelemetry-api-1.56:javaagent'
@@ -1171,9 +1168,6 @@ targets:
     - type: gradle
       path: ./
       target: ':instrumentation:spring:spring-jms:spring-jms-6.0:javaagent'
-    - type: gradle
-      path: ./
-      target: ':instrumentation:spring:spring-web:spring-web-3.1:javaagent'
     - type: gradle
       path: ./
       target: ':instrumentation:spring:spring-web:spring-web-3.1:library'

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

@@ -91,13 +91,15 @@ For each file in scope:
    - PR mode: changed lines only
    - File/directory mode: all lines
 4. Apply checklist rules (below) and insert comments above offending lines.
-5. Do not flag a non-capturing lambda or method reference as an unnecessary allocation,
-   because on modern JDKs these are typically cached at the call site rather than
-   allocated on every invocation.
-6. Flag a missing `// added in X.Y` comment on a single-class lower-bound
-   `hasClassesNamed(...)` check in `classLoaderMatcher()` only after validating that the
-   stated version is factually correct from repository or upstream evidence; the comment
-   documents why the matcher exists and which version boundary it enforces.
+5. Do not flag a non-capturing lambda or method reference as an allocation issue.
+   On HotSpot / OpenJDK 8+, these are cached at the call site.
+6. Flag a missing version-boundary comment on a single-class `hasClassesNamed(...)`
+   check in `classLoaderMatcher()` only after validating the stated boundary from
+   repository or upstream evidence. Use `// added in X.Y` for a pure lower bound.
+   If the same positive class also provides the upper bound because it disappears in a
+   newer sibling version, preserve or request `removed in Y.Z` too. For negated checks
+   such as `not(hasClassesNamed(...))`, use `// added in Y.Z` because the class's first
+   appearance is what starts the excluded version range.
 7. Prevent duplicates:
    - If equivalent `REVIEW:` already exists above the same line, do not add another.
 

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

@@ -11,12 +11,14 @@ Load only files relevant to the current scope to reduce noise and avoid over-con
 | `api-deprecation-policy.md` | Public API removal, rename, or deprecation; stable vs alpha breaking changes |
 | `config-property-stability.md` | `otel.instrumentation.*` property add, remove, rename, or deprecation |
 | `general-rules.md` | Always — review checklist table and core rules enforced on every review |
+| `metadata-yaml-format.md` | Always — mandatory review of metadata.yaml for config coverage |
 | `gradle-conventions.md` | `build.gradle.kts` or `settings.gradle.kts` changes, custom test task registration or wiring |
 | `javaagent-advice-patterns.md` | ByteBuddy `@Advice` class or advice-method changes |
-| `javaagent-module-patterns.md` | `InstrumentationModule`, `TypeInstrumentation`, `Singletons`, `VirtualField`, `CallDepth` |
+| `javaagent-module-patterns.md` | `InstrumentationModule`, `TypeInstrumentation`, `VirtualField`, `CallDepth` |
+| `javaagent-singletons-patterns.md` | `*Singletons` holder classes, singleton accessors, callers of singleton accessors/fields |
 | `library-patterns.md` | Library instrumentation telemetry, builder, getter, or setter pattern changes |
 | `module-naming.md` | New or renamed modules or packages; settings includes |
-| `testing-general-patterns.md` | Test files in scope — assertion style, attribute assertion patterns, `satisfies()` lambda usage |
+| `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 |
 | `testing-experimental-flags.md` | `testExperimental` task or experimental span-attribute assertions |
 | `testing-semconv-stability.md` | Semconv opt-in modes, `emitOld*`/`emitStable*`, `maybeStable`, Semconv test tasks |
 

.github/agents/knowledge/README.md

@@ -52,7 +52,7 @@ Defined in [VERSIONING.md](../../../VERSIONING.md):
 Examples (flat ↔ YAML):
 
 - `otel.instrumentation.http.client.capture-request-headers` ↔ `request_captured_headers` — **stable**
-- `otel.instrumentation.common.experimental.db-sqlcommenter.enabled` ↔ `sqlcommenter/development: { enabled: true }` — **experimental**
+- `otel.instrumentation.common.db.experimental.sqlcommenter.enabled` ↔ `sqlcommenter/development: { enabled: true }` — **experimental**
 - `otel.instrumentation.http.client.emit-experimental-telemetry` ↔ `emit_experimental_telemetry/development: true` — **experimental**
 
 ## Deprecation Communication

.github/agents/knowledge/config-property-stability.md

@@ -15,22 +15,26 @@ When a "Knowledge File" is listed, load it from `knowledge/` before reviewing th
 | --- | --- | --- | --- |
 | General | Logic, correctness, reliability, safety, copy/paste mistakes, incorrect comments | Always | — |
 | Style | Style guide | Always | — |
-| Style | Uppercase field names should reflect semantic constants, not simply `static final` | Always | — |
+| Style | Uppercase field names should reflect semantic constants or immutable value constants such as `Duration` timeouts/intervals, not simply `static final` | Always | — |
 | Naming | Getter naming (`get` / `is`) | Always | — |
+| Style | Prefer `e` for used exceptions, prefer `f` for used exceptions in nested catch clauses when an outer catch already uses `e`, allow `error` for specific `*Error` catch types, prefer `t` for used `Throwable` catch values, prefer `ignored` for intentionally unused catch parameters, and use `ignore` for nested intentionally unused catch parameters when `ignored` would shadow an outer catch variable | Catch clauses | — |
 | Naming | Module/package naming | New or renamed modules/packages | `module-naming.md` |
 | Javaagent | Advice patterns | `@Advice` classes | `javaagent-advice-patterns.md` |
-| Javaagent | Module structure patterns | `InstrumentationModule`, `TypeInstrumentation`, `Singletons` | `javaagent-module-patterns.md` |
+| Javaagent | Module structure patterns | `InstrumentationModule`, `TypeInstrumentation` | `javaagent-module-patterns.md` |
+| Javaagent | Singletons patterns | `*Singletons` holder classes, singleton accessors, callers of singleton accessors/fields | `javaagent-singletons-patterns.md` |
 | 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` |
 | Semconv | Library vs javaagent semconv constant usage | Semconv constants/assertions | — |
 | Semconv | Dual semconv testing | `SemconvStability`, `maybeStable`, semconv Gradle tasks | `testing-semconv-stability.md` |
-| Testing | General test patterns | Test files in scope | `testing-general-patterns.md` |
+| Testing | General test patterns | Test files in scope — assertion style, test method signatures and throws clauses, resource cleanup, attribute assertions | `testing-general-patterns.md` |
 | Testing | Experimental flag tests | `testExperimental`, experimental attribute assertions, `experimental` flags in JVM args or system properties | `testing-experimental-flags.md` |
 | Library | TelemetryBuilder/getter/setter patterns | Library instrumentation classes | `library-patterns.md` |
 | API | Deprecation and breaking-change policy | Public API changes | `api-deprecation-policy.md` |
 | Config | Config property stability/renames/removals | `otel.instrumentation.*` property changes, `DeclarativeConfigUtil` or `ConfigProperties` usage | `config-property-stability.md` |
+| 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` |
 | Build | Gradle conventions, muzzle, test tasks, plugins | `build.gradle.kts`, `settings.gradle.kts` | `gradle-conventions.md` |
 | Build | `testcontainersBuildService` declaration | Testcontainers dependency without `usesService` | `gradle-conventions.md` |
 | 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 | — |
+| Style | Prefer `value == null` / `value != null` over `null == value` / `null != value` | Null comparisons | — |
 | Style | No unnecessary explicit type witnesses on generic method calls (`Collections.<String>emptyList()`) | Java generic method calls with explicit type parameters | — |
 | Style | Remove redundant null guards on attribute puts | `AttributesBuilder.put`, `onStart`, `onEnd`, attribute extraction methods | — |
 | General | No redundant `ByteBuffer.duplicate()` on `Value.getValue()` | `Value.getValue()` with `BYTES` type, `ByteBuffer` handling | — |
@@ -54,13 +58,37 @@ Flag real defects, including:
 
 Only flag substantive problems, not stylistic preference.
 
+## [Javaagent] Best-Effort Suppressed Failures
+
+When javaagent runtime code intentionally suppresses a `Throwable` to avoid breaking the
+application, do not silently swallow it unless the failure is an expected optional probe.
+
+- In ordinary instrumentation implementation code, local JUL `logger.log(FINE, ...)` is an
+  established pattern and preserves module / class logger identity.
+- Silent suppression is acceptable for expected optional-probe paths, such as classpath or
+  version-detection lookups where failure is routine and logging would be noisy.
+
 ## [Style] Style Guide
 
 Read and apply `docs/contributing/style-guide.md`.
 
 Do not flag the following patterns (common false positives):
 
 - FQCN is acceptable when class-name collision makes import impossible.
+- Do not claim that a Java non-capturing lambda or method reference allocates per
+  call. On HotSpot / OpenJDK 8+, these are cached at the call site.
+
+## [Style] Visibility modifiers
+
+Follow the principle of minimal necessary visibility. Use the most restrictive access modifier that
+still allows the code to function correctly.
+
+**Exception — Single public class**: If a module has only one public class then don't change it to
+package-private. Javadoc task fails when module has no public classes.**
+
+**Exception — Used from advice**: All classes and methods used from methods annotated with
+`@Advice.OnMethodEnter` or `@Advice.OnMethodExit` must be public. These methods are inlined into
+transformed classes and must be accessible from those classes, which may be in different packages.
 
 ## [Style] `@SuppressWarnings` Usage
 
@@ -71,11 +99,44 @@ Do not flag the following patterns (common false positives):
   The project disables javac's `-Xlint:deprecation` globally and uses a custom Error Prone
   check (`OtelDeprecatedApiUsage`) instead. Only add the annotation when it is actually
   required to fix an Error Prone error — not speculatively.
+- Preserve concise explanatory comments attached to existing `@SuppressWarnings` annotations
+  when they explain why the suppression exists (for example `// using deprecated semconv`
+  or `// using deprecated config property`). This repository has established precedent for
+  such comments, and test code disables Error Prone's `SuppressWarningsWithoutExplanation`
+  check only because enforcing it everywhere causes too many failures, not because these
+  comments are undesirable.
+- When normalizing or moving an existing `@SuppressWarnings`, keep any accurate explanatory
+  comment with it. Remove the comment only if it is incorrect, obsolete, or contradicted by
+  the code after your change.
 
 ## [Naming] Getter Naming
 
 Public API getters should use `get*` (or `is*` for booleans).
 
+## [Style] Catch Exception Variable Naming
+
+Prefer `e` as the exception variable name in catch clauses when the exception is
+used.
+
+For nested catch clauses, when an outer catch already uses `e`, prefer `f` as the
+inner exception variable name when the exception is used.
+
+`error` is also acceptable when the caught type is a specific `*Error` subtype.
+
+Prefer `t` for used `Throwable` catch values, including `catch (Throwable t)`.
+
+Do not apply these catch-variable naming preferences to method parameters,
+lambda parameters, fields, or other non-catch identifiers.
+
+If a catch parameter is intentionally unused, prefer `ignored` over `ignore`.
+
+For nested catch clauses, if `ignored` would shadow an outer catch variable,
+prefer `ignore` for the inner intentionally unused catch parameter.
+
+Both `ignored` and `ignore` are recognized by IntelliJ's `CatchMayIgnoreException`
+inspection as intentional unused-catch names. In test sources, IntelliJ also
+recognizes `expected` and `ok`.
+
 ## [Style] Prefer Instance Creation Over Singletons
 
 Stateless implementations of telemetry interfaces — `TextMapGetter`, `TextMapSetter`,
@@ -108,6 +169,11 @@ All `put` / `setAttribute` methods on `AttributesBuilder`, `Span`, `SpanBuilder`
 `LogRecordBuilder` are no-ops when the value is `null` (upstream SDK guarantee).
 Do not wrap these calls in `if (value != null)` guards — pass the value directly.
 
+This rule applies only when the guarded value can be passed through directly to the
+attribute setter. If the null check is guarding a dereference or other derived
+computation, keep the explicit guard instead of rewriting it into a ternary
+expression just to feed `null` to `put()`.
+
 **Exception — `AttributeKey<Long>` with `Integer` value**: the only primitive-typed
 overload on these interfaces is a convenience method that accepts `int`:
 
@@ -142,6 +208,16 @@ Preferred:
 attributes.put(SOME_KEY, getSomething());
 ```
 
+Do **not** flag (the guard is preserving a safe dereference and is clearer than a
+ternary rewrite):
+
+```java
+View view = modelAndView.getView();
+if (view != null) {
+  attributes.put("spring-webmvc.view.type", view.getClass().getName());
+}
+```
+
 Also flag (the guard is unnecessary — types match, generic overload handles null):
 
 ```java
@@ -323,6 +399,13 @@ instrumentation enable/disable bootstrapping (`otel.instrumentation.<name>.enabl
 `otel.instrumentation.common.default-enabled`). All other config reads go through the
 declarative API. Do not flag `DeclarativeConfigUtil` usage as incorrect.
 
+## [Config] Mandatory metadata.yaml Review
+
+**For every instrumentation module in scope, you MUST review and update its `metadata.yaml` file.**
+
+This is mandatory regardless of whether metadata.yaml was modified in the PR. Load
+`metadata-yaml-format.md` and execute its full validation procedure for each module.
+
 ## [Testing] General Test Patterns
 
 See `testing-general-patterns.md` (loaded when test files are in scope).