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

Prometheus Exporter: Duplicate HELP-Line when using multiple caches #700

Closed
StefanPenndorf opened this issue Nov 28, 2017 · 6 comments
Closed

Comments

@StefanPenndorf
Copy link
Contributor

When scraping my applications statistics using the brand new prometheus exporter (thanks for this cool feature @evernat & @slynn1324 ) I'm getting the error:

text format parsing error in line 121: second HELP line for metric name "javamelody_cache_in_memory_count"

After reading the prometheus docs on HELP format I think there may be just one HELP line and TYPE per metric name. Using labels (e.g. cache_name="...") produces no separate metric.

According to the format documentation

All lines for a given metric must be provided as one uninterrupted group, with the optional HELP and TYPE lines first (in no particular order). Beyond that, reproducible sorting in repeated expositions is preferred but not required, i.e. do not sort if the computational cost is prohibitive.

I also think that those metrics must be reordered mixing the different caches but having each metric in one uninterrupted group.

Here are the relevant lines. I've inserted to blank lines before the mentioned "line 121"

[...]
# HELP javamelody_cache_in_memory_count cache in memory count
# TYPE javamelody_cache_in_memory_count gauge
javamelody_cache_in_memory_count{cache_name="sprachencache"} 6
# HELP javamelody_cache_in_memory_used_pct in memory used percent
# TYPE javamelody_cache_in_memory_used_pct gauge
javamelody_cache_in_memory_used_pct{cache_name="sprachencache"} 0.12
# HELP javamelody_cache_in_memory_hits_pct cache in memory hit percent
# TYPE javamelody_cache_in_memory_hits_pct gauge
javamelody_cache_in_memory_hits_pct{cache_name="sprachencache"} -0.01
# HELP javamelody_cache_on_disk_count cache on disk count
# TYPE javamelody_cache_on_disk_count gauge
javamelody_cache_on_disk_count{cache_name="sprachencache"} 0
# HELP javamelody_cache_hits_pct cache hits percent
# TYPE javamelody_cache_hits_pct gauge
javamelody_cache_hits_pct{cache_name="sprachencache"} -0.01
# HELP javamelody_cache_in_memory_hits_count cache in memory hit count
# TYPE javamelody_cache_in_memory_hits_count counter
javamelody_cache_in_memory_hits_count{cache_name="sprachencache"} 0
# HELP javamelody_cache_hits_count cache  hit count
# TYPE javamelody_cache_hits_count counter
javamelody_cache_hits_count{cache_name="sprachencache"} 0
# HELP javamelody_cache_misses_count cache misses count
# TYPE javamelody_cache_misses_count counter
javamelody_cache_misses_count{cache_name="sprachencache"} 0


# HELP javamelody_cache_in_memory_count cache in memory count
# TYPE javamelody_cache_in_memory_count gauge
javamelody_cache_in_memory_count{cache_name="uebersetzungencache"} 371
# HELP javamelody_cache_in_memory_used_pct in memory used percent
# TYPE javamelody_cache_in_memory_used_pct gauge
javamelody_cache_in_memory_used_pct{cache_name="uebersetzungencache"} 0.03
# HELP javamelody_cache_in_memory_hits_pct cache in memory hit percent
# TYPE javamelody_cache_in_memory_hits_pct gauge
javamelody_cache_in_memory_hits_pct{cache_name="uebersetzungencache"} -0.01
# HELP javamelody_cache_on_disk_count cache on disk count
# TYPE javamelody_cache_on_disk_count gauge
javamelody_cache_on_disk_count{cache_name="uebersetzungencache"} 0
# HELP javamelody_cache_hits_pct cache hits percent
# TYPE javamelody_cache_hits_pct gauge
javamelody_cache_hits_pct{cache_name="uebersetzungencache"} -0.01
# HELP javamelody_cache_in_memory_hits_count cache in memory hit count
# TYPE javamelody_cache_in_memory_hits_count counter
javamelody_cache_in_memory_hits_count{cache_name="uebersetzungencache"} 0
# HELP javamelody_cache_hits_count cache  hit count
# TYPE javamelody_cache_hits_count counter
javamelody_cache_hits_count{cache_name="uebersetzungencache"} 0
# HELP javamelody_cache_misses_count cache misses count
# TYPE javamelody_cache_misses_count counter
javamelody_cache_misses_count{cache_name="uebersetzungencache"} 0
[...]

I think required order/format would be:

# HELP javamelody_cache_in_memory_count cache in memory count
# TYPE javamelody_cache_in_memory_count gauge
javamelody_cache_in_memory_count{cache_name="sprachencache"} 6
javamelody_cache_in_memory_count{cache_name="uebersetzungencache"} 371

# HELP javamelody_cache_in_memory_used_pct in memory used percent
# TYPE javamelody_cache_in_memory_used_pct gauge
javamelody_cache_in_memory_used_pct{cache_name="sprachencache"} 0.12
javamelody_cache_in_memory_used_pct{cache_name="uebersetzungencache"} 0.03

# HELP javamelody_cache_in_memory_hits_pct cache in memory hit percent
# TYPE javamelody_cache_in_memory_hits_pct gauge
javamelody_cache_in_memory_hits_pct{cache_name="sprachencache"} -0.01
javamelody_cache_in_memory_hits_pct{cache_name="uebersetzungencache"} -0.01

