-
Notifications
You must be signed in to change notification settings - Fork 694
feat: Allow utf-8 label names in write path #4489
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
base: main
Are you sure you want to change the base?
Conversation
69072a0
to
4df725b
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.
The write path should not use allow-utf8-labelnames=true
. The capability is a read path only capability.
When profile producers (SDK, profilecli upload, Alloy pyroscope.write) support utf8 label names, they should:
Send the labels in utf8, if they have any.
Then there are two options:
- Backend support utf8 label names/has enabled support: All is fine
- Backend doens't support: The first request fails and the client falls back to filtering/translating label names.
So basically you should create a tenant override maybe IngestionAllowUtf8LabelNames
, around here:
pyroscope/pkg/validation/limits.go
Line 65 in 019bac2
IngestionRelabelingDefaultRulesPosition RelabelRulesPosition `yaml:"ingestion_relabeling_default_rules_position" json:"ingestion_relabeling_default_rules_position" category:"advanced"` |
Then the ingest path should not consider client flags, they should just call limits.IngestionAllowUtf8LabelNames()
, and the act accordingly.
Adding a metric to determine scope and impact of removing write path sanitization by default #4501 |
Utf-8 label names are valid in the write path if the config
client-capability.allow_utf_8_label_names=true
is set, and the client capabilityallow-utf8-labelnames=true
is passed in theAccept
header for API calls.Legacy label name validation and sanitization will continue to be enforced on write path, otherwise.
Follow up to #4442
@simonswine @bryanhuhta