refactor: enable JSON logging in authcontroller
What type of PR is this?
What this PR does / why we need it:
JSON logging support. More details are highlighted in https://github.com/karmada-io/karmada/issues/6230
Which issue(s) this PR fixes:
Part of https://github.com/karmada-io/karmada/issues/6230
Special notes for your reviewer: NA
Does this PR introduce a user-facing change?:
NONE
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please assign xishanyongye-chang for approval. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
:warning: Please install the to ensure uploads and comments are reliably processed by Codecov.
Codecov Report
Attention: Patch coverage is 50.00000% with 9 lines in your changes missing coverage. Please review.
Project coverage is 49.05%. Comparing base (
cf1a15c) to head (d8a03dc).
| Files with missing lines | Patch % | Lines |
|---|---|---|
| ...controllers/unifiedauth/unified_auth_controller.go | 50.00% | 9 Missing :warning: |
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@ Coverage Diff @@
## master #6458 +/- ##
=======================================
Coverage 49.04% 49.05%
=======================================
Files 687 687
Lines 56031 56038 +7
=======================================
+ Hits 27478 27487 +9
+ Misses 26772 26770 -2
Partials 1781 1781
| Flag | Coverage Δ | |
|---|---|---|
| unittests | 49.05% <50.00%> (+<0.01%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
please review - @jabellard
kindly ping @jabellard
Hi @RainbowMango - can this be merged in?
Well, this PR tries to ingest the information injected by the controller-runtime framework. I like this idea, so that we can have more useful information in log. But this also brings concerns like hard to maintain a consistent logger in the whole code base. So, now let's focus on migrating the existing logs to structural logs, just like what we did at #6435.
More details, you can refer to the discussion at https://github.com/karmada-io/karmada/pull/6435#discussion_r2149147779.
Let me know what you think.
Sure, let's start with the log migration like we did in #6435.
Sure, let's start with the log migration like we did in #6435.
@abhinav-1305 , please take a look and address when you get a change.
/assign
Sure, let's start with the log migration like we did in #6435.
@abhinav-1305 , please take a look and address when you get a change.
Hey @abhinav-1305 , any updates?
kindly ping @abhinav-1305~
@jabellard is this the last item on the tracking list?
kindly ping @abhinav-1305~
@jabellard is this the last item on the tracking list?
Yes. I've allowed ample grace period and am hoping @abhinav-1305 can complete this task, but have not heard back in over a month.
Will allow some more time by waiting a few days. If I don't hear back by then, I have someone who's interested in contributing and will pass the ticket along so we can wrap up the feature on time.
Sounds good to me. I hope this will be included in the release v1.15.0, which will be cut on August 31st, by the way.
Will allow some more time by waiting a few days. If I don't hear back by then, I have someone who's interested in contributing and will pass the ticket along so we can wrap up the feature on time.
@jabellard it's time to go with plan B :)
Will allow some more time by waiting a few days. If I don't hear back by then, I have someone who's interested in contributing and will pass the ticket along so we can wrap up the feature on time.
@jabellard it's time to go with plan B :)
Yup. Will push PR in a few to close the party!
/close in favor of #6700
@RainbowMango: Closed this PR.
In response to this:
/close in favor of #6700
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.