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

Integration of Kubernetes support #94

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

opvizordz
Copy link

@ncabatoff we reworked the changes to support Kubernetes and map pod names to processes.
For that to work curl and jq are required and we need access to the Kubernetes API (key is shared by default and we use kubernetes variables similar to other projects).

More than happy to provide more details or guidelines if you want.

It would be great if you consider to merge the PR, so we can work together on improvements.

prometheus info looks like that:

app:"prometheus-process-exporter"
chart:"prometheus-process-exporter-0.2.2"
ctxswitchtype:"voluntary"
groupname:"promtail"
heritage:"Tiller"
instance:"10.1.74.221:9100"
job:"kubernetes-service-endpoints"
kubernetes_name:"process-exporter-prometheus-process-exporter"
kubernetes_namespace:"default"
kubernetes_node:"juju-e1b2fa-1"
pod:"loki-promtail-l6jvm"
release:"process-exporter"

Thanks!

@ncabatoff
Copy link
Owner

Hi @opvizordz / @nredko ,

Thanks for the PR! It's very cool to be able to provide this information.

Unfortunately, as I explained in #90, this goes beyond the scope of what I want process-exporter to do. I know you made it an opt-in feature, but even as such I don't want this in the core project.

As I see it there are a few options besides the one you chose of embedding all this functionality right in process-exporter.

  1. Run the existing process-exporter as a sidecar. That way prometheus operator should be able to attach pod label automatically. Since each instance of process-exporter will only see the processes in that pod, there shouldn't be much additional overhead compared to running a single process-exporter on the node directly. But I understand you may not have enough control over the workloads you want to monitor to be able to take this path.

  2. The bulk of process-exporter isn't in main.go. You could create a PR that extends the process-exporter libraries to expose containerID* and allow grouping by it, then write a new exporter modelled on main.go which provides the enhanced view. Note that I'm not keen on invoking subprocesses as part of the scrape operation; if your existing PR were philosophically compatible with what I want for process-exporter, I'd be taking issue with some implementation details like your use of grep/jq/set/etc.

  3. Similar to (2). A PR to make main.go expose a new label (containerID*) which will be empty for procs not running within containers. This would require an opt-in command-line option since it would potentially change groupings: two procs that matched by name or cmdline before but are running within different containers would appear in different metrics now (same groupname but different labelset.) Then you write an exporter-wrapper that consumes the output of process-exporter, invokes the k8s apis, and merges the data to replace the container id label with a pod name label. My https://github.com/ncabatoff/script-exporter project does something similar if you want an example. The only virtue of this compared to (2) is a bit less code duplication, at the cost of a more complicated deployment process. Probably not worth it.

The advantage of these approaches, besides keeping process-exporter from having hard to test dependencies and less predictable execution time, is that it makes it easier to support other container orchestrators like Nomad, Docker Swarm, etc.

(* When I say "containerID" above I'm not 100% clear if that's what it is we want, I'm still pretty ignorant when it comes to the low-level details of containers and cgroups.)

@mitchellmaler
Copy link

Is thing still something that is going to be added? Being able to connect a process to a container or even pod would help up so much.

@tusharbhasme
Copy link

@opvizordz any plan to maintain and update this fork? the last docker image is throwing errors for me:

Reading metrics from /host/proc based on "/var/process-exporter/config.yml"
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x7e3dbe]

goroutine 1 [running]:
github.com/opvizor/process-exporter.(*Labeler).GetLabels(0x0, 0x1, 0xc0001050c8, 0x7, 0xc000166b90, 0x5, 0x5, 0xc0001094a0, 0x4, 0x0, ...)
    /go/src/github.com/opvizor/process-exporter/common.go:55 +0x6e
github.com/opvizor/process-exporter/proc.(*Tracker).Update(0xc0001008c0, 2023-12-12T11:31:02.973605249Z 0x9f9b20, 0xc000100900, 0xc0001ce000, 0x0, 0xc000100900, 0xc00005fcc0, 0x40c458, 0xc0001ce000, 0x19a)
    /go/src/github.com/opvizor/process-exporter/proc/tracker.go:419 +0x301
github.com/opvizor/process-exporter/proc.(*Grouper).Update(0xc00011bb80, 0x9f9b20, 0xc000100900, 0xc000102f00, 0x0, 0x0, 0xc00005fd38, 0x413955)
    /go/src/github.com/opvizor/process-exporter/proc/grouper.go:102 +0x4a
main.NewProcessCollector(0x7ffc9d5ee060, 0xa, 0x9e0101, 0x9ef980, 0xc000102cf0, 0x0, 0x0, 0xc0001065a0, 0x1, 0x472032)
    /go/src/github.com/opvizor/process-exporter/cmd/process-exporter/main.go:539 +0x273
main.main()
    /go/src/github.com/opvizor/process-exporter/cmd/process-exporter/main.go:459 +0x8e7
Stream closed EOF for monitoring/process-exporter-prometheus-process-exporter-gmspn (process-exporter)

@nredko
Copy link

nredko commented Dec 13, 2023

@opvizordz any plan to maintain and update this fork? the last docker image is throwing errors for me:

@tusharbhasme
I don't maintain this project, it was feature request from @opvizordz

You can try to contact them via https://opvizor.com/ or wait if they will answer here.

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.

5 participants