Attention is currently required from: laforge.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/5815 )
Change subject: logging: add timestamp fmt 'date' = 2018-01-16,01:44:34.681
......................................................................
Patch Set 7:
This change is ready for review.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/5815
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Icbd5192ea835e24b12fe057cc1ab56e9572d75c0
Gerrit-Change-Number: 5815
Gerrit-PatchSet: 7
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Tue, 28 Mar 2023 03:54:39 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/5861 )
Change subject: logging vty: add 'logging print timestamp', deprecate other timestamp cmds
......................................................................
Patch Set 4:
This change is ready for review.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/5861
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I58c792dda3cbcf8618648ba4429c27fa398a9e15
Gerrit-Change-Number: 5861
Gerrit-PatchSet: 4
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Mar 2023 03:54:32 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: laforge.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/5857 )
Change subject: logging: use enum to set timestamp format
......................................................................
Patch Set 6:
This change is ready for review.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/5857
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I70ecee543c266df54191d3e86401466cd1a5c0a6
Gerrit-Change-Number: 5857
Gerrit-PatchSet: 6
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Tue, 28 Mar 2023 03:54:24 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: laforge, fixeria, pespin.
Hello Jenkins Builder, laforge,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmocore/+/32043
to look at the new patch set (#2).
Change subject: logging: add 'logging timezone (localtime|utc)'
......................................................................
logging: add 'logging timezone (localtime|utc)'
Add ability to print logging timestamps in UTC.
I want to test logging timestamps in vty transcript tests. To get
deterministic timestamps, I can use osmo_gettimeofday_override().
However, so far we log in the server's local time zone: the dates
printed after osmo_gettimeofday_override() still change according to the
locally configured timezone -- not good for regression testing. For UTC,
it is always the same.
The intended use is for regression testing of timestamps, but maybe
logging in UTC may be useful to some user out there, too.
Change-Id: I7f868b47bf8f8dfcf85e735f490ae69b18111af4
---
M configure.ac
M include/osmocom/core/logging.h
M src/core/libosmocore.map
M src/core/logging.c
M src/vty/logging_vty.c
M tests/logging/logging_vty_test.vty
6 files changed, 118 insertions(+), 12 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/43/32043/2
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32043
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I7f868b47bf8f8dfcf85e735f490ae69b18111af4
Gerrit-Change-Number: 32043
Gerrit-PatchSet: 2
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: laforge, fixeria, pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32043 )
Change subject: logging: add 'logging timezone (localtime|utc)'
......................................................................
Patch Set 1:
(8 comments)
Patchset:
PS1:
> I would probably have used a bool rather than an enum as I see no chance we ever want to deal with a […]
yes but i figured after converting lots of bools to enums, it would be weird to go on to add a new bool again in something that *could* conceptually have more than two preferences...
PS1:
(new patch set coming up)
File include/osmocom/core/logging.h:
https://gerrit.osmocom.org/c/libosmocore/+/32043/comment/2d045bdd_9b9fe8af
PS1, Line 400: /* Set timezone for logging of timestamps: 0 for local time, 1 for UTC. Other values are reserved for future
> 0 and 1? don't we have enums for that in this patch?
right, thx
File src/core/logging.c:
https://gerrit.osmocom.org/c/libosmocore/+/32043/comment/7cf368c8_78c510f5
PS1, Line 474: return 0;
> I'd really return an error if the requested timestamp cannot be obtained.
the code before this patch didn't care about the return value of localtime_r(), but if you insist...
https://gerrit.osmocom.org/c/libosmocore/+/32043/comment/a14a672d_6516ed7b
PS1, Line 528: } else if (target->print_timestamp && get_timestamp(&tv, &tm, target) == 0) {
> you are calling get_timestamp twice here every time we log something. […]
There is an unlikely corner case where it gets called twice, i.e. only if calling the first time failed. In all practical cases it is called once.
This code changes to a cleaner switch() in upcoming
https://gerrit.osmocom.org/c/libosmocore/+/5857/5/src/core/logging.c#520https://gerrit.osmocom.org/c/libosmocore/+/32043/comment/f17936bf_4162a716
PS1, Line 874: target->timezone = timezone;
> Ack
i don't agree. i would not want to duplicate some enum validation code here. It can be assumed that the caller only passes valid enum values, and if it is an unknown enum value, then the logging.c switch() deals with that. Otherwise we always have to keep this function "artificially" updated with what logging.c supports.
File src/vty/logging_vty.c:
https://gerrit.osmocom.org/c/libosmocore/+/32043/comment/89946725_1c316042
PS1, Line 203: log_set_timezone(tgt, LOG_TIMEZONE_UTC);
> Ack
if i can defend above argument, then no here as well =)
File tests/logging/logging_vty_test.vty:
https://gerrit.osmocom.org/c/libosmocore/+/32043/comment/5058ecde_1a21934f
PS1, Line 75: logging_vty_test# logging timestamp ?
> See https://gerrit.osmocom.org/c/libosmocore/+/32044.
pau has a solid point here, i failed to add usability tests for the 'logging timestamp' command itself, which now unconvered that i so far fail to write back the new vty command.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32043
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I7f868b47bf8f8dfcf85e735f490ae69b18111af4
Gerrit-Change-Number: 32043
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Mar 2023 03:27:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment