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

Add new logging format option, 'json-ts', for JSON formatted logs with timestamps #24307

Merged

Conversation

learnitall
Copy link
Contributor

This commit adds a new log formatter named 'json-ts', which can be used to enable json logs that include timestamps. Normally, timestamps for logs can be collected through the use of k8s container logging system, ie by passing --timestamps to kubectl logs. Relying on k8s for log timestamps is not always available however, therefore it is useful to have the ability to add timestamps if needed in special cases.

One case in which this is useful is for programmatically parsing through the cilium-cni log file, in order to observe and record CNI events.

Correct me if I'm wrong, but I'm pretty sure this needs the following labels:

  • release-note/misc
  • area/misc

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Thanks for contributing!

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Mar 10, 2023
@learnitall learnitall force-pushed the pr/learnitall/add-json-ts-format branch 3 times, most recently from 7dd3294 to 179a43a Compare March 10, 2023 23:41
@learnitall learnitall force-pushed the pr/learnitall/add-json-ts-format branch 2 times, most recently from c68cde5 to 482c14a Compare March 13, 2023 18:44
@learnitall learnitall marked this pull request as ready for review March 13, 2023 18:44
@learnitall learnitall requested a review from a team as a code owner March 13, 2023 18:44
@learnitall learnitall requested a review from rolinh March 13, 2023 18:44
@learnitall
Copy link
Contributor Author

Looks like pushing the images failed due to a bad gateway error from quay:

Error: signing [quay.io/cilium/docker-plugin-ci@sha256:58222f0bf0673f28834046b6317b57540fba20eeb68df400bbd32128cfd525ef]: accessing image: GET https://quay.io/v2/: unexpected status code 502 Bad Gateway: <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2 Final//EN">

I'm going to rebase to re-trigger the tests.

@learnitall learnitall force-pushed the pr/learnitall/add-json-ts-format branch from 482c14a to bf35cc4 Compare March 14, 2023 15:06
@rolinh rolinh added area/misc Impacts miscellaneous areas of the code not otherwise owned by another area. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Mar 29, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Mar 29, 2023
@learnitall learnitall force-pushed the pr/learnitall/add-json-ts-format branch from bf35cc4 to 06d46fe Compare March 30, 2023 16:46
This commit adds a new log formatter named 'json-ts', which can be used
to enable json logs that include timestamps. Normally, timestamps for
logs can be collected through the use of k8s container logging system,
ie by passing `--timestamps` to `kubectl logs`. Relying on k8s for log
timestamps is not always available however, therefore it is useful to
have the ability to add timestamps if needed in special cases.

One case in which this is useful is for programmatically parsing through
the `cilium-cni` log file, in order to observe and record CNI events.

Signed-off-by: Ryan Drew <ryan.drew@isovalent.com>
@learnitall learnitall force-pushed the pr/learnitall/add-json-ts-format branch from 06d46fe to 5ba11e0 Compare April 3, 2023 20:30
@learnitall
Copy link
Contributor Author

Hey @cilium/cli just want to double-check and see if there is anything you need from me to get this merged?

@michi-covalent
Copy link
Contributor

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Apr 4, 2023
@michi-covalent michi-covalent merged commit c311616 into cilium:master Apr 4, 2023
57 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/misc Impacts miscellaneous areas of the code not otherwise owned by another area. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants