-
Notifications
You must be signed in to change notification settings - Fork 218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add SafePtr wrapper #393
Add SafePtr wrapper #393
Conversation
Welcome @kaisoz! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small nits, looks good overall.
Thanks for the review @pohly , I've added your suggestions 😊 |
Done @pohly, I've rewritten the function description, let me know what you think 😊 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
/lgtm |
/assign @dims For approval. Should we update the OWNERS so that I can approve and remove @serathius? |
/lgtm |
Yes please @pohly |
I'll fix the lint fails. Not sure why the check is failing with 1.19 though, I'll have a closer look |
/assign @pohly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
We might need to do something about the Go version dependency, but let's see whether this triggers another test run.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dims, harshanarayana, kaisoz, pohly 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:
Approvers can indicate their approval by writing |
/ok-to-test |
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [k8s.io/klog/v2](https://togithub.com/kubernetes/klog) | `v2.110.1` -> `v2.120.1` | [![age](https://developer.mend.io/api/mc/badges/age/go/k8s.io%2fklog%2fv2/v2.120.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/go/k8s.io%2fklog%2fv2/v2.120.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/go/k8s.io%2fklog%2fv2/v2.110.1/v2.120.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/go/k8s.io%2fklog%2fv2/v2.110.1/v2.120.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>kubernetes/klog (k8s.io/klog/v2)</summary> ### [`v2.120.1`](https://togithub.com/kubernetes/klog/releases/tag/v2.120.1): Prepare klog release for Kubernetes v1.30 (Take 2) [Compare Source](https://togithub.com/kubernetes/klog/compare/v2.120.0...v2.120.1) #### What's Changed - textlogger: allow caller to override stack unwinding by [@​pohly](https://togithub.com/pohly) in [https://github.com/kubernetes/klog/pull/397](https://togithub.com/kubernetes/klog/pull/397) **Full Changelog**: kubernetes/klog@v2.120.0...v2.120.1 ### [`v2.120.0`](https://togithub.com/kubernetes/klog/releases/tag/v2.120.0): Prepare klog release for Kubernetes v1.30 (Take 1) [Compare Source](https://togithub.com/kubernetes/klog/compare/v2.110.1...v2.120.0) #### What's Changed - OWNERS: remove serathius, add mengjiao-liu, promote pohly by [@​pohly](https://togithub.com/pohly) in [https://github.com/kubernetes/klog/pull/394](https://togithub.com/kubernetes/klog/pull/394) - docs: clarify relationship between different features by [@​pohly](https://togithub.com/pohly) in [https://github.com/kubernetes/klog/pull/395](https://togithub.com/kubernetes/klog/pull/395) - Add SafePtr wrapper by [@​kaisoz](https://togithub.com/kaisoz) in [https://github.com/kubernetes/klog/pull/393](https://togithub.com/kubernetes/klog/pull/393) - logr v1.4.1 + SetSlogLogger by [@​pohly](https://togithub.com/pohly) in [https://github.com/kubernetes/klog/pull/396](https://togithub.com/kubernetes/klog/pull/396) #### New Contributors - [@​kaisoz](https://togithub.com/kaisoz) made their first contribution in [https://github.com/kubernetes/klog/pull/393](https://togithub.com/kubernetes/klog/pull/393) **Full Changelog**: kubernetes/klog@v2.110.1...v2.120.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "on tuesday" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/open-telemetry/opentelemetry-collector-contrib). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yMzguMSIsInVwZGF0ZWRJblZlciI6IjM3LjIzOC4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9--> --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: opentelemetrybot <107717825+opentelemetrybot@users.noreply.github.com>
…1721) [![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [k8s.io/klog/v2](https://togithub.com/kubernetes/klog) | `v2.110.1` -> `v2.120.1` | [![age](https://developer.mend.io/api/mc/badges/age/go/k8s.io%2fklog%2fv2/v2.120.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/go/k8s.io%2fklog%2fv2/v2.120.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/go/k8s.io%2fklog%2fv2/v2.110.1/v2.120.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/go/k8s.io%2fklog%2fv2/v2.110.1/v2.120.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>kubernetes/klog (k8s.io/klog/v2)</summary> ### [`v2.120.1`](https://togithub.com/kubernetes/klog/releases/tag/v2.120.1): Prepare klog release for Kubernetes v1.30 (Take 2) [Compare Source](https://togithub.com/kubernetes/klog/compare/v2.120.0...v2.120.1) #### What's Changed - textlogger: allow caller to override stack unwinding by [@&open-telemetry#8203;pohly](https://togithub.com/pohly) in [https://github.com/kubernetes/klog/pull/397](https://togithub.com/kubernetes/klog/pull/397) **Full Changelog**: kubernetes/klog@v2.120.0...v2.120.1 ### [`v2.120.0`](https://togithub.com/kubernetes/klog/releases/tag/v2.120.0): Prepare klog release for Kubernetes v1.30 (Take 1) [Compare Source](https://togithub.com/kubernetes/klog/compare/v2.110.1...v2.120.0) #### What's Changed - OWNERS: remove serathius, add mengjiao-liu, promote pohly by [@&open-telemetry#8203;pohly](https://togithub.com/pohly) in [https://github.com/kubernetes/klog/pull/394](https://togithub.com/kubernetes/klog/pull/394) - docs: clarify relationship between different features by [@&open-telemetry#8203;pohly](https://togithub.com/pohly) in [https://github.com/kubernetes/klog/pull/395](https://togithub.com/kubernetes/klog/pull/395) - Add SafePtr wrapper by [@&open-telemetry#8203;kaisoz](https://togithub.com/kaisoz) in [https://github.com/kubernetes/klog/pull/393](https://togithub.com/kubernetes/klog/pull/393) - logr v1.4.1 + SetSlogLogger by [@&open-telemetry#8203;pohly](https://togithub.com/pohly) in [https://github.com/kubernetes/klog/pull/396](https://togithub.com/kubernetes/klog/pull/396) #### New Contributors - [@&open-telemetry#8203;kaisoz](https://togithub.com/kaisoz) made their first contribution in [https://github.com/kubernetes/klog/pull/393](https://togithub.com/kubernetes/klog/pull/393) **Full Changelog**: kubernetes/klog@v2.110.1...v2.120.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "on tuesday" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/open-telemetry/opentelemetry-collector-contrib). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yMzguMSIsInVwZGF0ZWRJblZlciI6IjM3LjIzOC4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9--> --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: opentelemetrybot <107717825+opentelemetrybot@users.noreply.github.com>
What this PR does / why we need it:
If a nil pointer value is provided when logging, it will be dereferenced leading to a panic message. This PR adds a simple wrapper called
SafePtr
which helps preventing this by returning a nil interface in case the provided pointer is nil, or the same pointer if it's not nil.This function is available when compiling with Go >= 1.18 since it uses generics.
Release note: