Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable SPM in Jaeger v2 #5681

Merged
merged 71 commits into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from 62 commits
Commits
Show all changes
71 commits
Select commit Hold shift + click to select a range
6ed4f23
Create setup for enabling SPM in v2
FlamingSaint Jun 26, 2024
dc85461
Add connector to the config and add suggested changes to the Makefile
FlamingSaint Jun 27, 2024
e5d9850
Add integration test for SPM v2
FlamingSaint Jun 27, 2024
1bf0ece
fix
FlamingSaint Jun 27, 2024
8ba9818
Add suggested changes
FlamingSaint Jun 28, 2024
6f61680
Add suggested changes
FlamingSaint Jun 28, 2024
ff42224
fix CI
FlamingSaint Jun 28, 2024
0dbda14
Add changes
FlamingSaint Jun 28, 2024
591fda9
fix
FlamingSaint Jun 28, 2024
31ffea5
Add quotes
FlamingSaint Jun 28, 2024
ea0d5ec
Fix CI
FlamingSaint Jun 28, 2024
fb4b0ce
Add prometheusexporter to components.go
FlamingSaint Jun 28, 2024
231e397
Merge branch 'main' into spm
FlamingSaint Jul 3, 2024
6f42591
fix conflicts
FlamingSaint Jul 3, 2024
0cb5f7a
Use microsim v0.4.1 in docker-compose-v2.yml
FlamingSaint Jul 3, 2024
91987df
Added suggested changes and ran go mod tidy
FlamingSaint Jul 3, 2024
c49e322
Merge branch 'main' into spm
FlamingSaint Jul 3, 2024
43b2e72
Add back the endpoint
FlamingSaint Jul 3, 2024
5e9d0e4
Merge branch 'main' into spm
FlamingSaint Jul 3, 2024
977c9d1
Merge branch 'main' into spm
FlamingSaint Jul 3, 2024
e926e8a
Merge branch 'jaegertracing:main' into spm
FlamingSaint Jul 6, 2024
4ba0ad0
Add config for metrics_storage
FlamingSaint Jul 6, 2024
5675055
Separate TracesBackend and MetricsBackend
FlamingSaint Jul 6, 2024
6479836
Add suggested change
FlamingSaint Jul 6, 2024
14b94fe
Initialize the metrics storage in extensions.go
FlamingSaint Jul 7, 2024
c986181
Use singular naming
FlamingSaint Jul 8, 2024
5c5d701
fix naming
FlamingSaint Jul 8, 2024
21ea847
Add config options for prometheus
FlamingSaint Jul 8, 2024
1b6c9ca
Merge branch 'main' into spm
FlamingSaint Jul 8, 2024
118c8fb
Merge branch 'main' into spm
FlamingSaint Jul 8, 2024
44f2627
Resolve conflicts
FlamingSaint Jul 8, 2024
be42117
initialize metrics storage in extension.go
FlamingSaint Jul 8, 2024
c872291
Add changes
FlamingSaint Jul 8, 2024
85442e8
fix
FlamingSaint Jul 8, 2024
865d2ee
Add changes
FlamingSaint Jul 9, 2024
c6abf8b
WIP: Passed metric storage to the query service
FlamingSaint Jul 14, 2024
21f66f9
Merge branch 'main' into spm
FlamingSaint Jul 14, 2024
fa04c99
Use telset.logger instead of logger
FlamingSaint Jul 14, 2024
636191b
fix: generate mocks
FlamingSaint Jul 14, 2024
c39f8ec
Use Initialize instead of InitializeMetricsFactory
FlamingSaint Jul 14, 2024
f871540
Remove comments
FlamingSaint Jul 14, 2024
78ca11f
fix
FlamingSaint Jul 14, 2024
ed90e30
add changes
FlamingSaint Jul 14, 2024
7d4ee8e
resolve conflicts
FlamingSaint Jul 20, 2024
bae9e39
Merge branch 'main' into spm
FlamingSaint Jul 20, 2024
90176e2
Simplify
yurishkuro Jul 20, 2024
3a937b3
Simplify
yurishkuro Jul 20, 2024
f9bf6ba
Clean-up
yurishkuro Jul 20, 2024
176c332
fix
yurishkuro Jul 20, 2024
50e312c
Clean-up
yurishkuro Jul 20, 2024
f105bc9
fix
yurishkuro Jul 20, 2024
0cdb5ca
Fixes
yurishkuro Jul 20, 2024
db32afe
missed the file
yurishkuro Jul 20, 2024
2034502
Catch-all via wildcard
yurishkuro Jul 20, 2024
9d2f9b2
DRY code, rename public methods to be more consistent
yurishkuro Jul 20, 2024
4f87e23
Add changes
FlamingSaint Jul 21, 2024
7da08f3
Merge branch 'main' into spm
FlamingSaint Jul 22, 2024
0aefd7b
Increase code coverage
FlamingSaint Jul 22, 2024
81bc142
fix lint
FlamingSaint Jul 22, 2024
2e36abe
fix
FlamingSaint Jul 22, 2024
060740b
Increase coverage
FlamingSaint Jul 22, 2024
8f0ee2b
Increase coverage
FlamingSaint Jul 22, 2024
b17125a
Add changes
FlamingSaint Jul 23, 2024
f7a749e
fix
FlamingSaint Jul 23, 2024
64afe89
Merge branch 'main' into spm
FlamingSaint Jul 23, 2024
0301e64
fix
FlamingSaint Jul 24, 2024
4c51ab2
Remove extra spaces
FlamingSaint Jul 24, 2024
96649c9
Increase code coverage
FlamingSaint Jul 24, 2024
032a860
Increase code cov
FlamingSaint Jul 24, 2024
3ec5360
Increase code cov
FlamingSaint Jul 24, 2024
d641e36
Add changes
FlamingSaint Jul 24, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions .github/workflows/ci-e2e-spm.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@ permissions:
jobs:
spm:
runs-on: ubuntu-latest
strategy:
matrix:
mode:
- name: v1
binary: all-in-one
- name: v2
binary: jaeger

