Skip to content

Conversation

@britaniar
Copy link
Collaborator

@britaniar britaniar commented Apr 30, 2025

Description of your changes

Fixes #

I have: setup controller runtime logger to remove trace stack and share logs

  • Run make reviewable to ensure this PR is ready for review.

How has this code been tested

Special notes for your reviewer

Using DevMode to make it more human readable.

Example logs:
image

Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com>
@britaniar britaniar marked this pull request as ready for review April 30, 2025 22:14
@ryanzhang-oss ryanzhang-oss requested a review from Copilot April 30, 2025 22:19
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR sets up the controller-runtime logger using the zap package to improve log readability by removing trace stack clutter. It adds the necessary import and logger initialization in both the memberagent and hubagent main packages.

  • In cmd/memberagent/main.go, the logger is configured with zap in development mode.
  • In cmd/hubagent/main.go, similar logger initialization is added.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
cmd/memberagent/main.go Added zap import and initialized the controller-runtime logger using DevMode.
cmd/hubagent/main.go Added zap import and similarly set up the controller-runtime logger.

})

// Set up controller-runtime logger
ctrl.SetLogger(zap.New(zap.UseDevMode(true)))
Copy link

Copilot AI Apr 30, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider making the logger mode configurable (e.g., via a command-line flag or environment variable) instead of hard-coding dev mode to better support production environments.

Suggested change
ctrl.SetLogger(zap.New(zap.UseDevMode(true)))
ctrl.SetLogger(zap.New(zap.UseDevMode(*devMode)))

Copilot uses AI. Check for mistakes.
}

// Set up controller-runtime logger
ctrl.SetLogger(zap.New(zap.UseDevMode(true)))
Copy link

Copilot AI Apr 30, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider making the zap logger's development mode configurable to allow switching between development and production logging settings.

Suggested change
ctrl.SetLogger(zap.New(zap.UseDevMode(true)))
ctrl.SetLogger(zap.New(zap.UseDevMode(devMode)))

Copilot uses AI. Check for mistakes.
@codecov
Copy link

codecov bot commented Apr 30, 2025

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
cmd/hubagent/main.go 0.00% 2 Missing ⚠️
cmd/memberagent/main.go 0.00% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@britaniar britaniar merged commit 4fb56df into kubefleet-dev:main Apr 30, 2025
17 of 19 checks passed
@britaniar britaniar deleted the setupLogger branch April 30, 2025 23:36
audrastump pushed a commit to audrastump/kubefleet that referenced this pull request May 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants