fix(metrics): sanitize custom field names before Prometheus & OTLP validation #1290
Open
ooye-sanket wants to merge 3 commits intofalcosecurity:masterfrom
Open
fix(metrics): sanitize custom field names before Prometheus & OTLP validation #1290ooye-sanket wants to merge 3 commits intofalcosecurity:masterfrom
ooye-sanket wants to merge 3 commits intofalcosecurity:masterfrom
Conversation
Replace dots with underscores in custom field names before regex validation to prevent label cardinality mismatches. **Changes:** - Line ~45-50: Sanitize customfields key using `strings.ReplaceAll(i, ".", "_")` - Store sanitized name in variable for reuse in validation and append - Update error log to show both original and sanitized field names **Why:** Custom fields with dots (e.g., `k8s.cluster.name`) were failing validation but still being counted in expected label count, causing panic: "inconsistent label cardinality: expected 8 label values but got 7" **Related:** - Part of fix for issue #3748 - Works in conjunction with handlers.go and otlp_metrics.go fixes Signed-off-by: Sanket Kalekar <kalekarsanket005@gmail.com>
Replace dots with underscores in custom field names to ensure OTLP attribute name compliance and prevent cardinality panics. Changes: - Line ~34-40: Sanitize customfields key using `strings.ReplaceAll(i, ".", "_")` - Store sanitized name in variable for reuse - Change log level from ErrorLvl to WarningLvl (sanitization is automatic now) - Update log message to show both original and sanitized field names Why: OTLP metric attribute names must match `^[a-zA-Z_:][a-zA-Z0-9_:]*$` regex. Dots cause validation failures and label count mismatches. Related: - Part of fix for issue #3748 - Complements prometheus and handlers.go fixes Signed-off-by: Sanket Kalekar <kalekarsanket005@gmail.com>
Replace dots with underscores when building OTLP attribute list to ensure attribute name compliance. Changes: - Line ~230-237: In `newFalcoPayload()` function - Sanitize key using `strings.ReplaceAll(key, ".", "_")` before: - Regex validation check - Creating attribute.String() with sanitized key Why: OTLP metric attributes must follow naming conventions. Without sanitization, custom fields with dots would be rejected, causing similar cardinality issues as Prometheus. Related: - Part of fix for issue #3748 - Complements otlp_metrics.go changes - Prevents OTLP-related panics Signed-off-by: Sanket Kalekar <kalekarsanket005@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ooye-sanket The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @ooye-sanket! It looks like this is your first PR to falcosecurity/falcosidekick 🎉 |
Member
|
Hey @ooye-sanket out of curiosity: is this patch AI generated? 🤔 |
|
Issues go stale after 90d of inactivity. Mark the issue as fresh with Stale issues rot after an additional 30d of inactivity and eventually close. If this issue is safe to close now please do so with Provide feedback via https://github.com/falcosecurity/community. /lifecycle stale |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind bug
Any specific area of the project related to this PR?
/area config
/area outputs
What this PR does / why we need it:
Falco Sidekick currently panics when a custom field contains dots (e.g.,
k8s.cluster.name).Prometheus and OTLP both reject such attribute/label names, causing:
expected X values, got Y)The root cause was that custom fields were validated before sanitization.
✔ This PR fixes the issue by:
"." → "_"before validationhandlers.gostats_prometheus.gootlp_metrics.goOTLP.Metrics.ExtraAttributesListThis prevents crashes, ensures correct metric cardinality, and keeps behavior consistent across all backends.
Which issue(s) this PR fixes:
Fixes #3748
Special notes for your reviewer:
"Custom field 'k8s.cluster.name' (sanitized to 'k8s_cluster_name') is not valid and will be ignored"cc @maintainer