Skip to content

Commit 2321594

Browse files
authored
chore: update AGENTS.md for improved types and assertions (#7209)
1 parent e48b161 commit 2321594

1 file changed

Lines changed: 69 additions & 11 deletions

File tree

AGENTS.md

Lines changed: 69 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
dd-trace is the Datadog client library for Node.js.
1818

1919
**Key Directories:**
20+
2021
- `packages/dd-trace/` - Main library (APM, profiling, debugger, appsec, llmobs, CI visibility, etc)
2122
- `packages/datadog-core/` - Async context storage, shared utilities
2223
- `packages/datadog-instrumentations/` - Instrumentation implementations
@@ -27,6 +28,7 @@ dd-trace is the Datadog client library for Node.js.
2728
**Package Structure:**
2829

2930
Each package under `packages/` follows a consistent structure:
31+
3032
- `src/` - Source code for the package
3133
- `test/` - Unit tests for the package
3234
- Unit test files always follow the `*.spec.js` naming convention
@@ -36,38 +38,56 @@ Each package under `packages/` follows a consistent structure:
3638

3739
### Running Individual Tests
3840

39-
**IMPORTANT**: Never run `yarn test` directly. Use `mocha` directly on test files.
41+
**IMPORTANT**: Never run the root `npm test`. Run specific related test files directly or run targeted related `npm run test:<area>` scripts.
42+
43+
**Unit tests:**
4044

41-
**Mocha unit tests:**
4245
```bash
4346
./node_modules/.bin/mocha -r "packages/dd-trace/test/setup/mocha.js" path/to/test.spec.js
4447
```
4548

4649
**Integration tests:**
50+
4751
```bash
4852
./node_modules/.bin/mocha --timeout 60000 -r "packages/dd-trace/test/setup/core.js" path/to/test.spec.js
4953
```
5054

55+
**If a test expects “spec file is entrypoint” semantics (tap-like):**
56+
57+
```bash
58+
node scripts/mocha-run-file.js path/to/test.spec.js
59+
```
60+
61+
You can inject mocha options via `MOCHA_RUN_FILE_CONFIG` (JSON), including `require` hooks.
62+
5163
**Target specific tests:**
64+
5265
- Add `--grep "test name pattern"` flag
5366

5467
**Enable debug logging:**
68+
5569
- Prefix with `DD_TRACE_DEBUG=true`
5670

5771
### Plugin Tests
5872

5973
**Use `PLUGINS` env var:**
74+
6075
```bash
61-
PLUGINS="amqplib" yarn test:plugins
62-
PLUGINS="amqplib|bluebird" yarn test:plugins # pipe-delimited for multiple
76+
PLUGINS="amqplib" npm run test:plugins
77+
PLUGINS="amqplib|bluebird" npm run test:plugins # pipe-delimited for multiple
6378
./node_modules/.bin/mocha -r "packages/dd-trace/test/setup/mocha.js" packages/datadog-plugin-amqplib/test/index.spec.js
6479
```
6580

81+
**Narrow within plugin tests (optional):**
82+
83+
- Use `SPEC` to filter which `*.spec.js` files run within the selected plugins.
84+
6685
**With external services** (check `.github/workflows/apm-integrations.yml` for `SERVICES`):
86+
6787
```bash
6888
export SERVICES="rabbitmq" PLUGINS="amqplib"
6989
docker compose up -d $SERVICES
70-
yarn services && yarn test:plugins
90+
yarn services && npm run test:plugins
7191
```
7292

7393
**ARM64 incompatible:** `aerospike`, `couchbase`, `grpc`, `oracledb`
@@ -81,6 +101,7 @@ yarn services && yarn test:plugins
81101
```
82102

83103
**Philosophy:**
104+
84105
- Integration tests (running in sandboxes) don't count towards nyc coverage metrics
85106
- Don't add redundant unit tests solely to improve coverage numbers
86107
- Focus on covering important production code paths with whichever test type makes sense
@@ -98,23 +119,35 @@ assert.equal(actual, expected)
98119
assertObjectContains(response, { status: 200, body: { user: { name: 'Alice' } } })
99120
```
100121

122+
Favor fewer `assert.deepStrictEqual`/`assertObjectContains` calls over many `assert.strictEqual` calls. Combine with existing `assert.strictEqual` calls, if possible.
123+
124+
Never use the `doesNotThrow()` assertion. Instead, execute the method directly.
125+
101126
### Time-Based Testing
102127

103128
**Never rely on actual time passing in unit tests.** Use sinon's fake timers to mock time and make tests deterministic and fast.
104129

105130
## Code Style & Linting
106131

107132
### Linting & Naming
108-
- Lint: `yarn lint` / `yarn lint:fix`
133+
134+
- Lint: `npm run lint` / `npm run lint:fix`
109135
- Files: kebab-case
110136

111137
### JSDoc
138+
112139
- Use TypeScript-compatible syntax (`@param {string}`, `@returns {Promise<void>}`, `@typedef`)
113140
- Never use `any` (be specific or use `unknown` if type is truly unknown)
141+
- Write the most specific types possible by reading the overall context
142+
- Always define types for method arguments as method params
143+
- Never define argument types inside of a method
144+
- Only define types inside of a method, if it can not be inferred otherwise
145+
- Only rewrite code for better types in case it was explicitly requested by the user
114146

115147
### Import Ordering
116148

117149
Separate groups with empty line, sort alphabetically within each:
150+
118151
1. Node.js core modules (with `node:` prefix)
119152
2. Third-party modules
120153
3. Internal imports (by path proximity, then alpha)
@@ -134,8 +167,11 @@ const log = require('../log')
134167
### ECMAScript and Node.js API Standards
135168

136169
**Target Node.js 18.0.0 compatibility:**
170+
137171
- Use modern JS features supported by Node.js (e.g., optional chaining `?.`, nullish coalescing `??`)
172+
- Use `undefined` over `null`, if not required otherwise
138173
- Guard newer APIs with version checks using [`version.js`](./version.js):
174+
139175
```js
140176
const { NODE_MAJOR } = require('./version')
141177
if (NODE_MAJOR >= 20) { /* Use Node.js 20+ API */ }
@@ -145,12 +181,18 @@ const log = require('../log')
145181

146182
**CRITICAL: Tracer runs in application hot paths - every operation counts.**
147183

184+
- Use fast paths to skip unnecessary steps
185+
- Use most performant APIs
186+
- Understand the use case to write ideal CPU and memory performant code
187+
148188
**Async/Await:**
189+
149190
- Do NOT use `async/await` or promises in production code (npm package)
150191
- Allowed ONLY in: test files, worker threads (e.g., `packages/dd-trace/src/debugger/devtools_client/`)
151192
- Use callbacks or synchronous patterns instead
152193

153194
**Memory:**
195+
154196
- Minimize allocations in frequently-called paths
155197
- Avoid unnecessary objects, closures, arrays
156198
- Reuse objects and buffers
@@ -159,16 +201,19 @@ const log = require('../log')
159201
#### Array Iteration
160202

161203
**Prefer `for-of`, `for`, `while` loops over functional methods (`map()`, `forEach()`, `filter()`):**
204+
162205
- Avoid `items.forEach(item => process(item))` → use `for (const item of items) { process(item) }`
163206
- Avoid chaining `items.filter(...).map(...)` → use single loop with conditional push
164207
- Functional methods create closures and intermediate arrays
165208

166209
**Functional methods acceptable in:**
210+
167211
- Test files
168212
- Non-hot-path code where readability benefits
169213
- One-time initialization code
170214

171215
**Loop selection:**
216+
172217
- `for-of` - Simple iteration
173218
- `for` with index - Need index or better performance in hot paths
174219
- `while` - Custom iteration logic
@@ -195,29 +240,34 @@ Avoid try/catch in hot paths - validate inputs early
195240
## Development Workflow
196241

197242
### Core Principles
243+
198244
- **Search first**: Check for existing utilities/patterns before creating new code
199245
- **Small PRs**: Break large efforts into incremental, reviewable changes
200246
- **Descriptive code**: Self-documenting with verbs in function names; comment when needed
201247
- **Readable formatting**: Empty lines for grouping, split complex objects, extract variables
202248
- **Avoid large refactors**: Iterative changes, gradual pattern introduction
203-
- **Test changes**: Test logic (not mocks), failure cases, edge cases - always update tests
249+
- **Test changes**: Test logic (not mocks), failure cases, edge cases - always update tests. Write blackbox tests instead of testing internal exports directly
204250

205251
### Implementation and Testing Workflow
206252

207253
**When making any code or type change, the following MUST be followed:**
208254

209255
1. **Understand** - Read relevant code and tests to understand the current implementation
210-
2. **Implement** - Make the necessary code changes
211-
3. **Update Tests** - Modify or add tests to cover the changes
212-
4. **Run Tests** - Execute the relevant test files to verify everything works
213-
5. **Verify** - Confirm all tests pass before marking the task as complete
256+
2. **Optimize** - Identify the cleanest architectural approach to solve the request
257+
3. **Ask** - Make a proposal with the two best solutions to the user and let them choose. Explain trade-offs
258+
4. **Implement** - Make the necessary code changes
259+
5. **Update Tests** - Modify or add tests to cover the changes
260+
6. **Run Tests** - Execute the relevant test files to verify everything works
261+
7. **Verify** - Confirm all tests pass before marking the task as complete
214262

215263
### Always Consider Backportability
216264

217265
**We always backport `master` to older versions.**
266+
218267
- Keep breaking changes to a minimum
219268
- Don't use language/runtime features that are too new
220269
- **Guard breaking changes with version checks** using [`version.js`](./version.js):
270+
221271
```js
222272
const { DD_MAJOR } = require('./version')
223273
if (DD_MAJOR >= 6) {
@@ -239,6 +289,10 @@ Avoid try/catch in hot paths - validate inputs early
239289

240290
**Naming Convention:** Size/time-based config options should have unit suffixes (e.g., `timeoutMs`, `maxBytes`, `intervalSeconds`).
241291

292+
## Upstream changes
293+
294+
In case an issue is actually happening outside of dd-trace, suggest to fix it upstream instead of creating a work-around.
295+
242296
## Adding New Instrumentation
243297

244298
**New instrumentations go in `packages/datadog-instrumentations/`.** The instrumentation system uses diagnostic channels for communication.
@@ -248,21 +302,25 @@ Many integrations have corresponding plugins in `packages/datadog-plugin-*/` tha
248302
### What Are Plugins?
249303

250304
Plugins are modular code components in `packages/datadog-plugin-*/` directories that:
305+
251306
- Subscribe to diagnostic channels to receive instrumentation events
252307
- Handle APM tracing logic (spans, metadata, error tracking)
253308
- Manage feature-specific logic (e.g., code origin tracking, LLM observability)
254309

255310
**Plugin Base Classes:**
311+
256312
- **`Plugin`** - Base class with diagnostic channel subscription, storage binding, enable/disable lifecycle. Use for non-tracing functionality.
257313
- **`TracingPlugin`** - Extends `Plugin` with APM tracing helpers (`startSpan()`, automatic trace events, `activeSpan` getter). Use for plugins creating trace spans.
258314
- **`CompositePlugin`** - Extends `Plugin` to compose multiple sub-plugins. Use when one integration needs multiple feature plugins (e.g., `express` combines tracing and code origin plugins).
259315

260316
**Plugin Loading:**
317+
261318
- Plugins load lazily when application `require()`s the corresponding library
262319
- Disable with `DD_TRACE_DISABLED_PLUGINS` or `DD_TRACE_<PLUGIN>_ENABLED=false`
263320
- Test framework plugins only load when Test Optimization mode (`isCiVisibility`) is enabled
264321

265322
**When to Create a New Plugin:**
323+
266324
1. Adding support for a new third-party library/framework
267325
2. Adding a new product feature that integrates with existing libraries (use `CompositePlugin`)
268326

0 commit comments

Comments
 (0)