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.

115 changes: 115 additions & 0 deletions cmd/jaeger/internal/extension/jaegerstorage/extension_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
// 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"
)

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

func (host storageHost) GetExtensions() map[component.ID]component.Component {
myMap := make(map[component.ID]component.Component)
myMap[ID] = host.storageExtension
return myMap
chirag-ghosh marked this conversation as resolved.
Show resolved Hide resolved
}

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 TestStartStorageExtensionError(t *testing.T) {
chirag-ghosh marked this conversation as resolved.
Show resolved Hide resolved
ctx := context.Background()
const memstoreName = "memstore"
chirag-ghosh marked this conversation as resolved.
Show resolved Hide resolved

storageExtension := makeStorageExtension(t, memstoreName)

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

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) {
ctx := context.Background()
const memstoreName = "memstore"

storageExtension := makeStorageExtension(t, memstoreName)

host := componenttest.NewNopHost()
err := storageExtension.Start(ctx, host)
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, storageExtension.Shutdown(ctx)) })
chirag-ghosh marked this conversation as resolved.
Show resolved Hide resolved

_, 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) {
ctx := context.Background()
const memstoreName = "memstore"

storageExtension := makeStorageExtension(t, memstoreName)

host := storageHost{t: t, storageExtension: storageExtension}
yurishkuro marked this conversation as resolved.
Show resolved Hide resolved
err := storageExtension.Start(ctx, host)
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, storageExtension.Shutdown(ctx)) })

_, 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},
},
}

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

return storageExtension
}
Loading