Send job_run_uuid to status report telemetry
#1685
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
src/actions-util.test.ts
Dismissed
| t.assert(statusReport.job_name === (process.env["GITHUB_JOB"] || "")); | ||
| t.assert(statusReport.analysis_key === "analysis-key"); | ||
| t.assert(statusReport.commit_oid === process.env["GITHUB_SHA"]); | ||
| t.assert(statusReport.ref === process.env["GITHUB_REF"]); |
Check warning
Code scanning / CodeQL
Some environment variables may not exist in default setup workflows Warning
There was a problem hiding this comment.
Closed alert because we are only using the environment variable in a test.
|
I realized we hadn't gotten back to this PR, and resolved a merge conflict with |
| process.env["GITHUB_REF"] = "refs/heads/main"; | ||
| process.env["GITHUB_SHA"] = "a".repeat(40); | ||
| process.env["GITHUB_RUN_ID"] = "100"; | ||
| process.env["GITHUB_RUN_ATTEMPT"] = "2"; | ||
| process.env["GITHUB_REPOSITORY"] = "octocat/HelloWorld"; |
There was a problem hiding this comment.
These are supposed to be protected environment variables in the Actions runtime.
So two questions:
- Do these assignments actually affect the environment variable?
- If you change the assertions below to compare
statusReport.refetc with the constant values, not withprocess.env["GITHUB_REF"], do the tests still pass?
There was a problem hiding this comment.
Good point! It seems they actually do affect the environment variable. I changed the assertions to t.assert(statusReport.ref === "refs/heads/main"); and t.assert(statusReport.commit_oid === "a".repeat(40)); for the SHA, and the tests still passed.
If I don't populate these variables manually, I get
Rejected promise returned by test. Reason:
Error {
message: 'GITHUB_SHA environment variable must be set',
}
errors. I assume that these just aren't populated at all in the unit tests.
There was a problem hiding this comment.
Following up on this after our 1:1: these tests failed locally because I wasn't in the Actions runtime. When run in Actions, the protected Actions environment variables are able to be overridden for the purpose of the checks, but are not populated across steps and tests.
@henrymercer and/or @aeisenberg, does this match your understanding of how this unit test works?
There was a problem hiding this comment.
That's my understanding. I would go further and say that we should be running our unit tests starting from an empty environment — our tests shouldn't depend on the environment we run them in.
There was a problem hiding this comment.
Ok great, I'm happy enough with this test checking the status report is populated from process.env, and we can leave further test cleanup for a future PR:
- starting from clean environment
- passing locally when not running in an Actions environment
| process.env["GITHUB_REF"] = "refs/heads/main"; | ||
| process.env["GITHUB_SHA"] = "a".repeat(40); | ||
| process.env["GITHUB_RUN_ID"] = "100"; | ||
| process.env["GITHUB_RUN_ATTEMPT"] = "2"; | ||
| process.env["GITHUB_REPOSITORY"] = "octocat/HelloWorld"; |
There was a problem hiding this comment.
Ok great, I'm happy enough with this test checking the status report is populated from process.env, and we can leave further test cleanup for a future PR:
- starting from clean environment
- passing locally when not running in an Actions environment
This change generates a
job_run_uuidin theinitAction, and then exports it as an environment variable to propagate it across steps. When any status report is created, the value is read from the environment variable and then sent to our internal telemetry API. This will allow us to join status reports from various steps to one another without using multiple fields.The PR also tests the telemetry fields in
statusReportBaseare assigned appropriately and the proper type (eg.stringfor thejob_run_uuid).[GitHub internal only]: I validated against this branch, with a logging statement, on this job. Note that in a single attempt, the
job_run_uuidlogged is the same across steps, and in each attempt it is unique.Merge / deployment checklist