Configure contextual and structured logging for NEG controller #1746

gauravkghildiyal · 2022-07-11T22:37:35Z

This PR incorporates changes suggested by structured logging (KEP) and contextual logging (KEP) into the NEG controller, which improves the overall observability of the logs.

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

Changes introduced:

Update klog version from v1 to v2
All NEG controller log messages will be prefixed by NEGController
Example:
```
"NEGController: Starting network endpoint group controller"
```
Various sub-components like Syncer, ReadinessReflector, ClusterL4ILBEndpointsCalculator etc will also have their component name prefixed.
Example:
```
"NEGController/Syncer: Stopping NEG syncer for service port"
...
"NEGController/ReadinessReflector: Starting NEG readiness reflector"
```

Syncer (as well as other some other sub-component logs), will include additional context like the name of the service and NEG.
Example:

"NEGController/Syncer: Stats for NEG" service="default/neg-demo-svc" negName="my-neg-name" description="desired NEG endpoints" stats="3 endpoints in zone \"us-central1-c\""

k8s-ci-robot · 2022-07-11T22:37:42Z

Welcome @gauravkghildiyal!

It looks like this is your first PR to kubernetes/ingress-gce 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/ingress-gce has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2022-07-11T22:37:43Z

Hi @gauravkghildiyal. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

gauravkghildiyal · 2022-07-11T22:43:53Z

/assign @swetharepakula

It would make sense to review the commits separately. The delta looks large, but the major contributor in this delta is the changes to the vendor directory introduced in the first commit "Update klog version to v2". The second commit "Add contextual logging to NEG controller and its components" is the one which includes the changes to the logs.

swetharepakula · 2022-07-12T22:05:31Z

/ok-to-test

swetharepakula

Thanks Gaurav! Comments are mostly on improving the log level of log lines and some suggestions to make the log line clearer.

cmd/glbc/main.go

pkg/neg/manager.go

pkg/neg/readiness/poller.go

pkg/neg/syncers/transaction.go

spencerhance · 2022-07-28T22:05:15Z

lgtm

swetharepakula · 2022-07-28T22:39:52Z

pkg/neg/controller.go

 			return
 		}
 		if endpointSlice, ok = tombstone.Obj.(*discovery.EndpointSlice); !ok {
-			klog.Errorf("Unexpected tombstone object type: %T, expected *discovery.EndpointSlice", obj)
+			c.logger.Error(nil, "Unexpected tombstone object type: %T, expected *discovery.EndpointSlice", "objectTypeFound", fmt.Sprintf("%T", obj))


remove the format %T in the error message.

swetharepakula · 2022-07-28T22:46:15Z

pkg/neg/syncers/transaction.go

@@ -541,16 +547,17 @@ func mergeTransactionIntoZoneEndpointMap(endpointMap map[string]negtypes.Network

 // logStats logs aggregated stats of the input endpointMap
 func (s *transactionSyncer) logStats(endpointMap map[string]negtypes.NetworkEndpointSet, desc string) {
-	stats := []string{}
+	stats := []interface{}{}


nit: var stats []interface{}

swetharepakula · 2022-07-28T22:47:20Z

Thanks for all your work Gaurav. A few more small comments and this should be ready to merge.

swetharepakula

Thanks for the change!

/lgtm
/approve

k8s-ci-robot · 2022-07-29T20:02:38Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gauravkghildiyal, swetharepakula

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [swetharepakula]

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

Configure contextual and structured logging for NEG controller

This PR is the continuation of kubernetes#1746(contextual logging for components in NEG controller). * Functions will accept a logger object from its caller, so the prefix will be determined based on the caller objects.

Update klog version to v2

b8f11ed

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 11, 2022

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 11, 2022

k8s-ci-robot requested review from kl52752 and thockin July 11, 2022 22:38

k8s-ci-robot assigned swetharepakula Jul 11, 2022

gauravkghildiyal mentioned this pull request Jul 12, 2022

Add minor logs to track processing track for neg controller #1747

Merged

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 12, 2022

swetharepakula reviewed Jul 26, 2022

View reviewed changes

gauravkghildiyal force-pushed the contextLogging branch 2 times, most recently from 6473054 to 892bf61 Compare July 28, 2022 20:59

swetharepakula reviewed Jul 28, 2022

View reviewed changes

Add contextual logging to NEG controller and its components

149e5e1

gauravkghildiyal force-pushed the contextLogging branch from 892bf61 to 149e5e1 Compare July 29, 2022 08:12

swetharepakula reviewed Jul 29, 2022

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 29, 2022

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 29, 2022

k8s-ci-robot merged commit c577834 into kubernetes:master Jul 29, 2022

gauravkghildiyal deleted the contextLogging branch July 29, 2022 20:34

gauravkghildiyal pushed a commit to gauravkghildiyal/ingress-gce that referenced this pull request Aug 26, 2022

Merge pull request kubernetes#1746 from gauravkghildiyal/contextLogging

43ec4e1

Configure contextual and structured logging for NEG controller

sawsa307 mentioned this pull request Oct 24, 2023

Replace klog with context logging for NEG Controller #2297

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Configure contextual and structured logging for NEG controller #1746

Configure contextual and structured logging for NEG controller #1746

gauravkghildiyal commented Jul 11, 2022 •

edited

Loading

k8s-ci-robot commented Jul 11, 2022

k8s-ci-robot commented Jul 11, 2022

gauravkghildiyal commented Jul 11, 2022

swetharepakula commented Jul 12, 2022

swetharepakula left a comment

spencerhance commented Jul 28, 2022

swetharepakula Jul 28, 2022

gauravkghildiyal Jul 29, 2022

swetharepakula Jul 28, 2022

gauravkghildiyal Jul 29, 2022

swetharepakula commented Jul 28, 2022

swetharepakula left a comment

k8s-ci-robot commented Jul 29, 2022

Configure contextual and structured logging for NEG controller #1746

Configure contextual and structured logging for NEG controller #1746

Conversation

gauravkghildiyal commented Jul 11, 2022 • edited Loading

k8s-ci-robot commented Jul 11, 2022

k8s-ci-robot commented Jul 11, 2022

gauravkghildiyal commented Jul 11, 2022

swetharepakula commented Jul 12, 2022

swetharepakula left a comment

Choose a reason for hiding this comment

spencerhance commented Jul 28, 2022

swetharepakula Jul 28, 2022

Choose a reason for hiding this comment

gauravkghildiyal Jul 29, 2022

Choose a reason for hiding this comment

swetharepakula Jul 28, 2022

Choose a reason for hiding this comment

gauravkghildiyal Jul 29, 2022

Choose a reason for hiding this comment

swetharepakula commented Jul 28, 2022

swetharepakula left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jul 29, 2022

gauravkghildiyal commented Jul 11, 2022 •

edited

Loading