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

Add config-file to init status report #2443

Merged
merged 4 commits into from
Aug 26, 2024
Merged

Conversation

dbartol
Copy link
Contributor

@dbartol dbartol commented Aug 23, 2024

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 dbartol requested a review from a team as a code owner August 23, 2024 21:08
@aeisenberg
Copy link
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?

@angelapwen
Copy link
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
Copy link
Contributor Author

dbartol commented Aug 26, 2024

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