Skip to content
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

Add telemetry #404

Merged
merged 2 commits into from
Oct 22, 2023
Merged

Add telemetry #404

merged 2 commits into from
Oct 22, 2023

Conversation

quesabe
Copy link
Collaborator

@quesabe quesabe commented Oct 18, 2023

Closes PLA-263.

A common module for telemetry defines:

  • a function to setup telemetry in a project (basically adds a URL for reporting and sets up a Github workflow);
  • a function to collect the data;
  • the data sending is done with node-fetch package since we claim Node16 support;
  • so far escape hatches are found in a projenrc file via a simple test search - maybe for better results some kind of AST parsing would be a better fit.

@quesabe quesabe requested a review from gvidon October 18, 2023 06:29
@quesabe quesabe self-assigned this Oct 18, 2023
@linear
Copy link

linear bot commented Oct 18, 2023

PLA-263

README.md Show resolved Hide resolved
src/common/telemetry/collect-telemetry/__tests__/index.ts Outdated Show resolved Hide resolved
src/common/telemetry/collect-telemetry/clone-workflow.ts Outdated Show resolved Hide resolved
import {diff} from './diff'
import {EscapeHatches, getEscapeHatches} from './get-escape-hatches'

interface TelemetryPayload {
Copy link
Contributor

Choose a reason for hiding this comment

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

@quesabe should be type.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've configured the consistent-type-definitions rule to require interfaces, since JSII does not work with type for exported members.

This one is not exported and thus can be converted to type. However it would require either to disable the eslint rule per line or completely switch it off for the repo.

Copy link
Contributor

Choose a reason for hiding this comment

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

@quesabe do you mean that the aslant rule in the repo requires interfaces instead of types? If so, then I'm ok with interface. Just wanted it to stay consistent.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please resolve the conversation if what I'm saying above is correct.

src/common/telemetry/setup-telemetry.ts Show resolved Hide resolved
src/common/telemetry/with-telemetry.ts Outdated Show resolved Hide resolved
@quesabe quesabe force-pushed the experiment-scan-projenrc branch 2 times, most recently from b599682 to 9b9ed74 Compare October 21, 2023 15:18
@quesabe quesabe requested a review from gvidon October 21, 2023 15:26
@gvidon gvidon merged commit 1fb30d0 into main Oct 22, 2023
6 checks passed
@gvidon gvidon deleted the experiment-scan-projenrc branch October 22, 2023 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants