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

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

Merged
merged 6 commits into from
Jun 30, 2023

Conversation

angelapwen
Copy link
Contributor

@angelapwen angelapwen commented Jun 28, 2023

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 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
src/analyze.ts Show resolved Hide resolved
src/analyze.ts Outdated Show resolved Hide resolved
src/analyze.ts Outdated Show resolved Hide resolved
henrymercer
henrymercer previously approved these changes Jun 30, 2023
@angelapwen
Copy link
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
328 checks passed
@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
6 tasks
@esbena
Copy link
Contributor

esbena commented Jul 3, 2023

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
Copy link
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.

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.

3 participants