Need to improve the readability of the log about kueue HOT 26 CLOSED

I'm fine waiting for v3 or replacing zapper with klogr.

I don't want to loose contextual logging.

from kueue.

/cc @ahg-g @alculquicondor

from kueue.

/kind cleanup

from kueue.

much agree, klog is soo much better

from kueue.

/assign

from kueue.

Let's reach to some level of agreement before we jump into implementing :)

If this is juts about format, I believe there are different zap configurations we can use. This is actually my first time using zap, so I'm not really sure what's possible.

While klog format might be better, the framework doesn't currently support contextual logging, which simplifies building structured logging a lot.
But this is changing soon https://github.com/kubernetes/enhancements/tree/master/keps/sig-instrumentation/3077-contextual-logging

In the meantime, if we stick with logr for now, I think it will be easier to switch to klog (v3?) once it supports contextual logging.

from kueue.

let's defer this to klog/v3 but keep circling back here. I am a klog fan for sure

from kueue.

is there a better format we can use for zap?

from kueue.

This is why I don't like logr

1.6454820377420967e+09  ERROR   job-reconciler  Unsuspending job        {"job": {"name":"sample-job","namespace":"default"}, "error": "Operation cannot be fulfilled on jobs.batch \"sample-jo
b\": the object has been modified; please apply your changes to the latest version and try again"}                                                                                            sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile                 
        /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:114                                
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler          
        /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:311
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem       
        /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:266
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2             
        /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:227
1.6454820377421455e+09  ERROR   controller.job  Reconciler error        {"reconciler group": "batch", "reconciler kind": "Job", "name": "sample-job", "namespace": "default", "error": "Operat
ion cannot be fulfilled on jobs.batch \"sample-job\": the object has been modified; please apply your changes to the latest version and try again"}
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem       
        /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:266
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2             
        /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:227

had the similar erro/logs on NFD when using logr, and got rid by moving to klog/v2

from kueue.

Can you try to migrate to https://github.com/kubernetes/klog/tree/main/klogr?

from kueue.

I'm fine waiting for v3 or replacing zapper with klogr.

I don't want to loose contextual logging.

Since we are on a hold here
/unassign

from kueue.

You could always give klogr a try

from kueue.

/kind cleanup

from kueue.

What is the status of this?

from kueue.

I disabled the dev mode for the binary, so the output is all structured now:

{"level":"Level(-2)","ts":1649793642.0015297,"logger":"controller.clusterqueue","msg":"Reconciling ClusterQueue","reconciler group":"kueue.x-k8s.io","reconciler kind":"ClusterQueue","name":"cluster-total","namespace":"","clusterQueue":{"name":"cluster-total"}}

It looks like klog made progress towards contextual logging https://github.com/kubernetes/klog/blob/main/contextual.go

Let me ask what's the status of it.

from kueue.

Documentation is up here:

https://github.com/kubernetes/community/blob/master/contributors/devel/sig-instrumentation/migration-to-structured-logging.md

Kueue using it will help it mature.

/help

from kueue.

@alculquicondor:
This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

• Why are we solving this issue?
• To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
• Does this issue have zero to low barrier of entry?
• How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

from kueue.

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

from kueue.

/help
to migrate to klog's contextual logging.

from kueue.

/remove-lifecycle stale
/priority backlog

from kueue.

I just realized that kubernetes uses zap to do json output through klog.

klog's contextual logging implements the logr.Logger interface. So it doesn't look like we benefit much migrating to klog, given that controller-runtime already expects a Logger.

However, there are some logs for which we are receiving the wrong call reference. I'll see if there is something we can do kubernetes-sigs/controller-runtime#1737

from kueue.

I managed to fix it, but we'll have to leave this until there's a controller-runtime release kubernetes-sigs/controller-runtime#1975

from kueue.

/help cancel

from kueue.

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

from kueue.

/lifecycle stale

We should be able to upgrade controller-runtime to have the fix

/help

from kueue.

I'll take a look at this. /assign @kannon92

from kueue.