PR Review: Kubelet Stats OpenTelemetry Input Package

Overall this is a well-structured package that follows the existing patterns in the repository. The package passes elastic-package check validation. I've compared it against the upstream kubeletstatsreceiver and other input packages in this repo. Below are my findings.

🟢 Looks Good

1. Package structure follows the established pattern (LICENSE.txt, manifest.yml, docs, _dev/test/policy, agent/input, img)
2. Configuration coverage is comprehensive - supports all four auth types (none, tls, serviceAccount, kubeConfig)
3. Handlebars template correctly handles conditional rendering for optional fields
4. Documentation is thorough with troubleshooting tips and RBAC examples
5. Test policy validates the expected output structure

🟡 Suggestions (Non-blocking)

1. insecure_skip_verify conditional may output false when not wanted

In input.yml.hbs:16-18, the current pattern:

{{#if insecure_skip_verify}}
    insecure_skip_verify: {{insecure_skip_verify}}
{{/if}}

Since insecure_skip_verify defaults to false in manifest.yml, this conditional will be falsy and won't render. This is actually the desired behavior! However, if a user explicitly sets it to true, it will render correctly. Just noting this is working as expected.

2. Consider using type: duration for interval fields

The nginx_input_otel package uses type: duration for collection_interval and initial_delay, which provides validation. This package uses type: text. Either approach works, but duration type provides additional validation.

manifest.yml:39-45:

- name: collection_interval
  type: text  # Could be: type: duration
  ...

3. RBAC documentation may need nodes/proxy permission

According to upstream docs, when using extra_metadata_labels (like container.id) or node utilization metrics, the RBAC requires access to nodes/proxy in addition to nodes/stats. Consider expanding the RBAC example in docs/README.md:

rules:
- apiGroups: [""]
  resources: ["nodes/stats", "nodes/proxy"]  # nodes/proxy needed for extra_metadata_labels
  verbs: ["get"]

4. Missing tls auth type in k8s_api_config options

The main auth_type field has four options (none, tls, serviceAccount, kubeConfig), but k8s_api_config_auth_type only has three options (missing tls). Based on upstream docs, the K8s API config supports the same auth types.

manifest.yml:119-131:

- name: k8s_api_config_auth_type
  type: select
  options:
    - value: none
      text: None
    - value: serviceAccount  
      text: Service Account
    - value: kubeConfig
      text: Kube Config
    # Consider adding:
    # - value: tls
    #   text: TLS

5. Endpoint default may need adjustment for auth_type: none

The default endpoint is https://localhost:10250 (secure port), but the description mentions port 10255 is used for auth_type: none (read-only). Users selecting none auth would need to manually change the endpoint and protocol to http://localhost:10255.

Consider adding guidance in the description or providing a more explicit warning in the documentation.

⚪ Minor Observations

• The logo SVG looks like a Kubernetes helm logo - which is appropriate for kubelet stats
• Changelog properly links to the originating issue #17140
• The _dev/build/docs/README.md and docs/README.md files are identical (expected for packages without template variables)

Summary

This is a solid contribution that correctly implements the kubeletstats receiver. The suggestions above are all non-blocking improvements. The package is ready for merge from my perspective, though you may want to address the tls auth type omission in k8s_api_config if upstream supports it.

🤖 Review performed by Claude | View job run

This is a well-structured new OTel input package for Kubelet Stats. The package follows the established patterns for input packages in this repository.

The template correctly handles the complex conditional logic for network interface collection and TLS configuration. The policy test validates the default configuration correctly.

One minor documentation improvement suggested (LOW severity, non-blocking).

Why is Claude responding? | Type @claude to interact further

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

This PR adds a well-structured OpenTelemetry input package for collecting Kubelet stats. The implementation follows established patterns used by other OTel input packages in this repository.

The package correctly:

• Defines the kubeletstatsreceiver configuration with appropriate variables
• Supports multiple authentication methods (serviceAccount, TLS, kubeConfig, none)
• Includes proper policy tests
• Provides comprehensive documentation

Two low-severity suggestions are provided as inline comments:

1. The endpoint description could be clarified regarding the port/protocol differences
2. The build template should include the generatedHeader directive for consistency

CI is passing and the package builds successfully.

Why is Claude responding? | Type @claude to interact further

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

The package has a build issue that causes elastic-package lint to fail:

Primary Issue: The docs/README.md file is out of sync with the _dev/build/docs/README.md template. It appears that the docs/README.md was manually edited with additional content (extra metadata labels section, Kubernetes API configuration, node utilization metrics, kubeConfig authentication example) instead of being generated from the template.

Resolution: Either:

1. Copy the additional documentation content from docs/README.md to _dev/build/docs/README.md, then run elastic-package build to regenerate docs/README.md, OR
2. Delete docs/README.md and run elastic-package build to regenerate it from the template (if the simpler template documentation is acceptable)

After resolving, verify with elastic-package lint that the package passes validation.

Why is Claude responding? | Type @claude to interact further

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

The package structure and configuration coverage are good, but there is a blocking issue that must be fixed before merge:

Must Fix:
The docs/README.md file is out of sync with _dev/build/docs/README.md. The docs file contains additional content (extra metadata labels section, Kubernetes API configuration, node utilization metrics documentation) that was manually added instead of being added to the build template. This causes elastic-package lint to fail.

To resolve: Copy the additional documentation content from docs/README.md to _dev/build/docs/README.md, then run elastic-package build to regenerate docs/README.md from the template.

Minor Observation:
The package has been updated to use type: duration for interval fields (which is good), and the manifest now includes the tls option in the k8s_api_config_auth_type select field. These improvements from earlier feedback have been addressed.

Why is Claude responding? | Type @claude to interact further

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

The package follows established patterns for OTel input packages and is well-structured. The template conditionals for the optional metrics block (lines 64-104 in input.yml.hbs) are complex but functionally correct. Configuration options appropriately mirror the upstream kubeletstatsreceiver options. No security, correctness, or significant quality issues identified.

Why is Claude responding? | Type @claude to interact further

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

This PR adds a well-structured OpenTelemetry input package for collecting Kubernetes metrics from the Kubelet API.

The package follows established patterns for OTel input packages in this repository:

• Correct manifest structure with appropriate variables for authentication (serviceAccount, TLS, kubeConfig, none)
• Proper Handlebars template for generating OTel collector configuration
• Policy tests that validate the expected output
• Comprehensive documentation covering configuration, RBAC requirements, and troubleshooting

No blocking issues identified.

The code passes linting and builds successfully. The complex conditional logic in the input template for optional metrics is correctly implemented, though somewhat verbose. The TLS configuration follows the upstream kubeletstatsreceiver format.

Why is Claude responding? | Type @claude to interact further

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

The test file test-k8s-api-config.expected has a component ID mismatch that will cause policy tests to fail. The processor is defined as transform/componentid-1 but the pipeline (line 62) references transform/componentid-0.

Why is Claude responding? | Type @claude to interact further

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

This is a well-structured input package that follows established patterns from similar OTel packages in the repository.

Unresolved Discussion Items:
Several open discussion threads need author/reviewer decisions before merge:

1. Endpoint default value (line 36-38): The current default https://localhost:10250 requires users to always override it. Consider using an environment variable default like https://${env:K8S_NODE_IP}:10250 per the discussion thread, which would work out-of-the-box in typical Kubernetes deployments.

2. metric_groups default (line 100-108): While @mrodm noted upstream defaults cover this, having explicit defaults in the manifest improves discoverability and documentation.

3. metrics YAML field (line 145-160): The field was added for advanced configuration. Awaiting decision on whether to keep it in the initial release.

No blocking technical issues were identified in this review.

Why is Claude responding? | Type @claude to interact further

Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.

👍🏻 nice testing!!