Skip to content

Commit

Permalink
Merge branch 'main' into test-spm-build
Browse files Browse the repository at this point in the history
  • Loading branch information
yurishkuro authored Jun 19, 2024
2 parents 98e2a99 + 3f6f5fe commit b15de19
Show file tree
Hide file tree
Showing 33 changed files with 512 additions and 197 deletions.
2 changes: 1 addition & 1 deletion .github/actions/setup-node.js/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ runs:
run: |
echo "JAEGER_UI_NODE_JS_VERSION=$(cat jaeger-ui/.nvmrc)" >> ${GITHUB_ENV}
- uses: actions/setup-node@64ed1c7eab4cce3362f8c340dee64e5eaeef8f7c # v4.0.2
- uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4.0.2
with:
node-version: ${{ env.JAEGER_UI_NODE_JS_VERSION }}
cache: 'yarn'
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/dependency-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,4 @@ jobs:
- name: 'Checkout Repository'
uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
- name: 'Dependency Review'
uses: actions/dependency-review-action@0c155c5e8556a497adf53f2c18edabf945ed8e70 # v4.3.2
uses: actions/dependency-review-action@72eb03d02c7872a771aacd928f3123ac62ad6d3a # v4.3.3
2 changes: 1 addition & 1 deletion .github/workflows/scorecard.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ jobs:
# Upload the results as artifacts (optional). Commenting out will disable uploads of run results in SARIF
# format to the repository Actions tab.
- name: "Upload artifact"
uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3 # v4.3.1
uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # v4.3.3
with:
name: SARIF file
path: results.sarif
Expand Down
4 changes: 2 additions & 2 deletions cmd/collector/app/handler/otlp_receiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func startOTLPReceiver(
// from here: params that can be mocked in tests
otlpFactory receiver.Factory,
newTraces func(consume consumer.ConsumeTracesFunc, options ...consumer.Option) (consumer.Traces, error),
createTracesReceiver func(ctx context.Context, set receiver.CreateSettings,
createTracesReceiver func(ctx context.Context, set receiver.Settings,
cfg component.Config, nextConsumer consumer.Traces) (receiver.Traces, error),
) (receiver.Traces, error) {
otlpReceiverConfig := otlpFactory.CreateDefaultConfig().(*otlpreceiver.Config)
Expand All @@ -76,7 +76,7 @@ func startOTLPReceiver(
// TODO this could be wired into changing healthcheck.HealthCheck
logger.Info("OTLP receiver status change", zap.Stringer("status", ev.Status()))
}
otlpReceiverSettings := receiver.CreateSettings{
otlpReceiverSettings := receiver.Settings{
TelemetrySettings: component.TelemetrySettings{
Logger: logger,
TracerProvider: nooptrace.NewTracerProvider(),
Expand Down
2 changes: 1 addition & 1 deletion cmd/collector/app/handler/otlp_receiver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func TestStartOtlpReceiver_Error(t *testing.T) {
assert.Contains(t, err.Error(), "could not create the OTLP consumer")

createTracesReceiver := func(
context.Context, receiver.CreateSettings, component.Config, consumer.Traces,
context.Context, receiver.Settings, component.Config, consumer.Traces,
) (receiver.Traces, error) {
return nil, errors.New("mock error")
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/collector/app/handler/zipkin_receiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func startZipkinReceiver(
// from here: params that can be mocked in tests
zipkinFactory receiver.Factory,
newTraces func(consume consumer.ConsumeTracesFunc, options ...consumer.Option) (consumer.Traces, error),
createTracesReceiver func(ctx context.Context, set receiver.CreateSettings,
createTracesReceiver func(ctx context.Context, set receiver.Settings,
cfg component.Config, nextConsumer consumer.Traces) (receiver.Traces, error),
) (receiver.Traces, error) {
receiverConfig := zipkinFactory.CreateDefaultConfig().(*zipkinreceiver.Config)
Expand All @@ -60,7 +60,7 @@ func startZipkinReceiver(
CORS: options.HTTP.CORS,
// TODO keepAlive not supported?
})
receiverSettings := receiver.CreateSettings{
receiverSettings := receiver.Settings{
TelemetrySettings: component.TelemetrySettings{
Logger: logger,
TracerProvider: nooptrace.NewTracerProvider(),
Expand Down
2 changes: 1 addition & 1 deletion cmd/collector/app/handler/zipkin_receiver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func TestStartZipkinReceiver_Error(t *testing.T) {
assert.Contains(t, err.Error(), "could not create Zipkin consumer")

createTracesReceiver := func(
context.Context, receiver.CreateSettings, component.Config, consumer.Traces,
context.Context, receiver.Settings, component.Config, consumer.Traces,
) (receiver.Traces, error) {
return nil, errors.New("mock error")
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/jaeger/internal/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func Command() *cobra.Command {
},
}

cmd := otelcol.NewCommand(settings)
cmd := otelcol.NewCommandMustSetProvider(settings)

// We want to support running the binary in all-in-one mode without a config file.
// Since there are no explicit hooks in OTel Collector for that today (as of v0.87),
Expand Down
31 changes: 8 additions & 23 deletions cmd/jaeger/internal/exporters/storageexporter/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,20 @@ package storageexporter

import (
"context"
"errors"
"fmt"

otlp2jaeger "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/jaeger"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/pdata/ptrace"
"go.uber.org/zap"

"github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage"
"github.com/jaegertracing/jaeger/storage/spanstore"
"github.com/jaegertracing/jaeger/storage_v2/spanstore"
)

type storageExporter struct {
config *Config
logger *zap.Logger
spanWriter spanstore.Writer
config *Config
logger *zap.Logger
traceWriter spanstore.Writer
}

func newExporter(config *Config, otel component.TelemetrySettings) *storageExporter {
Expand All @@ -31,13 +29,13 @@ func newExporter(config *Config, otel component.TelemetrySettings) *storageExpor
}

func (exp *storageExporter) start(_ context.Context, host component.Host) error {
f, err := jaegerstorage.GetStorageFactory(exp.config.TraceStorage, host)
f, err := jaegerstorage.GetStorageFactoryV2(exp.config.TraceStorage, host)
if err != nil {
return fmt.Errorf("cannot find storage factory: %w", err)
}

if exp.spanWriter, err = f.CreateSpanWriter(); err != nil {
return fmt.Errorf("cannot create span writer: %w", err)
if exp.traceWriter, err = f.CreateTraceWriter(); err != nil {
return fmt.Errorf("cannot create trace writer: %w", err)
}

return nil
Expand All @@ -49,18 +47,5 @@ func (*storageExporter) close(_ context.Context) error {
}

func (exp *storageExporter) pushTraces(ctx context.Context, td ptrace.Traces) error {
batches, err := otlp2jaeger.ProtoFromTraces(td)
if err != nil {
return fmt.Errorf("cannot transform OTLP traces to Jaeger format: %w", err)
}
var errs []error
for _, batch := range batches {
for _, span := range batch.Spans {
if span.Process == nil {
span.Process = batch.Process
}
errs = append(errs, exp.spanWriter.WriteSpan(ctx, span))
}
}
return errors.Join(errs...)
return exp.traceWriter.WriteTraces(ctx, td)
}
68 changes: 47 additions & 21 deletions cmd/jaeger/internal/exporters/storageexporter/exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@ package storageexporter

import (
"context"
"errors"
"testing"

"github.com/open-telemetry/opentelemetry-collector-contrib/extension/storage/storagetest"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/component"
Expand All @@ -32,29 +34,28 @@ import (
"github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage"
"github.com/jaegertracing/jaeger/model"
memoryCfg "github.com/jaegertracing/jaeger/pkg/memory/config"
"github.com/jaegertracing/jaeger/storage"
factoryMocks "github.com/jaegertracing/jaeger/storage/mocks"
)

type storageHost struct {
t *testing.T
storageExtension component.Component
type mockStorageExt struct {
name string
factory *factoryMocks.Factory
}

func (host storageHost) GetExtensions() map[component.ID]component.Component {
myMap := make(map[component.ID]component.Component)
myMap[jaegerstorage.ID] = host.storageExtension
return myMap
func (*mockStorageExt) Start(context.Context, component.Host) error {
panic("not implemented")
}

func (host storageHost) ReportFatalError(err error) {
host.t.Fatal(err)
func (*mockStorageExt) Shutdown(context.Context) error {
panic("not implemented")
}

func (storageHost) GetFactory(_ component.Kind, _ component.Type) component.Factory {
return nil
}

func (storageHost) GetExporters() map[component.DataType]map[component.ID]component.Component {
return nil
func (m *mockStorageExt) Factory(name string) (storage.Factory, bool) {
if m.name == name {
return m.factory, true
}
return nil, false
}

func TestExporterConfigError(t *testing.T) {
Expand All @@ -63,8 +64,10 @@ func TestExporterConfigError(t *testing.T) {
require.EqualError(t, err, "TraceStorage: non zero value required")
}

func TestExporterStartError(t *testing.T) {
host := makeStorageExtension(t, "foo")
func TestExporterStartBadNameError(t *testing.T) {
host := storagetest.NewStorageHost()
host.WithExtension(jaegerstorage.ID, &mockStorageExt{name: "foo"})

exporter := &storageExporter{
config: &Config{
TraceStorage: "bar",
Expand All @@ -75,6 +78,26 @@ func TestExporterStartError(t *testing.T) {
require.ErrorContains(t, err, "cannot find storage factory")
}

func TestExporterStartBadSpanstoreError(t *testing.T) {
factory := new(factoryMocks.Factory)
factory.On("CreateSpanWriter").Return(nil, errors.New("mocked error"))

host := storagetest.NewStorageHost()
host.WithExtension(jaegerstorage.ID, &mockStorageExt{
name: "foo",
factory: factory,
})

exporter := &storageExporter{
config: &Config{
TraceStorage: "foo",
},
}
err := exporter.start(context.Background(), host)
require.Error(t, err)
require.ErrorContains(t, err, "mocked error")
}

func TestExporter(t *testing.T) {
exporterFactory := NewFactory()

Expand All @@ -92,7 +115,7 @@ func TestExporter(t *testing.T) {
err := config.Validate()
require.NoError(t, err)

tracesExporter, err := exporterFactory.CreateTracesExporter(ctx, exporter.CreateSettings{
tracesExporter, err := exporterFactory.CreateTracesExporter(ctx, exporter.Settings{
ID: ID,
TelemetrySettings: telemetrySettings,
BuildInfo: component.NewDefaultBuildInfo(),
Expand Down Expand Up @@ -133,11 +156,11 @@ func TestExporter(t *testing.T) {
assert.Equal(t, spanID.String(), requiredTrace.Spans[0].SpanID.String())
}

func makeStorageExtension(t *testing.T, memstoreName string) storageHost {
func makeStorageExtension(t *testing.T, memstoreName string) component.Host {
extensionFactory := jaegerstorage.NewFactory()
storageExtension, err := extensionFactory.CreateExtension(
context.Background(),
extension.CreateSettings{
extension.Settings{
TelemetrySettings: component.TelemetrySettings{
Logger: zap.L(),
TracerProvider: nooptrace.NewTracerProvider(),
Expand All @@ -147,10 +170,13 @@ func makeStorageExtension(t *testing.T, memstoreName string) storageHost {
memstoreName: {MaxTraces: 10000},
}})
require.NoError(t, err)
host := storageHost{t: t, storageExtension: storageExtension}

host := storagetest.NewStorageHost()
host.WithExtension(jaegerstorage.ID, storageExtension)

err = storageExtension.Start(context.Background(), host)
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, storageExtension.Shutdown(context.Background())) })

return host
}
2 changes: 1 addition & 1 deletion cmd/jaeger/internal/exporters/storageexporter/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func createDefaultConfig() component.Config {
return &Config{}
}

func createTracesExporter(ctx context.Context, set exporter.CreateSettings, config component.Config) (exporter.Traces, error) {
func createTracesExporter(ctx context.Context, set exporter.Settings, config component.Config) (exporter.Traces, error) {
cfg := config.(*Config)
ex := newExporter(cfg, set.TelemetrySettings)
return exporterhelper.NewTracesExporter(ctx, set, cfg,
Expand Down
2 changes: 1 addition & 1 deletion cmd/jaeger/internal/extension/jaegerquery/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,6 @@ func createDefaultConfig() component.Config {
}

// createExtension creates the extension based on this config.
func createExtension(_ context.Context, set extension.CreateSettings, cfg component.Config) (extension.Extension, error) {
func createExtension(_ context.Context, set extension.Settings, cfg component.Config) (extension.Extension, error) {
return newServer(cfg.(*Config), set.TelemetrySettings), nil
}
2 changes: 1 addition & 1 deletion cmd/jaeger/internal/extension/jaegerquery/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func Test_NewFactory(t *testing.T) {
}

func Test_CreateExtension(t *testing.T) {
set := extension.CreateSettings{
set := extension.Settings{
ID: ID,
}
cfg := createDefaultConfig()
Expand Down
11 changes: 11 additions & 0 deletions cmd/jaeger/internal/extension/jaegerstorage/extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"go.opentelemetry.io/collector/extension"
"go.uber.org/zap"

"github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage/factoryadapter"
esCfg "github.com/jaegertracing/jaeger/pkg/es/config"
memoryCfg "github.com/jaegertracing/jaeger/pkg/memory/config"
"github.com/jaegertracing/jaeger/pkg/metrics"
Expand All @@ -22,6 +23,7 @@ import (
"github.com/jaegertracing/jaeger/plugin/storage/grpc"
"github.com/jaegertracing/jaeger/plugin/storage/memory"
"github.com/jaegertracing/jaeger/storage"
"github.com/jaegertracing/jaeger/storage_v2/spanstore"
)

var _ Extension = (*storageExt)(nil)
Expand Down Expand Up @@ -62,6 +64,15 @@ func GetStorageFactory(name string, host component.Host) (storage.Factory, error
return f, nil
}

func GetStorageFactoryV2(name string, host component.Host) (spanstore.Factory, error) {
f, err := GetStorageFactory(name, host)
if err != nil {
return nil, err
}

return factoryadapter.NewFactory(f), nil
}

func newStorageExt(config *Config, otel component.TelemetrySettings) *storageExt {
return &storageExt{
config: config,
Expand Down
12 changes: 11 additions & 1 deletion cmd/jaeger/internal/extension/jaegerstorage/extension_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,22 @@ func TestStorageFactoryBadShutdownError(t *testing.T) {
require.ErrorIs(t, err, shutdownError)
}

func TestStorageFactoryV2Error(t *testing.T) {
host := componenttest.NewNopHost()
_, err := GetStorageFactoryV2("something", host)
require.ErrorContains(t, err, "cannot find extension")
}

func TestStorageExtension(t *testing.T) {
const name = "foo"
host := storageHost{t: t, storageExtension: startStorageExtension(t, name)}
f, err := GetStorageFactory(name, host)
require.NoError(t, err)
require.NotNil(t, f)

f2, err := GetStorageFactoryV2(name, host)
require.NoError(t, err)
require.NotNil(t, f2)
}

func TestBadgerStorageExtension(t *testing.T) {
Expand Down Expand Up @@ -206,7 +216,7 @@ func makeStorageExtenion(t *testing.T, config *Config) component.Component {
extensionFactory := NewFactory()
ctx := context.Background()
storageExtension, err := extensionFactory.CreateExtension(ctx,
extension.CreateSettings{
extension.Settings{
ID: ID,
TelemetrySettings: noopTelemetrySettings(),
BuildInfo: component.NewDefaultBuildInfo(),
Expand Down
2 changes: 1 addition & 1 deletion cmd/jaeger/internal/extension/jaegerstorage/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,6 @@ func createDefaultConfig() component.Config {
}

// createExtension creates the extension based on this config.
func createExtension(_ context.Context, set extension.CreateSettings, cfg component.Config) (extension.Extension, error) {
func createExtension(_ context.Context, set extension.Settings, cfg component.Config) (extension.Extension, error) {
return newStorageExt(cfg.(*Config), set.TelemetrySettings), nil
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Storage Factory Converter

A temporary v1 storage factory wrapper to implement v2 storage APIs.
This way, the existing v1 storage factories declared in `jaegerstorageextension`
can act as v2 storage while we migrate to v2 storage APIs.
Loading

0 comments on commit b15de19

Please sign in to comment.