application failover klog structured logging #6566

SujoyDutta · 2025-07-21T20:01:43Z

What type of PR is this?

structured logging support. More details are highlighted in
#6440 and
#6230

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

karmada-bot · 2025-07-21T20:01:48Z

[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 garrybest for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

pkg/controllers/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

gemini-code-assist

Summary of Changes

Hello @SujoyDutta, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request begins the process of adopting structured logging within the application failover controllers. The core change involves updating existing klog statements to use structured logging functions, aiming to make log data more consistent, parsable, and easier to analyze for debugging and monitoring purposes.

Highlights

Structured Logging Adoption: Initiated the migration of klog calls from traditional printf-style formatting (klog.Errorf, klog.Infof) to structured logging equivalents (klog.ErrorS, klog.InfoS).
Improved Log Parsability: Enhanced log messages by converting variable arguments into explicit key-value pairs, which significantly improves machine readability and programmatic analysis of logs.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request migrates logging calls to use klog's structured logging capabilities across the application failover controllers. The changes correctly adopt the InfoS and ErrorS functions. My feedback focuses on improving the consistency and clarity of log keys in crb_application_failover_controller.go to make them more descriptive and align better with structured logging best practices.

gemini-code-assist · 2025-07-21T20:03:18Z

pkg/controllers/applicationfailover/crb_application_failover_controller.go

@@ -59,7 +59,7 @@ type CRBApplicationFailoverController struct {
 // The Controller will requeue the Request to be processed again if an error is non-nil or
 // Result.Requeue is true, otherwise upon completion it will remove the work from the queue.
 func (c *CRBApplicationFailoverController) Reconcile(ctx context.Context, req controllerruntime.Request) (controllerruntime.Result, error) {
-	klog.V(4).Infof("Reconciling ClusterResourceBinding %s.", req.Name)
+	klog.V(4).InfoS("Reconciling ClusterResourceBinding", "name", req.Name)


For consistency and clarity in structured logging, it's better to use a more specific key for the resource name. The corresponding controller for ResourceBinding uses resourceBinding as a key. Here, clusterResourceBinding would be a more descriptive key than name.

Suggested change

klog.V(4).InfoS("Reconciling ClusterResourceBinding", "name", req.Name)

klog.V(4).InfoS("Reconciling ClusterResourceBinding", "clusterResourceBinding", req.Name)

gemini-code-assist · 2025-07-21T20:03:18Z

pkg/controllers/applicationfailover/crb_application_failover_controller.go

@@ -132,7 +132,7 @@ func (c *CRBApplicationFailoverController) syncBinding(ctx context.Context, bind

 	err := c.evictBinding(binding, needEvictClusters)
 	if err != nil {
-		klog.Errorf("Failed to evict binding(%s), err: %v.", binding.Name, err)
+		klog.ErrorS(err, "Failed to evict binding", "binding", binding.Name)


The key binding is a bit generic and could be ambiguous. A more descriptive key like clusterResourceBinding would improve log clarity and consistency with other parts of the codebase.

Suggested change

klog.ErrorS(err, "Failed to evict binding", "binding", binding.Name)

klog.ErrorS(err, "Failed to evict binding", "clusterResourceBinding", binding.Name)

gemini-code-assist · 2025-07-21T20:03:18Z

pkg/controllers/applicationfailover/crb_application_failover_controller.go

@@ -153,7 +153,9 @@ func (c *CRBApplicationFailoverController) evictBinding(binding *workv1alpha2.Cl
 	for _, cluster := range clusters {
 		taskOpts, err := buildTaskOptions(binding.Spec.Failover.Application, binding.Status.AggregatedStatus, cluster, CRBApplicationFailoverControllerName, clustersBeforeFailover)
 		if err != nil {
-			klog.Errorf("failed to build TaskOptions for ClusterResourceBinding(%s) under Cluster(%s): %v", binding.Name, cluster, err)
+			klog.ErrorS(err, "Failed to build TaskOptions for ClusterResourceBinding under Cluster", 
+				"binding", binding.Name, 


The key binding is a bit generic. To improve log clarity and consistency, consider using a more descriptive key like clusterResourceBinding.

Suggested change

"binding", binding.Name,

"clusterResourceBinding", binding.Name,

gemini-code-assist · 2025-07-21T20:03:18Z

pkg/controllers/applicationfailover/crb_application_failover_controller.go

@@ -225,7 +227,7 @@ func (c *CRBApplicationFailoverController) clusterResourceBindingFilter(crb *wor
 	resourceKey, err := helper.ConstructClusterWideKey(crb.Spec.Resource)
 	if err != nil {
 		// Never reach
-		klog.Errorf("Failed to construct clusterWideKey from clusterResourceBinding(%s)", crb.Name)
+		klog.ErrorS(err, "Failed to construct clusterWideKey from clusterResourceBinding", "binding", crb.Name)


The key binding is a bit generic. A more descriptive key like clusterResourceBinding would improve log clarity and consistency.

Suggested change

klog.ErrorS(err, "Failed to construct clusterWideKey from clusterResourceBinding", "binding", crb.Name)

klog.ErrorS(err, "Failed to construct clusterWideKey from clusterResourceBinding", "clusterResourceBinding", crb.Name)

codecov-commenter · 2025-07-21T20:17:48Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 38.88889% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.34%. Comparing base (84359ef) to head (3bd480e).
⚠️ Report is 32 commits behind head on master.

Files with missing lines	Patch %	Lines
...tionfailover/rb_application_failover_controller.go	25.00%	6 Missing ⚠️
...ionfailover/crb_application_failover_controller.go	28.57%	5 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6566      +/-   ##
==========================================
- Coverage   45.36%   45.34%   -0.03%     
==========================================
  Files         687      687              
  Lines       56358    56389      +31     
==========================================
  Hits        25568    25568              
- Misses      29194    29225      +31     
  Partials     1596     1596

Flag	Coverage Δ
unittests	`45.34% <38.88%> (-0.03%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

jabellard · 2025-07-22T00:15:14Z

/assign

jabellard · 2025-07-24T13:42:01Z

@SujoyDutta , thanks for this contribution!

Generally looks good. Please update to use the keys name and namespace (where applicable) in this change set for consistency. Other than that, things look good.

SujoyDutta · 2025-07-24T14:02:17Z

@SujoyDutta , thanks for this contribution!

Generally looks good. Please update to use the keys name and namespace (where applicable) in this change set for consistency. Other than that, things look good.

Thanks @jabellard updated, please take a look and lmk if i missed anything . Thanks !

jabellard · 2025-08-03T16:27:01Z

/lgtm

/cc @RainbowMango for another look.

jabellard · 2025-08-03T16:37:31Z

@SujoyDutta , Also, please make sure to squash your commits after making change to make sure the PR is ready to get merged.

RainbowMango · 2025-08-04T07:19:59Z

In addition, it is recommended to add your email to GitHub Emails, otherwise, GitHub can't recognize you and count your contributions.

PS: It is exactly the reason why GitHub still recognizes you as the first-time contributor (in PR description).

Signed-off-by: sdutta133 <[email protected]> fix lint issues fix cicd Signed-off-by: sdutta133 <[email protected]> consistent naming Signed-off-by: sdutta133 <[email protected]>

karmada-bot · 2025-08-05T18:12:18Z

New changes are detected. LGTM label has been removed.

jabellard · 2025-08-05T21:02:03Z

New changes are detected. LGTM label has been removed.

Generally looks good to me.

/cc @RainbowMango for another look.

karmada-bot requested review from chaunceyjiang and zhzhuang-zju July 21, 2025 20:01

karmada-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 21, 2025

gemini-code-assist bot reviewed Jul 21, 2025

View reviewed changes

SujoyDutta changed the title ~~klog structured logging~~ application failover klog structured logging Jul 21, 2025

gemini-code-assist bot reviewed Jul 21, 2025

View reviewed changes

SujoyDutta force-pushed the sdutta/application-failover-logs branch from 22f77ff to e16fa60 Compare July 21, 2025 21:06

karmada-bot assigned jabellard Jul 22, 2025

jabellard mentioned this pull request Jul 22, 2025

Structured Logging for Karmada Components #6230

Open

34 tasks

karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 3, 2025

klog structured logging

3bd480e

Signed-off-by: sdutta133 <[email protected]> fix lint issues fix cicd Signed-off-by: sdutta133 <[email protected]> consistent naming Signed-off-by: sdutta133 <[email protected]>

SujoyDutta force-pushed the sdutta/application-failover-logs branch from 79ce9de to 3bd480e Compare August 5, 2025 18:12

karmada-bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 5, 2025

	klog.V(4).InfoS("Reconciling ClusterResourceBinding", "name", req.Name)
	klog.V(4).InfoS("Reconciling ClusterResourceBinding", "clusterResourceBinding", req.Name)

	klog.ErrorS(err, "Failed to evict binding", "binding", binding.Name)
	klog.ErrorS(err, "Failed to evict binding", "clusterResourceBinding", binding.Name)

	"binding", binding.Name,
	"clusterResourceBinding", binding.Name,

	klog.ErrorS(err, "Failed to construct clusterWideKey from clusterResourceBinding", "binding", crb.Name)
	klog.ErrorS(err, "Failed to construct clusterWideKey from clusterResourceBinding", "clusterResourceBinding", crb.Name)

application failover klog structured logging #6566

Are you sure you want to change the base?

application failover klog structured logging #6566

Uh oh!

Conversation

SujoyDutta commented Jul 21, 2025

Uh oh!

karmada-bot commented Jul 21, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jabellard commented Jul 22, 2025

Uh oh!

jabellard commented Jul 24, 2025

Uh oh!

SujoyDutta commented Jul 24, 2025

Uh oh!

jabellard commented Aug 3, 2025

Uh oh!

jabellard commented Aug 3, 2025

Uh oh!

RainbowMango commented Aug 4, 2025

Uh oh!

karmada-bot commented Aug 5, 2025

Uh oh!

jabellard commented Aug 5, 2025

Uh oh!

Uh oh!

codecov-commenter commented Jul 21, 2025 •

edited

Loading