Skip to content

Commit 5b8f19d

Browse files
dougqhdougqh
authored
Merge branch 'master' into dougqh/tagset
2 parents 2f8c503 + 5e68348 commit 5b8f19d

732 files changed

Lines changed: 19838 additions & 13171 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.

‎.agents/skills/apm-integrations/SKILL.md‎

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
---
2+
name: apm-integrations
3+
description: Write a new library instrumentation end-to-end. Use when the user ask to add a new APM integration or a library instrumentation.
4+
context: fork
5+
allowed-tools:
6+
- Bash
7+
- Read
8+
- Write
9+
- Edit
10+
- Glob
11+
- Grep
12+
---
13+
14+
Write a new APM end-to-end integration for dd-trace-java, based on library instrumentations, following all project conventions.
15+
16+
## Step 1 – Read the authoritative docs and sync this skill (mandatory, always first)
17+
18+
Before writing any code, read all three files in full:
19+
20+
1. [`docs/how_instrumentations_work.md`](docs/how_instrumentations_work.md) — full reference (types, methods, advice, helpers, context stores, decorators)
21+
2. [`docs/add_new_instrumentation.md`](docs/add_new_instrumentation.md) — step-by-step walkthrough
22+
3. [`docs/how_to_test.md`](docs/how_to_test.md) — test types and how to run them
23+
24+
These files are the single source of truth. Reference them while implementing.
25+
26+
## Step 2 – Clarify the task
27+
28+
If the user has not already provided all of the following, ask before proceeding:
29+
30+
- **Framework name** and **minimum supported version** (e.g. `okhttp-3.0`)
31+
- **Target class(es) and method(s)** to instrument (fully qualified class names preferred)
32+
- **Target system**: one of `Tracing`, `Profiling`, `AppSec`, `Iast`, `CiVisibility`, `Usm`, `ContextTracking`
33+
- **Whether this is a bootstrap instrumentation** (affects allowed imports)
34+
35+
## Step 3 – Find a reference instrumentation
36+
37+
Search `dd-java-agent/instrumentation/` for a structurally similar integration:
38+
- Same target system
39+
- Comparable type-matching strategy (single type, hierarchy, known types)
40+
41+
Read the reference integration's `InstrumenterModule`, Advice, Decorator, and test files to understand the established
42+
pattern before writing new code. Use it as a template.
43+
44+
## Step 4 – Set up the module
45+
46+
1. Create directory: `dd-java-agent/instrumentation/$framework/$framework-$minVersion/`
47+
2. Under it, create the standard Maven source layout:
48+
- `src/main/java/` — instrumentation code
49+
- `src/test/groovy/` — Groovy/Spock instrumentation tests (see Step 9.1)
50+
3. Create `build.gradle` with:
51+
- `compileOnly` dependencies for the target framework
52+
- `testImplementation` dependencies for tests
53+
- `muzzle { pass { } }` directives (see Step 9.2)
54+
4. Register the new module in `settings.gradle.kts` in **alphabetical order**
55+
5. Register all integration names in `metadata/supported-configurations.json` — **read [Supported Configurations](references/supported-configurations.md)** for the exact key shapes and CI checks involved. Declaring several names (`super("a", "b")`) means one entry each.
56+
57+
**See [Naming Conventions](references/naming-conventions.md) — module directory name must end with a version or an allowed suffix (`-common`, `-stubs`, `-iast`). Java filename and `public class` name MUST match character-for-character including acronym casing (CRITICAL — see § "Java naming consistency").**
58+
59+
## Step 4.1 – Span-creating vs context-tracking instrumentation
60+
61+
Read [Context-Tracking Instrumentation](references/context-tracking.md) and decide whether the library needs `InstrumenterModule.Tracing` (I/O operations that create spans) or `InstrumenterModule.ContextTracking` (async-boundary bridging, no spans).
62+
63+
## Step 5 – Write the InstrumenterModule
64+
65+
**Read [InstrumenterModule Guidance](references/instrumenter-module.md).**
66+
67+
## Step 6 – Write the Decorator
68+
69+
- Extend the most specific available base decorator:
70+
- `HttpClientDecorator`, `DatabaseClientDecorator`, `ServerDecorator`, `MessagingClientDecorator`, etc.
71+
- One `public static final DECORATE` instance
72+
- Define `UTF8BytesString` constants for the component name and operation name
73+
- Keep all tag/naming/error logic here — not in the Advice class
74+
- Override `spanType()`, `component()`, `spanKind()` as appropriate
75+
- Override `instrumentationNames()` to return the primary integration name without a version suffix: `return new String[] {"jedis"};` not `"jedis-3.0"`.
76+
- `BaseDecorator` uses those names to resolve analytics settings (`DD_TRACE_<NAME>_ANALYTICS_ENABLED`, `DD_TRACE_<NAME>_ANALYTICS_SAMPLE_RATE`). Search `metadata/supported-configurations.json` for each returned name — if analytics keys are absent, add them (see [Supported Configurations](references/supported-configurations.md) for the JSON shape).
77+
78+
## Step 7 – Write the Advice class (highest-risk step)
79+
80+
**Read [Writing the Advice Class](references/advice-class.md) — the highest-risk step.** Pay particular attention to: `@Advice.OnMethodEnter/Exit` annotations; `CallDepthThreadLocalMap` reentrancy guarding; span lifecycle order; and the "Must NOT do" list.
81+
82+
## Step 8 – Add SETTER/GETTER adapters (if applicable)
83+
84+
For context propagation to and from upstream services, like HTTP headers,
85+
implement `AgentPropagation.Setter` / `AgentPropagation.Getter` adapters that wrap the framework's specific header API.
86+
Place them in the helpers package, declare them in `helperClassNames()`.
87+
88+
## Step 9 – Write tests
89+
90+
Cover all mandatory test types:
91+
92+
### 1. Instrumentation test (mandatory)
93+
94+
**Read [Writing Tests](references/tests.md).** Instrumentation tests are Groovy/Spock (`src/test/groovy/`) — add the `tag: override groovy enforcement` label to suppress the `Enforce Groovy Migration` CI check (which blocks new `.groovy` files by default — instrumentation tests are intentionally Groovy/Spock). Must cover error/exception scenarios. When adding new integration names, register them per [Supported Configurations](references/supported-configurations.md). When `compileOnly` and `testImplementation` use different versions, comment the specific class that requires the higher version. Include sibling version modules as `testImplementation` dependencies for mutual-exclusion tests.
95+
96+
### 2. Muzzle directives (mandatory)
97+
98+
**Read [Muzzle Directives](references/muzzle.md)** — it covers all three valid patterns and their `assertInverse` rules. Search adjacent module `build.gradle` files for `skipVersions` before declaring a new version-bounded module's muzzle directives.
99+
100+
### 3. Latest dependency test (mandatory)
101+
102+
Use `latestDepTestImplementation` in `build.gradle` to pin the latest available version. Run with:
103+
```bash
104+
./gradlew :dd-java-agent:instrumentation:$framework:$framework-$version:latestDepTest
105+
```
106+
107+
**`latestDepTestImplementation` version range must match the instrumented range.** If your module instruments version `2.x`, use `2.+` as the version constraint, not `3.+`:
108+
109+
```groovy
110+
// WRONG — latestDep tests against 3.x but the module only instruments 2.x
111+
latestDepTestImplementation group: 'commons-httpclient', name: 'commons-httpclient', version: '3.+'
112+
113+
// CORRECT — latestDep tests against the highest 2.x release
114+
latestDepTestImplementation group: 'commons-httpclient', name: 'commons-httpclient', version: '2.+'
115+
```
116+
117+
Using `3.+` for a `2.x` instrumentation means `latestDepTest` runs against an incompatible API version and will either fail or silently test nothing.
118+
119+
### 4. Smoke test (optional)
120+
121+
Add a smoke test in `dd-smoke-tests/` only if the framework warrants a full end-to-end demo-app test.
122+
123+
## Step 10 – Build and verify
124+
125+
Run these commands in order and fix any failures before proceeding:
126+
127+
```bash
128+
./gradlew :dd-java-agent:instrumentation:$framework:$framework-$version:muzzle
129+
./gradlew :dd-java-agent:instrumentation:$framework:$framework-$version:test
130+
./gradlew :dd-java-agent:instrumentation:$framework:$framework-$version:latestDepTest
131+
./gradlew checkInstrumenterModuleConfigurations
132+
./gradlew checkDecoratorAnalyticsConfigurations
133+
./gradlew spotlessApply
134+
./gradlew :dd-java-agent:updateAgentJarIntegrationsGoldenFile
135+
```
136+
137+
After `updateAgentJarIntegrationsGoldenFile` runs, commit the updated `metadata/agent-jar-checks.properties` file alongside your instrumentation changes. The `verifyAgentJarIntegrations` check runs automatically in CI and fails if this file is out of date.
138+
139+
**If muzzle fails:**
140+
- Missing helper class names in `helperClassNames()` — the most common cause; add any missing inner, anonymous, or enum synthetic classes.
141+
- Wrong version range — the declared `versions` in `build.gradle` doesn't cover the versions actually used by tests; adjust the bounds.
142+
- API mismatch — the instrumented class or method doesn't exist in the declared version; check the library's changelog and narrow the `compileOnly` version or the muzzle range.
143+
144+
**If `checkInstrumenterModuleConfigurations` fails:** an integration name from `super(...)` is missing
145+
(or mismatched) in `metadata/supported-configurations.json` — see [Supported Configurations](references/supported-configurations.md).
146+
147+
**If `checkDecoratorAnalyticsConfigurations` fails:** a name returned by the decorator's `instrumentationNames()` is missing `DD_TRACE_<NAME>_ANALYTICS_ENABLED` / `DD_TRACE_<NAME>_ANALYTICS_SAMPLE_RATE` entries in `metadata/supported-configurations.json` — add them per [Supported Configurations](references/supported-configurations.md).
148+
149+
**If tests fail:** verify span lifecycle order (start → activate → error → close → finish), helper registration,
150+
and `contextStore()` map entries match actual usage.
151+
152+
## Step 11 – Checklist before finishing
153+
154+
Output this checklist and confirm each item is satisfied:
155+
156+
- [ ] `settings.gradle.kts` entry added in alphabetical order
157+
- [ ] `metadata/supported-configurations.json` has a `DD_TRACE_<NAME>_ENABLED` entry (+ the two aliases) for every name passed to `super(...)`
158+
- [ ] `metadata/supported-configurations.json` has `DD_TRACE_<NAME>_ANALYTICS_ENABLED` and `DD_TRACE_<NAME>_ANALYTICS_SAMPLE_RATE` entries for every name returned by the decorator's `instrumentationNames()`
159+
- [ ] `build.gradle` has `compileOnly` deps and `muzzle` directives
160+
- [ ] Muzzle pattern is correct (see [Muzzle Directives](references/muzzle.md))
161+
- [ ] `latestDepTestImplementation` version range matches the instrumented version range (e.g. `2+` not `3+` for a `2.x` module)
162+
- [ ] `@AutoService(InstrumenterModule.class)` annotation present on the module class
163+
- [ ] `helperClassNames()` lists ALL referenced helpers (including inner, anonymous, and enum synthetic classes)
164+
- [ ] Advice methods are `static` with `@Advice.OnMethodEnter` / `@Advice.OnMethodExit` annotations
165+
- [ ] `@Advice.OnMethodEnter(suppress = Throwable.class)` on enter; `@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)` on exit (omit `onThrowable` when hooking a constructor)
166+
- [ ] No static constants holding return values of one-shot instrumenter methods (`triggerClasses()`, `contextStore()`, etc.)
167+
- [ ] No logger field in the Advice class or InstrumenterModule class
168+
- [ ] No `inline=false` left in production code
169+
- [ ] No `java.util.logging.*` / `java.nio.file.*` / `javax.management.*` in bootstrap path
170+
- [ ] `metadata/agent-jar-checks.properties` updated via `./gradlew :dd-java-agent:updateAgentJarIntegrationsGoldenFile` and committed
171+
- [ ] Span lifecycle order is correct: startSpan → afterStart → activateSpan (enter); onError → beforeFinish → close → finish (exit)
172+
- [ ] All Step 10 verification commands passed with no failures
173+
174+
## Step 12 – Retrospective: update this skill with what was learned
175+
176+
After the instrumentation is complete (or abandoned), review the full session and improve this skill for future use.
177+
178+
**Collect lessons from four sources:**
179+
180+
1. **Build/test failures** — did any Gradle task fail with an error that this skill did not anticipate or gave wrong
181+
guidance for? (e.g. a muzzle failure that wasn't caused by missing helpers, a test pattern that didn't work)
182+
2. **Docs vs. skill gaps** — did Step 1's sync miss anything? Did you consult the docs for something not captured here?
183+
3. **Reference instrumentation insights** — did the reference integration use a pattern, API, or convention not
184+
reflected in any step of this skill?
185+
4. **User corrections** — did the user correct an output, override a decision, or point out a mistake?
186+
187+
**For each lesson identified**, edit this file (`.agents/skills/apm-integrations/SKILL.md`) or its referenced files
188+
using the `Edit` tool:
189+
- Wrong rule → fix it in place
190+
- Missing rule → add it to the most relevant step
191+
- Wrong failure guidance → update the relevant "If X fails" section in Step 10
192+
- Misleading or obsolete content → remove it
193+
194+
Keep each change minimal and targeted. Do not rewrite sections that worked correctly.
195+
After editing, confirm to the user which improvements were made to the skill.

