Migrated server.go, ipvs/proxier.go(partial) to structured logging

[shivanshuraj1333]

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Fixed incorrectly migrated logs
Migrated some missing logs to structured logging

Which issue(s) this PR fixes:

Ref: #104872

Special notes for your reviewer:

In this PR, I've cleaned up some improperly migrated logs, going forward we need to figure out a way to make every migration follow the guide, #105718 (comment)
Also, this PR contains partial migration of ipvs/proxier.go since it's very huge file, sending another PR for remaining part

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

[k8s-ci-robot]

@shivanshu1333: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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.

[shivanshuraj1333]

/assign @serathius

[shivanshuraj1333]

/wg structured-logging
/area logging
/priority important-soon
/kind cleanup
/cc @kubernetes/wg-structured-logging-reviews

[serathius]

/lgtm

[aojea]

@serathius for education, why replacing the .String() ?

[shivanshuraj1333]

@aojea we don't need to use the stringer while logging, we do it by default in klog ref https://github.com/kubernetes/klog/blob/331d2323a192188a9aca7006c8dedd0c61c03833/klog.go#L816-L840

[shivanshuraj1333]

/retest

[serathius]

@aojea This is a difference in behavior between C like format methods like "Infof" and structured ones like "InfoS".

In C string formats it is responsibility of log caller to decide how to marshal (represent as text/bytes) arguments passed to logger. For example you need to pick proper format "%d" for decimal numbers, for more complicated types like structs you can pick "%v", "%#v", "#+v" or implement stringer interface and use "%s". This allows users to have more control, however it leads to inconsistency throughout codebase.

In structured logging we explicitly delegate decision about marshalling to logging library, we pass whole object and depend on library to pick a best serialization methods (we can add some hints, but they should be applied per type and not per line). Good example is one that @shivanshu1333 linked https://github.com/kubernetes/klog/blob/331d2323a192188a9aca7006c8dedd0c61c03833/klog.go#L816-L840. It's the code that K8s uses to pick a marshalling when writing in default text format. However Kubernetes supports multiple log formats ("text", "json"), so best marshalling format will depend on logging format we use.

By removing .String() we avoid marshaling structs ourselfs and leave it to logging library. It should be able to pick best representation of the struct that we pass which can depend on external factors for example if user prefers logs in text or json.

[aojea]

Thanks both, appreciate it
/approve
we definitively need this structured logging linter running on presubmit :)

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aojea, shivanshu1333

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:

• cmd/kube-proxy/OWNERS [aojea]
• pkg/proxy/ipvs/OWNERS [aojea]

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

[shivanshuraj1333]

using label to squash
/label tide/merge-method-squash