Visitar URL original
Send new per-query alert count event reports for QA telemetry by angelapwen · Pull Request #1741 · github/codeql-action · GitHub
Skip to content

Send new per-query alert count event reports for QA telemetry - #1741

Merged
angelapwen merged 6 commits into
github:mainfrom
angelapwen:per-query-alert-counts
Jun 30, 2023
Merged

Send new per-query alert count event reports for QA telemetry#1741
angelapwen merged 6 commits into
github:mainfrom
angelapwen:per-query-alert-counts

Conversation

@angelapwen

@angelapwen angelapwen commented Jun 28, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

This change sends a list of event reports representing the per-query alert counts from a run, appended as a field to the analyze status report, when the qa_telemetry feature flag is set. The event_report schema in the data warehouse is mimicked in the EventReport interface here.

We generate an event report just after codeql database-interpret results is run for each language, so the event reports are language-specific. The properties field of the event report is an object representing the per-query alert counts processed from a single SARIF file.

I have manually tested this by setting the environment variable CODEQL_ACTION_QA_TELEMETRY: true in a test run (GitHub internal only link) to confirm that the event report and alert counts are being generated appropriately. I have also modified the existing runQueries test to confirm that the appropriate event_report status report field is set with event property codeql database interpret-results.

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
angelapwen force-pushed the per-query-alert-counts branch 5 times, most recently from 46b340a to b84eeb1 Compare June 28, 2023 15:22
@angelapwen
angelapwen force-pushed the per-query-alert-counts branch from b84eeb1 to c5f557c Compare June 29, 2023 14:59
@angelapwen
angelapwen force-pushed the per-query-alert-counts branch from c5f557c to 5156052 Compare June 29, 2023 15:02
@angelapwen
angelapwen marked this pull request as ready for review June 29, 2023 15:09
@angelapwen
angelapwen requested a review from a team as a code owner June 29, 2023 15:09
@angelapwen
angelapwen requested a review from henrymercer June 29, 2023 15:09
henrymercer
henrymercer reviewed Jun 29, 2023
View reviewed changes
Comment thread src/analyze.ts Resolved
Comment thread src/analyze.ts Resolved
Comment thread src/analyze.ts Resolved
henrymercer
henrymercer previously approved these changes Jun 30, 2023
View reviewed changes
@angelapwen
angelapwen requested a review from henrymercer June 30, 2023 12:37
@angelapwen

angelapwen commented Jun 30, 2023

Copy link
Copy Markdown
Contributor Author

I've set the feature to true in the existing runQueries test, and confirmed that the appropriate event_report status report field is set with event property codeql database interpret-results.

@angelapwen
angelapwen merged commit 46a6823 into github:main Jun 30, 2023
@angelapwen
angelapwen deleted the per-query-alert-counts branch June 30, 2023 14:53
@github-actions github-actions Bot mentioned this pull request Jul 3, 2023
Merged
5 of 6 tasks
@esbena

esbena commented Jul 3, 2023

Copy link
Copy Markdown
Contributor

and confirmed that the appropriate event_report status report field

I can see that in your test run. But it does not look like the data enters the database. Is that intentional?

database("hydro").code_scanning_v0_status_report
| where timestamp between (now(-30d) .. now())
| summarize count() by array_length(event_reports)

Results in 34035006 reports with length 0 event_reports, and nothing else.

@angelapwen

angelapwen commented Jul 5, 2023

Copy link
Copy Markdown
Contributor Author

@esbena Thank you for pointing this out. I looked at the debug logs of the run more closely and it looks like the request returned an error because I had sent the date/times for the event_report with the incorrect format. I'll put up a new PR, start a test run, and check the data warehouse for the report.

@angelapwen angelapwen mentioned this pull request Jul 5, 2023
Merged
3 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@henrymercer 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

@angelapwen @esbena @henrymercer