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

Consider defaulting model.NameEscapingScheme to model.UnderscoreEscaping #689

Closed
dashpole opened this issue Sep 4, 2024 · 1 comment · Fixed by #690
Closed

Consider defaulting model.NameEscapingScheme to model.UnderscoreEscaping #689

dashpole opened this issue Sep 4, 2024 · 1 comment · Fixed by #690

Comments

@dashpole
Copy link

dashpole commented Sep 4, 2024

Forked from open-telemetry/opentelemetry-go#5755 (comment)

When trying utf-8 support with OpenTelemetry, I found the default model.NameEscapingScheme of model.ValueEncodingEscaping surprising. It converted my metric name to U__rpc_2e_durations_2e_histogram_2e_seconds.

Underscore escaping is the most human-readable of the escaping schemes. For anyone using a system that doesn't support the new UTF-8 stuff, or even curl, being able to read the output or find/read the names when querying is probably more important. I also suspect, but haven't verified, that most existing escaping schemes replace with underscores (at least OpenCensus/OpenTelemetry currently do). If we defaulted to underscore replacement, it would make the migration easier on that front.

@ywwg

@ywwg
Copy link
Member

ywwg commented Sep 4, 2024

I am ok with this approach. The main reason to escape to ValuesEncoding is to preserve round-tripability of the escaped names. Once we convert to underscores, that possibility is lost. However simple underscore replacement has been the chosen approach for Prometheus for a long time, so it's probably better to keep things familiar. I'll prepare a PR.

ywwg added a commit to ywwg/common that referenced this issue Sep 4, 2024
fixes prometheus#689

Signed-off-by: Owen Williams <owen.williams@grafana.com>
ywwg added a commit to ywwg/common that referenced this issue Sep 4, 2024
fixes prometheus#689

Signed-off-by: Owen Williams <owen.williams@grafana.com>
@ywwg ywwg closed this as completed in #690 Sep 5, 2024
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 a pull request may close this issue.

2 participants