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

logging: use human readable timestamp in JSON based log formats #88741

Closed
fabiog1901 opened this issue Sep 26, 2022 · 10 comments · Fixed by #104265
Closed

logging: use human readable timestamp in JSON based log formats #88741

fabiog1901 opened this issue Sep 26, 2022 · 10 comments · Fixed by #104265
Assignees
Labels
A-observability-inf C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@fabiog1901
Copy link
Contributor

fabiog1901 commented Sep 26, 2022

Currently, the timestamp in the JSON based log messages is using epoch, which is surely parseable by computers but not so much by humans. More often then not, however, devops engs read through log files on the server, rather than on Splunk.

It would be good to give an option to use something like ISO 8601 instead of epoch. Something like 2022-09-26 13:28:45.123456789 is a good example. No T in the middle and no need to add the tz since we default to UTC.

gz#14106

Jira issue: CRDB-19901

Epic CRDB-21266

@fabiog1901 fabiog1901 added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Sep 26, 2022
@dhartunian
Copy link
Collaborator

@fabiog1901 would it be possible for a customer to configure two separate file sinks for the same channels and use json for one and crdb-v2 for another? The latter would be much easier to read by humans and the former could be reserved for use with automation etc.

@dhartunian
Copy link
Collaborator

Another option is to use the debug merge-logs feature to transform the JSON log output.

@fabiog1901
Copy link
Contributor Author

fabiog1901 commented Jan 9, 2023

True, but these would be customer workarounds, and long term I'd prefer we address it db side. Also, we would have to double write every line of log, or run a command to transform the logs. Neither is truly appealing. a flag to specify how to show the timestamp would be ideal IMHO

@thtruo
Copy link
Contributor

thtruo commented Jan 10, 2023

Hey @fabiog1901 the team had a quick discussion around scoping potential solutions. Ideally we should introduce a new flag in the CLI command and introduce a cluster setting to specify log timestamp formats. However, for the sake of scoping down and getting something out sooner, we're considering starting with a cluster setting first. The advantage of this is that it won't require the cluster to restart in order for you to view logs with different timestamp formats.

Before we go down this path, we wanted to validate this and get your feedback. Any concerns here? cc @data-matt who might be interested in chiming in as well

@fabiog1901
Copy link
Contributor Author

No issue with cluster setting, I prefer it over a CLI flag. I however trust more your team's experience to choose the best approach :-)

@data-matt
Copy link

Any reason we can't have it in the logging yaml file?

@thtruo
Copy link
Contributor

thtruo commented Jan 20, 2023

No reason not to include it in the logging yaml file @data-matt :) but IIUC this will still require a cluster to restart. So we'll consider first adding a cluster setting to start

@knz
Copy link
Contributor

knz commented Jun 2, 2023

The original description of this issue mentions ISO 8601.

Note that in ISO 8601, the date and time must be separated with a capital T, and the UTC timezone must be indicated with the final Z after the time.

Using a space and no final Z is an allowance permitted by RFC 3339, not ISO 8601.

@data-matt
Copy link

@knz , I think we would be happy with anything closer to human readable ISO 8601 would work for me.

@fabiog1901 will most likely agree.

@knz
Copy link
Contributor

knz commented Jun 2, 2023

yes, i understood this. thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-observability-inf C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants