Skip to content

add and use alternative APIs which support contextual logging #126379

@pohly

Description

@pohly

What would you like to be added?

Several staging repos, for example client-go, only have APIs which don't support contextual logging. Changing those APIs is typically not desirable because it can break large parts of the downstream ecosystem. Instead, we need to add alternative APIs (usually called <something>WithContext) which accept a context.

Such a context can replace an existing stop channel, add cancellation when there was no such stop channel, and last but not least be used to retrieve a logger for contextual logging. When using context.WithCancelCause in the caller and context.Cause(ctx) in the callee, additional information about the reason for canceling can be passed down, thus avoiding the uninformative "context canceled" messages.

The process for adding such alternatives is

  • Add an alternative <something>WithContext API to some package in staging

  • Mark the original API with // Contextual logging: <something>WithContext should be used instead of <something> in code which supports contextual logging.

  • Review that change with maintainers and merge it

  • Temporarily replace // Contextual logging: ... with //logcheck:context // ...:

    sed -i -e 's;// *Contextual logging: ;//logcheck:context // ;' -e 's/Handle\(Error\|Crash\)WithContext should be used instead of/Handle\1WithContext or Handle\1WithLogger should be used instead of/'  $(git grep -l "// *Contextual logging: ")
    
  • Run hack/verify-golang-lint.sh to find code which is supposed to support contextual logging and now needs to be updated to use <something>WithContext or (where available and applicable) <something>WithLogger

  • Sometimes tests can or even should (for example, in the package itself) continue to use the function that got replaced. Use common sense to decide and //nolint:logcheck // Intentionally using original function. to suppress such warnings.

  • Fix relevant issues, submit in one or more PRs:

    • try to align with OWNERS files so that a single approver is sufficient for a PR
    • reference https://github.com/kubernetes/kubernetes/issues/126379 in the issue section of the PR description
  • Once all of those PRs are merged, submit a PR which replaces the // Contextual logging: ... with //logcheck:context // ...

This issue will be kept open and (if needed) reopened as long as there is work remaining, which can be checked with:

git grep  -e '// Contextual logging:'

Special notes for contributors:

Please reach out on Slack in the #wg-structured-logging channel if there are questions.

Why is this needed?

Several of our binaries (e.g. kube-controller-manager, kube-scheduler) have been converted to structured, contextual logging. However, they still emit unstructured, context-unaware log output because of the helper packages that they use.

Metadata

Metadata

Labels

good first issueDenotes an issue ready for a new contributor, according to the "help wanted" guidelines.help wantedDenotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.kind/featureCategorizes issue or PR as related to a new feature.sig/instrumentationCategorizes an issue or PR as relevant to SIG Instrumentation.triage/acceptedIndicates an issue or PR is ready to be actively worked on.

Type

No type

Projects

Status

In Progress

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions