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 new collectors and remove fetch-on-checkpoint mechanism (New) #1788

Merged
merged 11 commits into from
Mar 27, 2025

Conversation

Hook25
Copy link
Collaborator

@Hook25 Hook25 commented Mar 11, 2025

Description

It is often useful to analyze the logs after a session is done to see why something went south. In order to do this, this PR adds a journal collector to system information.

This PR also moves the collection to the end of the session. I was about to create some complex machinery to run some collectors at the start of a run and some at the end, but I talked myself out of it. The collection was placed where it was placed because we thought we could use it during the run somehow. This idea went out of the window and all that is remaining now is the cost of running the collection at the start of the run, which is about 3s just to run inxi on my machine, for any session that is started, even if no submission is ever generated. I don't think the value we get (which is nothing right now) justifies this cost. If we will ever want to do something with that data during the run we can always revert (partially) this commit and tag the collectors/create functions to call them at finalize and first checkpoint, for now, I think we should do the easy thing and just collect at the end.

Resolved issues

Fixes: CHECKBOX-1753

Documentation

N/A

Tests

Tried this locally, in general I don't expect this to cause major issues as collectors can safely fail

Copy link

codecov bot commented Mar 11, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 49.83%. Comparing base (5baa4d0) to head (7ca70c0).
Report is 20 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1788      +/-   ##
==========================================
- Coverage   49.83%   49.83%   -0.01%     
==========================================
  Files         377      377              
  Lines       40719    40715       -4     
  Branches     6851     6837      -14     
==========================================
- Hits        20294    20290       -4     
  Misses      19700    19700              
  Partials      725      725              
Flag Coverage Δ
checkbox-ng 69.87% <100.00%> (∅)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Hook25 Hook25 force-pushed the endof_session_collection branch from 9d4ee3d to 0e5567f Compare March 18, 2025 15:55
pieqq
pieqq previously approved these changes Mar 25, 2025
@Hook25 Hook25 marked this pull request as draft March 25, 2025 08:18
@Hook25 Hook25 marked this pull request as ready for review March 25, 2025 10:01
pieqq
pieqq previously approved these changes Mar 26, 2025
Copy link
Collaborator

@pieqq pieqq left a comment

Choose a reason for hiding this comment

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

Glad we caught the weird JSON formatting of journalctl for each line in the journal!

+1

@Hook25 Hook25 merged commit 00ae622 into main Mar 27, 2025
28 checks passed
@Hook25 Hook25 deleted the endof_session_collection branch March 27, 2025 14:21
stanley31huang pushed a commit that referenced this pull request Mar 28, 2025
)

* Add new collectors and remove fetch-on-checkpoint mechanism

* Remove also the tests for ResumeSessionHelper8

* Remove dmesg as it is included in journalctl

* Rollback v8 removal and add v9 to disable v8 removal

* Test peeker and resume helper

Minor: fix sloppy copy paste comment

* Fix the actual suspend mechanism and actually test it

* Also test positive branch

* Journalctl json doesn't output json, but json objects stream

* Fix test, all should inherit the parser and exception

* Test weird journalctl output

* Change default value to  instead of
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.

None yet

2 participants