Code review agent: Add declarative config to metadata (open-telemetry#17871)

jaydeluca · web-flow · commit 1163e7b77ff6 · 2026-04-17T08:23:22.000-07:00
diff --git a/.github/agents/knowledge/README.md b/.github/agents/knowledge/README.md
@@ -11,6 +11,7 @@ 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` | `metadata.yaml` changes, declarative config name conversion |
 | `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` |
diff --git a/.github/agents/knowledge/general-rules.md b/.github/agents/knowledge/general-rules.md
@@ -30,6 +30,7 @@ When a "Knowledge File" is listed, load it from `knowledge/` before reviewing th
 | 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 | `metadata.yaml` changes | `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 | — |
diff --git a/.github/agents/knowledge/metadata-yaml-format.md b/.github/agents/knowledge/metadata-yaml-format.md
@@ -0,0 +1,150 @@
+# [Config] metadata.yaml Format and Declarative Name Conversion
+
+## Quick Reference
+
+- Use when: reviewing or creating `metadata.yaml` files, converting config names
+- Review focus: declarative_name format, examples guidelines, special mappings, config validation
+
+## Entry Structure
+
+Each configuration entry includes:
+
+- `name`: Flat system property (e.g., `otel.instrumentation.grpc.emit-message-events`)
+- `declarative_name`: YAML key path (e.g., `java.grpc.emit_message_events`)
+- `type`: `boolean`, `string`, `list`, `integer`
+- `description`: Human-readable explanation
+- `default`: Default value
+- `examples` (optional): Only for module-specific configs with non-obvious format
+
+## Special Mappings
+
+Non-standard mappings (see `ConfigPropertiesBackedDeclarativeConfigProperties.java` for latest):
+
+| Flat Property                                                                   | Declarative YAML Key                                              |
+|---------------------------------------------------------------------------------|-------------------------------------------------------------------|
+| `otel.instrumentation.http.client.capture-request-headers`                      | `general.http.client.request_captured_headers`                    |
+| `otel.instrumentation.http.client.capture-response-headers`                     | `general.http.client.response_captured_headers`                   |
+| `otel.instrumentation.http.server.capture-request-headers`                      | `general.http.server.request_captured_headers`                    |
+| `otel.instrumentation.http.server.capture-response-headers`                     | `general.http.server.response_captured_headers`                   |
+| `otel.instrumentation.sanitization.url.experimental.sensitive-query-parameters` | `general.sanitization.url.sensitive_query_parameters/development` |
+| `otel.semconv-stability.opt-in`                                                 | `general.semconv_stability.opt_in`                                |
+| `otel.instrumentation.http.known-methods`                                       | `java.common.http.known_methods`                                  |
+| `otel.instrumentation.http.client.experimental.redact-query-parameters`         | `java.common.http.client.redact_query_parameters/development`     |
+| `otel.instrumentation.http.client.emit-experimental-telemetry`                  | `java.common.http.client.emit_experimental_telemetry/development` |
+| `otel.instrumentation.http.server.emit-experimental-telemetry`                  | `java.common.http.server.emit_experimental_telemetry/development` |
+| `otel.instrumentation.messaging.experimental.receive-telemetry.enabled`         | `java.common.messaging.receive_telemetry/development.enabled`     |
+| `otel.instrumentation.messaging.experimental.capture-headers`                   | `java.common.messaging.capture_headers/development`               |
+| `otel.instrumentation.genai.capture-message-content`                            | `java.common.gen_ai.capture_message_content`                      |
+| `otel.instrumentation.experimental.span-suppression-strategy`                   | `java.common.span_suppression_strategy/development`               |
+| `otel.instrumentation.opentelemetry-annotations.exclude-methods`                | `java.opentelemetry_extension_annotations.exclude_methods`        |
+| `otel.experimental.javascript-snippet`                                          | `java.servlet.javascript_snippet/development`                     |
+| `otel.jmx.enabled`                                                              | `java.jmx.enabled`                                                |
+| `otel.jmx.config`                                                               | `java.jmx.config`                                                 |
+| `otel.jmx.target.system`                                                        | `java.jmx.target.system`                                          |
+
+## Standard Conversion
+
+For `otel.instrumentation.*` properties not in SPECIAL_MAPPINGS:
+
+1. Strip `otel.instrumentation.` prefix
+2. Replace `-` with `_` (kebab-case → snake_case)
+3. Handle `experimental`:
+   - `experimental.` path segment → remove, add `/development` to next segment
+   - `experimental-` in name → add `/development` to that segment
+4. Prepend `java.`
+
+Examples:
+
+| Flat Property                                                                | Declarative YAML Key                                        |
+|------------------------------------------------------------------------------|-------------------------------------------------------------|
+| `otel.instrumentation.grpc.emit-message-events`                              | `java.grpc.emit_message_events`                             |
+| `otel.instrumentation.grpc.experimental-span-attributes`                     | `java.grpc.experimental_span_attributes/development`        |
+| `otel.instrumentation.logback-appender.experimental.capture-code-attributes` | `java.logback_appender.capture_code_attributes/development` |
+| `otel.instrumentation.common.experimental.controller-telemetry.enabled`      | `java.common.controller_telemetry/development.enabled`      |
+
+## Examples Guidelines
+
+Add `examples` only for module-specific configs with non-obvious format (lists, patterns, enums).
+
+**Never add for**: `general.*`, `java.common.*`, or boolean configs.
+
+```yaml
+- name: otel.instrumentation.grpc.capture-metadata.client.request
+  declarative_name: java.grpc.capture_metadata.client.request
+  type: list
+  examples:
+    - "custom-request-header"
+    - "header1,header2,header3"
+```
+
+## Validation Procedure
+
+### 1. Validate experimental markers match
+
+- `/development` in declarative_name ↔ `experimental` in flat name (MUST match both ways)
+- WRONG: `otel.instrumentation.servlet.capture-request-parameters` + `java.servlet.capture_request_parameters/development`
+- RIGHT: `otel.instrumentation.servlet.experimental.capture-request-parameters` + `java.servlet.capture_request_parameters/development`
+
+### 2. Verify config is used
+
+Search module source for `DeclarativeConfigUtil.getInstrumentationConfig()`, `config.getBoolean()`, etc.
+
+If a module has a dependency on other modules (for example, a "-common" module, check those as well since they may read the config.
+
+### 3. Verify type and default
+
+Match type and default value with actual code usage.
+
+## Automated Test (MANDATORY)
+
+**Run after any metadata.yaml changes:**
+
+```bash
+./gradlew :instrumentation-docs:test --tests DeclarativeConfigValidationTest
+```
+
+This validates round-trip conversion (flat property → bridge → declarative path) and catches:
+
+- Wrong flat property names (e.g., missing `experimental.`)
+- Wrong declarative paths
+- Type mismatches
+
+Example failure when flat name is wrong:
+
+```
+FAIL in ../instrumentation/liberty/liberty-20.0/metadata.yaml:
+  flat property: otel.instrumentation.servlet.capture-request-parameters
+  expected: [item1, item2, item3]
+  got: null
+```
+
+**Test must pass before completion.**
+
+## Validation Outcomes
+
+| Issue                        | Action                                  |
+|------------------------------|-----------------------------------------|
+| Config not used              | Flag for removal                        |
+| Default/type mismatch        | Update metadata.yaml to match code      |
+| Missing config (in code)     | Add to metadata.yaml                    |
+| Experimental marker mismatch | Fix flat and declarative names to agree |
+
+## Output Format
+
+```yaml
+- name: otel.instrumentation.grpc.emit-message-events
+  declarative_name: java.grpc.emit_message_events
+  type: boolean
+  description: Determines whether to emit a span event for each message.
+  default: true
+- name: otel.instrumentation.grpc.experimental-span-attributes
+  declarative_name: java.grpc.experimental_span_attributes/development
+  type: boolean
+  description: Enable capture of experimental span attributes.
+  default: false
+```
+
+## Edge Cases
+
+- Properties without `otel.instrumentation.` prefix → check SPECIAL_MAPPINGS
+- Already has declarative_name → skip conversion
diff --git a/instrumentation/liberty/liberty-20.0/metadata.yaml b/instrumentation/liberty/liberty-20.0/metadata.yaml
@@ -8,31 +8,37 @@ semantic_conventions:
   - HTTP_SERVER_METRICS
 configurations:
   - name: otel.instrumentation.http.known-methods
+    declarative_name: java.common.http.known_methods
     description: >
       Configures the instrumentation to recognize an alternative set of HTTP request methods. All
       other methods will be treated as `_OTHER`.
     type: list
     default: "CONNECT,DELETE,GET,HEAD,OPTIONS,PATCH,POST,PUT,TRACE"
   - name: otel.instrumentation.http.server.capture-request-headers
+    declarative_name: general.http.server.request_captured_headers
     description: List of HTTP request headers to capture in HTTP server telemetry.
     type: list
     default: ""
   - name: otel.instrumentation.http.server.capture-response-headers
+    declarative_name: general.http.server.response_captured_headers
     description: List of HTTP response headers to capture in HTTP server telemetry.
     type: list
     default: ""
   - name: otel.instrumentation.http.server.emit-experimental-telemetry
+    declarative_name: java.common.http.server.emit_experimental_telemetry/development
     description: >
       Enable the capture of experimental HTTP server telemetry. Adds the `http.request.body.size`
       and `http.response.body.size` attributes to spans, and records `http.server.request.size` and
       `http.server.response.size` metrics.
     type: boolean
     default: false
   - name: otel.instrumentation.servlet.experimental-span-attributes
+    declarative_name: java.servlet.experimental_span_attributes/development
     description: Enables capturing the experimental `servlet.timeout` span attribute.
     type: boolean
     default: false
-  - name: otel.instrumentation.servlet.capture-request-parameters
+  - name: otel.instrumentation.servlet.experimental.capture-request-parameters
+    declarative_name: java.servlet.capture_request_parameters/development
     description: List of request parameter names to capture as span attributes.
     type: list
     default: ""
diff --git a/instrumentation/liberty/liberty-dispatcher-20.0/metadata.yaml b/instrumentation/liberty/liberty-dispatcher-20.0/metadata.yaml
@@ -8,20 +8,24 @@ semantic_conventions:
   - HTTP_SERVER_METRICS
 configurations:
   - name: otel.instrumentation.http.known-methods
+    declarative_name: java.common.http.known_methods
     description: >
       Configures the instrumentation to recognize an alternative set of HTTP request methods. All
       other methods will be treated as `_OTHER`.
     type: list
     default: "CONNECT,DELETE,GET,HEAD,OPTIONS,PATCH,POST,PUT,TRACE"
   - name: otel.instrumentation.http.server.capture-request-headers
+    declarative_name: general.http.server.request_captured_headers
     description: List of HTTP request headers to capture in HTTP server telemetry.
     type: list
     default: ""
   - name: otel.instrumentation.http.server.capture-response-headers
+    declarative_name: general.http.server.response_captured_headers
     description: List of HTTP response headers to capture in HTTP server telemetry.
     type: list
     default: ""
   - name: otel.instrumentation.http.server.emit-experimental-telemetry
+    declarative_name: java.common.http.server.emit_experimental_telemetry/development
     description: >
       Enable the capture of experimental HTTP server telemetry. Adds the `http.request.body.size`
       and `http.response.body.size` attributes to spans, and records `http.server.request.size` and
diff --git a/instrumentation/tomcat/tomcat-7.0/metadata.yaml b/instrumentation/tomcat/tomcat-7.0/metadata.yaml
@@ -8,31 +8,37 @@ features:
   - HTTP_ROUTE
 configurations:
   - name: otel.instrumentation.http.known-methods
+    declarative_name: java.common.http.known_methods
     description: >
       Configures the instrumentation to recognize an alternative set of HTTP request methods. All
       other methods will be treated as `_OTHER`.
     type: list
     default: "CONNECT,DELETE,GET,HEAD,OPTIONS,PATCH,POST,PUT,TRACE"
   - name: otel.instrumentation.http.server.capture-request-headers
+    declarative_name: general.http.server.request_captured_headers
     description: List of HTTP request headers to capture in HTTP server telemetry.
     type: list
     default: ""
   - name: otel.instrumentation.http.server.capture-response-headers
+    declarative_name: general.http.server.response_captured_headers
     description: List of HTTP response headers to capture in HTTP server telemetry.
     type: list
     default: ""
   - name: otel.instrumentation.http.server.emit-experimental-telemetry
+    declarative_name: java.common.http.server.emit_experimental_telemetry/development
     description: >
       Enable the capture of experimental HTTP server telemetry. Adds the `http.request.body.size`
       and `http.response.body.size` attributes to spans, and records `http.server.request.body.size`
       and `http.server.response.body.size` metrics.
     type: boolean
     default: false
   - name: otel.instrumentation.servlet.experimental-span-attributes
+    declarative_name: java.servlet.experimental_span_attributes/development
     description: Enables capturing the experimental `servlet.timeout` span attribute.
     type: boolean
     default: false
-  - name: otel.instrumentation.servlet.capture-request-parameters
+  - name: otel.instrumentation.servlet.experimental.capture-request-parameters
+    declarative_name: java.servlet.capture_request_parameters/development
     description: List of request parameter names to capture as span attributes.
     type: list
     default: ""
