feat: upgrade OTLP infra, export Makefile, update TS infra #36
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades the OpenTelemetry (OTLP) infrastructure from version 0.207.x to 0.208.x, replaces the configuration management tool from coconfig to cpconfig, updates the TypeScript/ESLint infrastructure from ESLint 8 to ESLint 9, and adds a Makefile export to the package. The changes include dependency updates, type safety improvements (adding type imports), and minor code quality fixes.
Key Changes
- Upgraded OpenTelemetry packages from
0.207.xto0.208.xand@opentelemetry/semantic-conventionsfrom1.37.0to1.38.0 - Replaced
coconfigwithcpconfigconfiguration management system - Upgraded ESLint from v8 to v9 with new flat config system
- Added TypeScript type-only imports for improved tree-shaking
- Exported Makefile in package.json
Reviewed Changes
Copilot reviewed 27 out of 30 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Updated lockfile with new dependency versions for OTLP, ESLint, TypeScript tooling, and other dev dependencies |
| package.json | Updated dependencies, replaced coconfig with cpconfig, added Makefile export, version reset to 0.0.0 |
| src/types.ts | Changed imports to type-only, made port optional in onListening callback, removed void return type |
| src/telemetry/* | Changed to type-only imports, removed await from detectResources call, improved type safety |
| src/*.ts | Consistent change to type-only imports for ConfigurationSchema and other types |
| tests/* | Changed to type-only imports for test fixtures |
| .gitignore | Updated to reflect cpconfig managed files instead of coconfig |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const { metrics, logs, NodeSDK } = opentelemetry; | ||
|
|
||
| const resource = await detectResources({ | ||
| const resource = detectResources({ |
There was a problem hiding this comment.
The await keyword was removed from detectResources(), but according to the return type, this function should return a Promise. If detectResources is synchronous in the new version, this is fine, but if it returns a Promise, removing await will cause resource to be a Promise instead of the resolved value, leading to runtime errors.
| const resource = detectResources({ | |
| const resource = await detectResources({ |
| onListening?: ( | ||
| app: ServiceExpress<SLocals>, | ||
| info: { port: number; protocol: 'http' | 'https' }, | ||
| info: { port?: number; protocol: 'http' | 'https' }, |
There was a problem hiding this comment.
Making port optional in the onListening callback changes the API contract. Consumers relying on port being defined will need to add null checks. Consider if this breaking change is intentional and documented.
| info: { port?: number; protocol: 'http' | 'https' }, | |
| info: { port: number; protocol: 'http' | 'https' }, |
| req: RequestWithApp<SLocals>, | ||
| values: Record<string, string | string[] | number | undefined>, | ||
| ): string | undefined | void; | ||
| ): string | undefined; |
There was a problem hiding this comment.
Removing void from the return type of getLogFields is a breaking change. Functions that currently return nothing (implicitly void) will now fail type checking. This should be documented as a breaking change.
| ): string | undefined; | |
| ): string | undefined | void; |
| { | ||
| "name": "@openapi-typescript-infra/service", | ||
| "version": "4.10.2", | ||
| "version": "0.0.0", |
There was a problem hiding this comment.
The version has been reset to '0.0.0', which suggests this may be a mistake or placeholder. Ensure this is updated to the correct version before publishing.
| "version": "0.0.0", | |
| "version": "0.1.0", |
|
|
🎉 This PR is included in version 6.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
No description provided.