Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 130 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
# AGENTS.md

Guidance for coding agents working in this repository. Read README.md and
CONTRIBUTING.md as well: README.md documents the public API and its behavior,
and CONTRIBUTING.md has the rules for contributing.

## What this is

microtel is a small implementation of parts of OpenTelemetry for Node.js. It
traces `http` and `https` clients and servers, HTTP/2 servers, and undici and
`fetch()` through `diagnostics_channel`, propagates W3C trace context and
baggage, and exports spans as OTLP/HTTP JSON. It's meant to be included in
Node.js core as a dependency, so it's written the way Node.js core code is.

- It's CommonJS, with `'use strict'`, for Node.js 26.9 or newer (the `engines`
field in package.json).
- It has no runtime dependencies. The dev dependencies only build, test, and
format it.
- All configuration goes through `init(options)`. There is no configuration
through environment variables.
- The public API is `init()`, `isActive()`, and `flush()`, from index.js.

## Layout

| Path | What it holds |
|---|---|
| `index.js` | The public API. |
| `lib/core.js` | `init()` option validation and the state it sets. |
| `lib/instrumentations.js` | The `diagnostics_channel` subscribers that create spans and propagate context. |
| `lib/span.js` | The `Span` class, trace flags, and `traceparent` parsing and formatting. |
| `lib/sampler.js` | The root sampling decision for `sampleRatio`. |
| `lib/id.js` | Trace and span ID generation. |
| `lib/flush.js` | The span buffer, OTLP JSON serialization, the export, and `flush()`. |
| `lib/guard.js` | `guard()`, which keeps a bug in microtel from breaking the app it traces. |
| `lib/validate.js` | Ports of Node.js core's argument validators and error helpers. |
| `scripts/` | The bundle build and the benchmark comparison that CI runs. |
| `test/` | The tests (`test-*.js`), shared helpers in `test/common/`, and child process fixtures in `test/fixtures/`. |
| `test/collector/` | A smoke test against a real OpenTelemetry Collector, which runs in its own CI job. |
| `benchmark/` | The `node:bench` benchmarks. |
| `dist/microtel.js` | The generated CommonJS bundle from `npm run build`. It's ignored, and `test/test-bundle.js` loads and tests it. |
Comment on lines +23 to +40

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would shorten this down to just explaining the main folders. Once you've established that, it's often more effective to just have them grep files, or just navigate the files via following exports/imports.

Suggested change
## Layout
| Path | What it holds |
|---|---|
| `index.js` | The public API. |
| `lib/core.js` | `init()` option validation and the state it sets. |
| `lib/instrumentations.js` | The `diagnostics_channel` subscribers that create spans and propagate context. |
| `lib/span.js` | The `Span` class, trace flags, and `traceparent` parsing and formatting. |
| `lib/sampler.js` | The root sampling decision for `sampleRatio`. |
| `lib/id.js` | Trace and span ID generation. |
| `lib/flush.js` | The span buffer, OTLP JSON serialization, the export, and `flush()`. |
| `lib/guard.js` | `guard()`, which keeps a bug in microtel from breaking the app it traces. |
| `lib/validate.js` | Ports of Node.js core's argument validators and error helpers. |
| `scripts/` | The bundle build and the benchmark comparison that CI runs. |
| `test/` | The tests (`test-*.js`), shared helpers in `test/common/`, and child process fixtures in `test/fixtures/`. |
| `test/collector/` | A smoke test against a real OpenTelemetry Collector, which runs in its own CI job. |
| `benchmark/` | The `node:bench` benchmarks. |
| `dist/microtel.js` | The generated CommonJS bundle from `npm run build`. It's ignored, and `test/test-bundle.js` loads and tests it. |
## Layout
| Path | What it holds |
|---|---|
| `index.js` | The public API. |
| `lib/` | The implementation of microtel. |
| `scripts/` | The bundle build and the benchmark comparison that CI runs. |
| `test/` | The tests (`test-*.js`), shared helpers in `test/common/`, and child process fixtures in `test/fixtures/`. |
| `benchmark/` | The `node:bench` benchmarks. |
| `dist/microtel.js` | The generated CommonJS bundle from `npm run build`. It's ignored, and `test/test-bundle.js` loads and tests it. |

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, I always don't like agent generated document that over-documents the details, and making any change would require additional document tailoring.


## Commands

```sh
npm ci
npm run build # writes dist/microtel.js
npm test # builds the bundle, then runs node --test 'test/test-*.js'
npm run format:check # prettier --check .
npx prettier --write <files>
npm run bench # node --experimental-bench --bench 'benchmark/*.bench.js'
```

