Fix parameter encoding issue in TA (#930)

* logging

* Fixed encoding issue of params

* Do string replacement

* Another test

* Removed dockerfile changes

* Fix broken test

* Newline

* Change placement of the log import

cmd/otel-allocator/allocation/allocator_test.go

@@ -4,15 +4,16 @@ import (
 	"math"
 	"testing"
 
-	"github.com/go-logr/logr"
 	"github.com/prometheus/common/model"
 	"github.com/stretchr/testify/assert"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
 )
 
+var logger = logf.Log.WithName("unit-tests")
+
 // Tests least connection - The expected collector after running findNextCollector should be the collector with the least amount of workload
 func TestFindNextCollector(t *testing.T) {
-	var log logr.Logger
-	s := NewAllocator(log)
+	s := NewAllocator(logger)
 
 	defaultCol := collector{Name: "default-col", NumTargets: 1}
 	maxCol := collector{Name: "max-col", NumTargets: 2}
@@ -25,9 +26,7 @@ func TestFindNextCollector(t *testing.T) {
 }
 
 func TestSetCollectors(t *testing.T) {
-
-	var log logr.Logger
-	s := NewAllocator(log)
+	s := NewAllocator(logger)
 
 	cols := []string{"col-1", "col-2", "col-3"}
 	s.SetCollectors(cols)
@@ -42,8 +41,7 @@ func TestSetCollectors(t *testing.T) {
 
 func TestAddingAndRemovingTargets(t *testing.T) {
 	// prepare allocator with initial targets and collectors
-	var log logr.Logger
-	s := NewAllocator(log)
+	s := NewAllocator(logger)
 
 	cols := []string{"col-1", "col-2", "col-3"}
 	s.SetCollectors(cols)
@@ -88,8 +86,7 @@ func TestAddingAndRemovingTargets(t *testing.T) {
 func TestCollectorBalanceWhenAddingAndRemovingAtRandom(t *testing.T) {
 
 	// prepare allocator with 3 collectors and 'random' amount of targets
-	var log logr.Logger
-	s := NewAllocator(log)
+	s := NewAllocator(logger)
 
 	cols := []string{"col-1", "col-2", "col-3"}
 	s.SetCollectors(cols)

cmd/otel-allocator/allocation/http_test.go

@@ -0,0 +1,118 @@
+package allocation
+
+import (
+	"github.com/prometheus/common/model"
+	"reflect"
+	"testing"
+)
+
+func TestGetAllTargetsByCollectorAndJob(t *testing.T) {
+	baseAllocator := NewAllocator(logger)
+	baseAllocator.SetCollectors([]string{"test-collector"})
+	statefulAllocator := NewAllocator(logger)
+	statefulAllocator.SetCollectors([]string{"test-collector-0"})
+	type args struct {
+		collector string
+		job       string
+		cMap      map[string][]TargetItem
+		allocator *Allocator
+	}
+	var tests = []struct {
+		name string
+		args args
+		want []targetGroupJSON
+	}{
+		{
+			name: "Empty target map",
+			args: args{
+				collector: "test-collector",
+				job:       "test-job",
+				cMap:      map[string][]TargetItem{},
+				allocator: baseAllocator,
+			},
+			want: nil,
+		},
+		{
+			name: "Single entry target map",
+			args: args{
+				collector: "test-collector",
+				job:       "test-job",
+				cMap: map[string][]TargetItem{
+					"test-collectortest-job": {
+						TargetItem{
+							JobName: "test-job",
+							Label:   model.LabelSet{
+								"test-label": "test-value",
+							},
+							TargetURL: "test-url",
+							Collector: &collector{
+								Name:       "test-collector",
+								NumTargets: 1,
+							},
+						},
+					},
+				},
+				allocator: baseAllocator,
+			},
+			want: []targetGroupJSON{
+				{
+					Targets: []string{"test-url"},
+					Labels: map[model.LabelName]model.LabelValue{
+						"test-label": "test-value",
+					},
+				},
+			},
+		},
+		{
+			name: "Multiple entry target map",
+			args: args{
+				collector: "test-collector",
+				job:       "test-job",
+				cMap: map[string][]TargetItem{
+					"test-collectortest-job": {
+						TargetItem{
+							JobName: "test-job",
+							Label:   model.LabelSet{
+								"test-label": "test-value",
+							},
+							TargetURL: "test-url",
+							Collector: &collector{
+								Name:       "test-collector",
+								NumTargets: 1,
+							},
+						},
+					},
+					"test-collectortest-job2": {
+						TargetItem{
+							JobName: "test-job2",
+							Label:   model.LabelSet{
+								"test-label": "test-value",
+							},
+							TargetURL: "test-url",
+							Collector: &collector{
+								Name:       "test-collector",
+								NumTargets: 1,
+							},
+						},
+					},
+				},
+				allocator: baseAllocator,
+			},
+			want: []targetGroupJSON{
+				{
+					Targets: []string{"test-url"},
+					Labels: map[model.LabelName]model.LabelValue{
+						"test-label": "test-value",
+					},
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := GetAllTargetsByCollectorAndJob(tt.args.collector, tt.args.job, tt.args.cMap, tt.args.allocator); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("GetAllTargetsByCollectorAndJob() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

cmd/otel-allocator/main.go

@@ -185,18 +185,18 @@ func (s *server) TargetsHandler(w http.ResponseWriter, r *http.Request) {
 		compareMap[v.Collector.Name+v.JobName] = append(compareMap[v.Collector.Name+v.JobName], *v)
 	}
 	params := mux.Vars(r)
+	jobId, err := url.QueryUnescape(params["job_id"])
+	if err != nil {
+		errorHandler(err, w, r)
+		return
+	}
 
 	if len(q) == 0 {
-		jobId, err := url.QueryUnescape(params["job_id"])
-		if err != nil {
-			errorHandler(err, w, r)
-			return
-		}
 		displayData := allocation.GetAllTargetsByJob(jobId, compareMap, s.allocator)
 		jsonHandler(w, r, displayData)
 
 	} else {
-		tgs := allocation.GetAllTargetsByCollectorAndJob(q[0], params["job_id"], compareMap, s.allocator)
+		tgs := allocation.GetAllTargetsByCollectorAndJob(q[0], jobId, compareMap, s.allocator)
 		// Displays empty list if nothing matches
 		if len(tgs) == 0 {
 			jsonHandler(w, r, []interface{}{})

pkg/collector/reconcile/config_replace.go

@@ -16,6 +16,7 @@ package reconcile
 
 import (
 	"fmt"
+	"net/url"
 
 	"github.com/mitchellh/mapstructure"
 	promconfig "github.com/prometheus/prometheus/config"
@@ -61,9 +62,10 @@ func ReplaceConfig(params Params) (string, error) {
 	}
 
 	for i := range cfg.PromConfig.ScrapeConfigs {
+		escapedJob := url.QueryEscape(cfg.PromConfig.ScrapeConfigs[i].JobName)
 		cfg.PromConfig.ScrapeConfigs[i].ServiceDiscoveryConfigs = discovery.Configs{
 			&http.SDConfig{
-				URL: fmt.Sprintf("http://%s:80/jobs/%s/targets?collector_id=$POD_NAME", naming.TAService(params.Instance), cfg.PromConfig.ScrapeConfigs[i].JobName),
+				URL: fmt.Sprintf("http://%s:80/jobs/%s/targets?collector_id=$POD_NAME", naming.TAService(params.Instance), escapedJob),
 			},
 		}
 	}

pkg/collector/reconcile/configmap_test.go

@@ -128,6 +128,58 @@ service:
 
 	})
 
+	t.Run("should return expected escaped collector config map with http_sd_config", func(t *testing.T) {
+		expectedLables["app.kubernetes.io/component"] = "opentelemetry-collector"
+		expectedLables["app.kubernetes.io/name"] = "test-collector"
+		expectedLables["app.kubernetes.io/version"] = "latest"
+
+		expectedData := map[string]string{
+			"collector.yaml": `exporters:
+  logging: null
+processors: null
+receivers:
+  prometheus:
+    config:
+      global:
+        scrape_interval: 1m
+        scrape_timeout: 10s
+        evaluation_interval: 1m
+      scrape_configs:
+      - job_name: serviceMonitor/test/test/0
+        honor_timestamps: true
+        scrape_interval: 1m
+        scrape_timeout: 10s
+        metrics_path: /metrics
+        scheme: http
+        follow_redirects: true
+        http_sd_configs:
+        - follow_redirects: false
+          url: http://test-targetallocator:80/jobs/serviceMonitor%2Ftest%2Ftest%2F0/targets?collector_id=$POD_NAME
+service:
+  pipelines:
+    metrics:
+      exporters:
+      - logging
+      processors: []
+      receivers:
+      - prometheus
+`,
+		}
+
+		param, err := newParams("test/test-img", "../testdata/http_sd_config_servicemonitor_test.yaml")
+		assert.NoError(t, err)
+		param.Instance.Spec.TargetAllocator.Enabled = true
+		actual := desiredConfigMap(context.Background(), param)
+
+		assert.Equal(t, "test-collector", actual.Name)
+		assert.Equal(t, expectedLables, actual.Labels)
+		assert.Equal(t, expectedData, actual.Data)
+
+		// Reset the value
+		expectedLables["app.kubernetes.io/version"] = "0.47.0"
+
+	})
+
 	t.Run("should return expected target allocator config map", func(t *testing.T) {
 		expectedLables["app.kubernetes.io/component"] = "opentelemetry-targetallocator"
 		expectedLables["app.kubernetes.io/name"] = "test-targetallocator"

pkg/collector/testdata/http_sd_config_servicemonitor_test.yaml

@@ -0,0 +1,25 @@
+processors:
+receivers:
+  prometheus:
+    config:
+      scrape_configs:
+      - job_name: serviceMonitor/test/test/0
+
+        static_configs:
+        - targets: ["prom.domain:1001", "prom.domain:1002", "prom.domain:1003"]
+          labels:
+            my: label
+
+        file_sd_configs:
+        - files:
+          - file2.json
+
+exporters:
+  logging:
+
+service:
+  pipelines:
+    metrics:
+      receivers: [prometheus]
+      processors: []
+      exporters: [logging]