‎.agents/skills/apm-integrations/references/advice-class.md‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
# Writing the Advice Class
2+
3+
> Referenced from `SKILL.md` Step 7. The highest-risk step — every rule in this file exists because someone's PR broke on it.
4+
5+
## Must do
6+
7+
- Advice methods **must** be `static`
8+
- Annotate enter: `@Advice.OnMethodEnter(suppress = Throwable.class)`
9+
- Annotate exit: `@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)`
10+
- **Exception**: do NOT use `onThrowable` when hooking a constructor — if the constructor throws, `@Advice.This` is a partially initialized object
11+
- Use `@Advice.Local("...")` for values shared between enter and exit (span, scope)
12+
- Use the correct parameter annotations:
13+
- `@Advice.This` — the receiver object
14+
- `@Advice.Argument(N)` — a method argument by index
15+
- `@Advice.Return` — the return value (exit only)
16+
- `@Advice.Thrown` — the thrown exception (exit only)
17+
- `@Advice.Enter` — the return value of the enter method (exit only)
18+
- Use `CallDepthThreadLocalMap` to guard against recursive instrumentation of the same method
19+
- **Instrument the single delegate method, not all overloads**: when a library has multiple overloads of the same operation (e.g. `executeMethod(String)`, `executeMethod(HostConfig)`, `executeMethod(HostConfig, HttpMethod)`), check if they all delegate to a single internal method. If yes, instrument ONLY the delegate — not each overload. Instrumenting all overloads without a proper reentrancy guard creates **duplicate spans per request** (one per overload in the call chain) and injects context propagation headers multiple times. Use `CallDepthThreadLocalMap` when you must instrument at a higher level.
20+
21+
## Span lifecycle (in order)
22+
23+
Enter method:
24+
1. `AgentSpan span = startSpan(DECORATE.operationName(), ...)`
25+
2. `DECORATE.afterStart(span)` + set domain-specific tags
26+
3. `AgentScope scope = activateSpan(span)` — return or store via `@Advice.Local`
27+
28+
Exit method:
29+
4. `DECORATE.onError(span, throwable)` — only if throwable is non-null
30+
5. `DECORATE.beforeFinish(span)`
31+
6. `scope.close()`
32+
7. `span.finish()`
33+
34+
### onExit handling when the target method throws
35+
36+
The `onThrowable = Throwable.class` attribute on `@Advice.OnMethodExit` controls whether the exit advice fires when the **instrumented target method** throws. You **must** set it explicitly to `Throwable.class` for any exit advice that closes a scope or finishes a span — the default skips exceptional termination, which leaks active scopes when the instrumented method throws.
37+
38+
```java
39+
// Standard pattern — exit fires whether the target method returned or threw
40+
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
41+
public static void exit(
42+
@Advice.Enter final AgentScope scope,
43+
@Advice.Thrown final Throwable thrown) {
44+
if (scope != null) {
45+
AgentSpan span = scope.span();
46+
DECORATE.onError(span, thrown);
47+
DECORATE.beforeFinish(span);
48+
scope.close();
49+
span.finish();
50+
}
51+
}
52+
```
53+
54+
**`onThrowable` does NOT compensate for `onEnter` throwing.** Per `docs/how_instrumentations_work.md`: "If the `Advice.OnMethodEnter` method throws an exception, the `Advice.OnMethodExit` method is not invoked" — this is unconditional. To keep `onEnter` from throwing in the first place, use `suppress = Throwable.class` on the enter advice.
55+
56+
When using `CallDepthThreadLocalMap`, only the outermost call (the one where `incrementCallDepth` returned 0) should reset the counter. Recursive inner calls that returned early on enter must also return early on exit without resetting — otherwise an inner exit clears the counter while the outer call is still active, allowing subsequent nested calls to create duplicate spans. The exit guard must mirror the enter guard exactly.
57+
58+
### Specify charset explicitly when converting byte[] to String
59+
60+
```java
61+
// WRONG — uses platform default charset
62+
String cmd = new String(commandBytes);
63+
64+
// CORRECT — explicit charset
65+
import java.nio.charset.StandardCharsets;
66+
String cmd = new String(commandBytes, StandardCharsets.UTF_8);
67+
```
68+
69+
### Do NOT catch `NullPointerException`; use null-check guards instead
70+
71+
Catching `NullPointerException` is always a sign of an unguarded precondition — fix the root cause with an explicit null check instead. dd-trace-java enforces this via SpotBugs rule `DCN_NULLPOINTER_EXCEPTION`; violations fail `:spotbugsMain` and block the PR.
72+
73+
```java
74+
// WRONG — SpotBugs DCN_NULLPOINTER_EXCEPTION
75+
@Override
76+
protected int status(final HttpMethod httpMethod) {
77+
try {
78+
return httpMethod.getStatusCode();
79+
} catch (NullPointerException e) {
80+
// getStatusCode() throws NPE when statusLine is null
81+
return 0;
82+
}
83+
}
84+
85+
// CORRECT — null-check guard (this is the canonical master pattern)
86+
@Override
87+
protected int status(final HttpMethod httpMethod) {
88+
final StatusLine statusLine = httpMethod.getStatusLine();
89+
return statusLine == null ? 0 : statusLine.getStatusCode();
90+
}
91+
```
92+
93+
**How to discover**: when implementing a method that calls library code which may NPE on null internal state, READ the master module's analogous method for the canonical null-check pattern. The master typically exposes the nullable intermediate (e.g. `getStatusLine()`) so you can guard it.
94+
95+
## Multiple advice classes and `@AppliesOn`
96+
97+
If your instrumentation needs to apply multiple advices to the same method (e.g. separate context-tracking from tracing logic), use `applyAdvices()` inside `methodAdvice()`. Use the `@AppliesOn` annotation to control which target systems each advice applies to.
98+
99+
See the `@AppliesOn Annotation` section of `docs/how_instrumentations_work.md` for the full API and examples.
100+
101+
## Must NOT do
102+
103+
- **No logger fields** in the Advice class or the Instrumentation class (loggers only in helpers/decorators)
104+
- **No code in the Advice constructor** — it is never called
105+
- **Do not use lambdas in advice methods** — they create synthetic classes that will be missing from helper declarations
106+
- **No references** to other methods in the same Advice class or in the InstrumenterModule class
107+
- **No `InstrumentationContext.get()`** outside of Advice code
108+
- **No `inline=false`** in production code (only for debugging; must be removed before committing)
109+
- **No `java.util.logging.*`, `java.nio.file.*`, or `javax.management.*`** in bootstrap instrumentations

0 commit comments

Comments
 (0)