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

[monitoring/prometheus] No separator between MetricFactory.Prefix and metric names #1376

Closed
DazWilkin opened this issue Dec 12, 2018 · 1 comment · Fixed by #2745
Closed
Assignees
Labels
docs Related to documentation. Low Priority

Comments

@DazWilkin
Copy link
Contributor

If not intentional (developer provides separator if wanted), consider inserting a separator here:

Name: pmf.Prefix + name,

Name: pmf.Prefix + name,

Name: pmf.Prefix + name,

Perhaps simply:
Name: pmf.Prefix + "_" + name

Avoids e.g.:

# HELP someprefixmysql_queue_leaf_latency_entry Latency of insert-entry part of queue (single) leaf operation in seconds
# TYPE someprefixmysql_queue_leaf_latency_entry histogram

With:

# HELP someprefix_mysql_queue_leaf_latency_entry Latency of insert-entry part of queue (single) leaf operation in seconds
# TYPE someprefix_mysql_queue_leaf_latency_entry histogram

I've written a simple OpenCensus monitoring example. I'd used "[namespace]/[metric]" because I have been testing with Stackdriver monitoring but realize this is a poor assumption and should probably use "_" there too. Will be consistent with this Prometheus implementation.

@RJPercival RJPercival self-assigned this Dec 13, 2018
@RJPercival
Copy link
Contributor

I believe it was intentional, i.e. the developer is expected to end their prefix with a separator if desired. We should probably document that Prefix variable though, to state that explicitly.

@RJPercival RJPercival added the docs Related to documentation. label Jan 8, 2019
@RJPercival RJPercival removed their assignment Feb 26, 2020
@mhutchinson mhutchinson self-assigned this May 26, 2022
mhutchinson added a commit to mhutchinson/trillian that referenced this issue May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Related to documentation. Low Priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants