passenger_exporter icon indicating copy to clipboard operation
passenger_exporter copied to clipboard

Changing label on AppGroup Queue

Open phenom5886 opened this issue 6 years ago • 7 comments

This change allowed this metric to function when having multiple Rails applications running on the same server.

Related to issue - https://github.com/stuartnelson3/passenger_exporter/issues/8

phenom5886 avatar Feb 05 '19 08:02 phenom5886

@phenom5886 I have a very similar patch I had to roll when we added an additional queue - it would be the following diff instead - how do you feel about that? We are running this in production currently and I would love to get this upstreamed by @stuartnelson3. If you like this you can either pull it into this PR or I can roll a separate PR from my fork. Thoughts?

diff --git a/main.go b/main.go
index a7f7512..5db7da9 100644
--- a/main.go
+++ b/main.go
@@ -53,6 +53,7 @@ type Exporter struct {
 
        // App metrics.
        appQueue         *prometheus.Desc
+       appGroupQueue    *prometheus.Desc
        appProcsSpawning *prometheus.Desc
 
        // Process metrics.
@@ -110,6 +111,12 @@ func NewExporter(cmd string, timeout time.Duration) *Exporter {
                        []string{"name"},
                        nil,
                ),
+               appGroupQueue: prometheus.NewDesc(
+                       prometheus.BuildFQName(namespace, "", "app_group_queue"),
+                       "Number of requests in app group process queues.",
+                       []string{"group", "default"},
+                       nil,
+               ),
                appProcsSpawning: prometheus.NewDesc(
                        prometheus.BuildFQName(namespace, "", "app_procs_spawning"),
                        "Number of processes spawning.",
@@ -159,6 +166,8 @@ func (e *Exporter) Collect(ch chan<- prometheus.Metric) {
                ch <- prometheus.MustNewConstMetric(e.appQueue, prometheus.GaugeValue, parseFloat(sg.RequestsInQueue), sg.Name)
                ch <- prometheus.MustNewConstMetric(e.appProcsSpawning, prometheus.GaugeValue, parseFloat(sg.Group.ProcessesSpawning), sg.Name)
 
+               ch <- prometheus.MustNewConstMetric(e.appGroupQueue, prometheus.GaugeValue, parseFloat(sg.Group.GetWaitListSize), sg.Group.Name, sg.Group.Default)
+
                // Update process identifiers map.
                processIdentifiers = updateProcesses(processIdentifiers, sg.Group.Processes)
                for _, proc := range sg.Group.Processes {
@@ -214,6 +223,7 @@ func (e *Exporter) Describe(ch chan<- *prometheus.Desc) {
        ch <- e.currentProcessCount
        ch <- e.appCount
        ch <- e.appQueue
+       ch <- e.appGroupQueue
        ch <- e.appProcsSpawning
        ch <- e.requestsProcessed
        ch <- e.procStartTime

daveworth avatar Jul 17 '19 17:07 daveworth

hey @stuartnelson3 - I know you aren't so actively working on this or maintaining this project but I think we have a couple of changes (mine or @phenom5886 ) - mine is in production today and I'd really love this upstream to be the source of truth.

daveworth avatar Jul 23 '19 18:07 daveworth

Ah, I missed this ages ago and never got back to it!

I'm more than happy to add these changes.

@daveworth would it be sufficient to merge this PR for you? Or do you need your patch as well?

stuartnelson3 avatar Jul 23 '19 20:07 stuartnelson3

I've tested the changes in this PR (not the patch by @daveworth ) and it seems to work fine. (This was tested in my dev env not in production).

rwky avatar Aug 07 '19 10:08 rwky

ah - and I missed your response @stuartnelson3

I need my patch if that's possible - I think it would also solve @phenom5886 's issue. Would you like a separate PR for that? I can make that happen later today. /cc @rwky

daveworth avatar Aug 07 '19 13:08 daveworth

@daveworth sorry for the long delay, yeah open a separate PR and we can move this ahead.

stuartnelson3 avatar Oct 21 '19 09:10 stuartnelson3

@stuartnelson3 - done in #13! Thank you!

daveworth avatar Oct 28 '19 18:10 daveworth