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

feat: support to hostname includes dots #21

Merged

Conversation

yuta1024
Copy link
Contributor

cpu_idlestate, cpu_usage and zfs_pool accept hostname that includes dots. However, other metrics does not accept it. Therefore, this PR fix all patterns to use regex same like cpu_idlestate etc.

Before

truenas_nas_example_com_cputemp_temperatures_0 28.45

After

cpu_temperature{cpu="cpu0",instance="nas.example.com",job="truenas"} 28.3666667

@Supporterino
Copy link
Owner

Supporterino commented Mar 14, 2024

Regular expression matching is significantly slower than glob mapping as all mappings must be tested in order. Because of this, regex mappings are only executed after all glob mappings. In other words, glob mappings take preference over regex matches, irrespective of the order in which they are specified. Regular expression matches are always evaluated in order, and the first match wins.

https://github.com/prometheus/statsd_exporter

The documentation for the statsd_exporter mentions that regex parsing can be slow. I would like to test the speed of the exporter with this change and the resource usage before merging. If you have time feel free to do a comparison. I will mostly likely get to it on the weekend. @yuta1024

@yuta1024
Copy link
Contributor Author

Thank you for you reply!

I add --log.level=debug to graphite-exporter and check processing time for one push.

It seems to be no significant peformance degration. However I don't know what happen until time passed. So I will wait about 1 day and check again. I hope this will be useful to you.

  • main branch: 0.144 sec
ts=2024-03-14T15:56:59.432Z caller=collector.go:131 level=debug msg="Incoming line" line="truenas.(snip: hostname).system.cpu.guest_nice 0.0000000 1710431816"
(snip)
ts=2024-03-14T15:56:59.576Z caller=collector.go:183 level=debug msg="Processing sample" sample="collector.graphiteSample{OriginalName:\"truenas.(snip: hostname).smart_log_smart.disktemp.nvme0n1.nvme0n1\", Name:\"truenas_(snip: hostname)_smart_log_smart_disktemp_nvme0n1_nvme0n1\", Labels:prometheus.Labels{}, Help:\"Graphite metric truenas_(snip: hostname)smart_log_smart_disktemp_nvme0n1_nvme0n1\", Value:45, Type:2, Timestamp:time.Date(2024, time.March, 15, 0, 53, 0, 0, time.Local)}"
  • this branch: 0.122 sec
ts=2024-03-14T15:50:59.432Z caller=collector.go:131 level=debug msg="Incoming line" line="truenas.(snip: hostname).system.cpu.guest_nice 0.0000000 1710431456"
(snip)
ts=2024-03-14T15:50:59.544Z caller=collector.go:183 level=debug msg="Processing sample" sample="collector.graphiteSample{OriginalName:\"truenas.(snip: hostname).smart_log_smart.disktemp.nvme0n1.nvme0n1\", Name:\"disk_temperature\", Labels:prometheus.Labels{\"instance\":\"(snip: hostname)\", \"job\":\"truenas\", \"serial\":\"nvme0n1\"}, Help:\"Graphite metric disk_temperature\", Value:44, Type:2, Timestamp:time.Date(2024, time.March, 15, 0, 47, 0, 0, time.Local)}"

Machine specs(Docker on Ubuntu 22.04.2 LTS on Proxmox VE):

  • CPU: 1 vCPU(Intel Core i5 9400)
  • Mem: 2 GiB

@Supporterino
Copy link
Owner

@yuta1024 Could you rebase your branch real quick then I get a docker image for my testing as well. But your results look promising

@yuta1024 yuta1024 force-pushed the feature/support-dot-include-hostname branch from ddaaaf1 to efb2686 Compare March 15, 2024 14:46
@yuta1024
Copy link
Contributor Author

@Supporterino OK, I rebased branch. I checked processing time again for one push. The result is 0.137 secs.

@Supporterino
Copy link
Owner

I also did my fair checking and the performance hit isn't notable to I am going to merge it. @yuta1024 thank you for your patience

@Supporterino Supporterino merged commit c3f9f36 into Supporterino:main Mar 15, 2024
1 of 4 checks passed
@yuta1024
Copy link
Contributor Author

Thank you!

@yuta1024 yuta1024 deleted the feature/support-dot-include-hostname branch March 15, 2024 16:23
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.

2 participants