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

Display full job name and nested workflow details in log #2049

Merged
merged 16 commits into from
Aug 23, 2022
Merged

Display full job name and nested workflow details in log #2049

merged 16 commits into from
Aug 23, 2022

Conversation

nicholasbergesen
Copy link
Contributor

@nicholasbergesen nicholasbergesen commented Aug 9, 2022

This forms part of the work being done for the nested workflow feature. Jobs that are nested more than 1 level in depth have their job name truncated in the UI. The change displays the full job name as well as the calling workflow ref and inputs that were passed in, in the UI and raw logs

Complete job name:
image

Calling workflow ref with inputs:
image
Expanded:
image

Raw logs:
image

actions-dotnet change: https://github.com/github/actions-dotnet/pull/12910

Closes: https://github.com/github/c2c-actions-policy/issues/580
Epic: https://github.com/github/c2c-actions-policy/issues/399

@nicholasbergesen nicholasbergesen requested a review from a team as a code owner August 9, 2022 05:37
@yujincat
Copy link
Contributor

yujincat commented Aug 9, 2022

In the example snapshot, I believe we used ./.github/workflows/end.yml but it is adding the nwo.
What does it look like when we refer the workflow with nwo and version? Does it show the version as well?

Copy link
Contributor Author

@nicholasbergesen nicholasbergesen left a comment

Choose a reason for hiding this comment

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

remove unused JobWorkflowRef property

Copy link
Contributor Author

@nicholasbergesen nicholasbergesen left a comment

Choose a reason for hiding this comment

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

add mapping to dataWorkflowFilePathRaw in constructor

@nicholasbergesen
Copy link
Contributor Author

nicholasbergesen commented Aug 9, 2022

In the example snapshot, I believe we used ./.github/workflows/end.yml but it is adding the nwo. What does it look like when we refer the workflow with nwo and version? Does it show the version as well?

@yujincat if I use github/public-server/.github/workflows/end.yml@main in the workflow file it displays github/public-server/.github/workflows/end.yml in the log, with the nwo but without the version.

@TingluoHuang
Copy link
Member

Why do we need to log this info to the set up job step?
We might need to have PM sign off on this kind of change since these are informational message that is not really scoped for how the runner set up the job.

@yujincat
Copy link
Contributor

@TingluoHuang We already have discussed this with @chrispat before, hence we are working on this. The reason we want to log this is we want to provide more context of workflow execution in the log so that the users can understand it; mostly reusable/nested workflows perspective.

Users may want to know what inputs they are given. I thin it makes sense to have that in the "set up job" section because the inputs apply per job. Also we want to show the text given from user in the uses field in the yaml file to provide more context of using reusable workflows for this job.

Providing the full job name is also what we want, as the job name using nested workflows will be truncated in the UI.

@nicholasbergesen I think we should check if we want to make this change for all workflows or reusable case only.

@nicholasbergesen
Copy link
Contributor Author

I've moved the display of complete job name inside the check on system.workflowFileFullPath so it will only display in the log for reusable workflows.

@nicholasbergesen
Copy link
Contributor Author

nicholasbergesen commented Aug 11, 2022

@TingluoHuang Instead of using Set up job Do you think it would be better to create a new section? Something along the lines of Job trigger or Job source for displaying details relating to how the job was called?

@chrispat
Copy link
Member

I think this is good. We already display a bunch of information in that area that has nothing to do with the runner such as secret source and token permissions.

yujincat
yujincat previously approved these changes Aug 12, 2022
ajaykn
ajaykn previously approved these changes Aug 22, 2022
Copy link
Contributor

@ajaykn ajaykn left a comment

Choose a reason for hiding this comment

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

Looks good to me 🚀

ajaykn
ajaykn previously approved these changes Aug 22, 2022
TingluoHuang
TingluoHuang previously approved these changes Aug 22, 2022
@nicholasbergesen nicholasbergesen dismissed stale reviews from TingluoHuang, ghost , and ajaykn via 8fae189 August 23, 2022 00:06
@TingluoHuang TingluoHuang merged commit 42c8666 into actions:main Aug 23, 2022
@nicholasbergesen nicholasbergesen deleted the nick/complete_job_name branch August 23, 2022 00:23
ChristopherHX pushed a commit to ChristopherHX/runner.server that referenced this pull request Dec 29, 2022
nikola-jokic pushed a commit to nikola-jokic/runner that referenced this pull request May 12, 2023
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.

5 participants