Skip to content

Conversation

AlexFenlon
Copy link
Contributor

@AlexFenlon AlexFenlon commented Aug 19, 2025

Proposed changes

Add the option to change the timestamp of the logs in json and text format to unix in seconds, milliseconds and nanoseconds

eg. JSON with unix
{"time":"1755613721","level":"DEBUG","source":{"file":"controller.go","line":1019},"msg":"Syncing default/my-release-nginx-ingress-controller-7tj7f"}

default json
{"time":"2025-08-19T14:29:17.492029463Z","level":"DEBUG","source":{"file":"controller.go","line":1019},"msg":"Syncing default/my-release-nginx-ingress-controller-7tj7f"}

TEXT with unix-ms
time=1755619167305 level=DEBUG source=controller.go:1019 msg="Syncing default/my-release-nginx-ingress-controller-c828v"

default TEXT
time=2025-08-19T14:31:26.485Z level=DEBUG source=controller.go:1019 msg="Syncing default/my-release-nginx-ingress-controller-7tj7f"

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@AlexFenlon AlexFenlon requested a review from a team as a code owner August 19, 2025 15:54
@github-actions github-actions bot added enhancement Pull requests for new features/feature enhancements go Pull requests that update Go code labels Aug 19, 2025
Copy link

codecov bot commented Aug 19, 2025

Codecov Report

❌ Patch coverage is 67.50000% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 53.38%. Comparing base (54159f9) to head (773a3a0).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
cmd/nginx-ingress/flags.go 0.00% 12 Missing ⚠️
cmd/nginx-ingress/main.go 96.42% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8168      +/-   ##
==========================================
+ Coverage   53.08%   53.38%   +0.29%     
==========================================
  Files          90       90              
  Lines       21778    21950     +172     
==========================================
+ Hits        11561    11717     +156     
- Misses       9740     9756      +16     
  Partials      477      477              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@javorszky javorszky left a comment

Choose a reason for hiding this comment

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

tiny tiny bit, but otherwise g2g

Key: slog.TimeKey,
Value: slog.Int64Value(t.UnixNano()),
}
case "default":
Copy link
Contributor

Choose a reason for hiding this comment

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

this isn't needed here, because

  1. either we've already made sure that the time log format is one of the four allowed values, in which case if it's not the other three, then the switch's default arm is enough, or
  2. we haven't, and if we pass in some weird value like iso8601, which should not be allowed, that will also be ignored and rfc3339 will be printed, as per the original value coming from slog

@vepatel
Copy link
Contributor

vepatel commented Aug 19, 2025

imo we can extend --log-format to accept text-unix and json-unix instead of adding a very specific flag

@AlexFenlon AlexFenlon linked an issue Aug 19, 2025 that may be closed by this pull request
@AlexFenlon AlexFenlon marked this pull request as draft August 20, 2025 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Pull requests for new features/feature enhancements go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow NIC json logging to use UNIX time
3 participants