Skip to content

fix: use flagset when initializing klog#6329

Open
hairyhum wants to merge 1 commit into
mainfrom
hairyhum/klog-flagset
Open

fix: use flagset when initializing klog#6329
hairyhum wants to merge 1 commit into
mainfrom
hairyhum/klog-flagset

Conversation

@hairyhum
Copy link
Copy Markdown
Contributor

Issue Reference

NA

Description

When using kargo as a library, logger will initialize klog in the init() function with default flagset. This means that if any other tool initializes the klog with default flagset in the main() function, that will panic with flag redefined error.

klog recommends calling InitFlags in main() or using a flagset. This PR uses a "kargo_logger" flagset for InitFlags

Checklist

  • The PR is linked to an existing issue.
  • The linked issue has no blocking labels (kind/proposal,
    needs discussion, needs research, maintainer only, area/security,
    size/large, size/x-large, size/xx-large).
  • I have added or updated tests as appropriate.
  • I have added or updated documentation as appropriate.

AI Use Disclosure

Select one:

  • This PR was written by a human without AI assistance.
  • This PR was written by a human with AI assistance. A human has reviewed every line prior to opening the PR.
  • This PR was written by an AI with human supervision. A human has reviewed every line prior to opening the PR.
  • This PR was written entirely by AI. No human has reviewed this prior to opening the PR.

Sign-Off

  • All commits are signed off (git commit -s) (required)
  • All commits are cryptographically signed (git commit -S) (encouraged)

When using kargo as a library, logger will initialize klog in the init() function with default flagset.
This means that if any other tool initializes the klog with default flagset in the main() function,
that will panic with `flag redefined` error.

klog recommends calling `InitFlags` in `main()` or using flagset, going for the second option here.

Signed-off-by: Daniil Fedotov <[email protected]>
@netlify
Copy link
Copy Markdown

netlify Bot commented May 21, 2026

Deploy Preview for docs-kargo-io ready!

Name Link
🔨 Latest commit bb2cab2
🔍 Latest deploy log https://app.netlify.com/projects/docs-kargo-io/deploys/6a0f5a0cf084eb000887911f
😎 Deploy Preview https://deploy-preview-6329.docs.kargo.io
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@kargo-governance-bot kargo-governance-bot Bot added needs/area Issue or PR needs to be labeled to indicate what parts of the code base are affected needs/kind Issue or PR needs to be labeled to clarify its nature needs/priority Priority has not yet been determined; a good signal that maintainers aren't fully committed policy/no-linked-issue labels May 21, 2026
@kargo-governance-bot
Copy link
Copy Markdown

Automated Policy Notice

This pull request has been converted to a draft because it does not reference an unblocked issue. Maintainers do not routinely review drafts.

All contributions to Kargo require a linked, unblocked issue. This ensures that proposed changes have been reviewed by maintainers before effort is invested.

To move forward:

  1. Open a Bug Report or Feature Request
  2. Wait for a maintainer to review the issue and remove any blocking labels
  3. Update this PR's description, linking to the issue using Closes #<number>
  4. Ensure the implementation in this PR is consistent with discussion points in the linked issue
  5. Mark the PR ready for review

See the Contributor Guide for full details.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 21, 2026

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 58.05%. Comparing base (4caa1ee) to head (bb2cab2).

Files with missing lines Patch % Lines
pkg/logging/logger.go 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6329   +/-   ##
=======================================
  Coverage   58.05%   58.05%           
=======================================
  Files         499      499           
  Lines       41748    41749    +1     
=======================================
+ Hits        24238    24239    +1     
  Misses      16050    16050           
  Partials     1460     1460           

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

@krancour
Copy link
Copy Markdown
Member

@hairyhum you're supposed to be exempt from the PR policy. Not sure what happened, but I will find out and fix it.

@krancour krancour marked this pull request as ready for review May 21, 2026 19:26
@krancour krancour requested a review from a team as a code owner May 21, 2026 19:26
@kargo-governance-bot kargo-governance-bot Bot marked this pull request as draft May 21, 2026 19:26
@akuity akuity deleted a comment from kargo-governance-bot Bot May 21, 2026
@akuity akuity deleted a comment from kargo-governance-bot Bot May 21, 2026
@akuity akuity deleted a comment from kargo-governance-bot Bot May 21, 2026
Copy link
Copy Markdown
Contributor

@EronWright EronWright left a comment

Choose a reason for hiding this comment

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

LGTM, but let me wait for non-draft

@EronWright EronWright self-requested a review May 21, 2026 19:59
@krancour krancour marked this pull request as ready for review May 21, 2026 22:21
@krancour krancour added priority/normal This is the priority for most work kind/bug Something isn't working as intended; If unsure that something IS a bug, start a discussion instead and removed needs/kind Issue or PR needs to be labeled to clarify its nature needs/priority Priority has not yet been determined; a good signal that maintainers aren't fully committed needs/area Issue or PR needs to be labeled to indicate what parts of the code base are affected labels May 21, 2026
@krancour krancour added this to the v1.11.0 milestone May 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Something isn't working as intended; If unsure that something IS a bug, start a discussion instead priority/normal This is the priority for most work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants