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

Add `config-file` to init status report by dbartol · Pull Request #2443 · github/codeql-action · GitHub

Add config-file to init status report - #2443

Merged
dbartol merged 4 commits into
mainfrom
dbartol/config-file-telemetry
Aug 26, 2024
Merged

dbartol merged 4 commits into
mainfrom
dbartol/config-file-telemetry

Conversation

dbartol commented Aug 23, 2024
edited
Loading

Copy link
Copy Markdown

Testing using a private branch confirm that we're already receiving the telemetry correctly.

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.

dbartol requested a review from a team as a code owner August 23, 2024 21:08

Copy link
Copy Markdown
Contributor

So, this will add the path to the config file to our telemetry? If so, how useful is this for us? More precisely, what questions will this help us answer? Would we get the same value by adding a boolean field something like uses_config_file: true/false?

Copy link
Copy Markdown
Contributor

So, this will add the path to the config file to our telemetry? If so, how useful is this for us? More precisely, what questions will this help us answer? Would we get the same value by adding a boolean field something like uses_config_file: true/false?

From internal issue: it looks like we want to know:

  • if a config file was used
  • whether the config file was remote or local

I guess having the path will answer both of those questions. Or we could alternatively have 2 fields in the status report, a boolean for if a config file was used and an enum/string.

The latter would probably make it easier to query (so we have the logic determining whether the file was remote or local in the Action, rather than in the query). On the other hand, we've already merged the changes to the schema/API so changing the shape of the data we send through would require updating those.

dbartol commented Aug 26, 2024

Copy link
Copy Markdown
Author

We already have to do a similar local vs. remote parsing in the Kusto query for the queries property, so we might as well do the same for the config file path.

dbartol merged commit 864b979 into main Aug 26, 2024
dbartol deleted the dbartol/config-file-telemetry branch August 26, 2024 23:38
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
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


Back | FazBrowse Home | New Git URL