Skip to content

Commit

Permalink
[target-allocator] Addtl server unit tests (open-telemetry#1357)
Browse files Browse the repository at this point in the history 
* add DiscoveryManager interface, ScrapeConfigsHandler tests

* add job handler tests

* add changelog entry

* make LinkJSON with keyed fields

* fix whitespace
  • Loading branch information
@frzifus
Kristina Pathak authored and frzifus committed Apr 15, 2023
1 parent 2b06f92 commit 6070063
Show file tree
Hide file tree
Showing 5 changed files with 279 additions and 0 deletions.
16 changes: 16 additions & 0 deletions .chloggen/improve-server-tests.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. operator, target allocator, github action)
component: target allocator

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Added test coverage for server handling.

# One or more tracking issues related to the change
issues: [1392]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:
19 changes: 19 additions & 0 deletions cmd/otel-allocator/server/bench_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,15 @@ import (
"testing"
"time"

<<<<<<< HEAD
"github.com/gin-gonic/gin"
"github.com/prometheus/common/model"
promconfig "github.com/prometheus/prometheus/config"
"github.com/stretchr/testify/assert"
=======
"github.com/prometheus/common/model"
promconfig "github.com/prometheus/prometheus/config"
>>>>>>> 1b66c8e ([target-allocator] Addtl server unit tests (#1357))

"github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation"
"github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/target"
Expand Down Expand Up @@ -54,7 +59,11 @@ func BenchmarkServerTargetsHandler(b *testing.B) {
listenAddr := ":8080"
a.SetCollectors(cols)
a.SetTargets(targets)
<<<<<<< HEAD
s := NewServer(logger, a, &listenAddr)
=======
s := NewServer(logger, a, nil, &listenAddr)
>>>>>>> 1b66c8e ([target-allocator] Addtl server unit tests (#1357))
b.Run(fmt.Sprintf("%s_num_cols_%d_num_jobs_%d", allocatorName, v.numCollectors, v.numJobs), func(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
Expand All @@ -78,6 +87,7 @@ func BenchmarkScrapeConfigsHandler(b *testing.B) {
tests := []int{0, 5, 10, 50, 100, 500}
for _, n := range tests {
data := makeNScrapeConfigs(n)
<<<<<<< HEAD
assert.NoError(b, s.UpdateScrapeConfigResponse(data))

b.Run(fmt.Sprintf("%d_targets", n), func(b *testing.B) {
Expand All @@ -88,6 +98,15 @@ func BenchmarkScrapeConfigsHandler(b *testing.B) {
c.Request = httptest.NewRequest("GET", "/scrape_configs", nil)

s.ScrapeConfigsHandler(c)
=======
b.Run(fmt.Sprintf("%d_targets", n), func(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
s.compareHash = 0
s.discoveryManager = &mockDiscoveryManager{m: data}
resp := httptest.NewRecorder()
s.ScrapeConfigsHandler(resp, nil)
>>>>>>> 1b66c8e ([target-allocator] Addtl server unit tests (#1357))
}
})
}
Expand Down
20 changes: 20 additions & 0 deletions cmd/otel-allocator/server/mocks_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,31 @@
package server

import (
<<<<<<< HEAD
=======
promconfig "github.com/prometheus/prometheus/config"

>>>>>>> 1b66c8e ([target-allocator] Addtl server unit tests (#1357))
"github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation"
"github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/target"
)

<<<<<<< HEAD
var _ allocation.Allocator = &mockAllocator{}
=======
var (
_ DiscoveryManager = &mockDiscoveryManager{}
_ allocation.Allocator = &mockAllocator{}
)

type mockDiscoveryManager struct {
m map[string]*promconfig.ScrapeConfig
}

func (m *mockDiscoveryManager) GetScrapeConfigs() map[string]*promconfig.ScrapeConfig {
return m.m
}
>>>>>>> 1b66c8e ([target-allocator] Addtl server unit tests (#1357))

// mockAllocator implements the Allocator interface, but all funcs other than
// TargetItems() are a no-op.
Expand Down
15 changes: 15 additions & 0 deletions cmd/otel-allocator/server/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,21 @@ type collectorJSON struct {
Jobs []*target.Item `json:"targets"`
}

type DiscoveryManager interface {
GetScrapeConfigs() map[string]*promconfig.ScrapeConfig
}

type Server struct {
<<<<<<< HEAD
logger logr.Logger
allocator allocation.Allocator
server *http.Server
=======
logger logr.Logger
allocator allocation.Allocator
discoveryManager DiscoveryManager
server *http.Server
>>>>>>> 1b66c8e ([target-allocator] Addtl server unit tests (#1357))

// Use RWMutex to protect scrapeConfigResponse, since it
// will be predominantly read and only written when config
Expand All @@ -62,7 +73,11 @@ type Server struct {
scrapeConfigResponse []byte
}

<<<<<<< HEAD
func NewServer(log logr.Logger, allocator allocation.Allocator, listenAddr *string) *Server {
=======
func NewServer(log logr.Logger, allocator allocation.Allocator, discoveryManager DiscoveryManager, listenAddr *string) *Server {
>>>>>>> 1b66c8e ([target-allocator] Addtl server unit tests (#1357))
s := &Server{
logger: log,
allocator: allocator,
Expand Down
209 changes: 209 additions & 0 deletions cmd/otel-allocator/server/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,13 +190,21 @@ func TestServer_ScrapeConfigsHandler(t *testing.T) {
description: "nil scrape config",
scrapeConfigs: nil,
expectedCode: http.StatusOK,
<<<<<<< HEAD
expectedBody: []byte("{}"),
=======
expectedBody: []byte{},
>>>>>>> 1b66c8e ([target-allocator] Addtl server unit tests (#1357))
},
{
description: "empty scrape config",
scrapeConfigs: map[string]*promconfig.ScrapeConfig{},
expectedCode: http.StatusOK,
<<<<<<< HEAD
expectedBody: []byte("{}"),
=======
expectedBody: []byte{},
>>>>>>> 1b66c8e ([target-allocator] Addtl server unit tests (#1357))
},
{
description: "single entry",
Expand Down Expand Up @@ -445,9 +453,14 @@ func TestServer_ScrapeConfigsHandler(t *testing.T) {
for _, tc := range tests {
t.Run(tc.description, func(t *testing.T) {
listenAddr := ":8080"
<<<<<<< HEAD
s := NewServer(logger, nil, &listenAddr)
assert.NoError(t, s.UpdateScrapeConfigResponse(tc.scrapeConfigs))

=======
dm := &mockDiscoveryManager{m: tc.scrapeConfigs}
s := NewServer(logger, nil, dm, &listenAddr)
>>>>>>> 1b66c8e ([target-allocator] Addtl server unit tests (#1357))
request := httptest.NewRequest("GET", "/scrape_configs", nil)
w := httptest.NewRecorder()

Expand All @@ -469,6 +482,198 @@ func TestServer_ScrapeConfigsHandler(t *testing.T) {
}
}

<<<<<<< HEAD
=======
func TestScrapeConfigsHandler_Hashing(t *testing.T) {
s := &Server{logger: logger}
// these tests are meant to be run sequentially in this order, to test
// that hashing doesn't cause us to send the wrong information.
tests := []struct {
description string
scrapeConfigs map[string]*promconfig.ScrapeConfig
}{
{
description: "base config",
scrapeConfigs: map[string]*promconfig.ScrapeConfig{
"serviceMonitor/testapp/testapp/0": {
JobName: "serviceMonitor/testapp/testapp/0",
HonorTimestamps: true,
ScrapeInterval: model.Duration(30 * time.Second),
ScrapeTimeout: model.Duration(30 * time.Second),
MetricsPath: "/metrics",
Scheme: "http",
HTTPClientConfig: config.HTTPClientConfig{
FollowRedirects: true,
},
RelabelConfigs: []*relabel.Config{
{
SourceLabels: model.LabelNames{model.LabelName("job")},
Separator: ";",
Regex: relabel.MustNewRegexp("(.*)"),
TargetLabel: "__tmp_prometheus_job_name",
Replacement: "$$1",
Action: relabel.Replace,
},
},
},
},
},
{
description: "different bool",
scrapeConfigs: map[string]*promconfig.ScrapeConfig{
"serviceMonitor/testapp/testapp/0": {
JobName: "serviceMonitor/testapp/testapp/0",
HonorTimestamps: false,
ScrapeInterval: model.Duration(30 * time.Second),
ScrapeTimeout: model.Duration(30 * time.Second),
MetricsPath: "/metrics",
Scheme: "http",
HTTPClientConfig: config.HTTPClientConfig{
FollowRedirects: true,
},
RelabelConfigs: []*relabel.Config{
{
SourceLabels: model.LabelNames{model.LabelName("job")},
Separator: ";",
Regex: relabel.MustNewRegexp("(.*)"),
TargetLabel: "__tmp_prometheus_job_name",
Replacement: "$$1",
Action: relabel.Replace,
},
},
},
},
},
{
description: "different job name",
scrapeConfigs: map[string]*promconfig.ScrapeConfig{
"serviceMonitor/testapp/testapp/0": {
JobName: "serviceMonitor/testapp/testapp/1",
HonorTimestamps: false,
ScrapeInterval: model.Duration(30 * time.Second),
ScrapeTimeout: model.Duration(30 * time.Second),
MetricsPath: "/metrics",
Scheme: "http",
HTTPClientConfig: config.HTTPClientConfig{
FollowRedirects: true,
},
RelabelConfigs: []*relabel.Config{
{
SourceLabels: model.LabelNames{model.LabelName("job")},
Separator: ";",
Regex: relabel.MustNewRegexp("(.*)"),
TargetLabel: "__tmp_prometheus_job_name",
Replacement: "$$1",
Action: relabel.Replace,
},
},
},
},
},
{
description: "different key",
scrapeConfigs: map[string]*promconfig.ScrapeConfig{
"serviceMonitor/testapp/testapp/1": {
JobName: "serviceMonitor/testapp/testapp/1",
HonorTimestamps: false,
ScrapeInterval: model.Duration(30 * time.Second),
ScrapeTimeout: model.Duration(30 * time.Second),
MetricsPath: "/metrics",
Scheme: "http",
HTTPClientConfig: config.HTTPClientConfig{
FollowRedirects: true,
},
RelabelConfigs: []*relabel.Config{
{
SourceLabels: model.LabelNames{model.LabelName("job")},
Separator: ";",
Regex: relabel.MustNewRegexp("(.*)"),
TargetLabel: "__tmp_prometheus_job_name",
Replacement: "$$1",
Action: relabel.Replace,
},
},
},
},
},
{
description: "unset scrape interval",
scrapeConfigs: map[string]*promconfig.ScrapeConfig{
"serviceMonitor/testapp/testapp/1": {
JobName: "serviceMonitor/testapp/testapp/1",
HonorTimestamps: false,
ScrapeTimeout: model.Duration(30 * time.Second),
MetricsPath: "/metrics",
Scheme: "http",
HTTPClientConfig: config.HTTPClientConfig{
FollowRedirects: true,
},
RelabelConfigs: []*relabel.Config{
{
SourceLabels: model.LabelNames{model.LabelName("job")},
Separator: ";",
Regex: relabel.MustNewRegexp("(.*)"),
TargetLabel: "__tmp_prometheus_job_name",
Replacement: "$$1",
Action: relabel.Replace,
},
},
},
},
},
//{
//
// TODO: fix handler logic so this test passes.
// This test currently fails due to the regexp struct not having any
// exported fields for the hashing algorithm to hash on, causing the
// hashes to be the same even though the data is different.
//
// description: "different regex",
// scrapeConfigs: map[string]*promconfig.ScrapeConfig{
// "serviceMonitor/testapp/testapp/1": {
// JobName: "serviceMonitor/testapp/testapp/1",
// HonorTimestamps: false,
// ScrapeTimeout: model.Duration(30 * time.Second),
// MetricsPath: "/metrics",
// HTTPClientConfig: config.HTTPClientConfig{
// FollowRedirects: true,
// },
// RelabelConfigs: []*relabel.Config{
// {
// SourceLabels: model.LabelNames{model.LabelName("job")},
// Separator: ";",
// Regex: relabel.MustNewRegexp("something else"),
// TargetLabel: "__tmp_prometheus_job_name",
// Replacement: "$$1",
// Action: relabel.Replace,
// },
// },
// },
// },
//},
}
for _, tc := range tests {
t.Run(tc.description, func(t *testing.T) {
dm := &mockDiscoveryManager{m: tc.scrapeConfigs}
s.discoveryManager = dm
request := httptest.NewRequest("GET", "/scrape_configs", nil)
w := httptest.NewRecorder()

s.ScrapeConfigsHandler(w, request)
result := w.Result()

assert.Equal(t, http.StatusOK, result.StatusCode)
bodyBytes, err := io.ReadAll(result.Body)
require.NoError(t, err)
scrapeConfigResult := map[string]*promconfig.ScrapeConfig{}
err = yaml.Unmarshal(bodyBytes, scrapeConfigResult)
require.NoError(t, err)
assert.Equal(t, tc.scrapeConfigs, scrapeConfigResult)
})
}
}

>>>>>>> 1b66c8e ([target-allocator] Addtl server unit tests (#1357))
func TestServer_JobHandler(t *testing.T) {
tests := []struct {
description string
Expand Down Expand Up @@ -518,7 +723,11 @@ func TestServer_JobHandler(t *testing.T) {
t.Run(tc.description, func(t *testing.T) {
listenAddr := ":8080"
a := &mockAllocator{targetItems: tc.targetItems}
<<<<<<< HEAD
s := NewServer(logger, a, &listenAddr)
=======
s := NewServer(logger, a, nil, &listenAddr)
>>>>>>> 1b66c8e ([target-allocator] Addtl server unit tests (#1357))
request := httptest.NewRequest("GET", "/jobs", nil)
w := httptest.NewRecorder()

Expand Down

0 comments on commit 6070063

Please sign in to comment.