Skip to content
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

remove non-ascii characters from invalid metric names in error message #7146

Merged
merged 3 commits into from
Jan 17, 2024

Conversation

replay
Copy link
Contributor

@replay replay commented Jan 17, 2024

When a metric with a non-ascii characters gets received the input validation correctly rejects it, but depending on the invalid character the serialization of the generated error message can fail, for example:

2024/01/16 17:24:39 ERROR: [transport] [server-transport 0xc05a8c21a0] Failed to marshal rpc status: code:400 message:"received a series with invalid metric name: 'invalid_metric_name_\xb0c' (err-mimir-metric-name-invalid)\n" details:{type_url:"type.googleapis.com/httpgrpc.HTTPResponse" value:"\x08\x90\x03\x12)\n\x0cContent-Type\x12\x19text/plain; charset=utf-8\x12!\n\x16X-Content-Type-Options\x12\x07nosniff\x1a\x80\x01received a series with invalid metric name: 'invalid_metric_name_\xb0c' (err-mimir-metric-name-invalid)\n"}, error: string field contains invalid UTF-8

This change removes the non-ascii characters from the metric name when generating the invalid metric name error message. It appends the string (non-ascii characters removed) to the modified metric name in the error message because if we'd just return the cleaned up string as part of the error message then the user who's looking at the error might not understand what the problem was.

Signed-off-by: Mauro Stettler <[email protected]>
Signed-off-by: Mauro Stettler <[email protected]>
Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@replay replay marked this pull request as ready for review January 17, 2024 08:16
@replay replay requested a review from a team as a code owner January 17, 2024 08:16
@replay replay merged commit ca1e7bc into main Jan 17, 2024
28 checks passed
@replay replay deleted the remove-non-ascii-chars-from-invalid-metric-names branch January 17, 2024 16:44
@pstibrany
Copy link
Member

This change removes the non-ascii characters from the metric name when generating the invalid metric name error message.

If this removes all characters, what's left is useless. :( Could we skip only invalid utf-8 "characters"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants