-
Notifications
You must be signed in to change notification settings - Fork 13
docs: Update AGENTS.md #20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- summarize repo structure, workflows, coding patterns\n- highlight quality standards and critical rules for agents\n- add common tasks and reference materials
Completed Working on "Auto PR Description"✅ Workflow completed successfully. |
Completed Working on "Code Review"✅ Workflow completed successfully. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Findings: 0 BLOCKER, 0 CRITICAL, 1 MAJOR, 0 MINOR, 0 SUGGESTION, 0 PRAISE.
Key theme:
- The new AGENTS.md entry for
forRootAsync()documents a factory return shape ({ instrumentation?: Provider<Instrumentation>[] }) that the module does not actually consume; it currently iterates over the injected value directly, so following the doc leads to a runtimeTypeError. The documentation needs to match the implementation (or vice versa).
Next steps:
- Update AGENTS.md to describe the actual factory contract in
OpenTelemetryModule.forRootAsync, or adjust the module to read.instrumentationbefore iterating. - Re-run a sample module using the documented factory to confirm the instructions work end-to-end.
| ]; | ||
| ``` | ||
| `OpenTelemetryModule.forRoot()` registers these providers globally and eagerly calls `setupInstrumentation()` on each (plus `DecoratorInstrumentation`) via the `Constants.SDK_INJECTORS` factory. When supplying `config.instrumentation`, always pass providers extending the `Instrumentation` interface exported from `src/trace/instrumentation/Instrumentation`. | ||
| - **Async bootstrapping**: `forRootAsync()` uses `ModuleRef.create()` to instantiate `DecoratorInstrumentation` and any supplied instrumentation classes lazily; ensure async factories in `OpenTelemetryModuleAsyncOptions` return `{ instrumentation?: Provider<Instrumentation>[] }`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[MAJOR]: AGENTS.md L55-L57 says forRootAsync() factories should return { instrumentation?: Provider<Instrumentation>[] }, but the module currently injects that value and iterates over it directly (see src/open-telemetry.module.ts L100-L111: it treats the injected value itself as the iterable instrumentation list and never dereferences .instrumentation). Following the doc therefore throws TypeError: instrumentation is not iterable whenever the documented object shape is returned. Update this section to describe the shape that actually works today or adjust the code before documenting the object contract.
Suggested fix: Document that the async factory must currently return the instrumentation array itself (or update the module to read .instrumentation before iterating) so readers do not follow a crashing example.
Summary
This PR adds a contributor-focused AGENTS.md with the essential information agents need when working in this repository.
Changes
Why
Keeping AGENTS.md up to date ensures anyone touching the repo can quickly align with current practices, tooling, and release processes.
Summary
This PR introduces a contributor-focused `AGENTS.md` that centralizes the project overview, repository layout, development workflow, and key references for working on `@amplication/opentelemetry-nestjs`. It captures the conventions, quality standards, and critical rules contributors must follow to keep tracing instrumentation, SDK helpers, and release processes aligned.
Changes
Commits
Testing