From 8359cf814a3be9cc8e539b9b6b8cb2e497fca329 Mon Sep 17 00:00:00 2001
From: Aaron Elkiss <aelkiss@umich.edu>
Date: Thu, 16 Jan 2025 16:39:53 -0500
Subject: [PATCH] Bugfix: Send instance label with URL

For instance to be used as part of the group, it needs to be included
with the URL we post to. While other labels can be used as part of the
group, it appears customary to use instance. If we need additional
groups, we could add support for arbitrary group labels here.

* Stricter tests around where we expect instance to appear
* New test ensuring instances don't clobber each other
---
 lib/ProgressTracker.pm |  5 ++++-
 t/progress_tracker.t   | 16 +++++++++++++---
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/lib/ProgressTracker.pm b/lib/ProgressTracker.pm
index 7264f8c..8c15827 100644
--- a/lib/ProgressTracker.pm
+++ b/lib/ProgressTracker.pm
@@ -24,7 +24,7 @@ sub new {
   $self->warn_not_reporting if !$self->{pushgateway};
 
   my $instance =  $params{instance} || $ENV{'JOB_NAMESPACE'};
-  $self->{labels}{instance} = $instance if $instance;
+  $self->{instance} = $instance if $instance;
 
   my $app =  $params{app} || $ENV{'JOB_APP'};
   $self->{labels}{app} = $app if $app;
@@ -118,7 +118,10 @@ sub push_metrics {
   }
 
   my $job = $self->{job};
+  my $instance = $self->{instance};
   my $url = $self->{pushgateway} . "/metrics/job/$job";
+  $url .= "/instance/$instance" if $instance;
+
   my $data = $self->{prom}->render;
 
   $self->{ua}->post($url, Content => $data);
diff --git a/t/progress_tracker.t b/t/progress_tracker.t
index c113440..b4c9a91 100644
--- a/t/progress_tracker.t
+++ b/t/progress_tracker.t
@@ -81,7 +81,7 @@ describe "ProgressTracker" => sub {
       my $tracker = ProgressTracker->new();
       $tracker->update_metrics;
 
-      ok(metrics =~ /^job_duration_seconds.*instance=""/m);
+      ok(metrics =~ /^job_duration_seconds\{instance="",job="progress_tracker.t"\}/m);
     };
 
     it "uses instance param if given" => sub {
@@ -89,7 +89,7 @@ describe "ProgressTracker" => sub {
       my $tracker = ProgressTracker->new(instance=>'override-instance');
       $tracker->update_metrics;
 
-      ok(metrics =~ /^job_duration_seconds\S*instance="override-instance"/m);
+      ok(metrics =~ /^job_duration_seconds\{instance="override-instance",job="progress_tracker.t"\}/m);
     };
 
     it "uses JOB_NAMESPACE env var as instance if given" => sub {
@@ -97,8 +97,18 @@ describe "ProgressTracker" => sub {
       my $tracker = ProgressTracker->new();
       $tracker->update_metrics;
 
-      ok(metrics =~ /^job_duration_seconds\S*instance="some-namespace"/m);
+      ok(metrics =~ /^job_duration_seconds\{instance="some-namespace",job="progress_tracker.t"\}/m);
     };
+
+    it "pushes in such a way that doesn't wipe other instances" => sub {
+      my $tracker1 = ProgressTracker->new(instance => "instance1");
+      $tracker1->update_metrics;
+      my $tracker2 = ProgressTracker->new(instance => "instance2");
+      $tracker2->update_metrics;
+
+      ok(metrics =~ /^job_duration_seconds\{instance="instance1",job="progress_tracker.t"\}/m);
+      ok(metrics =~ /^job_duration_seconds\{instance="instance2",job="progress_tracker.t"\}/m);
+    }
   };
 
   describe "app label" => sub {