Skip to content
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

docs: clarify relationship between different features #395

Merged
merged 1 commit into from Nov 17, 2023

Conversation

pohly
Copy link

@pohly pohly commented Nov 16, 2023

What this PR does / why we need it:

With klogr being deprecated there might be some confusion whether textlogger or klog.Background should be used instead.

Release note:

NONE

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Nov 16, 2023
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 16, 2023
@pohly
Copy link
Author

pohly commented Nov 16, 2023

/cc @mythi

@k8s-ci-robot
Copy link

@pohly: GitHub didn't allow me to request PR reviews from the following users: mythi.

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

In response to this:

/cc @mythi

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.

// Package klog implements logging analogous to the Google-internal C++ INFO/ERROR/V setup.
// It provides functions Info, Warning, Error, Fatal, plus formatting variants such as
// Infof. It also provides V-style logging controlled by the -v and -vmodule=file=2 flags.
// Package klog contains the following functionality:
Copy link
Author

@pohly pohly Nov 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've checked with go install golang.org/x/pkgsite/cmd/pkgsite@latest && pkgsite that this renders okay.

@dgrisonnet
Copy link
Member

/triage accepted
/assign @mengjiao-liu

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Nov 16, 2023
@pohly
Copy link
Author

pohly commented Nov 16, 2023

/hold

For second approver.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 16, 2023
Copy link

@mythi mythi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

klog.go Outdated
// be turned into no-ops ([EnableContextualLogging], [NewContext], [FromContext],
// [LoggerWithValues], [LoggerWithName]); if the ability to turn of
// contextual logging is not needed, then go-logr can also be used directly
// - type aliases for go-logr types to simplifies imports in code which uses both (e.g. [Logger])
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// - type aliases for go-logr types to simplifies imports in code which uses both (e.g. [Logger])
// - type aliases for go-logr types to simplify imports in code which uses both (e.g. [Logger])

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Fixed.

@pohly pohly force-pushed the readme-update branch 3 times, most recently from 95101f6 to fdbe4f2 Compare November 17, 2023 07:35
klog.go Outdated
// - helper functions for logging values ([Format]) and managing the state of klog ([CaptureState], [State.Restore])
// - wrappers for [logr] APIs for contextual logging where the wrappers can
// be turned into no-ops ([EnableContextualLogging], [NewContext], [FromContext],
// [LoggerWithValues], [LoggerWithName]); if the ability to turn of
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"turn of" ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, should be "turn off". Fixed.

With klogr being deprecated there might be some confusion whether
textlogger or klog.Background should be used instead.
Copy link
Member

@mengjiao-liu mengjiao-liu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
The preview page looks good to me.
image

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 17, 2023
@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mengjiao-liu, 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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mengjiao-liu
Copy link
Member

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 17, 2023
@mengjiao-liu
Copy link
Member

/hold

For second approver.

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 17, 2023
@k8s-ci-robot k8s-ci-robot merged commit 8dd3f2e into kubernetes:main Nov 17, 2023
17 checks passed
@mengjiao-liu
Copy link
Member

Looks like the block is a little late.😅 sorry, my bad.

Please confirm if you have time. @dims @thockin
If there is any problem, we can submit a new PR to follow up.

codeboten pushed a commit to open-telemetry/opentelemetry-collector-contrib that referenced this pull request Mar 12, 2024
[![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
[@&#8203;pohly](https://togithub.com/pohly) in
[kubernetes/klog#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
[@&#8203;pohly](https://togithub.com/pohly) in
[kubernetes/klog#394
- docs: clarify relationship between different features by
[@&#8203;pohly](https://togithub.com/pohly) in
[kubernetes/klog#395
- Add SafePtr wrapper by [@&#8203;kaisoz](https://togithub.com/kaisoz)
in
[kubernetes/klog#393
- logr v1.4.1 + SetSlogLogger by
[@&#8203;pohly](https://togithub.com/pohly) in
[kubernetes/klog#396

#### New Contributors

- [@&#8203;kaisoz](https://togithub.com/kaisoz) made their first
contribution in
[kubernetes/klog#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>
DougManton pushed a commit to DougManton/opentelemetry-collector-contrib that referenced this pull request Mar 13, 2024
…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
[kubernetes/klog#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
[kubernetes/klog#394
- docs: clarify relationship between different features by
[@&open-telemetry#8203;pohly](https://togithub.com/pohly) in
[kubernetes/klog#395
- Add SafePtr wrapper by [@&open-telemetry#8203;kaisoz](https://togithub.com/kaisoz)
in
[kubernetes/klog#393
- logr v1.4.1 + SetSlogLogger by
[@&open-telemetry#8203;pohly](https://togithub.com/pohly) in
[kubernetes/klog#396

#### New Contributors

- [@&open-telemetry#8203;kaisoz](https://togithub.com/kaisoz) made their first
contribution in
[kubernetes/klog#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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants