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

rhsmcertd: use ISO 8601 timestamps for the log file #3491

Merged
merged 2 commits into from
Jan 15, 2025

Conversation

ptoscano
Copy link
Contributor

@ptoscano ptoscano commented Jan 8, 2025

Print the timestamp in the rhsmcertd.log log file using an ISO 8601
format with subseconds precision. The implementation needs multiple
passes/fixes due to the limitations of the C APIs.

Card ID: CCT-965

Copy link

github-actions bot commented Jan 8, 2025

Coverage

Coverage (computed on Fedora latest) •
FileStmtsMissCoverMissing
TOTAL17432446174% 
report-only-changed-files is enabled. No files were changed during this commit :)

Tests Skipped Failures Errors Time
2416 14 💤 0 ❌ 0 🔥 31.163s ⏱️

@jsefler
Copy link
Contributor

jsefler commented Jan 8, 2025

[root@kvm-03-guest23 ~]# rpm -q subscription-manager
subscription-manager-1.30.3-1.el10.x86_64
[root@kvm-03-guest23 ~]# rhsmcertd --now
[root@kvm-03-guest23 ~]# tail -1 /var/log/rhsm/rhsmcertd.log
Wed Jan 8 17:40:33 2025 [ERROR] Unable to get lock, exiting

^^^^^^^^^^^^^^^^^^^^^ ORIGINAL TIME STAMP FORMAT

[root@kvm-03-guest23 ~]# yum update subscription-manager --enablerepo=rhsm_RHEL10.0 -y --quiet

Upgraded:
libdnf-plugin-subscription-manager-1.30.3+8.gdba81aa0a-1.git.0.680c108.x86_64
python3-cloud-what-1.30.3+8.gdba81aa0a-1.git.0.680c108.x86_64
python3-subscription-manager-rhsm-1.30.3+8.gdba81aa0a-1.git.0.680c108.x86_64
subscription-manager-1.30.3+8.gdba81aa0a-1.git.0.680c108.x86_64

[root@kvm-03-guest23 ~]# date
Wed Jan 8 05:42:20 PM EST 2025
[root@kvm-03-guest23 ~]# rhsmcertd --now
[root@kvm-03-guest23 ~]# tail -1 /var/log/rhsm/rhsmcertd.log
2025-01-08T17:42:39.322-05:00 [ERROR] Unable to get lock, exiting

^^^^^^^^^^^^^^^^^^^^^ NEW ISO 8601 TIME STAMP FORMAT

This PR successfully writes timestamps to /var/rhsm/rhsmcertd.log in ISO 8601 format as desired on a RHEL-10 system.

@ptoscano ptoscano force-pushed the ptoscano/rhsmcertd-logging branch from dba81aa to 364217d Compare January 9, 2025 12:45
Copy link
Contributor

@pkoprda pkoprda 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, I just left some comments what could be improved.

src/daemons/rhsmcertd.c Outdated Show resolved Hide resolved
src/daemons/rhsmcertd.c Outdated Show resolved Hide resolved
src/daemons/rhsmcertd.c Outdated Show resolved Hide resolved
@ptoscano ptoscano force-pushed the ptoscano/rhsmcertd-logging branch from 364217d to 0a76814 Compare January 14, 2025 12:35
Rather than modifying and returning the static buffer of asctime(),
provide a buffer for it on which to copy the resulting timestamp.
Because of this, give the function a new name that indicates what it
does now.

While the static buffer of asctime() is still used, this will make it
possible to change the implementation of this helper function to (also)
not rely on that.

There should be no behaviour changes.
Print the timestamp in the rhsmcertd.log log file using an ISO 8601
format with subseconds precision. The implementation needs multiple
passes/fixes due to the limitations of the C APIs.

Card ID: CCT-965
@ptoscano ptoscano force-pushed the ptoscano/rhsmcertd-logging branch from 0a76814 to e9acccb Compare January 15, 2025 10:48
@ptoscano
Copy link
Contributor Author

/packit retest-failed

Copy link
Contributor

@pkoprda pkoprda left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @ptoscano

@pkoprda pkoprda merged commit bedc697 into main Jan 15, 2025
20 of 22 checks passed
@ptoscano ptoscano deleted the ptoscano/rhsmcertd-logging branch January 15, 2025 16:12
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.

3 participants