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

Add tests for cmd/jaeger/internal/extension/jaegerstorage #5096

Merged
merged 10 commits into from
Jan 18, 2024
2 changes: 0 additions & 2 deletions cmd/jaeger/internal/extension/jaegerstorage/.nocover

This file was deleted.

12 changes: 12 additions & 0 deletions cmd/jaeger/internal/extension/jaegerstorage/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
package jaegerstorage

import (
"fmt"
"reflect"

memoryCfg "github.com/jaegertracing/jaeger/pkg/memory/config"
)

Expand All @@ -19,3 +22,12 @@ type MemoryStorage struct {
Name string `mapstructure:"name"`
memoryCfg.Configuration
}

func (cfg *Config) Validate() error {
emptyCfg := createDefaultConfig().(*Config)
if reflect.DeepEqual(*cfg, *emptyCfg) {
return fmt.Errorf("%s: no storage type present in config", ID)
} else {
return nil
}
}
112 changes: 112 additions & 0 deletions cmd/jaeger/internal/extension/jaegerstorage/extension_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
// Copyright (c) 2024 The Jaeger Authors.
// SPDX-License-Identifier: Apache-2.0

package jaegerstorage

import (
"context"
"fmt"
"testing"

"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/component/componenttest"
"go.opentelemetry.io/collector/extension"
noopmetric "go.opentelemetry.io/otel/metric/noop"
nooptrace "go.opentelemetry.io/otel/trace/noop"
"go.uber.org/zap"

memoryCfg "github.com/jaegertracing/jaeger/pkg/memory/config"
)

const memstoreName = "memstore"

type storageHost struct {
t *testing.T
storageExtension component.Component
}

func (host storageHost) GetExtensions() map[component.ID]component.Component {
return map[component.ID]component.Component{
ID: host.storageExtension,
}
}

func (host storageHost) ReportFatalError(err error) {
host.t.Fatal(err)
}

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 TestExtensionConfigError(t *testing.T) {
config := createDefaultConfig().(*Config)
err := config.Validate()
require.EqualError(t, err, fmt.Sprintf("%s: no storage type present in config", ID))
}

func TestStartStorageExtensionError(t *testing.T) {
chirag-ghosh marked this conversation as resolved.
Show resolved Hide resolved
ctx := context.Background()

storageExtension := makeStorageExtension(t, memstoreName)

host := componenttest.NewNopHost()
err := storageExtension.Start(ctx, host)
require.Error(t, err)
require.EqualError(t, err, fmt.Sprintf("duplicate memory storage name %s", memstoreName))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is the error in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are starting the storageExtension again. This will lead to an error. Ref

Copy link
Member

@yurishkuro yurishkuro Jan 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but the error message doesn't seem to be about extension starting.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're testing specifically the dual entry condition, it would be better to test it explicitly, not via a side-effect of starting extension twice. I assume it's possible to create dual entry in the config if using a different type of storage and then giving it the same name.

Copy link
Contributor Author

@chirag-ghosh chirag-ghosh Jan 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently only Memory type storage is supported. So I was unable to recreate this using config. Have changed the function name for now.

}

func TestGetStorageFactoryError(t *testing.T) {
makeStorageExtension(t, memstoreName)

host := componenttest.NewNopHost()
_, err := GetStorageFactory(memstoreName, host)
require.Error(t, err)
require.EqualError(t, err, fmt.Sprintf("cannot find extension '%s' (make sure it's defined earlier in the config)", ID))
}

func TestStorageExtension(t *testing.T) {
storageExtension := makeStorageExtension(t, memstoreName)

host := storageHost{t: t, storageExtension: storageExtension}
yurishkuro marked this conversation as resolved.
Show resolved Hide resolved

_, err := GetStorageFactory(memstoreName, host)
require.NoError(t, err)
}

func makeStorageExtension(t *testing.T, memstoreName string) component.Component {
extensionFactory := NewFactory()

ctx := context.Background()
telemetrySettings := component.TelemetrySettings{
Logger: zap.L(),
TracerProvider: nooptrace.NewTracerProvider(),
MeterProvider: noopmetric.NewMeterProvider(),
}
config := &Config{
Memory: map[string]memoryCfg.Configuration{
memstoreName: {MaxTraces: 10000},
},
}
err := config.Validate()
require.NoError(t, err)

storageExtension, err := extensionFactory.CreateExtension(ctx, extension.CreateSettings{
ID: ID,
TelemetrySettings: telemetrySettings,
BuildInfo: component.NewDefaultBuildInfo(),
}, config)
require.NoError(t, err)

host := componenttest.NewNopHost()
err = storageExtension.Start(ctx, host)
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, storageExtension.Shutdown(ctx)) })

return storageExtension
}
Loading