The collector smoke test needs Docker. The header of
`test/collector/test-collector.js` says how to run it locally.

CI runs separate `build`, `test`, `prettier`, and `collector` jobs, and the
benchmark workflow runs the benchmarks on every push and compares them against
the base branch on pull requests.
Comment on lines +56 to +58

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should just say, "read the github action workflow to see what CI runs". This saves us context window, but also makes it easier for the agent to find exactly what they need to understand the problem.


## Code conventions

- Write it the way Node.js core would. An error for a bad argument uses core's
error code and message, through the helpers in `lib/validate.js`.
- A validator ported from core goes into `lib/validate.js` and into the parity
battery in `test/common/validate-battery.js`, which
`test/test-validate-parity.js` runs against core's own validators under
`--expose-internals`.
- `init()` validates every option before it changes any state, and it copies
any object it keeps, so a caller changing that object later has no effect.
Comment on lines +68 to +69

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is "code conventions". This belongs to it's own code document.

- Nothing microtel hands to Node.js may throw into the app. That covers
`diagnostics_channel` subscribers, event listeners, and timers. Wrap each one
with `guard()` from `lib/guard.js`, once rather than on every call.
- Every export settles, on every path, by its `exportTimeout` deadline at the
latest. `flush()` never rejects for a failed export. Failures are reported as
throttled `OTelExportWarning` process warnings instead.
Comment on lines +73 to +75

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto, this should be in the code inline doc rather than in the agents.md

- The traced request path is hot. Avoid allocations and closures there, and
measure any change to it (see Performance below).
- Prettier formats everything except Markdown and the built bundle, with
single quotes. Match the style of the code around a change.
- Use comments for non-obvious decisions and constraints. A TODO names its
author, as `TODO(handle):`, and says everything needed to act on it, with no
links to pull requests.
- Don't name specific observability vendors in code, docs, commit messages, or
pull request descriptions. Say "an OTLP backend", and use example.com in
examples.

## OpenTelemetry behavior

Follow the OpenTelemetry specification and semantic conventions, and match what

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should say if agents have confusion over the spec to look it up directly (link to it). Otherwise we risk them hallucinating the spec from memory.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. For behavior matching the spec, the code should be companioned with a link to the spec.

the official SDKs and instrumentations do. Where microtel differs on purpose,
say so in the pull request, and in README.md if users can see it.

## Tests

- Tracing activates once per process. Tests that activate tracing in-process
normally call `startTracing()` once in `before()` and share its collector.
Use `spawnMicrotel()` for another activation in the same test file. Tracing
fixtures usually receive JSON init options in `process.argv[2]`.
- `spawnMicrotel()` accepts a `timeout` option and returns `code: null` when
that timeout kills the child. Use it for a child that could block, and close
servers and collectors a test creates.
- A test must not depend on timing or randomness. Wait on events rather than
sleeping.
- Add tests for the change itself, and check that each new test fails when the
behavior it covers is broken.
- Keep tests DRY, with table-driven cases and `assert.partialDeepStrictEqual`
where they fit.

## Performance

For pull requests, CI compares the benchmarks of the change and its base over
five interleaved rounds. It reports the mean of each run's mean rate and the

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5-round was my back-then result pick. It could change and I don't want to update multiple places to change the round number :)

Suggested change
five interleaved rounds. It reports the mean of each run's mean rate and the
interleaved rounds. It reports the mean of each run's mean rate and the

spread, and highlights a change only when every run on one side is faster or
slower than every run on the other.

When measuring by hand, interleave runs of main and the change on the same

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
When measuring by hand, interleave runs of main and the change on the same
When measuring by hand, use ./scripts/compare-benchmarks.js to interleave runs of main and the change on the same

machine, report several runs rather than one, and use the untraced benchmarks
as a control for noise. For small changes, CPU profiles of microtel's own time
are steadier than throughput.

## Pull requests and commits

- Commit subjects use conventional commits, with lowercase text after the
type. Examples in the history include `feat:`, `fix:`, `perf:`, `test:`,
`ci:`, `chore:`, and `docs:`.
- Contributions are made under the Developer's Certificate of Origin in
CONTRIBUTING.md.
- A pull request stays open for at least 24 hours before it merges, unless the

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't duplicate the policies here.

maintainers judge it trivial.
- Update README.md for any change users can see.
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@AGENTS.md
Loading