enhancements icon indicating copy to clipboard operation
enhancements copied to clipboard

Published 20 hours ago • verified publisher icon kubernetes

KEP-2831: adding beta graduation criteria

Open sallyom opened this issue 2 years ago • 3 comments

  • updates to KEP-2831 to track connected traces in kubelet and beta graduation

  • Issue link: https://github.com/kubernetes/enhancements/issues/2831

/cc @dashpole /cc @saschagrunert /cc @vrutkovs

sallyom avatar Jan 09 '23 14:01 sallyom

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

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

In response to this:

  • updates to KEP-2831 to track connected traces in kubelet and beta graduation

  • Issue link: https://github.com/kubernetes/enhancements/issues/2831

/cc @dashpole /cc @saschagrunert /cc @vrutkovs

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.

k8s-ci-robot avatar Jan 09 '23 14:01 k8s-ci-robot

@sallyom wanna give this one another update? :)

saschagrunert avatar Jan 23 '23 11:01 saschagrunert

In my queue - just waiting for PRR shadow reviewer to take a look first :)

wojtek-t avatar Jan 30 '23 11:01 wojtek-t

👋 I'm shadowing PRR review this time around and gave this a review. It doesn't look like this PR has updated the PRR questionare (which was pretty complete for the alpha stage). Since this is proposing to graduate to beta, have you given any additional thought to the metrics that might inform a rollback or monitoring requirements? With a bad TracingConfiguration, I expect users will things in the kubelet logs to help correct that configuration, but are there any other changes expected as part of this that might introduce better ways to monitor/troubleshoot this feature or understand if they are achieving the SLO (99% of spans delivered)? Not sure if this something that can be done (maybe ref: https://github.com/open-telemetry/opentelemetry-go/issues/2547) , but I think it would be useful to add some clarity around this with an update.

Was also wondering if you added unit tests for the feature gate when this was implemented in alpha? If so, could you link those in an update to the PRR questionnaire?

jeremyrickard avatar Feb 02 '23 04:02 jeremyrickard

/assign @ehashman

logicalhan avatar Feb 02 '23 17:02 logicalhan

/assign

logicalhan avatar Feb 02 '23 17:02 logicalhan

@jeremyrickard @wojtek-t thanks for your review & apologies for not getting to it right away - I've updated based on your feedback, thank you.

sallyom avatar Feb 03 '23 17:02 sallyom

@wojtek-t @ehashman updated based on the latest review/feedback - thank you again

sallyom avatar Feb 09 '23 01:02 sallyom

I'm fine with it from PRR perspective. You still need SIG-level approval though.

/approve PRR

wojtek-t avatar Feb 09 '23 07:02 wojtek-t

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dashpole, ehashman, sallyom, saschagrunert, wojtek-t

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

k8s-ci-robot avatar Feb 09 '23 07:02 k8s-ci-robot

Actually - I see Elana already approved it above. So lgtm-ing to ensure it will make the release.

/lgtm

wojtek-t avatar Feb 09 '23 08:02 wojtek-t