Merge remote-tracking branch 'upstream/main' into rename-common-modules

Author: trask

.fossa.yml

@@ -420,7 +420,7 @@ targets:
       target: ':instrumentation:cassandra:cassandra-4.4:library'
     - type: gradle
       path: ./
-      target: ':instrumentation:clickhouse:clickhouse-client-common:javaagent'
+      target: ':instrumentation:clickhouse:clickhouse-client-common-0.5:javaagent'
     - type: gradle
       path: ./
       target: ':instrumentation:clickhouse:clickhouse-client-v1-0.5:javaagent'
@@ -1131,7 +1131,7 @@ targets:
       target: ':instrumentation:play:play-ws:play-ws-2.1:javaagent'
     - type: gradle
       path: ./
-      target: ':instrumentation:play:play-ws:play-ws-common:javaagent'
+      target: ':instrumentation:play:play-ws:play-ws-common-1.0:javaagent'
     - type: gradle
       path: ./
       target: ':instrumentation:reactor:reactor-netty:reactor-netty-0.9:javaagent'

.github/agents/knowledge/README.md

@@ -15,7 +15,7 @@ Load only files relevant to the current scope to reduce noise and avoid over-con
 | `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`, `VirtualField`, `CallDepth` |
-| `javaagent-singletons-patterns.md` | `*Singletons` holder classes, singleton accessors, callers of singleton accessors/fields |
+| `javaagent-singletons-patterns.md` | `*Singletons`, `*SpanNaming`, and similar 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, test method signatures and throws clauses, resource cleanup patterns, attribute assertion patterns, `satisfies()` lambda usage |

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

@@ -21,7 +21,7 @@ When a "Knowledge File" is listed, load it from `knowledge/` before reviewing th
 | 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` | `javaagent-module-patterns.md` |
-| Javaagent | Singletons patterns | `*Singletons` holder classes, singleton accessors, callers of singleton accessors/fields | `javaagent-singletons-patterns.md` |
+| Javaagent | Singletons patterns | `*Singletons`, `*SpanNaming`, and similar 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` |
@@ -105,7 +105,8 @@ Reason about visibility from "what does the advice method directly reference?".
 ## [Style] `@SuppressWarnings` Usage
 
 - Place `@SuppressWarnings` on the single member that needs it, or on the class when two
-  or more members in the class need the same suppression.
+  or more members in the class need the same suppression. Do not move an existing
+  suppression from a member to the class unless multiple members need it.
 - **Do not add `@SuppressWarnings("deprecation")` unless the build fails without it.**
   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

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

