search

kubernetes-sigs / sig-storage-lib-external-provisioner

Apache License 2.0
548 stars 174 forks source link

Support structured logging: broadcaster & eventRecorder #171

Closed bells17 closed 6 months ago

bells17 commented 6 months ago

What type of PR is this?

/kind feature

What this PR does / why we need it:

In the following PR, support for contextual logging was implemented, but the broadcaster and eventRecorder were not updated to support contextual logging. https://github.com/kubernetes-sigs/sig-storage-lib-external-provisioner/pull/154

Therefore, I made changes to enable the use of contextual logging within the broadcaster and eventRecorder as well.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

https://github.com/kubernetes/kubernetes/pull/120729

Does this PR introduce a user-facing change?:

Signature of NewProvisionController has been changed to support contextual logging for eventRecorder. This is a breaking change that requires updates to code that calls this function.
bells17 commented 6 months ago

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

https://github.com/kubernetes/community/blob/master/contributors/devel/sig-instrumentation/migration-to-structured-logging.md#what-to-include-in-the-pull-request

k8s-ci-robot commented 6 months ago

@bells17: GitHub didn't allow me to request PR reviews from the following users: kubernetes/wg-structured-logging-reviews.

Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to [this](https://github.com/kubernetes-sigs/sig-storage-lib-external-provisioner/pull/171#issuecomment-2093445826): >/wg structured-logging >/area logging >/priority important-longterm >/kind cleanup >/cc @kubernetes/wg-structured-logging-reviews > >https://github.com/kubernetes/community/blob/master/contributors/devel/sig-instrumentation/migration-to-structured-logging.md#what-to-include-in-the-pull-request 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.
k8s-ci-robot commented 6 months ago

@bells17: The label(s) area/logging cannot be applied, because the repository doesn't have them.

In response to [this](https://github.com/kubernetes-sigs/sig-storage-lib-external-provisioner/pull/171#issuecomment-2093445826): >/wg structured-logging >/area logging >/priority important-longterm >/kind cleanup >/cc @kubernetes/wg-structured-logging-reviews > >https://github.com/kubernetes/community/blob/master/contributors/devel/sig-instrumentation/migration-to-structured-logging.md#what-to-include-in-the-pull-request 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.
bells17 commented 6 months ago

/cc @jsafrane

https://github.com/kubernetes-sigs/sig-storage-lib-external-provisioner/pull/154#discussion_r1575873607

jsafrane commented 6 months ago

Signature of NewProvisionController changes and therefore it's a breaking change. Please update the release note accordingly.

bells17 commented 6 months ago

@jsafrane, I'm thinking of using the following text for the release notes. Do you think this is appropriate?

Signature of NewProvisionController has been changed to support contextual logging for eventRecorder. This is a breaking change that requires updates to code that calls this function.
jsafrane commented 6 months ago

Signature of NewProvisionController has been changed to support contextual logging for eventRecorder. This is a breaking change that requires updates to code that calls this function.

That would work nicely.

bells17 commented 6 months ago

@jsafrane Thank you! I've updated the release note.

jsafrane commented 6 months ago

/lgtm /approvee

jsafrane commented 6 months ago

/approve

k8s-ci-robot commented 6 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bells17, jsafrane

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](https://github.com/kubernetes-sigs/sig-storage-lib-external-provisioner/blob/master/OWNERS)~~ [jsafrane] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment