feat: Release @appland/telemetry as a package #2330
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
dustinbyrne
force-pushed
the
feat/telemetry
branch
4 times, most recently
from
f1ba05d to
c857979
Compare
This includes a refactor and support for additional backends (such as Splunk).
Killing the process on failure with an exit code exits immediately, without allowing the event loop to drain. This means any pending promises will not resolve.
dustinbyrne
force-pushed
the
feat/telemetry
branch
from
91000e4 to
734de43
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the telemetry functionality by extracting it from the existing monolithic file into a standalone @appland/telemetry package, making it reusable across different projects. The refactor includes significant architectural improvements with a more modular design pattern.
- Extracted telemetry functionality into a standalone package with a proper backend architecture
- Updated all dependent packages to use the new telemetry package instead of local imports
- Improved configuration management with automatic package detection and configurable backends
Reviewed Changes
Copilot reviewed 42 out of 43 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/telemetry/tsconfig.json | Updated TypeScript configuration for package build |
| packages/telemetry/tests/*.spec.ts | New comprehensive test suite for all telemetry components |
| packages/telemetry/src/ | New modular source structure with separate files for client, session, git, etc. |
| packages/telemetry/package.json | Package configuration updates for publishing as standalone package |
| packages/scanner/src/ | Updated to use @appland/telemetry package instead of local import |
| packages/cli/src/ | Updated to use @appland/telemetry package with proper configuration |
|
|
||
| touch(): void { | ||
| this._expiration = Session.expirationFromNow(); | ||
| this.config.set('sessionExpiration', this.expiration); |
There was a problem hiding this comment.
The touch() method uses this.expiration which calls the getter that checks validity and may renew the session. This creates a circular dependency where touch() could indirectly call renew() if the session is invalid, which is not the intended behavior. Use this._expiration instead.
| this.config.set('sessionExpiration', this.expiration); | |
| this.config.set('sessionExpiration', this._expiration); |
Copilot uses AI. Check for mistakes.
| @@ -165,7 +173,7 @@ yargs(process.argv.slice(2)) | |||
| console.error(err); | |||
| } | |||
| } | |||
| process.exit(1); | |||
| process.exitCode = 1; | |||
There was a problem hiding this comment.
[nitpick] This change from process.exit(1) to process.exitCode = 1 allows async operations like telemetry flushing to complete before exit, but the original behavior may have been intentional for immediate termination. Consider if this change aligns with the error handling requirements.
| process.exitCode = 1; | |
| process.exit(1); |
Copilot uses AI. Check for mistakes.
| if (gitCache.has(cacheKey)) { | ||
| return gitCache.get(cacheKey); | ||
| } | ||
| /* eslint-disable-next-line @typescript-eslint/no-unsafe-function-type */ |
There was a problem hiding this comment.
The comment suggests this should be @typescript-eslint/ban-types but the rule name is incorrect. The correct rule name is @typescript-eslint/ban-types, not @typescript-eslint/no-unsafe-function-type.
| /* eslint-disable-next-line @typescript-eslint/no-unsafe-function-type */ | |
| /* eslint-disable-next-line @typescript-eslint/ban-types */ |
Copilot uses AI. Check for mistakes.
| "read-pkg-up": "^7.0.1", | ||
| "splunk-logging": "^0.11.1" | ||
| }, | ||
| "devDependencies": { | ||
| "@types/jest": "^29.4.1", | ||
| "@typescript-eslint/eslint-plugin": "^5.7.0", | ||
| "@typescript-eslint/parser": "^5.7.0", | ||
| "eslint": "^8.4.1", | ||
| "eslint-config-prettier": "^8.3.0", | ||
| "@types/node": "^22", | ||
| "@types/splunk-logging": "^0.11.8", |
There was a problem hiding this comment.
The splunk-logging dependency is added but not used in any of the visible code. Consider removing unused dependencies to keep the package lean.
Copilot uses AI. Check for mistakes.
This includes a refactor. Additional details to be provided.