FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Send tools telemetry to `init` status report by angelapwen · Pull Request #1497 · github/codeql-action · GitHub

/ codeql-action Public

Send tools telemetry to init status report - #1497

Merged
angelapwen merged 17 commits into
github:mainfrom
angelapwen:tools-telemetry-report
Jan 25, 2023
ConversationCommits (17)ChecksFiles changed
Merged

Send tools telemetry to init status report#1497
angelapwen merged 17 commits into
github:mainfrom
angelapwen:tools-telemetry-report

Conversation

angelapwen commented Jan 21, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

This PR sends the following fields to the init status report on success and failure:

  • tools_source
  • if tools_source was DOWNLOAD, tools_download_duration_ms and tools_feature_flags_valid

Note that previously we only sent additional fields on init success, but because we would like to support the tools fields on failure as well, the PR changes the status report behavior so that the init report is sent on failure too, with as many fields populated as possible.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.
  • Confirm the changelog has been updated if necessary.

angelapwen force-pushed the tools-telemetry-report branch from a9545a6 to f24d78c Compare January 21, 2023 00:37
angelapwen changed the title Send tools telemetry to init success status report Send tools telemetry to init status report Jan 21, 2023
angelapwen marked this pull request as ready for review January 21, 2023 01:06
angelapwen requested a review from a team as a code owner January 21, 2023 01:06
angelapwen force-pushed the tools-telemetry-report branch from a217825 to 4fa58ed Compare January 21, 2023 01:10

angelapwen commented Jan 21, 2023

Copy link
Copy Markdown
Contributor Author

Looks like a bit of an inscrutable error, init action failed: TypeError: (0 , actions_util_1.getActionVersion) is not a function when it is correctly defined in actions-util and I haven't changed anything about it.

Also, oddly enough the error fails in the analyze step even though it originates from init

henrymercer commented Jan 23, 2023

Copy link
Copy Markdown
Contributor

I think this could be as boring as just needing to merge in main and recompile the Action — some of the compiled files seem to be missing at the moment.

henrymercer reviewed Jan 23, 2023
View reviewed changes
Comment thread src/init.ts Outdated
github-advanced-security AI found potential problems Jan 23, 2023
View reviewed changes
Comment thread src/init-action.ts Fixed
Comment thread src/init-action.ts Fixed
Comment thread src/init-action.ts Fixed
Comment thread src/init-action.ts Fixed

angelapwen commented Jan 23, 2023

Copy link
Copy Markdown
Contributor Author

I have an issue with node in VSCode so the automated linting isn't working very well.. trying to resolve now.

angelapwen force-pushed the tools-telemetry-report branch from 8ca90f9 to 919c531 Compare January 23, 2023 18:30
henrymercer reviewed Jan 23, 2023
View reviewed changes

henrymercer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I like how you've used this additional telemetry information to improve the tests for setting up the CodeQL tools! Here's a couple of initial comments:

Comment thread src/codeql.test.ts Outdated
Comment thread src/init-action.ts Outdated
Comment thread src/init-action.ts Outdated
Comment thread src/init-action.ts Outdated

angelapwen commented Jan 24, 2023

Copy link
Copy Markdown
Contributor Author

I think I've addressed all comments and PR is ready for review again!

henrymercer reviewed Jan 24, 2023
View reviewed changes
Comment thread src/feature-flags.ts Outdated
Comment thread src/init-action.ts Outdated
Comment thread src/init-action.ts Outdated
Comment thread src/init-action.ts Outdated
Comment thread src/init-action.ts Outdated
Comment thread src/init-action.ts Outdated
henrymercer previously approved these changes Jan 25, 2023
View reviewed changes

henrymercer left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Great stuff! A couple of minor suggestions to improve the types, and there are a few conflicts to resolve before we can merge this.

Comment thread src/feature-flags.ts
Comment thread src/feature-flags.ts Outdated
henrymercer approved these changes Jan 25, 2023
View reviewed changes

henrymercer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

🎉

angelapwen merged commit 24ca6b0 into github:main Jan 25, 2023
angelapwen deleted the tools-telemetry-report branch January 25, 2023 19:09
github-actions Bot mentioned this pull request Jan 26, 2023
Merged
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

henrymercer henrymercer approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL