-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
contextual logging blog post #32656
contextual logging blog post #32656
Conversation
✅ Pull request preview available for checking
To edit notification comments on pull requests, go to your Netlify site settings. |
601b15b
to
dbf8925
Compare
The date and final title still needs to be decided. Other than that the text is ready for review. |
dbf8925
to
a1c2aa1
Compare
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.
I'm confident that we should actually publish this article under https://k8s.dev/blog/, and then mirror it to https://blog.k8s.io/
It should still be part of release comms but we'd also want to involve SIG Contributor Experience and make sure that we only publish the article on the right day.
Code has to be modified to take advantage of contextual logging. The | ||
traditional klog calls for logging continue to work unchanged but then don't | ||
support contextual logging because output still goes through the global klog | ||
logger. The [migration |
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.
The migration guide doesn't seem to have anything specific about contextual logging. I couldn't locate any specific text that dealt with deriving logger from Context or using APIs like FromName etc.
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.
I should have mentioned that kubernetes/community#6560 needs to be merged first.
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.
It is indeed more targeted towards developers. Does that mean that I should move the post into a different PR? Where?
Publishing it in both blogs makes sense, too. For example, I was only following the older blog.k8s.io but not k8s.dev/blog. |
I would copy the article text into a PR that targets https://github.com/kubernetes/contributor-site Please be sure to /hold that other PR in light of kubernetes/contributor-site#281 Then reference one PR from the other and we can get them published on the same day. Update this PR with a canonical URL in the front matter, that matches the expected URL of the primary blog once it publishes. |
Let's pick a date first. When do you want the two blog posts to get published? Is the "contextual-logging" slug and title okay although the post also touches on progress with structured logging and mentions other klog enhancements? The majority of the enhancements are for contextual logging and it makes for a more unique title compared to, say, "logging enhancements in Kubernetes in 1.24". |
a1c2aa1
to
5565b4d
Compare
@mickeyboxell could you assign a tentative date? That will help @pohly move this work forward. |
I think this kind of question is best posed on the other PR, so that folks from SIG ContribEx can help answer them. This PR should turn into just mirroring the text of the other PR, perhaps with minor changes. |
@sftim Is there an official way for me to assign a date to a blog post other than suggesting an edit to the slug and blog title? I would like to aim to have a draft done by 26-April and release some time the following week. |
I have copied the post into kubernetes/contributor-site#304 with the 2022-05-25 date and also updated it here. |
79febe3
to
b2e9304
Compare
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.
Initial comments/suggestions. Hope you find them useful.
|
||
Before Kubernetes 1.24, some log calls in kube-scheduler still used `klog.Info` | ||
for multi-line strings to avoid the unreadable output. Now kube-scheduler has | ||
been converted completely to structured logging. |
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.
Do we want to say that kube-scheduler now implements structured logging? Or would it be using structured logging? I don't think convert is the right term here.
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.
The log calls have been converted.
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.
I've replace the last sentence with:
Now all log calls have been updated to support structured logging.
|
||
This enables additional use cases: | ||
|
||
- The caller can attach a prefix |
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.
This section as a whole is confusing. Are we intending to say that the caller can attach a prefix WithName
and additional values from the WithValues
prefix to the logger that it then passes into a function? Also, with the "and" in between do we imply that WithValues
is necessary when we pass the WithName
prefix?
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.
Are we intending to say that the caller can attach a prefix WithName and additional values from the WithValues prefix to the logger that it then passes into a function?
Yes.
Also, with the "and" in between do we imply that WithValues is necessary when we pass the WithName prefix?
No. That's an "and/or".
I've rewritten the first part of this use case.
Contextual logging is an enhancement that landed as alpha in Kubernetes 1.24. Because it targets primarily developers of Kubernetes at this point, publishing it under https://k8s.dev/blog on the same day as it also gets published on the main blog (see kubernetes/website#32656) makes sense.
@divya-mohan0209: let me take your suggestions over to kubernetes/contributor-site#304 and continue the discussion there. Otherwise I'll end up making changes to the same text in two different PRs 😢 |
Contextual logging is an enhancement that landed as alpha in Kubernetes 1.24. Because it targets primarily developers of Kubernetes at this point, publishing it under https://k8s.dev/blog on the same day as it also gets published on the main blog (see kubernetes/website#32656) makes sense.
Contextual logging is an enhancement that landed as alpha in Kubernetes 1.24. Because it targets primarily developers of Kubernetes at this point, publishing it under https://k8s.dev/blog on the same day as it also gets published on the main blog (see kubernetes/website#32656) makes sense.
/hold Let's finalize kubernetes/contributor-site#304 first, then I'll update here. |
Contextual logging is an enhancement that landed as alpha in Kubernetes 1.24. Because it targets primarily developers of Kubernetes at this point, publishing it under https://k8s.dev/blog on the same day as it also gets published on the main blog (see kubernetes/website#32656) makes sense.
This is a copy of https://github.com/kubernetes/contributor-site/blob/11c75c25cfb0aa1f50686e40f4a64e8ccfbb8a44/content/en/blog/2022/2022-05-25-contextual-logging.md with the canonical URL added to the header.
ab353d9
to
1ec1d50
Compare
@sftim, @mickeyboxell: this needs to be merged by tomorrow to meet the publishing date. I'll unhold kubernetes/contributor-site#304 tomorrow, so that blog post should go out on time. /hold cancel |
Thanks /lgtm |
LGTM label has been added. Git tree hash: ee127fbec872cf410dd2a429fbaab38a97dca619
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sftim 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 |
Contextual logging is an enhancement that landed as alpha in Kubernetes 1.24. Because it targets primarily developers of Kubernetes at this point, publishing it under https://k8s.dev/blog on the same day as it also gets published on the main blog (see kubernetes/website#32656) makes sense.
kubernetes/enhancements#3077: contextual logging