Skip to content

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 was introduced in CodeQL CLI v2.8.0.

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

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

@yo-h yo-h left a comment

Choose a reason for hiding this comment

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

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) |

@bmuskalla
Copy link
Contributor

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?

Co-authored-by: yo-h <55373593+yo-h@users.noreply.github.com>
@henrymercer
Copy link
Contributor Author

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

The queries that we'll run on Code Scanning are defined within https://github.com/github/codeql/tree/main/java/ql/src/codeql-suites. I can't find FrameworkCoverageMetrics.ql, but I agree we likely want to avoid running queries like these that are targetted at query writers in Code Scanning. We can do this for instance by adding another exclusion rule to https://github.com/github/codeql/blob/main/misc/suite-helpers/code-scanning-selectors.yml like the one we have for experimental.

@henrymercer henrymercer marked this pull request as ready for review February 7, 2022 15:48
@henrymercer henrymercer requested a review from a team as a code owner February 7, 2022 15:48
@henrymercer henrymercer requested a review from yo-h February 7, 2022 15:48
@henrymercer
Copy link
Contributor Author

@github/codeql-java Would this PR require a changenote?

Copy link
Contributor

@yo-h yo-h left a comment

Choose a reason for hiding this comment

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

Would this PR require a changenote?

We are still evaluating the output of these queries, so I would merge without a change note for now. Unless @turbo knows of a reason to advertise more widely at this time?

@henrymercer henrymercer added the no-change-note-required This PR does not need a change note label Feb 8, 2022
@henrymercer henrymercer merged commit eff0ca0 into main Feb 8, 2022
@henrymercer henrymercer deleted the henrymercer/java/update-telemetry-query-metadata branch February 8, 2022 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Java no-change-note-required This PR does not need a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants