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

Feat CCT-965: Include timezone in the logs #3475

Merged
merged 1 commit into from
Dec 2, 2024

Conversation

grunwmar
Copy link

In subscription-manager/src/rhsm/logutil.py:

  • New loggin formatter class RhsmISO8601Formatter is made to ensure that time stamp entry is in ISO-8810 format and that it contains info about milliseconds and time zone.

  • Then variable LOG_FORMATTER is made and RhsmISO8601Formatter(LOG_FORMAT) is assigned to it. Then distributed to places where .setFormatter(Formatter(LOG_FORMAT, ...)) occured.

CARD CCT-965

Copy link
Contributor

@ptoscano ptoscano left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! It seems to work fine.

I have notes some things to tweak, however they are mostly stylistic.

note.txt Outdated Show resolved Hide resolved
src/rhsm/logutil.py Outdated Show resolved Hide resolved
src/rhsm/logutil.py Outdated Show resolved Hide resolved
src/rhsm/logutil.py Outdated Show resolved Hide resolved
src/rhsm/logutil.py Outdated Show resolved Hide resolved
@grunwmar grunwmar force-pushed the feat/965 branch 4 times, most recently from 14adec7 to d86505a Compare November 28, 2024 12:52
Copy link
Contributor

@ptoscano ptoscano left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Waiting for QE verification before merging.

Copy link
Contributor

@zpetrace zpetrace left a comment

Choose a reason for hiding this comment

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

Hi, based on the pre-verification against this PR only rhsm.log gets updated timestamps while rhsmcertd.log doesn't - more info in the corresponding RHEL ticket.

@ptoscano
Copy link
Contributor

ptoscano commented Dec 2, 2024

Hi, based on the pre-verification against this PR only rhsm.log gets updated timestamps while rhsmcertd.log doesn't - more info in the corresponding RHEL ticket.

ACK, thanks. I'll merge this one (which is good), and deal with the C bits for rhsmcertd.log in a separate PR.

In `subscription-manager/src/rhsm/logutil.py`i:

* New loggin formater class `RhsmISO8601Formatter` was made
to ensure that time stamp entry is in ISO-8810 format and
that it contains info about milliseconds and time zone.

CARD CCT-965
@ptoscano ptoscano merged commit 55e250d into candlepin:main Dec 2, 2024
21 checks passed
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.

4 participants