Skip to content

Commit d8b2110

Browse files
Enable SPM in Jaeger v2 (#5681)
## Which problem is this PR solving? - This PR is a part of the issue #5632 ## Description of the changes - Added a new docker-compose file - Added the configuration file which will be used for enabling SPM in v2 ## How was this change tested? - Run `make dev-v2` inside `docker-compose/monitor` ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: FlamingSaint <[email protected]> Signed-off-by: Raghuram Kannan <[email protected]> Signed-off-by: Yuri Shkuro <[email protected]> Signed-off-by: Yuri Shkuro <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]>
1 parent 74ee5d6 commit d8b2110

File tree

26 files changed

+738
-87
lines changed

26 files changed

+738
-87
lines changed

.github/workflows/ci-e2e-spm.yml

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,14 @@ permissions:
1818
jobs:
1919
spm:
2020
runs-on: ubuntu-latest
21+
strategy:
22+
matrix:
23+
mode:
24+
- name: v1
25+
binary: all-in-one
26+
- name: v2
27+
binary: jaeger
28+
2129
steps:
2230
- name: Harden Runner
2331
uses: step-security/harden-runner@17d0e2bd7d51742c71671bd19fa12bdc9d40a3d6 # v2.8.1
@@ -38,11 +46,6 @@ jobs:
3846

3947
- name: Setup Node.js version
4048
uses: ./.github/actions/setup-node.js
41-
42-
- name: Temporary - only run the build
43-
run:
44-
cd docker-compose/monitor && make build
45-
49+
4650
- name: Run SPM Test
47-
run: ./scripts/spm-integration-test.sh
48-
51+
run: bash scripts/spm-integration-test.sh -b ${{ matrix.mode.binary }}

cmd/jaeger/internal/components.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package internal
66
import (
77
"github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector"
88
"github.com/open-telemetry/opentelemetry-collector-contrib/exporter/kafkaexporter"
9+
"github.com/open-telemetry/opentelemetry-collector-contrib/exporter/prometheusexporter"
910
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/jaegerreceiver"
1011
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/kafkareceiver"
1112
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/zipkinreceiver"
@@ -90,6 +91,7 @@ func (b builders) build() (otelcol.Factories, error) {
9091
// add-ons
9192
storageexporter.NewFactory(), // generic exporter to Jaeger v1 spanstore.SpanWriter
9293
kafkaexporter.NewFactory(),
94+
prometheusexporter.NewFactory(),
9395
// elasticsearch.NewFactory(),
9496
)
9597
if err != nil {

cmd/jaeger/internal/exporters/storageexporter/exporter_test.go

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,13 @@ import (
3939
)
4040

4141
type mockStorageExt struct {
42-
name string
43-
factory *factoryMocks.Factory
42+
name string
43+
factory *factoryMocks.Factory
44+
metricsFactory *factoryMocks.MetricsFactory
4445
}
4546

47+
var _ jaegerstorage.Extension = (*mockStorageExt)(nil)
48+
4649
func (*mockStorageExt) Start(context.Context, component.Host) error {
4750
panic("not implemented")
4851
}
@@ -51,13 +54,20 @@ func (*mockStorageExt) Shutdown(context.Context) error {
5154
panic("not implemented")
5255
}
5356

54-
func (m *mockStorageExt) Factory(name string) (storage.Factory, bool) {
57+
func (m *mockStorageExt) TraceStorageFactory(name string) (storage.Factory, bool) {
5558
if m.name == name {
5659
return m.factory, true
5760
}
5861
return nil, false
5962
}
6063

64+
func (m *mockStorageExt) MetricStorageFactory(name string) (storage.MetricsFactory, bool) {
65+
if m.name == name {
66+
return m.metricsFactory, true
67+
}
68+
return nil, false
69+
}
70+
6171
func TestExporterConfigError(t *testing.T) {
6272
config := createDefaultConfig().(*Config)
6373
err := config.Validate()

cmd/jaeger/internal/extension/jaegerquery/config.go

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,12 @@ var _ component.ConfigValidator = (*Config)(nil)
1818
// Config represents the configuration for jaeger-query,
1919
type Config struct {
2020
queryApp.QueryOptionsBase `mapstructure:",squash"`
21-
22-
TraceStoragePrimary string `valid:"required" mapstructure:"trace_storage"`
23-
TraceStorageArchive string `valid:"optional" mapstructure:"trace_storage_archive"`
24-
25-
HTTP confighttp.ServerConfig `mapstructure:",squash"`
26-
GRPC configgrpc.ServerConfig `mapstructure:",squash"`
27-
28-
Tenancy tenancy.Options `mapstructure:"multi_tenancy"`
21+
TraceStoragePrimary string `valid:"required" mapstructure:"trace_storage"`
22+
TraceStorageArchive string `valid:"optional" mapstructure:"trace_storage_archive"`
23+
MetricStorage string `valid:"optional" mapstructure:"metric_storage"`
24+
HTTP confighttp.ServerConfig `mapstructure:",squash"`
25+
GRPC configgrpc.ServerConfig `mapstructure:",squash"`
26+
Tenancy tenancy.Options `mapstructure:"multi_tenancy"`
2927
}
3028

3129
func (cfg *Config) Validate() error {

cmd/jaeger/internal/extension/jaegerquery/server.go

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/jaegertracing/jaeger/pkg/telemetery"
1919
"github.com/jaegertracing/jaeger/pkg/tenancy"
2020
"github.com/jaegertracing/jaeger/plugin/metrics/disabled"
21+
"github.com/jaegertracing/jaeger/storage/metricsstore"
2122
)
2223

2324
var (
@@ -67,7 +68,12 @@ func (s *server) Start(_ context.Context, host component.Host) error {
6768
return err
6869
}
6970
qs := querysvc.NewQueryService(spanReader, depReader, opts)
70-
metricsQueryService, _ := disabled.NewMetricsReader()
71+
72+
mqs, err := s.createMetricReader(host)
73+
if err != nil {
74+
return err
75+
}
76+
7177
tm := tenancy.NewManager(&s.config.Tenancy)
7278

7379
// TODO OTel-collector does not initialize the tracer currently
@@ -89,7 +95,7 @@ func (s *server) Start(_ context.Context, host component.Host) error {
8995
s.server, err = queryApp.NewServer(
9096
// TODO propagate healthcheck updates up to the collector's runtime
9197
qs,
92-
metricsQueryService,
98+
mqs,
9399
s.makeQueryOptions(),
94100
tm,
95101
telset,
@@ -122,6 +128,24 @@ func (s *server) addArchiveStorage(opts *querysvc.QueryServiceOptions, host comp
122128
return nil
123129
}
124130

131+
func (s *server) createMetricReader(host component.Host) (metricsstore.Reader, error) {
132+
if s.config.MetricStorage == "" {
133+
s.telset.Logger.Info("Metric storage not configured")
134+
return disabled.NewMetricsReader()
135+
}
136+
137+
mf, err := jaegerstorage.GetMetricsFactory(s.config.MetricStorage, host)
138+
if err != nil {
139+
return nil, fmt.Errorf("cannot find metrics storage factory: %w", err)
140+
}
141+
142+
metricsReader, err := mf.CreateMetricsReader()
143+
if err != nil {
144+
return nil, fmt.Errorf("cannot create metrics reader %w", err)
145+
}
146+
return metricsReader, err
147+
}
148+
125149
func (s *server) makeQueryOptions() *queryApp.QueryOptions {
126150
return &queryApp.QueryOptions{
127151
QueryOptionsBase: s.config.QueryOptionsBase,

cmd/jaeger/internal/extension/jaegerquery/server_test.go

Lines changed: 94 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ import (
2626
"github.com/jaegertracing/jaeger/storage"
2727
"github.com/jaegertracing/jaeger/storage/dependencystore"
2828
depsmocks "github.com/jaegertracing/jaeger/storage/dependencystore/mocks"
29+
"github.com/jaegertracing/jaeger/storage/metricsstore"
30+
metricsstoremocks "github.com/jaegertracing/jaeger/storage/metricsstore/mocks"
2931
"github.com/jaegertracing/jaeger/storage/spanstore"
3032
spanstoremocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks"
3133
)
@@ -62,17 +64,43 @@ func (ff fakeFactory) Initialize(metrics.Factory, *zap.Logger) error {
6264
return nil
6365
}
6466

67+
type fakeMetricsFactory struct {
68+
name string
69+
}
70+
71+
// Initialize implements storage.MetricsFactory.
72+
func (fmf fakeMetricsFactory) Initialize(*zap.Logger) error {
73+
if fmf.name == "need-initialize-error" {
74+
return fmt.Errorf("test-error")
75+
}
76+
return nil
77+
}
78+
79+
func (fmf fakeMetricsFactory) CreateMetricsReader() (metricsstore.Reader, error) {
80+
if fmf.name == "need-metrics-reader-error" {
81+
return nil, fmt.Errorf("test-error")
82+
}
83+
return &metricsstoremocks.Reader{}, nil
84+
}
85+
6586
type fakeStorageExt struct{}
6687

6788
var _ jaegerstorage.Extension = (*fakeStorageExt)(nil)
6889

69-
func (fakeStorageExt) Factory(name string) (storage.Factory, bool) {
90+
func (fakeStorageExt) TraceStorageFactory(name string) (storage.Factory, bool) {
7091
if name == "need-factory-error" {
7192
return nil, false
7293
}
7394
return fakeFactory{name: name}, true
7495
}
7596

97+
func (fakeStorageExt) MetricStorageFactory(name string) (storage.MetricsFactory, bool) {
98+
if name == "need-factory-error" {
99+
return nil, false
100+
}
101+
return fakeMetricsFactory{name: name}, true
102+
}
103+
76104
func (fakeStorageExt) Start(context.Context, component.Host) error {
77105
return nil
78106
}
@@ -105,6 +133,7 @@ func TestServerStart(t *testing.T) {
105133
config: &Config{
106134
TraceStorageArchive: "jaeger_storage",
107135
TraceStoragePrimary: "jaeger_storage",
136+
MetricStorage: "jaeger_metrics_storage",
108137
},
109138
},
110139
{
@@ -136,6 +165,22 @@ func TestServerStart(t *testing.T) {
136165
},
137166
expectedErr: "cannot find archive storage factory",
138167
},
168+
{
169+
name: "metrics storage error",
170+
config: &Config{
171+
MetricStorage: "need-factory-error",
172+
TraceStoragePrimary: "jaeger_storage",
173+
},
174+
expectedErr: "cannot find metrics storage factory",
175+
},
176+
{
177+
name: " metrics reader error",
178+
config: &Config{
179+
MetricStorage: "need-metrics-reader-error",
180+
TraceStoragePrimary: "jaeger_storage",
181+
},
182+
expectedErr: "cannot create metrics reader",
183+
},
139184
}
140185

141186
for _, tt := range tests {
@@ -242,3 +287,51 @@ func TestServerAddArchiveStorage(t *testing.T) {
242287
})
243288
}
244289
}
290+
291+
func TestServerAddMetricsStorage(t *testing.T) {
292+
host := componenttest.NewNopHost()
293+
294+
tests := []struct {
295+
name string
296+
config *Config
297+
extension component.Component
298+
expectedOutput string
299+
expectedErr string
300+
}{
301+
{
302+
name: "Metrics storage unset",
303+
config: &Config{},
304+
expectedOutput: `{"level":"info","msg":"Metric storage not configured"}` + "\n",
305+
expectedErr: "",
306+
},
307+
{
308+
name: "Metrics storage set",
309+
config: &Config{
310+
MetricStorage: "random-value",
311+
},
312+
expectedOutput: "",
313+
expectedErr: "cannot find metrics storage factory: cannot find extension",
314+
},
315+
}
316+
317+
for _, tt := range tests {
318+
t.Run(tt.name, func(t *testing.T) {
319+
logger, buf := testutils.NewLogger()
320+
telemetrySettings := component.TelemetrySettings{
321+
Logger: logger,
322+
}
323+
server := newServer(tt.config, telemetrySettings)
324+
if tt.extension != nil {
325+
host = storagetest.NewStorageHost().WithExtension(jaegerstorage.ID, tt.extension)
326+
}
327+
_, err := server.createMetricReader(host)
328+
if tt.expectedErr == "" {
329+
require.NoError(t, err)
330+
} else {
331+
require.ErrorContains(t, err, tt.expectedErr)
332+
}
333+
334+
assert.Contains(t, buf.String(), tt.expectedOutput)
335+
})
336+
}
337+
}

cmd/jaeger/internal/extension/jaegerstorage/config.go

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import (
1313

1414
casCfg "github.com/jaegertracing/jaeger/pkg/cassandra/config"
1515
esCfg "github.com/jaegertracing/jaeger/pkg/es/config"
16+
promCfg "github.com/jaegertracing/jaeger/pkg/prometheus/config"
17+
"github.com/jaegertracing/jaeger/plugin/metrics/prometheus"
1618
"github.com/jaegertracing/jaeger/plugin/storage/badger"
1719
"github.com/jaegertracing/jaeger/plugin/storage/cassandra"
1820
"github.com/jaegertracing/jaeger/plugin/storage/es"
@@ -23,6 +25,7 @@ import (
2325
var (
2426
_ component.ConfigValidator = (*Config)(nil)
2527
_ confmap.Unmarshaler = (*Backend)(nil)
28+
_ confmap.Unmarshaler = (*MetricBackends)(nil)
2629
)
2730

2831
// Config contains configuration(s) for jaeger trace storage.
@@ -31,7 +34,8 @@ var (
3134
// We tried to alias this type directly to a map, but conf did not populated it correctly.
3235
// Note also that the Backend struct has a custom unmarshaler.
3336
type Config struct {
34-
Backends map[string]Backend `mapstructure:"backends"`
37+
Backends map[string]Backend `mapstructure:"backends"`
38+
MetricBackends map[string]MetricBackends `mapstructure:"metric_backends"`
3539
}
3640

3741
type Backend struct {
@@ -43,6 +47,10 @@ type Backend struct {
4347
Opensearch *esCfg.Configuration `mapstructure:"opensearch"`
4448
}
4549

50+
type MetricBackends struct {
51+
Prometheus *promCfg.Configuration `mapstructure:"prometheus"`
52+
}
53+
4654
// Unmarshal implements confmap.Unmarshaler. This allows us to provide
4755
// defaults for different configs. It cannot be done in createDefaultConfig()
4856
// because at that time we don't know which backends the user wants to use.
@@ -98,3 +106,12 @@ func (cfg *Config) Validate() error {
98106
}
99107
return nil
100108
}
109+
110+
func (cfg *MetricBackends) Unmarshal(conf *confmap.Conf) error {
111+
// apply defaults
112+
if conf.IsSet("prometheus") {
113+
v := prometheus.DefaultConfig()
114+
cfg.Prometheus = &v
115+
}
116+
return conf.Unmarshal(cfg)
117+
}

cmd/jaeger/internal/extension/jaegerstorage/config_test.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,3 +107,14 @@ backends:
107107
require.NoError(t, conf.Unmarshal(cfg))
108108
assert.NotEmpty(t, cfg.Backends["some_storage"].Opensearch.Servers)
109109
}
110+
111+
func TestConfigDefaultPrometheus(t *testing.T) {
112+
conf := loadConf(t, `
113+
metric_backends:
114+
some_metrics_storage:
115+
prometheus:
116+
`)
117+
cfg := createDefaultConfig().(*Config)
118+
require.NoError(t, conf.Unmarshal(cfg))
119+
assert.NotEmpty(t, cfg.MetricBackends["some_metrics_storage"].Prometheus.ServerURL)
120+
}

0 commit comments

Comments
 (0)