Skip to content

Conversation

@liangxin1300
Copy link
Collaborator

@liangxin1300 liangxin1300 commented Dec 17, 2025

  • Add milliseconds time format to crmsh.log
  • Change the time format of crmsh.log to the same as pacemaker.log
  • Add milliseconds time format to corosync.log

@codecov
Copy link

codecov bot commented Dec 17, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.31%. Comparing base (8dbfabd) to head (eaf4028).

Additional details and impacted files
Flag Coverage Δ
integration 55.64% <ø> (ø)
unit 52.37% <ø> (ø)

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

Files with missing lines Coverage Δ
crmsh/corosync.py 88.02% <ø> (ø)
crmsh/log.py 68.85% <ø> (ø)

☔ 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.

@liangxin1300 liangxin1300 marked this pull request as ready for review December 17, 2025 03:21
@liangxin1300
Copy link
Collaborator Author

liangxin1300 commented Dec 17, 2025

em, the datefmt is conflicted with

@nicholasyang2022 What do you think?

Now I think since crm report can guess the year (see #1295), it's fine to keep the time format the same with pacemaker.log

@liangxin1300 liangxin1300 requested a review from zzhou1 December 17, 2025 03:42
@nicholasyang2022
Copy link
Collaborator

I don't think this is a valid request. Parsing an ISO timestamp is much easier and reliable than guessing the year.

@liangxin1300
Copy link
Collaborator Author

I don't think this is a valid request. Parsing an ISO timestamp is much easier and reliable than guessing the year.

OK, then I will keep the datefmt unchanged, just add milliseconds

@nicholasyang2022
Copy link
Collaborator

OK, then I will keep the datefmt unchanged, just add milliseconds

No, ISO 8601 timestamp forbids milliseconds. I think the original request is to keep consistent format with pacemaker.log. So it won't help to add milliseconds.

This is a suitable use case of /etc/crm.conf. We can add some configurable options for log format.

@liangxin1300
Copy link
Collaborator Author

liangxin1300 commented Dec 18, 2025

No, ISO 8601 timestamp forbids milliseconds.

Add milliseconds

2025-12-18T14:18:22+0800.518 sle16-1 crmsh.bootstrap: INFO: Done (log saved to /var/log/crmsh/crmsh.log on sle16-1)

without milliseconds

2025-12-18T14:20:53+0800 sle16-1 crmsh.ui_context: INFO: crm(live/sle16-1)# status

This is a suitable use case of /etc/crm.conf. We can add some configurable options for log format.

If we add the log_date_format option, what kind of options do we support? syslog and rfc5424(default) ?

@nicholasyang2022
Copy link
Collaborator

2025-12-18T14:18:22+0800.518

This is totally wrong. If you must add milliseconds, it should be 2025-12-18T14:18:22.518+0800.

If we add the log_date_format option, what kind of options do we support? syslog and rfc5424(default) ?

I think it is OK to expose datefmt directly as a configurable option. Just let user to compose their own format using % directives.

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.

2 participants