Feat: scrape hot threads CPU consumption percentage #533
Conversation
Signed-off-by: avinash kumar <[email protected]>
|
@zwopir could I get some inputs on this PR ? |
|
Is there anything else that needs to be done to merge this PR ? |
|
Is there any updated progress for this PR ? |
| @@ -0,0 +1,246 @@ | |||
| // Copyright 2021 The Prometheus Authors | |||
There was a problem hiding this comment.
| // Copyright 2021 The Prometheus Authors | |
| // Copyright 2022 The Prometheus Authors |
| "github.com/go-kit/kit/log" | ||
| "github.com/go-kit/kit/log/level" |
There was a problem hiding this comment.
| "github.com/go-kit/kit/log" | |
| "github.com/go-kit/kit/log/level" | |
| "github.com/go-kit/log" | |
| "github.com/go-kit/log/level" |
| "github.com/go-kit/kit/log/level" | ||
| "github.com/prometheus/client_golang/prometheus" | ||
| "io/ioutil" | ||
| "net/http" |
There was a problem hiding this comment.
These imports are not formatted properly. The stdlib imports should be first in alphabetical order, followed by an empty line, followed by the other imports in alphabetical order. I believe go fmt should do that automatically, or one of the other go tools.
| HotThreads, | ||
| } | ||
| } | ||
| NODE_OUTPUT_SEPERATOR = ":::" |
There was a problem hiding this comment.
I'm not sure why all of this data is being parsed by regex. It seems error prone. I think some tests would be very useful.
| Labels func(HotThreadsIndex string, HotThreadsPolicy string, action string, step string) []string | ||
| } | ||
|
|
||
| func getEnv(key, defaultVal string) string { |
There was a problem hiding this comment.
I don't think env vars should be read in a collector. All configuration and values should be passed in.
| } | ||
|
|
||
| sb := string(body) | ||
| hotThreadsNodeOp := strings.Split(string(sb), NODE_OUTPUT_SEPERATOR) |
There was a problem hiding this comment.
Why is sb being cast to a string? It's cast to a string above.
| ) | ||
| } | ||
| t := &HotThreadsRsp{CpuPercentage: cpuPercentage, Node: nodeName, ThreadName: threadName, ThreadId: threadId} | ||
| *hotThreads = append(*hotThreads, *t) |
There was a problem hiding this comment.
Why are all of these pointer dereferences?
| allHotThreads := hotThreadsOpRegex.FindAllString(nodeData, -1) | ||
| cpuPercentageRegex := regexp.MustCompile(CPU_PERCENTAGE_REGEX) | ||
|
|
||
| for _, v := range allHotThreads { |
There was a problem hiding this comment.
The logic here seems like some complex parsing. I think it would be best for this to be in a separate function with good unit tests. The signature of the function and name should provide a clear definition of what the functionality is.
Is there not an alternative to this parsing? What format is the data in?
| // limitations under the License. | ||
| package collector | ||
|
|
||
| // _nodes/hot_threads response |
There was a problem hiding this comment.
This does not need to be a separate file.
| @@ -66,6 +66,7 @@ github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9 | |||
| github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= | |||
| github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= | |||
| github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= | |||
| github.com/go-kit/kit v0.9.0 h1:wDJmvq38kDhkVxi50ni9ykkdUr1PKgqKOoi01fa0Mdk= | |||
There was a problem hiding this comment.
I don't think there are any go.mod changes so I don't believe that there should be any go.sum changes either.
Signed-off-by: avinash kumar [email protected]
We found this information useful in root-causing CPU spikes on our benchmarking and production clusters.
API ( /_nodes/hot_threads ) output from elasticsearch is not JSON, hence some string parsing and formatting in the code. number of hot threads to scrape and scrape interval in configurable through - MAX_HOT_THREADS_COUNT and HOT_THREADS_SECOND_SAMPLING_INTERVAL env variables.