@@ -199,7 +199,10 @@ usesService(gradle.sharedServices.registrations["testcontainersBuildService"].se
 ```
 
 Place the declaration in `withType<Test>().configureEach` when **all** test tasks in the
-module use Testcontainers. Otherwise place it on **only** the specific task(s) that do.
+module use Testcontainers **and** the module has multiple test tasks. When the module has
+only a single test task, put the declaration directly inside `tasks.test { ... }` — do not
+introduce a `withType<Test>().configureEach` block just for `usesService`. Otherwise place
+it on **only** the specific task(s) that use Testcontainers.
 
 **Do not over-apply.** Adding `usesService` to a task that does not use Testcontainers
 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.
 
 ## Prefer `withType<Test>().configureEach` (ONLY when multiple test tasks exist)
 
-When a module has custom test tasks (e.g., `testStableSemconv`), system properties and JVM
-args that apply to **all** test tasks should be set once in a `withType<Test>().configureEach`
-block, not repeated on each individual task.
+When a module has custom test tasks (e.g., `testStableSemconv`), shared configuration
+(system properties, JVM args, `usesService` declarations, etc.) that applies to **all**
+test tasks should be set once in a `withType<Test>().configureEach` block, not repeated
+on each individual task.
 
 If a property or JVM arg is moved into `withType<Test>().configureEach`, remove any now-redundant
 copies from individual tasks unless a task intentionally overrides the shared value.

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

@@ -2,11 +2,13 @@
 
 ## Quick Reference
 
-- Use when: reviewing `*Singletons` holder classes and their callers
+- Use when: reviewing `*Singletons`, `*SpanNaming`, and similar holder classes and their callers
 - Review focus: field/accessor naming, eager initialization, singleton accessor call sites
 
 Javaagent modules keep shared `Instrumenter` instances and related collaborators in a dedicated
-`Singletons` holder class such as `MyLibrarySingletons`.
+`Singletons` holder class such as `MyLibrarySingletons`. Some modules also use focused helper
+holders such as `*ServerSpanNaming` for shared span-name or route-name collaborators; apply the
+same accessor and call-site rules when these classes expose stored singleton fields.
 
 ## Rules
 
@@ -18,8 +20,10 @@ Javaagent modules keep shared `Instrumenter` instances and related collaborators
 - For exported collaborators, keep the field `private`, use a lower camel case field name, and
   give the accessor method the exact same name as the field. Do not prefix these accessors with
   `get`. This rule applies only to zero-arg methods that directly return a stored singleton
-  field; methods that take arguments or compute a value are not singleton accessors and keep
-  their normal names (including `get*` when appropriate).
+  field. It applies regardless of whether the holder class is named `*Singletons`,
+  `*ServerSpanNaming`, or another focused holder name. Methods that take arguments or compute a
+  value are not singleton accessors and keep their normal names (including `get*` when
+  appropriate).
   - `instrumenter` -> `instrumenter()`
   - `helper` -> `helper()`
   - `setter` -> `setter()`
@@ -32,7 +36,9 @@ Javaagent modules keep shared `Instrumenter` instances and related collaborators
   - `RESPONSE_STATUS` stays `RESPONSE_STATUS`
 - Callers should static import only exported singleton accessors and uppercase constant-like
   fields, and use those members unqualified: accessors for lower camel collaborators, fields for
-  uppercase constant-like members.
+  uppercase constant-like members. This includes route/span naming accessors such as
+  `serverSpanName()` when they simply return a stored `HttpServerRouteGetter` or similar
+  collaborator.
 - Keep verb-named helper methods as verbs when they perform work instead of returning a stored
   field. These methods are not singleton accessors and should not be static imported under this
   rule.

.github/agents/knowledge/testing-general-patterns.md

@@ -83,6 +83,11 @@
   `hasAttributesSatisfyingExactly(...)` in the same assertion chain, because the exact
   variant already validates the total attribute count. Remove the `hasTotalAttributeCount`
   call.
+- These rules apply to **span** attribute assertions only. Metric point assertions are
+  different: metric point assertions do not have a `hasTotalAttributeCount(...)` method, so
+  the span guidance about preferring `hasTotalAttributeCount(0)` for zero-attribute checks
+  does not apply. For metric points, used `point.hasAttributes(Attributes.empty())` — it
+  reads more clearly than the no-arg `hasAttributesSatisfyingExactly()` form.
 - For non-semconv attribute keys in `equalTo(...)`, use inline `AttributeKey` factory
   methods — `longKey("name")`, `stringKey("name")`, etc. — directly in the assertion.
   Do **not** extract them into class-level `private static final AttributeKey<T>` constants.
@@ -92,6 +97,20 @@
   `equalTo(AttributeKey<Long>, int)` overload, so `equalTo(longKey("iteration"), iteration)` is
   preferred over `equalTo(longKey("iteration"), (long) iteration)`.
 
+## Metric Assertions
+
+- Prefer `InstrumentationExtension.waitAndAssertMetrics(...)` for metric assertions. It waits
+  until the supplied assertion passes, so fixed sleeps such as `Thread.sleep(100)`
+  usually unnecessary and make tests slower and more fragile.
+- `DbConnectionPoolMetricsAssertions.assertConnectionPoolEmitsMetrics()` already uses
+  `waitAndAssertMetrics(...)` for each expected pool metric. Keep the pool state being asserted
+  valid until this method returns; for example, if the assertion expects a `used` connection point,
+  keep the borrowed connection open until after the assertion.
+- After removing a metric-producing source or unregistering an observable callback, call
+  `testing().clearData()` and then use `waitAndAssertMetrics(..., AbstractIterableAssert::isEmpty)`
+  for absence checks. Do not add an exporter-interval sleep before or after `clearData()` solely to
+  wait for metrics; the test runners force-flush metrics when reading them.
+
 ## Attribute Assertion `satisfies()` Lambda Parameters
 
 **Attribute-assertion `satisfies()` lambda parameters are `AbstractAssert` instances, not raw

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

@@ -0,0 +1,171 @@
+---
+description: "Review a PR in opentelemetry-java-instrumentation and post a pending GitHub review with inline comments and code suggestions. The review stays as a draft until the caller submits it."
+tools: [read, edit, execute, search]
+---
+
+You are a code review agent for the `opentelemetry-java-instrumentation` repository.
+
+Primary responsibilities:
+
+- Review PR changes against repository standards and the knowledge base.
+- Post a **pending** (draft) GitHub review with inline comments and `suggestion` blocks.
+- The caller submits the review manually after inspecting it.
+
+Do not stop until all in-scope files are reviewed and the review is posted.
+
+## Knowledge Loading
+
+Always load first:
+
+- `docs/contributing/style-guide.md`
+- `.github/agents/knowledge/general-rules.md` — review checklist and core rules
+
+Then load additional knowledge files **only** when their scope trigger fires.
+Use the **Knowledge File** column in the checklist table inside `general-rules.md`.
+
+## Review Workflow
+
+### Phase 1: Resolve PR
+
+1. Get current branch:
+
+   ```
+   git branch --show-current
+   ```
+
+2. If branch is `main`, stop with:
+   > "Aborting: cannot review the main branch. Please check out a PR branch first."
+
+3. Resolve PR:
+
+   ```
+   gh pr list --head <branch-name> --json number,title,url,headRefOid --jq '.[0]'
+   ```
+
+4. If no PR exists, stop:
+   > "No open PR found for branch `<branch-name>`. Push the branch and open a PR first."
+
+5. Announce: `Reviewing PR #<number>: <title>`
+
+### Phase 2: Build Diff Scope
+
+1. Get changed file names:
+
+   ```
+   gh pr diff <number> --name-only
+   ```
+
+2. Get the full unified diff:
+
+   ```
+   gh pr diff <number> --color never
+   ```
+
+3. Parse the diff to build a map of `file → set of changed right-side line numbers`.
+
+4. For each changed file, also parse the diff **hunk boundaries** (the `@@ ... @@`
+   headers). Record the right-side line ranges that each hunk covers.
+   A review comment or suggestion can only target lines **inside a diff hunk**.
+
+### Phase 3: Read Files and Load Knowledge
+
+1. Skip non-reviewable files:
+   - binary files
+   - files under `licenses/`
+2. Read each changed file's full content.
+3. Scan file contents to decide which additional knowledge articles to load
+   (e.g., load `javaagent-advice-patterns.md` when `@Advice` classes are in scope).
+
+### Phase 4: Review
+
+For each file, apply all rules from the loaded knowledge articles.
+Only flag issues on lines that were changed in the PR diff.
+
+Do not flag:
+
+- Non-capturing lambdas or method references as unnecessary allocations.
+- Patterns explicitly allowed by the style guide or knowledge articles.
+
+For each finding, record:
+
+- `path` — repo-relative file path
+- `line` — right-side line number in the current file
+- `start_line` — (optional) first line, if the comment spans multiple lines
+- `category` — tag like `[Style]`, `[Concurrency]`, `[Javaagent]`, etc.
+- `body` — concise comment text
+- `suggestion` — (optional) replacement text for a `suggestion` block
+
+### Phase 5: Build and Post Review
+
+#### Comment Format
+
+Each comment body should be concise. When a concrete fix is possible, include a
+GitHub suggestion block:
+
+````
+<comment text>
+
+```suggestion
+<replacement lines>
+```
+````
+
+The suggestion block replaces the lines from `start_line` (or `line` if single-line)
+through `line` inclusive. The replacement text must be the **exact literal content**
+that should appear in those lines — no fenced-code markup inside the suggestion.
+
+#### Hunk Validation
+
+Before adding a comment, verify that **all** lines from `start_line` through `line`
+fall inside a diff hunk. If any line is outside a hunk:
+
+- Try narrowing the range (e.g., drop to single-line, remove the suggestion).
+- If the line cannot be commented on at all, skip it and note it in the summary.
+
+#### Multi-line Suggestion Rules
+
+- `start_line` and `start_side` are required for multi-line comments.
+- Both `side` and `start_side` must be `"RIGHT"`.
+- The suggestion text replaces the entire `start_line..line` range.
+
+#### Posting
+
+1. Collect all valid comments into a JSON array.
+
+2. Build the review payload:
+
+   ```json
+   {
+     "commit_id": "<head SHA from Phase 1>",
+     "body": "<summary text>",
+     "comments": [ ... ]
+   }
+   ```
+
+   Do **not** include an `"event"` field — omitting it creates a PENDING review.
+
+3. Write the JSON to a temp file in the workspace (e.g., `_review-payload.json`).
+
+4. Post the review:
+
+   ```
+   gh api repos/{owner}/{repo}/pulls/{number}/reviews --method POST --input _review-payload.json --jq '.id'
+   ```
+
+5. If the API returns a `422` with `"Line could not be resolved"`:
+   - One or more comments reference lines outside diff hunks.
+   - Use binary search: split comments into halves and post each half separately to
+     identify the offending comment(s).
+   - Fix or drop the offending comments, then retry.
+
+6. Delete the temp file after a successful post.
+
+7. Report the review ID and comment count:
+   > Posted pending review `<id>` with N comments on PR #<number>.
+   > Submit it from the GitHub UI or via:
+   > `gh api repos/{owner}/{repo}/pulls/{number}/reviews/{id}/events --method POST -f event=COMMENT`
+
+## Review Checklist and Core Rules
+
+Load `knowledge/general-rules.md` — it contains the review checklist table and all
+core rules that apply to every review.