Skip to content

Conversation

@kaizencc
Copy link
Contributor

@kaizencc kaizencc commented Jun 9, 2025

Introduces a generic telemetry sink to the cli. it is tested but not used anywhere.

There are 3 base types of telemetry sinks:

  • FileSink -> gathers events and writes them to a file
  • IoHostSink -> gathers events and sends them to the IoHost (for writing to stdout/err)
  • EndpointSink -> gathers events and sends them to an external endpoint, batching at intervals of 30 seconds.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@github-actions github-actions bot added the p2 label Jun 9, 2025
@aws-cdk-automation aws-cdk-automation requested a review from a team June 9, 2025 18:39
@kaizencc kaizencc marked this pull request as ready for review June 18, 2025 22:07
@kaizencc kaizencc added the pr/exempt-integ-test Skips the integ test steps if set. label Jul 2, 2025
@kaizencc
Copy link
Contributor Author

kaizencc commented Jul 2, 2025

exempt integ test because this PR only adds new classes, it does not use them anywhere besides unit tests

fs.appendFileSync(this.logFilePath, output);
} catch (e: any) {
// Never throw errors, just log them via ioHost
await this.ioHelper.defaults.warn(`Failed to add telemetry event: ${e.message}`);
Copy link
Contributor Author

@kaizencc kaizencc Jul 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test that this sends a trace msg but does not throw

@kaizencc kaizencc changed the title chore(cli): telemetry client chore(cli): telemetry sink Jul 3, 2025
Copy link
Contributor

@iliapolo iliapolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving in spite of comments to start creating smaller PRs. Looks good overall and all comments can be addressed in a quick follow.

*/
export class EndpointTelemetrySink implements ITelemetrySink {
private events: TelemetrySchema[] = [];
private endpoint: UrlWithStringQuery;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should accept a string and parse internally. It would avoid exposing nodejs types in the props interface. Also - external callers are not the ones who want to parse, its this class that needs it, so it should do it.

@iliapolo iliapolo added this pull request to the merge queue Jul 3, 2025
Merged via the queue into main with commit 4c14220 Jul 3, 2025
11 checks passed
@iliapolo iliapolo deleted the conroy/basic-telemetry-client branch July 3, 2025 02:36
github-merge-queue bot pushed a commit that referenced this pull request Jul 9, 2025
Follow up to #585

---
By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

p2 pr/exempt-integ-test Skips the integ test steps if set.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants