Skip to content

Commit

Permalink
Merge branch 'main' into kafka-config
Browse files Browse the repository at this point in the history
  • Loading branch information
yurishkuro authored Jul 24, 2024
2 parents 07f903e + d8b2110 commit 46121f5
Show file tree
Hide file tree
Showing 30 changed files with 799 additions and 108 deletions.
16 changes: 12 additions & 4 deletions .github/workflows/ci-docker-hotrod.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,21 @@ jobs:
- name: Export BRANCH variable
uses: ./.github/actions/setup-branch

- name: Install tools
run: make install-ci

- uses: docker/setup-qemu-action@5927c834f5b4fdf503fca6f4c7eccda82949e1ee # v3.1.0

- name: Define BUILD_FLAGS var if running on a Pull Request
run: |
case ${GITHUB_EVENT_NAME} in
pull_request)
echo "BUILD_FLAGS=-l -p linux/amd64" >> ${GITHUB_ENV}
;;
*)
echo "BUILD_FLAGS=" >> ${GITHUB_ENV}
;;
esac
- name: Build, test, and publish hotrod image
run: bash scripts/hotrod-integration-test.sh
run: bash scripts/hotrod-integration-test.sh ${{ env.BUILD_FLAGS }}
env:
DOCKERHUB_TOKEN: ${{ secrets.DOCKERHUB_TOKEN }}
QUAY_TOKEN: ${{ secrets.QUAY_TOKEN }}
Expand Down
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 }}
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ run `make changelog` to generate content

#### ⛔ Breaking Changes

* The OTEL Collector upgrade brought in a change where OTLP receivers started listening on `localhost` instead of `0.0.0.0` as before. As a result, when running in container environment the endpoints are likely unreachable from other containers (Issue [#5737](https://github.com/jaegertracing/jaeger/issues/5737)). The fix will be available in the next release. Meanwhile, the workaround is to instruct Jaeger to listen on `0.0.0.0`, as in [this fix](https://github.com/jaegertracing/jaeger/pull/5734/files#diff-299f817cc4ab077ddb763f1e6a023d9d042d714e2fd3736cc40af3f218d44f1eR15):
```
- COLLECTOR_OTLP_GRPC_HOST_PORT=0.0.0.0:4317
- COLLECTOR_OTLP_HTTP_HOST_PORT=0.0.0.0:4318
```
* Update opentelemetry-go to v1.28.0 and refactor references to semantic conventions ([@renovate-bot](https://github.com/renovate-bot) in [#5698](https://github.com/jaegertracing/jaeger/pull/5698))

#### ✨ New Features
Expand Down
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
28 changes: 26 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 @@ import (
"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 @@ func (s *server) Start(_ context.Context, host component.Host) error {
return err
}
qs := querysvc.NewQueryService(spanReader, depReader, opts)
metricsQueryService, _ := disabled.NewMetricsReader()

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

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

// TODO OTel-collector does not initialize the tracer currently
Expand All @@ -89,7 +95,7 @@ func (s *server) Start(_ context.Context, host component.Host) error {
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,24 @@ func (s *server) addArchiveStorage(opts *querysvc.QueryServiceOptions, host comp
return nil
}

func (s *server) createMetricReader(host component.Host) (metricsstore.Reader, error) {
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()
if err != nil {
return nil, fmt.Errorf("cannot create metrics reader %w", err)
}
return metricsReader, err
}

func (s *server) makeQueryOptions() *queryApp.QueryOptions {
return &queryApp.QueryOptions{
QueryOptionsBase: s.config.QueryOptionsBase,
Expand Down
95 changes: 94 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-metrics-reader-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 @@ -105,6 +133,7 @@ func TestServerStart(t *testing.T) {
config: &Config{
TraceStorageArchive: "jaeger_storage",
TraceStoragePrimary: "jaeger_storage",
MetricStorage: "jaeger_metrics_storage",
},
},
{
Expand Down Expand Up @@ -136,6 +165,22 @@ func TestServerStart(t *testing.T) {
},
expectedErr: "cannot find archive storage factory",
},
{
name: "metrics storage error",
config: &Config{
MetricStorage: "need-factory-error",
TraceStoragePrimary: "jaeger_storage",
},
expectedErr: "cannot find metrics storage factory",
},
{
name: " metrics reader error",
config: &Config{
MetricStorage: "need-metrics-reader-error",
TraceStoragePrimary: "jaeger_storage",
},
expectedErr: "cannot create metrics reader",
},
}

for _, tt := range tests {
Expand Down Expand Up @@ -242,3 +287,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.createMetricReader(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)
}
Loading

0 comments on commit 46121f5

Please sign in to comment.