upsidr / merge-gatekeeper

Get better merge control
MIT License
85 stars 14 forks source link

Better processing log format #51

Closed Jrc356 closed 1 year ago

Jrc356 commented 1 year ago

Problem

Currently, the logging for execution looks something like this:

Total job count:     3
    jobs: ["my_job" "my_job2" "my_job3"]
  Completed job count: 2
    jobs: ["my_job" "my_job2"]
  Failed job count:    0
    jobs: []

This is fine for small amounts of jobs but once the list expands it becomes difficult to read the logs. Example

Total job count:     X
    jobs: ["my_job" "my_job2" "my_job3" "my_job" "my_job2" "my_job3" "my_job" "my_job2" "my_job3" "my_job" "my_job2" "my_job3" "my_job" "my_job2" "my_job3" "my_job" "my_job2" "my_job3" "my_job" "my_job2" "my_job3"]
  Completed job count: Y
    jobs: ["my_job" "my_job2" "my_job" "my_job2" "my_job3" "my_job" "my_job2" "my_job3""my_job" "my_job2" "my_job3""my_job" "my_job2" "my_job3" "my_job" "my_job2" "my_job3" "my_job" "my_job2" "my_job3"]
  Failed job count:    0
    jobs: []

(formatting in issues is slightly different than what is output in GitHub actions so YMMV on this)

Request

Part 1

print each job on a new line (yaml - esk), for example

Total job count:     3
    jobs: 
      - my_job
      - my_job2 
      - my_job3
  Completed job count: 2
    jobs:
      - my_job
      - my_job2 
  Failed job count:    0
    jobs: []

Part 2

Add a new section of the log that prints incomplete jobs (waiting for)

Total job count:     3
    jobs: 
      - my_job
      - my_job2 
      - my_job3
  Incomplete job count: 1
    jobs:
      - my_job3 
  Completed job count: 2
    jobs:
      - my_job
      - my_job2 
  Failed job count:    0
    jobs: []

I can split this into separate issues if wanted, lemme know. I can get started on a PR for this if the idea is liked

rytswd commented 1 year ago

Thanks @Jrc356 for raising this! 👍 The current implementation was chosen for its simplicity (of implementation), and not the best format by any means, so this absolutely makes sense! 🥰

When there are more than a handful of jobs, and if Merge Gatekeeper needs to wait for multiple runs, the YAML-esque logs could be quite chatty and difficult to follow as well. Perhaps we should also consider adding a foldable log format, as documented here?

Jrc356 commented 1 year ago

the YAML-esque logs could be quite chatty and difficult to follow as well

As I began some implementation of this last night, yeah this gets reeaaaalllly noisy and I'm not sure if it's actually that much better. I'm thinking that perhaps just adding an Incomplete section might be enough to make it a bit easier to read without making it too chatty. Thoughts?

The foldable format could work as well but that kinda sounds like disguising the problem (noisy logs). Perhaps there's a question here of whether the status should be printed on every iteration or not 🤔 I'm torn on this... I like having the info, but the noisiness could make it less worthwhile in the end. We could give the grouping a shot to see what it looks like if you think that would work well

Jrc356 commented 1 year ago

@rytswd thinking about this again, what if we added debug logging which prints all the job names out but without it, it'll just print numbers. Example:

# with:
#   debug: false (default)

Total jobs: 25
Complete Jobs: 5
Incomplete Jobs: 5
Failed Jobs: 0
Ignored Jobs: 15
# with:
#   debug: true

Total jobs: 25
  jobs:
    - my
    - list
    - of
    - jobs
    - ...
Complete Jobs: 5
  jobs:
    - my
    - complete
    - jobs
Incomplete Jobs: 5
  jobs:
    - my
    - incomplete
    - jobs
Failed Jobs: 0
Ignored Jobs: 15
  jobs:
    - my
    - ignored
    - jobs

This should keep the logs pretty clean unless you need it otherwise

rytswd commented 1 year ago

@Jrc356 Merge Gatekeeper aims to be a simple, zero-config solution, so adding a flag like that isn't precisely what we are after. But, as you pointed out, we should highlight what matters most from the Merge Gatekeeper point of view - that is, the job counts (successful / incomplete / failed / ignored), and all other bits are unnecessary details you wouldn't need to see in most cases.

I suppose we can update the list of jobs with YAML-esque format as "debug" logging, and present it as folded content. Providing the details for each iteration is actually important, as GitHub "Runs" can be some third party jobs rather than GitHub Action, which get their status updated directly via API calls. With that, it is possible for the job status to go from complete to incomplete (which doesn't make too much sense, but is a possibility), and it would be really confusing and difficult to understand without the clear status for each iteration.

What I was thinking is something like below:

Start processing validator: merge-gatekeeper....

Job status summary at 00:00:00
  Total:     3
  Completed: 1
  Failed:    0
  Ignored:   1

::group::DETAILS
1 out of 3
  Total job count:     3
    - "A"
    - "B"
    - "C"
  Completed job count: 1
    - "B"
  Failed job count:    0
    []
  Ignored job count:   1
    - "D"
::endgroup::

 WARNING: Validation is yet to be completed. This is most likely due to some other jobs still running.
          Waiting for 5 seconds before retrying.

Also, if Merge Gatekeeper actually fails with some failed job at the end, the list of jobs should be presented as is. With those in mind, it would look like something like below

✅ GOOD

-------------------------------------------------
Start processing validator: merge-gatekeeper....

Job status summary at 00:00:00
  Total:     3
  Completed: 1
  Failed:    0
  Ignored:   1

 WARNING: Validation is yet to be completed. This is most likely due to some other jobs still running.
          Waiting for 5 seconds before retrying.

▶ DETAILS

-------------------------------------------------
Start processing validator: merge-gatekeeper....

Job status summary at 00:00:05
  Total:     3
  Completed: 3
  Failed:    0
  Ignored:   1

All validations were successful!

▶ DETAILS

🙅 NG

-------------------------------------------------
Start processing validator: merge-gatekeeper....

Job status summary at 00:00:00
  Total:     3
  Completed: 1
  Failed:    0
  Ignored:   1

 WARNING: Validation is yet to be completed. This is most likely due to some other jobs still running.
          Waiting for 5 seconds before retrying.

▶ DETAILS

-------------------------------------------------
Start processing validator: merge-gatekeeper....

Job status summary at 00:00:05
  Total:     3
  Completed: 2
  Failed:    1
  Ignored:   1

ERROR: Found failed job(s), Merge Gatekeeper is exiting as failed as well.

  Total job count:     3
    - "A"
    - "B"
    - "C"
  Completed job count: 2
    - "A"
    - "B"
  Failed job count:    1
    - "C"
  Ignored job count:   1
    - "D"

I think the current log is the bare minimum of what we can do, so I think any update would be better than what we currently have - but hopefully this would be a good middle ground for now at least. What do you think?

Jrc356 commented 1 year ago

@rytswd yeah that makes sense to me. I like the idea of grouping the details so that it can be accessed but keeps the logs generally clean unless needed. I'll see what I can do later tonight on that branch I started

Jrc356 commented 1 year ago

Never got around to working on this last week but I'll try to get to it this week

Jrc356 commented 1 year ago

Finally got around to this, opened https://github.com/upsidr/merge-gatekeeper/pull/54

@rytswd I took a little creative liberty in it in terms of grouping the jobs together. Lemme know what you think