# HELP javamelody_cache_on_disk_count cache on disk count
# TYPE javamelody_cache_on_disk_count gauge
javamelody_cache_on_disk_count{cache_name="sprachencache"} 0
javamelody_cache_on_disk_count{cache_name="uebersetzungencache"} 0

# HELP javamelody_cache_hits_pct cache hits percent
# TYPE javamelody_cache_hits_pct gauge
javamelody_cache_hits_pct{cache_name="sprachencache"} -0.01
javamelody_cache_hits_pct{cache_name="uebersetzungencache"} -0.01

# HELP javamelody_cache_in_memory_hits_count cache in memory hit count
# TYPE javamelody_cache_in_memory_hits_count counter
javamelody_cache_in_memory_hits_count{cache_name="sprachencache"} 0
javamelody_cache_in_memory_hits_count{cache_name="uebersetzungencache"} 0

# HELP javamelody_cache_hits_count cache  hit count
# TYPE javamelody_cache_hits_count counter
javamelody_cache_hits_count{cache_name="sprachencache"} 0
javamelody_cache_hits_count{cache_name="uebersetzungencache"} 0

# HELP javamelody_cache_misses_count cache misses count
# TYPE javamelody_cache_misses_count counter
javamelody_cache_misses_count{cache_name="sprachencache"} 0
javamelody_cache_misses_count{cache_name="uebersetzungencache"} 0
@StefanPenndorf
Copy link
Contributor Author

Note: I think the same applies for tomcat connectors exported. I've just one connector (http_nio_8080) thus I don't get an error there.

When fixing the PrometheusController I'm not sure whether we should go for iterating over caches and tomcat informations for each metric or whether reorganizing values in a map is worth the extra effort. We've got 6 caches in use at the moment and I don't know any setup with more than 3 tomcat connectors. If this is a "usual" setup I guess iterating per metric might be faster than reorganizing the information first. What do you think @evernat?

@evernat
Copy link
Member

evernat commented Nov 30, 2017

All lines for a given metric must be provided as one uninterrupted group

That requirement is frustrating for the javamelody code.

So yes, I think that we can iterate over caches and tomcat informations for each metric. (I don't see well what you mean by reorganizing values in a map.)
And yes having between 2 and 50 caches is very common, for example when caching entities in Hibernate.
Do you want to create a pull request?

@StefanPenndorf
Copy link
Contributor Author

I'm not sure whether I'll find some time next week to implement the change but we'll see. If someone else has time to jump in that will be fine.

By reorganizing I meant (in pseudo code)

Map<MetricName, Map<ListOfLabels, MetricData>> prometheusMetrics = new Map<>;
for(TomcatInformations t) {
  ListOfLabels labels = ....
  prometheusMetrics.get("javamelody_tomcat_threads_max").put(labels, t.getMaxThreads());
  prometheusMetrics.get("javamelody_tomcat_thread_busy_count").put(labels, t.getCurrentThreadsBusy());
  ....
}
for(CacheInformations c) {
  ListOfLabels labels = ....
  prometheusMetrics.get("javamelody_cache_in_memory_count").put(labels, c.getInMemoryObjectCount());
  ....
}

for(metricGroup: prometheusMetrics) {
  for(metricLine: metricGroup) {
     print(metricGroup, metricLine);
  }
}

Given t is the number of Tomcat Connectors, c is the number of caches and m_c and m_t is the number of metrics per type. If you iterate over TomcatInformations and CacheInformations per metric you'll have O(t * m_t) + O(c *m_c) but if you reorganize you'll have O(t) + O(c) + O(m_c+m_t) which will be lower for large t and/or c (assuming there is a constant number of metrics).

StefanPenndorf pushed a commit to StefanPenndorf/javamelody that referenced this issue Dec 12, 2017
…nnectors

- Reorganize output to match Prometheus requirements on help lines and
statistics output
- Optimized sanitation by extracting patterns into constants
- Optimized/cleaned up controller methods
@StefanPenndorf
Copy link
Contributor Author

I've added a first draft for the solution. Maybe you'll have a look at it and tell me what needs to be changed for the change to get accepted. I hadn't had the time to test the solution, thus it's only a draft right now. I'll check whether the commit really solves the problem by the end of the week.

I had to go with the "reorganize first than output" solution because the code would become clumsy otherwise having lots of repetitions. If you don't like the multiple String-Arrays created I can join the register* and report* methods to iterate over the caches/connectors per metric and directly output the stats.

StefanPenndorf pushed a commit to StefanPenndorf/javamelody that referenced this issue Dec 15, 2017
…nnectors

- Reorganize output to match Prometheus requirements on help lines and
statistics output
- Optimized sanitation by extracting patterns into constants
- Optimized/cleaned up controller methods
@StefanPenndorf
Copy link
Contributor Author

I've tested the changes and prometheus can read the output now. I've also cleaned up and reverted the line-ending change I've introduced in the first place. See #709 - I'm happy if you review the code and tell me what to change.

@evernat
Copy link
Member

evernat commented Dec 26, 2017

This issue is fixed by 6f06e46 which is merged and ready for the next release (1.71).
Thanks anyway for testing the fixed result and for your PR approach.
There will be a new snapshot build available in about 24h here:
https://javamelody.ci.cloudbees.com/job/javamelody/net.bull.javamelody$javamelody-core/

goldyliang pushed a commit to goldyliang/javamelody that referenced this issue Mar 17, 2022
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

No branches or pull requests

2 participants