-
Notifications
You must be signed in to change notification settings - Fork 87
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
chore: add k8sattributes processor for cluster metrics and events #606
Conversation
WalkthroughThe pull request modifies the OpenTelemetry Collector configuration templates in the Kubernetes infrastructure charts. The changes focus on enhancing the configuration of Kubernetes attributes and resource detection for deployments. A new function Changes
Sequence DiagramsequenceDiagram
participant OtelCollector as OpenTelemetry Collector
participant K8sAttributes as Kubernetes Attributes Processor
participant ResourceDetection as Resource Detection Processor
OtelCollector->>K8sAttributes: Enable Kubernetes Attributes
K8sAttributes-->>OtelCollector: Configure Attributes Extraction
OtelCollector->>ResourceDetection: Apply Resource Detection
ResourceDetection-->>OtelCollector: Detect and Add Resources
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
charts/k8s-infra/templates/_config.tpl (1)
795-809
: Document the rationale for separate deployment configuration.The implementation looks good, but consider expanding the TODO comment to better explain:
- Why a separate k8sattributes config is needed for deployment mode
- What "clearly defined" mode of operation means
- The differences from the base k8sattributes config (e.g., disabled passthrough, metadata-only extraction)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
charts/k8s-infra/templates/_config.tpl
(3 hunks)
🔇 Additional comments (2)
charts/k8s-infra/templates/_config.tpl (2)
92-94
: LGTM! The k8sattributes configuration is correctly integrated.The addition of k8sattributes processor for deployment mode is well-placed and aligns with the PR objectives.
681-696
: LGTM! The implementation follows established patterns.The function correctly handles all pipeline types and maintains proper processor ordering.
The processor is missing from the cluster metrics and events pipeline and the node filter doesn't apply in this context. While we could add a conditional filter, our deployment has way too many roles to properly type it. We need collectors that work across different setups - daemonset, deployment, statefulset, and singleton with clearly defined responsibilities. Instead of forcing it into the current definition, I opted for a separate deployment definition to handle this at the cost of a few lines of duplication.
Summary by CodeRabbit
New Features
Refactor