-
Notifications
You must be signed in to change notification settings - Fork 13
docs: Update AGENTS.md #21
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
Document repository structure, tooling commands, instrumentation patterns, and CI workflows for agents.
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.
Summary:
- Findings: 0 BLOCKER, 0 CRITICAL, 2 MAJOR, 1 MINOR
- Themes: AGENTS.md currently overstates available instrumentation (mentions transports that do not exist), mischaracterizes the releaseBeta workflow/quality gates, and references the wrong TraceWrapper path.
- Next steps: align the instrumentation list with actual source files, accurately describe the beta publishing workflow (dependency install + npm version + npm publish --tag beta, no semantic-release), and point the TraceWrapper guidance to
src/trace/trace-wrapper.ts. These updates will keep the guide trustworthy for future automation agents.
| Welcome! This document gives contributors a quick but thorough rundown of how the NestJS-focused OpenTelemetry integration is organized, built, and validated. | ||
|
|
||
| ## 🧭 Project Overview | ||
| - **Purpose:** Provide a complete OpenTelemetry toolkit tailored to NestJS applications, combining decorators (`@Span`, `@Traceable`), the `TraceService`, instrumentation classes (controllers, guards, interceptors, schedulers, and transports), and SDK helpers for observability across HTTP, microservices, GraphQL, and scheduled jobs. |
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]: > @@ line~6 @@
- Purpose: Provide a complete OpenTelemetry toolkit tailored to NestJS applications, combining decorators (
@Span,@Traceable), theTraceService, instrumentation classes (controllers, guards, interceptors, schedulers, and transports), and SDK helpers for observability across HTTP, microservices, GraphQL, and scheduled jobs.
The guide advertises a transport instrumentation, but src/trace/instrumentation only contains controller/guard/interceptor/pipe/schedule/decorator/event-emitter/graphql-resolver/console-logger implementations, so readers expect components that do not exist; please align the overview with the actual instrumentations.
Suggested fix: Replace the reference to transport instrumentation with the actual instrumentation types that are present (controller, guard, interceptor, pipe, schedule, event-emitter, graphql, decorator, console logger, etc.).
| - **CI/CD Workflows:** | ||
| - `.github/workflows/ci.yml` executes lint → `test:cov` → build on pushes/PRs against Node LTS. | ||
| - `.github/workflows/release.yml` repeats those gates on `main`, then runs `npm ci` and `npm run semantic-release` to publish packages via stored secrets. | ||
| - `.github/workflows/releaseBeta.yml` mirrors the release pipeline for manual beta publishing (triggered via workflow dispatch) and also relies on `npm ci` plus semantic-release prerelease configuration. |
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]: > @@ lines~51-55 @@
.github/workflows/releaseBeta.ymlmirrors the release pipeline for manual beta publishing (triggered via workflow dispatch) and also relies onnpm ciplus semantic-release prerelease configuration.
@@ line~61 @@- Publishing is exclusively handled through GitHub Actions +
semantic-release. Do not publish manually.
The beta workflow (.github/workflows/releaseBeta.yml) actually runs only npm ci, npm version ${{ inputs.version }}, and npm publish --tag beta, so no lint/test/build gates execute and semantic-release is bypassed; update the guide so contributors understand that beta releases are a manual npm publish step without semantic-release automation or CI quality gates.
Suggested fix: Document that releaseBeta installs dependencies, bumps the provided version, and runs npm publish --tag beta (with no lint/test/build/semantic-release), and clarify that this constitutes a manual publish step.
|
|
||
| ## 🧩 Code Patterns | ||
| - **Instrumentation Template:** Extend `BaseTraceInstrumentation` (`src/trace/instrumentation/base-trace.instrumentation.ts`). Override hooks for metadata scanning and ensure each instrumentation registers itself via dependency injection. See `controller.instrumentation.ts` for the standard pattern. | ||
| - **Trace Wrapper:** Always funnel manual instrumentation through `TraceWrapper` (`src/trace/trace-wrapper/trace-wrapper.ts`). It verifies whether a method/class has been wrapped already, preventing double spans. |
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.
[MINOR]: > @@ line~38 @@
- Trace Wrapper: Always funnel manual instrumentation through
TraceWrapper(src/trace/trace-wrapper/trace-wrapper.ts). It verifies whether a method/class has been wrapped already, preventing double spans.
The referenced path includes a nonexistent trace-wrapper directory—the helper lives at src/trace/trace-wrapper.ts—so following the documented path yields a 404; please point to the actual file location so contributors can find it quickly.
Suggested fix: Update the reference to src/trace/trace-wrapper.ts (or mention the spec file separately if desired).
🔄 Overcut automatically updated AGENTS.md with latest repository analysis.