Conversation
85844ff to
f8166e3
Compare
There was a problem hiding this comment.
Pull request overview
This pull request fixes OpenTelemetry instrumentation for Fastify and improves backend configuration and documentation. The primary change ensures HTTP request tracing works correctly with the Fastify adapter by explicitly enabling the Fastify instrumentation plugin and correcting the metric reader configuration.
Changes:
- Fixed OpenTelemetry configuration to properly instrument Fastify HTTP requests
- Added currency rate upsert behavior for idempotent date-based persistence
- Improved Docker signal handling for graceful shutdown
- Streamlined backend README and enhanced AGENTS.md documentation
Reviewed changes
Copilot reviewed 15 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/src/otel.ts | Enabled Fastify instrumentation explicitly, corrected metricReader to metricReaders, improved shutdown signal handling, and added configurable service name |
| backend/src/config.ts | Added OTEL_SERVICE_NAME environment variable with default value |
| backend/src/frameworks/relational-data-service/postgres/repositories/currency-rate.repository.ts | Added upsert logic using orUpdate to make currency rate saves idempotent by date |
| backend/src/frameworks/relational-data-service/postgres/repositories/tests/currency-rate.repository.spec.ts | Added test to verify upsert behavior for currency rates |
| backend/src/frameworks/relational-data-service/postgres/repositories/tests/snapshots/currency-rate.repository.spec.ts.snap | Updated snapshot to reflect ON CONFLICT clause in insert query |
| backend/Dockerfile | Changed CMD to use exec form with proper signal propagation, updated npm flags to modern syntax |
| backend/.dockerignore | Added more patterns to exclude from Docker build context |
| backend/AGENTS.md | Added OpenTelemetry and config file locations, documented OTEL_SERVICE_NAME, added local Postgres testing command |
| backend/README.md | Simplified to brief overview pointing to AGENTS.md for details |
| backend/example.env | Line renumbering only (contains pre-existing syntax error) |
| backend/src/api/http/filters/validation-exception.filter.ts | Removed spaces in type annotation formatting |
| backend/src/usecases/users/v2/tests/get-event-info-v2.usecase.test.ts | Updated imports to use barrel export and reorganized import ordering |
| backend/src/usecases/users/tests/delete-event.usecase.test.ts | Reorganized import ordering |
| backend/src/usecases/shared/tests/fetch-and-save-currency-rate.usecase.test.ts | Improved formatting for method chaining |
| docs/domain.md | Added blank lines for readability, improved line wrapping, documented currency rate upsert behavior |
| infra/AGENTS.md | Added quick local Postgres testing command |
| android/marathon/README.md | Added ANDROID_SDK_ROOT derivation note and PowerShell Gradle workaround |
Files not reviewed (1)
- web/package-lock.json: Language not supported
| private readonly queryName = 'currency_rate'; | ||
|
|
||
| constructor({dataSource, showQueryDetails}: {dataSource: DataSource; showQueryDetails: boolean}) { | ||
| constructor({dataSource, showQueryDetails}: { dataSource: DataSource; showQueryDetails: boolean }) { |
There was a problem hiding this comment.
The spacing in the type annotation is inconsistent with other repository constructors. Other repositories use {dataSource: DataSource; showQueryDetails: boolean} without spaces after the opening brace and before the closing brace. For consistency, this should match the pattern used in currency.repository.ts, event.repository.ts, and other repository files.
| constructor({dataSource, showQueryDetails}: { dataSource: DataSource; showQueryDetails: boolean }) { | |
| constructor({dataSource, showQueryDetails}: {dataSource: DataSource; showQueryDetails: boolean}) { |
| - optional `OTEL_SERVICE_NAME` (defaults to `commonex-backend` in `src/config.ts`; production sets it in | ||
| `infra/docker-compose-prod.yml`) |
There was a problem hiding this comment.
The documentation states that production sets OTEL_SERVICE_NAME in infra/docker-compose-prod.yml, but the docker-compose file doesn't actually include this environment variable for the nest-backend-green and nest-backend-blue services. Either add OTEL_SERVICE_NAME to the environment variables in infra/docker-compose-prod.yml for both backend services, or update this documentation to reflect that the default value is used in production.
No description provided.