steps:
- name: Harden Runner
uses: step-security/harden-runner@17d0e2bd7d51742c71671bd19fa12bdc9d40a3d6 # v2.8.1
Expand All @@ -38,11 +46,6 @@ jobs:

- name: Setup Node.js version
uses: ./.github/actions/setup-node.js

- name: Temporary - only run the build
run:
cd docker-compose/monitor && make build


- name: Run SPM Test
run: ./scripts/spm-integration-test.sh

run: bash scripts/spm-integration-test.sh -b ${{ matrix.mode.binary }}
2 changes: 2 additions & 0 deletions cmd/jaeger/internal/components.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package internal
import (
"github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector"
"github.com/open-telemetry/opentelemetry-collector-contrib/exporter/kafkaexporter"
"github.com/open-telemetry/opentelemetry-collector-contrib/exporter/prometheusexporter"
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/jaegerreceiver"
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/kafkareceiver"
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/zipkinreceiver"
Expand Down Expand Up @@ -90,6 +91,7 @@ func (b builders) build() (otelcol.Factories, error) {
// add-ons
storageexporter.NewFactory(), // generic exporter to Jaeger v1 spanstore.SpanWriter
kafkaexporter.NewFactory(),
prometheusexporter.NewFactory(),
// elasticsearch.NewFactory(),
)
if err != nil {
Expand Down
16 changes: 13 additions & 3 deletions cmd/jaeger/internal/exporters/storageexporter/exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,13 @@ import (
)

type mockStorageExt struct {
name string
factory *factoryMocks.Factory
name string
factory *factoryMocks.Factory
metricsFactory *factoryMocks.MetricsFactory
}

var _ jaegerstorage.Extension = (*mockStorageExt)(nil)

func (*mockStorageExt) Start(context.Context, component.Host) error {
panic("not implemented")
}
Expand All @@ -51,13 +54,20 @@ func (*mockStorageExt) Shutdown(context.Context) error {
panic("not implemented")
}

func (m *mockStorageExt) Factory(name string) (storage.Factory, bool) {
func (m *mockStorageExt) TraceStorageFactory(name string) (storage.Factory, bool) {
if m.name == name {
return m.factory, true
}
return nil, false
}

func (m *mockStorageExt) MetricStorageFactory(name string) (storage.MetricsFactory, bool) {
if m.name == name {
return m.metricsFactory, true
}
return nil, false
}

func TestExporterConfigError(t *testing.T) {
config := createDefaultConfig().(*Config)
err := config.Validate()
Expand Down
14 changes: 6 additions & 8 deletions cmd/jaeger/internal/extension/jaegerquery/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,12 @@ var _ component.ConfigValidator = (*Config)(nil)
// Config represents the configuration for jaeger-query,
type Config struct {
queryApp.QueryOptionsBase `mapstructure:",squash"`

TraceStoragePrimary string `valid:"required" mapstructure:"trace_storage"`
TraceStorageArchive string `valid:"optional" mapstructure:"trace_storage_archive"`

HTTP confighttp.ServerConfig `mapstructure:",squash"`
GRPC configgrpc.ServerConfig `mapstructure:",squash"`

Tenancy tenancy.Options `mapstructure:"multi_tenancy"`
TraceStoragePrimary string `valid:"required" mapstructure:"trace_storage"`
TraceStorageArchive string `valid:"optional" mapstructure:"trace_storage_archive"`
MetricStorage string `valid:"optional" mapstructure:"metric_storage"`
HTTP confighttp.ServerConfig `mapstructure:",squash"`
GRPC configgrpc.ServerConfig `mapstructure:",squash"`
Tenancy tenancy.Options `mapstructure:"multi_tenancy"`
}

func (cfg *Config) Validate() error {
Expand Down
25 changes: 23 additions & 2 deletions cmd/jaeger/internal/extension/jaegerquery/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"github.com/jaegertracing/jaeger/pkg/telemetery"
"github.com/jaegertracing/jaeger/pkg/tenancy"
"github.com/jaegertracing/jaeger/plugin/metrics/disabled"
"github.com/jaegertracing/jaeger/storage/metricsstore"
)

var (
Expand Down Expand Up @@ -67,7 +68,12 @@
return err
}
qs := querysvc.NewQueryService(spanReader, depReader, opts)
metricsQueryService, _ := disabled.NewMetricsReader()

mqs, err := s.createMetricStorage(host)
if err != nil {
return err

Check warning on line 74 in cmd/jaeger/internal/extension/jaegerquery/server.go

View check run for this annotation

Codecov / codecov/patch

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

Added line #L74 was not covered by tests
}

tm := tenancy.NewManager(&s.config.Tenancy)

// TODO OTel-collector does not initialize the tracer currently
Expand All @@ -89,7 +95,7 @@
s.server, err = queryApp.NewServer(
// TODO propagate healthcheck updates up to the collector's runtime
qs,
metricsQueryService,
mqs,
s.makeQueryOptions(),
tm,
telset,
Expand Down Expand Up @@ -122,6 +128,21 @@
return nil
}

func (s *server) createMetricStorage(host component.Host) (metricsstore.Reader, error) {
FlamingSaint marked this conversation as resolved.
Show resolved Hide resolved
if s.config.MetricStorage == "" {
s.telset.Logger.Info("Metric storage not configured")
return disabled.NewMetricsReader()
}

mf, err := jaegerstorage.GetMetricsFactory(s.config.MetricStorage, host)
if err != nil {
return nil, fmt.Errorf("cannot find metrics storage factory: %w", err)
}

metricsReader, err := mf.CreateMetricsReader()
return metricsReader, err

Check warning on line 143 in cmd/jaeger/internal/extension/jaegerquery/server.go

View check run for this annotation

Codecov / codecov/patch

cmd/jaeger/internal/extension/jaegerquery/server.go#L142-L143

Added lines #L142 - L143 were not covered by tests
}

func (s *server) makeQueryOptions() *queryApp.QueryOptions {
return &queryApp.QueryOptions{
QueryOptionsBase: s.config.QueryOptionsBase,
Expand Down
78 changes: 77 additions & 1 deletion cmd/jaeger/internal/extension/jaegerquery/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import (
"github.com/jaegertracing/jaeger/storage"
"github.com/jaegertracing/jaeger/storage/dependencystore"
depsmocks "github.com/jaegertracing/jaeger/storage/dependencystore/mocks"
"github.com/jaegertracing/jaeger/storage/metricsstore"
metricsstoremocks "github.com/jaegertracing/jaeger/storage/metricsstore/mocks"
"github.com/jaegertracing/jaeger/storage/spanstore"
spanstoremocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks"
)
Expand Down Expand Up @@ -62,17 +64,43 @@ func (ff fakeFactory) Initialize(metrics.Factory, *zap.Logger) error {
return nil
}

type fakeMetricsFactory struct {
name string
}

// Initialize implements storage.MetricsFactory.
func (fmf fakeMetricsFactory) Initialize(*zap.Logger) error {
if fmf.name == "need-initialize-error" {
return fmt.Errorf("test-error")
}
return nil
}

func (fmf fakeMetricsFactory) CreateMetricsReader() (metricsstore.Reader, error) {
if fmf.name == "need-span-writer-error" {
return nil, fmt.Errorf("test-error")
}
return &metricsstoremocks.Reader{}, nil
}

type fakeStorageExt struct{}

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

func (fakeStorageExt) Factory(name string) (storage.Factory, bool) {
func (fakeStorageExt) TraceStorageFactory(name string) (storage.Factory, bool) {
if name == "need-factory-error" {
return nil, false
}
return fakeFactory{name: name}, true
}

func (fakeStorageExt) MetricStorageFactory(name string) (storage.MetricsFactory, bool) {
if name == "need-factory-error" {
return nil, false
}
return fakeMetricsFactory{name: name}, true
}

func (fakeStorageExt) Start(context.Context, component.Host) error {
return nil
}
Expand Down Expand Up @@ -242,3 +270,51 @@ func TestServerAddArchiveStorage(t *testing.T) {
})
}
}

func TestServerAddMetricsStorage(t *testing.T) {
host := componenttest.NewNopHost()

tests := []struct {
name string
config *Config
extension component.Component
expectedOutput string
expectedErr string
}{
{
name: "Metrics storage unset",
config: &Config{},
expectedOutput: `{"level":"info","msg":"Metric storage not configured"}` + "\n",
expectedErr: "",
},
{
name: "Metrics storage set",
config: &Config{
MetricStorage: "random-value",
},
expectedOutput: "",
expectedErr: "cannot find metrics storage factory: cannot find extension",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
logger, buf := testutils.NewLogger()
telemetrySettings := component.TelemetrySettings{
Logger: logger,
}
server := newServer(tt.config, telemetrySettings)
if tt.extension != nil {
host = storagetest.NewStorageHost().WithExtension(jaegerstorage.ID, tt.extension)
}
_, err := server.createMetricStorage(host)
if tt.expectedErr == "" {
require.NoError(t, err)
} else {
require.ErrorContains(t, err, tt.expectedErr)
}

assert.Contains(t, buf.String(), tt.expectedOutput)
})
}
}
19 changes: 18 additions & 1 deletion cmd/jaeger/internal/extension/jaegerstorage/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import (

casCfg "github.com/jaegertracing/jaeger/pkg/cassandra/config"
esCfg "github.com/jaegertracing/jaeger/pkg/es/config"
promCfg "github.com/jaegertracing/jaeger/pkg/prometheus/config"
"github.com/jaegertracing/jaeger/plugin/metrics/prometheus"
"github.com/jaegertracing/jaeger/plugin/storage/badger"
"github.com/jaegertracing/jaeger/plugin/storage/cassandra"
"github.com/jaegertracing/jaeger/plugin/storage/es"
Expand All @@ -23,6 +25,7 @@ import (
var (
_ component.ConfigValidator = (*Config)(nil)
_ confmap.Unmarshaler = (*Backend)(nil)
_ confmap.Unmarshaler = (*MetricBackends)(nil)
)

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

type Backend struct {
Expand All @@ -43,6 +47,10 @@ type Backend struct {
Opensearch *esCfg.Configuration `mapstructure:"opensearch"`
}

type MetricBackends struct {
Prometheus *promCfg.Configuration `mapstructure:"prometheus"`
}

// Unmarshal implements confmap.Unmarshaler. This allows us to provide
// defaults for different configs. It cannot be done in createDefaultConfig()
// because at that time we don't know which backends the user wants to use.
Expand Down Expand Up @@ -98,3 +106,12 @@ func (cfg *Config) Validate() error {
}
return nil
}

func (cfg *MetricBackends) Unmarshal(conf *confmap.Conf) error {
// apply defaults
if conf.IsSet("prometheus") {
v := prometheus.DefaultConfig()
cfg.Prometheus = &v
}
return conf.Unmarshal(cfg)
}
11 changes: 11 additions & 0 deletions cmd/jaeger/internal/extension/jaegerstorage/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,3 +107,14 @@ backends:
require.NoError(t, conf.Unmarshal(cfg))
assert.NotEmpty(t, cfg.Backends["some_storage"].Opensearch.Servers)
}

func TestConfigDefaultPrometheus(t *testing.T) {
conf := loadConf(t, `
metric_backends:
some_metrics_storage:
prometheus:
`)
cfg := createDefaultConfig().(*Config)
require.NoError(t, conf.Unmarshal(cfg))
assert.NotEmpty(t, cfg.MetricBackends["some_metrics_storage"].Prometheus.ServerURL)
}
Loading
Loading