Merge remote-tracking branch 'upstream/main' into realtime-dash

# Conflicts:
#	.github/workflows/pr-review-dashboard.yml

Author: trask

.github/agents/knowledge/general-rules.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/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.

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

.github/labeler.yml

@@ -101,14 +101,16 @@
 # ---------------------------------------------------------------- gh helpers
 
 
-def gh_api(path: str, paginate: bool = False) -> Any:
+def gh_api(path: str, paginate: bool = False, token: str | None = None) -> Any:
     cmd = ["gh", "api", "-H", "Accept: application/vnd.github+json"]
     if paginate:
         cmd += ["--paginate", "--slurp"]
     cmd.append(path)
+    env = {**os.environ, "GH_TOKEN": token} if token else None
     proc = subprocess.run(
         cmd, capture_output=True, text=True, check=False,
         encoding="utf-8", errors="replace",
+        env=env,
     )
     if proc.returncode != 0:
         raise RuntimeError(f"gh api {path} failed: {proc.stderr.strip()}")
@@ -222,14 +224,22 @@ def detect_repo() -> str:
 
 
 def load_reviewer_set(org: str) -> set[str]:
+    # Reading org team membership requires a token with org:read scope.
+    # The default Actions GITHUB_TOKEN can't do this, so use OTELBOT_TOKEN
+    # (a GitHub App installation token) when present; fall back to the
+    # default GH_TOKEN otherwise (useful for local runs with a user token).
+    token = os.environ.get("OTELBOT_TOKEN") or None
     reviewers: set[str] = set()
     for slug in APPROVER_TEAM_SLUGS:
-        members = gh_api(f"/orgs/{org}/teams/{slug}/members?per_page=100", paginate=True)
+        members = gh_api(
+            f"/orgs/{org}/teams/{slug}/members?per_page=100",
+            paginate=True, token=token,
+        )
         reviewers.update(m["login"] for m in members)
     if not reviewers:
         raise RuntimeError(
             f"no reviewers found in teams {APPROVER_TEAM_SLUGS}; "
-            f"the GH_TOKEN must have org:read permission"
+            f"the token must have org:read permission"
         )
     return {r.lower() for r in reviewers}
 
@@ -774,6 +784,12 @@ def render_markdown_compact(
         res = results.get(pr["number"]) or {}
         decision = res.get("decision") or {}
         side = _infer_side(decision) if decision else "unknown"
+        # PRs authored by app/otelbot are never "waiting on authors" — the
+        # bot doesn't respond to review feedback, so the ball is always in
+        # an approver's court (review, close, or take over).
+        pr_author_login = ((pr.get("author") or {}).get("login") or "").lower()
+        if side == "author" and pr_author_login == "app/otelbot":
+            side = "approver"
         # Promote approved PRs that are waiting on approvers into the
         # "maintainer" bucket (deterministic): GitHub already says they have
         # the required approvals, so a maintainer can merge them.

.github/scripts/pull-request-dashboard.py

@@ -1,12 +1,26 @@
 name: Build pull request
 
+# Optional test variants (openj9, windows) are gated by `test openj9` /
+# `test windows` labels on the PR, read from the event payload. We
+# deliberately do NOT subscribe to `pull_request: [labeled]` — see
+# https://github.com/open-telemetry/opentelemetry-java-instrumentation/pull/18446
+# for the history of why every previous attempt to use that trigger failed.
+#
+# Practical consequence: adding a `test openj9` / `test windows` label
+# doesn't retrigger the build. Push another commit or close/reopen the
+# PR to pick it up. `comment-on-test-label.yml` posts a reminder when
+# one of those labels is added.
+#
+# `test native` is detected automatically from the PR's changed paths
+# (the `resolve-native` job below). The manual `test native` label is
+# still honored as a force-enable.
+
 on:
   pull_request:
     types:
       - opened
       - synchronize
       - reopened
-      - labeled
 
 concurrency:
   group: build-pull-request-${{ github.event.pull_request.number }}
@@ -16,22 +30,44 @@ permissions:
   contents: read
 
 jobs:
+  # Run native tests automatically when the PR touches a library
+  # instrumentation module with native support (logback-appender, jdbc,
+  # spring), the native smoke test harness, or the top-level build config
+  # that affects them.
+  resolve-native:
+    runs-on: ubuntu-latest
+    outputs:
+      run-native-tests: ${{ steps.filter.outputs.native }}
+    steps:
+      - name: Detect native-relevant changes
+        id: filter
+        env:
+          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          PR_NUMBER: ${{ github.event.pull_request.number }}
+        run: |
+          # `|| true` so an empty result from `grep -v` (everything filtered)
+          # doesn't fail the step under pipefail.
+          files=$(
+            gh api --paginate "/repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/files" \
+              --jq '.[].filename' |
+              grep -Ev '^instrumentation/spring/.+/javaagent/' || true
+          )
+          include='^instrumentation/logback/logback-appender-1\.0/library/'
+          include+='|^instrumentation/jdbc/library/'
+          include+='|^instrumentation/spring/'
+          include+='|^smoke-tests-otel-starter/'
+          include+='|^dependencyManagement/build\.gradle\.kts$'
+          include+='|^settings\.gradle\.kts$'
+          if grep -Eq "$include" <<<"$files"; then
+            echo "native=true" >> "$GITHUB_OUTPUT"
+          else
+            echo "native=false" >> "$GITHUB_OUTPUT"
+          fi
+
   build:
-    # On `labeled` events, only proceed when the added label affects what we
-    # build. All other event types (opened, synchronize, reopened) always
-    # proceed.
-    if: |
-      github.event.action != 'labeled' ||
-      github.event.label.name == 'test native' ||
-      github.event.label.name == 'test openj9' ||
-      github.event.label.name == 'test windows'
+    needs: [resolve-native]
     uses: ./.github/workflows/reusable-pr-build.yml
     with:
-      # it's rare for only the openj9 tests, openj9 smoke variants, the windows
-      # smoke tests, or the native tests to break, so they are gated by labels.
-      # `test native` is applied automatically by .github/labeler.yml when the
-      # PR diff touches native-relevant paths; `test openj9` and `test windows`
-      # are applied manually.
-      skip-native-tests: ${{ !contains(github.event.pull_request.labels.*.name, 'test native') }}
+      skip-native-tests: ${{ !(needs.resolve-native.outputs.run-native-tests == 'true' || contains(github.event.pull_request.labels.*.name, 'test native')) }}
       skip-openj9-tests: ${{ !contains(github.event.pull_request.labels.*.name, 'test openj9') }}
       skip-windows-smoke-tests: ${{ !contains(github.event.pull_request.labels.*.name, 'test windows') }}