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

Jump to bottom

Java: Start running telemetry queries on Code Scanning #7417

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Java: Start running telemetry queries on Code Scanning #7417

wants to merge 1 commit into from

Conversation

@henrymercer
Copy link
Contributor

@henrymercer henrymercer commented Dec 15, 2021

This PR incorporates the telemetry queries for Java into the summary metrics framework. Specifically we use the support for summary metrics with messages that'll be added in the next version of the CodeQL CLI.

This has a few effects:

  1. We'll start running these queries on Code Scanning.
  2. We will start displaying a summary output of these queries in the metrics summary table produced on Code Scanning and in the CLI (see example below).
  3. We can ingest the results of these queries as part of generic internal pipelines that operate on summary metrics.
Example metrics summary table additions 
Analysis produced the following metric data:

|                          Metric                          |      Value     |
+----------------------------------------------------------+----------------+
| Supported sinks in external libraries                    |  (574 results) |
| Usage of unsupported APIs coming from external libraries | (3949 results) |
| Supported sources in external libraries                  |   (15 results) |
| External libraries                                       |   (75 results) |
| Supported sinks in external libraries                    |   (22 results) |

/cc @bmuskalla @turbo @yo-h
@henrymercer
Java: Convert telemetry queries to summary metrics
5696146 
Use the support for summary metrics with messages that'll be in the next
version of the CodeQL CLI.
@github-actions github-actions bot added the Java label Dec 15, 2021
yo-h
yo-h reviewed Dec 19, 2021
View changes
Copy link
Contributor

@yo-h yo-h left a comment

Not caused by this PR, but it seems worth fixing what looks like a copy/paste error to avoid the following confusion (taken from the example in the PR description).

| Supported sinks in external libraries | (574 results) |
| Supported sinks in external libraries | (22 results) |

java/ql/src/Telemetry/SupportedExternalTaint.ql
@@ -1,9 +1,9 @@
/**
* @name Supported sinks in external libraries
Copy link
Contributor

@yo-h yo-h Dec 19, 2021

Choose a reason for hiding this comment

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

Suggested change
* @name Supported sinks in external libraries
* @name Supported flow steps in external libraries

java/ql/src/Telemetry/SupportedExternalTaint.ql
@@ -1,9 +1,9 @@
/**
* @name Supported sinks in external libraries
* @description A list of 3rd party APIs detected as sinks. Excludes test and generated code.
Copy link
Contributor

@yo-h yo-h Dec 19, 2021

Choose a reason for hiding this comment

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

Suggested change
* @description A list of 3rd party APIs detected as sinks. Excludes test and generated code.
* @description A list of 3rd party APIs detected as flow steps. Excludes test and generated code.

@bmuskalla bmuskalla mentioned this pull request Dec 21, 2021
Draft
@bmuskalla
Copy link
Contributor

@bmuskalla bmuskalla commented Dec 21, 2021

Thanks for fixing up the tags (I had a draft PR but never merged it, sorry)

@henrymercer - a more general question: Do we run all summary queries alongside every other invocation of codeql for regular users?
Because that makes sense for queries that are looking at the database in question (e.g. UnsupportedExternalAPIs.ql) but is not necessary for database-independant queries (e.g. the new FrameworkCoverageMetrics.ql). WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yo-h yo-h

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Java
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
@henrymercer @bmuskalla @yo-h