serathius commented 4 years ago

Enhancement Description

Define standard structure for Kubernetes log messages, add methods to klog to enforce this structure, add ability to configure Kubernetes components to produce logs in JSON format and initiate migration to structured logging.

Kubernetes Enhancement Proposal: Structured Logging KEP
Primary contact (assignee): @serathius
Responsible SIGs: sig-instrumentation
Enhancement target (which target equals to which milestone):
- Alpha release target 1.19
- Beta release target 1.23
- GA release target 1.27

Alpha

* [x] Implement `text` format by adding `klog.InfoS`, `klog.ErrorS` functions to k8s.io/klog (https://github.com/kubernetes/klog/issues/125, @yuzhiquan) * [x] Implement `klog.KObj`, `klog.KRef` to k8s.io/klog (https://github.com/kubernetes/klog/issues/127, @tahsinrahman) * [x] Upgrade klog to v2 (https://github.com/kubernetes/kubernetes/issues/89961. @dims) * [x] Implement `--logging-format` flag in k8s.io/component-base (https://github.com/kubernetes/kubernetes/issues/89190, @yuzhiquan) * [x] Prepare migration instructions (@serathius, https://github.com/kubernetes/community/pull/4793) * [x] Migrate selected log messages to `klog.InfoS`using [Migration Instructions](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-instrumentation/migration-to-structured-logging.md) * [x] staging/src/k8s.io/apiserver ([httplog.go:161](https://github.com/kubernetes/kubernetes/blob/15c3f1b11/staging/src/k8s.io/apiserver/pkg/server/httplog/httplog.go#L161), [get.go:251](https://github.com/kubernetes/kubernetes/blob/15c3f1b11/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/get.go#L251), [watcher.go:363](https://github.com/kubernetes/kubernetes/blob/15c3f1b11/staging/src/k8s.io/apiserver/pkg/storage/etcd3/watcher.go#L363), [watcher.go:256](https://github.com/kubernetes/kubernetes/blob/15c3f1b11/staging/src/k8s.io/apiserver/pkg/storage/etcd3/watcher.go#L256)) (@AkashSarda, https://github.com/kubernetes/kubernetes/pull/91833) * [x] pkg/controller/ ([garbagecollector.go:404](https://github.com/kubernetes/kubernetes/blob/15c3f1b11/pkg/controller/garbagecollector/garbagecollector.go#L404), [garbagecollector.go:517](https://github.com/kubernetes/kubernetes/blob/15c3f1b11/pkg/controller/garbagecollector/garbagecollector.go#L517), [controller_utils.go:603](https://github.com/kubernetes/kubernetes/blob/15c3f1b11/pkg/controller/controller_utils.go#L603), [replica_set.go:561](https://github.com/kubernetes/kubernetes/blob/15c3f1b11/pkg/controller/replicaset/replica_set.go#L561), [deployment_controller.go:484](https://github.com/kubernetes/kubernetes/blob/15c3f1b11/pkg/controller/deployment/deployment_controller.go#L484), [deployment_controller.go:575](https://github.com/kubernetes/kubernetes/blob/15c3f1b11/pkg/controller/deployment/deployment_controller.go#L575), [endpoints_controller.go:340](https://github.com/kubernetes/kubernetes/blob/15c3f1b11/pkg/controller/endpoint/endpoints_controller.go#L340), [cloud_cidr_allocator.go:233](https://github.com/kubernetes/kubernetes/blob/15c3f1b11/pkg/controller/nodeipam/ipam/cloud_cidr_allocator.go#L233), [ttl_controller.go:271](https://github.com/kubernetes/kubernetes/blob/15c3f1b11/pkg/controller/ttl/ttl_controller.go#L271), [replica_set.go:597](https://github.com/kubernetes/kubernetes/blob/15c3f1b11/pkg/controller/replicaset/replica_set.go#L597), [replica_set.go:225](https://github.com/kubernetes/kubernetes/blob/15c3f1b11/pkg/controller/replicaset/replica_set.go#L225)) (@tahsinrahman, https://github.com/kubernetes/kubernetes/pull/91576) * [x] /plugin/pkg/auth/authorizer ([node_authorizer.go:197](https://github.com/kubernetes/kubernetes/blob/15c3f1b11/plugin/pkg/auth/authorizer/node/node_authorizer.go#L197)) (@ankeesler, https://github.com/kubernetes/kubernetes/pull/91591) * [x] pkg/scheduler ([scheduler.go:604](https://github.com/kubernetes/kubernetes/blob/15c3f1b11/pkg/scheduler/scheduler.go#L604), [factory.go:431](https://github.com/kubernetes/kubernetes/blob/15c3f1b11/pkg/scheduler/factory.go#L431), [scheduler.go:715](https://github.com/kubernetes/kubernetes/blob/15c3f1b11/pkg/scheduler/scheduler.go#L715)) (@tanjunchen, https://github.com/kubernetes/kubernetes/pull/91679) * [x] pkg/kubelet ([kubelet_getters.go:173](https://github.com/kubernetes/kubernetes/blob/15c3f1b11/pkg/kubelet/kubelet_getters.go#L173)) (@hase1128, https://github.com/kubernetes/kubernetes/pull/91624) * [x] staging/src/k8s.io/client-go ([event.go:278](https://github.com/kubernetes/kubernetes/blob/15c3f1b11/staging/src/k8s.io/client-go/tools/record/event.go#L278)) (@KobayashiD27, https://github.com/kubernetes/kubernetes/pull/91712) * [x] Implement `json` log format (@yuzhiquan, https://github.com/kubernetes/kubernetes/issues/91490) * [x] Update flag help to list both format options (@serathius, https://github.com/kubernetes/kubernetes/pull/92177) * [x] Add `--logging-format` flag to kubernetes components * [x] kube-apiserver (https://github.com/kubernetes/kubernetes/pull/91501, @tahsinrahman) * [x] kube-controller-manager (https://github.com/kubernetes/kubernetes/pull/91521, @SataQiu) * [x] kubelet (https://github.com/kubernetes/kubernetes/pull/91532, @afrouzMashaykhi) * [x] kube-scheduler (https://github.com/kubernetes/kubernetes/pull/91522, @SataQiu) * [x] Write documentation for new logging format (https://github.com/kubernetes/website/pull/21202, @serathius) Follow-up from main tasks: * [x] https://github.com/kubernetes/klog/issues/153 (@tahsinrahman) * [x] https://github.com/kubernetes/klog/issues/156 (@yuzhiquan) * [x] https://github.com/kubernetes/klog/issues/158 (@tahsinrahman) * [x] Update klog in k/k to collect bug fixes above (@serathius, https://github.com/kubernetes/kubernetes/pull/91792) * [x] https://github.com/kubernetes/klog/issues/165 (@physcat) * [x] Update klog to collect bug fixes above (@serathius, https://github.com/kubernetes/kubernetes/pull/92554) * [x] Remove locking from logFormatRegistry as it's not needed (@rahulchheda, https://github.com/kubernetes/kubernetes/pull/92347) * [x] Validate if incompatible klog flags were used (@rahulchheda, https://github.com/kubernetes/kubernetes/pull/92394) * [x] Fix JSON logger verbosity (@yuzhiquan, https://github.com/kubernetes/kubernetes/pull/92788)

Beta

* [x] Directories can be marked as migrated, preventing regression via verify script in hack. (https://github.com/kubernetes/kubernetes/issues/98975, @adisky) * [x] Kubernetes Logging documentation is updated for structured logging. (https://github.com/kubernetes/community/issues/5593) * [x] Structured logging interface design is verify by migrating one whole component (Kubelet) to structured logging. (https://github.com/kubernetes/kubernetes/issues/98976) * [x] Log volume (i.e. size of output) will be benchmarked and documented for kubelet in non-JSON and JSON format. (https://github.com/kubernetes/kubernetes/issues/99803) * [x] Dedicated periodic e2e test scenario with Json enabled (https://github.com/kubernetes/kubernetes/issues/99802) * [x] Refactor Options to use LoggingConfiguration (https://github.com/kubernetes/kubernetes/issues/99267) * [x] Json format should support same set of feature flags as text format, minus those we will decide to deprecte. (https://github.com/kubernetes/kubernetes/issues/99270) * [x] Add source code location to JSON logs (https://github.com/kubernetes/kubernetes/issues/102353) * [x] Move klog flags to Logs flag group (https://github.com/kubernetes/kubernetes/issues/99265) * [x] Enabling Json format should work from component start (https://github.com/kubernetes/kubernetes/issues/100152)

Follow ups:

Migrate trace.go)

Please let me know if someone is interested in working on one of the tasks.

serathius commented 4 years ago

/sig instrumentation

serathius commented 4 years ago

/help

k8s-ci-robot commented 4 years ago

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

Please ensure the request meets the requirements listed here.

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

In response to [this](https://github.com/kubernetes/enhancements/issues/1602): >/help Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.

yuzhiquan commented 4 years ago

interested in

tanjunchen commented 4 years ago

/cc

serathius commented 4 years ago

/milestone v1.19

k8s-ci-robot commented 4 years ago

@serathius: You must be a member of the kubernetes/milestone-maintainers GitHub team to set the milestone. If you believe you should be able to issue the /milestone command, please contact your and have them propose you as an additional delegate for this responsibility.

In response to [this](https://github.com/kubernetes/enhancements/issues/1602#issuecomment-599659439): >/milestone v1.19 Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.

justaugustus commented 4 years ago

/milestone v1.19

hase1128 commented 4 years ago

/cc

serathius commented 4 years ago

KEP was marked as "implementable", so we can start working on it. I have created first two subtasks within specific repositories. I will add more as work progresses and new tasks are unblocked.

If your interested in contributing please volunteer directly on subtasks or ask for specific task here.

gavinfish commented 4 years ago

/cc

serathius commented 4 years ago

After discussing offline with @dims we concluded that upgrading klog will be non-trivial as we need to upgrade all references at once. Upgrade cannot be done partially as it would break klog configuration.

We considered two options:

[Preferred] Properly update of all dependencies
Aliasing in go modules

As we have a lot of time before 1.19 cut we would like to do it properly. I will create subtask with upgrade plan.

fenggw-fnst commented 4 years ago

/cc

johnbelamaric commented 4 years ago

@serathius Enhancements shadow for v1.19 here. Do you see this as moving to alpha in 1.19?

serathius commented 4 years ago

Yes, I think we are on track. We are finishing klog upgrade, after that next milestones will be really fast to implement.

johnbelamaric commented 4 years ago

Thanks, I'll add it to the tracking sheet.

/stage alpha

johnbelamaric commented 4 years ago

Hi @serathius, I was looking over the KEP and noticed that there is no test plan defined. Please add a bit about the test plan in order to target to 1.19. Enhancements freeze is next week, May 19.

Also, thanks for adding the PRR questions from the previous pilot. We have now updated those and integrated that with the updated KEP format. If you can, please take this opportunity to update the KEP to the new format and answer any additional questions. Thanks!

serathius commented 4 years ago

@johnbelamaric prepared update https://github.com/kubernetes/enhancements/pull/1787

johnbelamaric commented 4 years ago

Thanks @serathius for answering the updated questions. When you have a chance please also re-format to the new format. You do not need to do that before the freeze tomorrow though.

savitharaghunathan commented 4 years ago

Hi @serathius - I am Savitha,1.19 Docs lead. Does this enhancement work planned for 1.19 require any new docs (or modifications to existing docs)? If not, can you please update the 1.19 Enhancement Tracker Sheet, or let me know, I can do it for you :) If docs are required, just a friendly reminder that we are looking for a PR against k/website (branch dev-1.19) due by Friday, June 12, it can just be a placeholder PR at this time. Let me know if you have any questions!

serathius commented 4 years ago

We should document structured logging format I will create a placeholder PR

serathius commented 4 years ago

Placeholder created https://github.com/kubernetes/website/pull/21181

serathius commented 4 years ago

With --logging-format flag implemented and migration instructions draft, there are lot's of tasks that were unblocked. Some easy tasks:

Adding logging flag to k8s component (Kubelet little harder, discussed below)
Migrating logging call

Harder tasks:

Writing documentation
Implementing json format (https://github.com/kubernetes/kubernetes/issues/91490)

yuzhiquan commented 4 years ago

I notice that existing flags about log mostly in Global flags, and implement in klog or component/logs, such like:

Global flags:

      --alsologtostderr                                                                                                                                                               
                log to standard error as well as files
  -h, --help                                                                                                                                                                          
                help for kube-scheduler
      --log-backtrace-at traceLocation                                                                                                                                                
                when logging hits line file:N, emit a stack trace (default :0)
      --log-dir string                                                                                                                                                                
                If non-empty, write log files in this directory
      --log-file string                                                                                                                                                               
                If non-empty, use this log file
      --log-file-max-size uint                                                                                                                                                        
                Defines the maximum size a log file can grow to. Unit is megabytes. If the value is 0, the maximum file size is unlimited. (default 1800)
      --log-flush-frequency duration                                                                                                                                                  
                Maximum number of seconds between log flushes (default 5s)
      --logtostderr                                                                                                                                                                   
                log to standard error instead of files (default true)

kube-scheduler or apiserver does not impelment them in component code. So i don't know what's the propose of this subtask adding logging flag to k8s component?

serathius commented 4 years ago

Your right that currently there are a lot of global flags about logging. Those flags belong to klog which was introduced early in k8s development (before forking klog we used glog). With growth of k8s repo global flags started to be problematic (in k/k repo there are over 20 binaries which not always want to share command flags). Now it's an anti pattern to use global flags. I expect that in future when replacing klog, global flags will be removed.

With your change kube-scheduler or kube-apiserver do not implement this flag it requires another step which is the subtask adding logging flag to k8s component. https://github.com/kubernetes/kubernetes/pull/89683 implements special set of functions AddFlags, Validate and Apply that are meant to be invoked within component setup itself allowing picking which flag are needed and easier use outside of kubernetes/kubernetes repo.

Here is a PR that shows how to register additional flag in components https://github.com/kubernetes/kubernetes/pull/85266/files In this effort I decided to split task for flag "implementation" and flag "installation" to speed collection of approvals as different components are managed by different SIGs.

Hope this helps to explain reason and how to add flags to components. Please let me know if you have other questions.

yuzhiquan commented 4 years ago

OK, that's explained, thanks!

serathius commented 4 years ago

Created subtask for json https://github.com/kubernetes/kubernetes/issues/91490 For other tasks feel free to grab them. Please remember to request here them to make sure we don't duplicate work.

hase1128 commented 4 years ago

I try to do pkg/kubelet task in Migrate selected log messages to klog.InfoS

SataQiu commented 4 years ago

/cc

serathius commented 4 years ago

Hey @hase1128, Adding flag to Kubelet is little harder as it requires code to support dynamic configuration. This PR shows additional places that need to be changed https://github.com/kubernetes/kubernetes/pull/85282/files

hase1128 commented 4 years ago

@serathius Thanks the information, but I was going to take a task which migrate to infoS(not the task which adds flag). I try to see also task of flag. if it is difficult for me, I inform you.

serathius commented 4 years ago

@hase1128 Oh, sorry. Your right, I read your commend incorrectly.

AkashSarda commented 4 years ago

@serathius : can I take up migrate to InfoS for Apiserver?

serathius commented 4 years ago

Sure

serathius commented 4 years ago

Small announcement. One of the approvers requested that json format is implemented before we introduce --logging-format flag as it's not very useful without it. We are prioritizing json implementation to make sure we are code complete before freeze.

In mean time feel free to work on migrating log calls based on draft of instructions. https://github.com/kubernetes/community/pull/4793/files?short_path=af969dd#diff-af969ddd1593b66ad7330155ce792feb

tahsinrahman commented 4 years ago

@serathius can i work on migrate log calls for pkg/controller?

serathius commented 4 years ago

Ofc

ankeesler commented 4 years ago

@serathius - I can take /plugin/pkg/auth/authorizer (node_authorizer.go:197) if it hasn't already been claimed OOB. I am most involved in sig-auth so seems like a reasonable fit.

serathius commented 4 years ago

assigned

tanjunchen commented 4 years ago

I take pkg/scheduler for Migrate

KobayashiD27 commented 4 years ago

Can I try to migrate staging/src/k8s.io/client-go (event.go:278)? I assume that this migration is below:

StartLogging(klog.Infof) -> StartLogging(klog.InfoS)
change the variable in staging/src/k8s.io/client-go (event.go:278).

serathius commented 4 years ago

@KobayashiD27 assigned you to staging/src/k8s.io/client-go Please follow instructions here https://github.com/kubernetes/community/pull/4793/files?short_path=af969dd#diff-af969ddd1593b66ad7330155ce792feb

hase1128 commented 4 years ago

I take vendor/k8s.io/utils for Migrate.

serathius commented 4 years ago

Congrats to @hase1128 for getting first migration PR merged.

yuzhiquan commented 4 years ago

Awesome！

rahulchheda commented 4 years ago

Hi @serathius , any other tasks left to be done on this issue?

serathius commented 4 years ago

@rahulchheda, currently all coding tasks are taken. There is one tasks for writing documentation, but I expect it would be pretty hard and not very interesting for new contributors. Still please let me know if your interested.

serathius commented 4 years ago

Logging format was implemented, now we can move forward with PRs adding flags to components! Great work @yuzhiquan on JSON!!

kikisdeliveryservice commented 4 years ago

Hi @serathius !

To follow-up on the email sent to k-dev on Monday, I wanted to let you know that Code Freeze has been extended to Thursday, July 9th. You can see the revised schedule here: https://github.com/kubernetes/sig-release/tree/master/releases/release-1.19 We expect all PRs to be merged by that time. Please let me know if you have any questions. 😄

Best,  Kirsten

serathius commented 4 years ago

Thanks for info @kikisdeliveryservice

kubernetes / enhancements

Structured logging #1602

Enhancement Description