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 the rest of V2 factories and interfaces #626

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion consumer/consumer.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ type MetricsConsumer interface {
// MetricsConsumerV2 is the new metrics consumer interface that receives data.MetricData, processes it
// as needed, and sends it to the next processing node if any or to the destination.
type MetricsConsumerV2 interface {
ConsumeMetricsData(ctx context.Context, md data.MetricData) error
ConsumeMetricsV2(ctx context.Context, md data.MetricData) error
}

// BaseTraceConsumer defines a common interface for TraceConsumer and TraceConsumerV2.
Expand Down
6 changes: 6 additions & 0 deletions exporter/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,9 @@ type MetricsExporter interface {
consumer.MetricsConsumer
Exporter
}

// MetricsExporterV2 is a MetricsConsumerV2 that is also an Exporter.
type MetricsExporterV2 interface {
consumer.MetricsConsumerV2
Exporter
}
17 changes: 14 additions & 3 deletions exporter/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package exporter

import (
"context"
"fmt"

"go.uber.org/zap"
Expand All @@ -32,7 +33,7 @@ type BaseFactory interface {
// configuration and should not cause side-effects that prevent the creation
// of multiple instances of the Exporter.
// The object returned by this method needs to pass the checks implemented by
// 'conifgcheck.ValidateConfig'. It is recommended to have such check in the
// 'configcheck.ValidateConfig'. It is recommended to have such check in the
// tests of any implementation of the Factory interface.
CreateDefaultConfig() configmodels.Exporter
}
Expand All @@ -48,6 +49,13 @@ type Factory interface {
CreateMetricsExporter(logger *zap.Logger, cfg configmodels.Exporter) (MetricsExporter, error)
}

// CreationParams is passed to Create* functions in FactoryV2.
type CreationParams struct {
// Logger that the factory can use during creation and can pass to the created
// component to be used later as well.
Logger *zap.Logger
}

// FactoryV2 can create TraceExporterV2 and MetricsExporterV2. This is the
// new factory type that can create new style exporters.
type FactoryV2 interface {
Expand All @@ -56,9 +64,12 @@ type FactoryV2 interface {
// CreateTraceReceiverV2 creates a trace receiver based on this config.
// If the receiver type does not support tracing or if the config is not valid
// error will be returned instead.
CreateTraceExporterV2(logger *zap.Logger, cfg configmodels.Exporter) (TraceExporterV2, error)
CreateTraceExporterV2(ctx context.Context, params CreationParams, cfg configmodels.Exporter) (TraceExporterV2, error)
Copy link
Contributor

@jrcamp jrcamp Mar 16, 2020

Choose a reason for hiding this comment

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

Sorry, this didn't occur to me until just now. Do we need V2 in these names like CreateTraceExporterV2? Since the type signatures are different I think that is enough to distinguish Factory and FactoryV2 by keeping CreateTraceExporter with just different parameters. From some quick testing this seems to work.

If for some reason we want components to be explicit about their version we could have the factory have a Version() method but doesn't look like that's needed yet.

Copy link
Member Author

Choose a reason for hiding this comment

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

In that case it will make impossible for a factory to implement both interfaces, which may be desirable during transition period. Let me think a bit.

Side note: I cannot find the relevant part of Golang spec that describes interface method matching and thus making what you propose compliant with the spec - do you know where is it formally defined?

Copy link
Member Author

Choose a reason for hiding this comment

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

I suggest that we merge as is to avoid more delays and I will look into this separately.


// TODO: Add CreateMetricsExporterV2.
// CreateMetricsExporterV2 creates a metrics receiver based on this config.
// If the receiver type does not support metrics or if the config is not valid
// error will be returned instead.
CreateMetricsExporterV2(ctx context.Context, params CreationParams, cfg configmodels.Exporter) (MetricsExporterV2, error)
}

// Build takes a list of exporter factories and returns a map of type map[string]Factory
Expand Down
2 changes: 1 addition & 1 deletion extension/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type Factory interface {
// configuration and should not cause side-effects that prevent the creation
// of multiple instances of the Extension.
// The object returned by this method needs to pass the checks implemented by
// 'conifgcheck.ValidateConfig'. It is recommended to have such check in the
// 'configcheck.ValidateConfig'. It is recommended to have such check in the
// tests of any implementation of the Factory interface.
CreateDefaultConfig() configmodels.Extension

Expand Down
37 changes: 34 additions & 3 deletions processor/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package processor

import (
"context"
"fmt"

"go.uber.org/zap"
Expand All @@ -23,8 +24,8 @@ import (
"github.com/open-telemetry/opentelemetry-collector/consumer"
)

// Factory is factory interface for processors.
type Factory interface {
// BaseFactory defines the common functions for all processor factories.
type BaseFactory interface {
// Type gets the type of the Processor created by this factory.
Type() string

Expand All @@ -33,9 +34,14 @@ type Factory interface {
// configuration and should not cause side-effects that prevent the creation
// of multiple instances of the Processor.
// The object returned by this method needs to pass the checks implemented by
// 'conifgcheck.ValidateConfig'. It is recommended to have such check in the
// 'configcheck.ValidateConfig'. It is recommended to have such check in the
// tests of any implementation of the Factory interface.
CreateDefaultConfig() configmodels.Processor
}

// Factory is factory interface for processors.
type Factory interface {
BaseFactory

// CreateTraceProcessor creates a trace processor based on this config.
// If the processor type does not support tracing or if the config is not valid
Expand All @@ -50,6 +56,31 @@ type Factory interface {
cfg configmodels.Processor) (MetricsProcessor, error)
}

// CreationParams is passed to Create* functions in FactoryV2.
type CreationParams struct {
// Logger that the factory can use during creation and can pass to the created
// component to be used later as well.
Logger *zap.Logger
}

// FactoryV2 is factory interface for processors. This is the
// new factory type that can create new style processors.
type FactoryV2 interface {
BaseFactory

// CreateTraceProcessorV2 creates a trace processor based on this config.
// If the processor type does not support tracing or if the config is not valid
// error will be returned instead.
CreateTraceProcessorV2(ctx context.Context, params CreationParams,
nextConsumer consumer.TraceConsumerV2, cfg configmodels.Processor) (TraceProcessorV2, error)

// CreateMetricsProcessorV2 creates a metrics processor based on this config.
// If the processor type does not support metrics or if the config is not valid
// error will be returned instead.
CreateMetricsProcessorV2(ctx context.Context, params CreationParams,
nextConsumer consumer.MetricsConsumerV2, cfg configmodels.Processor) (MetricsProcessorV2, error)
}

// Build takes a list of processor factories and returns a map of type map[string]Factory
// with factory type as keys. It returns a non-nil error when more than one factories
// have the same type.
Expand Down
12 changes: 12 additions & 0 deletions processor/processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,18 @@ type MetricsProcessor interface {
Processor
}

// TraceProcessorV2 composes TraceConsumerV2 with some additional processor-specific functions.
type TraceProcessorV2 interface {
consumer.TraceConsumerV2
Processor
}

// MetricsProcessorV2 composes MetricsConsumerV2 with some additional processor-specific functions.
type MetricsProcessorV2 interface {
consumer.MetricsConsumerV2
Processor
}

type DualTypeProcessor interface {
consumer.TraceConsumer
consumer.MetricsConsumer
Expand Down
15 changes: 13 additions & 2 deletions receiver/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,13 @@ type Factory interface {
consumer consumer.MetricsConsumer) (MetricsReceiver, error)
}

// CreationParams is passed to Create* functions in FactoryV2.
type CreationParams struct {
// Logger that the factory can use during creation and can pass to the created
// component to be used later as well.
Logger *zap.Logger
}

// FactoryV2 can create TraceReceiverV2 and MetricsReceiverV2. This is the
// new factory type that can create new style receivers.
type FactoryV2 interface {
Expand All @@ -84,10 +91,14 @@ type FactoryV2 interface {
// CreateTraceReceiverV2 creates a trace receiver based on this config.
// If the receiver type does not support tracing or if the config is not valid
// error will be returned instead.
CreateTraceReceiverV2(ctx context.Context, logger *zap.Logger, cfg configmodels.Receiver,
CreateTraceReceiverV2(ctx context.Context, params CreationParams, cfg configmodels.Receiver,
nextConsumer consumer.TraceConsumerV2) (TraceReceiver, error)

// TODO: add CreateMetricsReceiverV2.
// CreateMetricsReceiverV2 creates a metrics receiver based on this config.
// If the receiver type does not support metrics or if the config is not valid
// error will be returned instead.
CreateMetricsReceiverV2(ctx context.Context, params CreationParams, cfg configmodels.Receiver,
nextConsumer consumer.MetricsConsumerV2) (MetricsReceiver, error)
}

// Build takes a list of receiver factories and returns a map of type map[string]Factory
Expand Down
5 changes: 3 additions & 2 deletions service/builder/receivers_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,17 +320,18 @@ func createTraceReceiver(
nextConsumer consumer.BaseTraceConsumer,
) (receiver.TraceReceiver, error) {
if factoryV2, ok := factory.(receiver.FactoryV2); ok {
creationParams := receiver.CreationParams{Logger: logger}

// If both receiver and consumer are of the new type (can manipulate on internal data structure),
// use FactoryV2.CreateTraceReceiverV2.
if nextConsumerV2, ok := nextConsumer.(consumer.TraceConsumerV2); ok {
return factoryV2.CreateTraceReceiverV2(ctx, logger, cfg, nextConsumerV2)
return factoryV2.CreateTraceReceiverV2(ctx, creationParams, cfg, nextConsumerV2)
}

// If receiver is of the new type, but downstream consumer is of the old type,
// use internalToOCTraceConverter compatibility shim.
traceConverter := consumer.NewInternalToOCTraceConverter(nextConsumer.(consumer.TraceConsumer))
return factoryV2.CreateTraceReceiverV2(ctx, logger, cfg, traceConverter)
return factoryV2.CreateTraceReceiverV2(ctx, creationParams, cfg, traceConverter)
}

// If both receiver and consumer are of the old type (can manipulate on OC traces only),
Expand Down
5 changes: 3 additions & 2 deletions service/builder/receivers_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -400,15 +400,16 @@ func (b *newStyleReceiverFactory) CustomUnmarshaler() receiver.CustomUnmarshaler

func (b *newStyleReceiverFactory) CreateTraceReceiverV2(
ctx context.Context,
logger *zap.Logger,
params receiver.CreationParams,
cfg configmodels.Receiver,
nextConsumer consumer.TraceConsumerV2,
) (receiver.TraceReceiver, error) {
return &config.ExampleReceiverProducer{}, nil
}

func (b *newStyleReceiverFactory) CreateMetricsReceiverV2(
logger *zap.Logger,
ctx context.Context,
params receiver.CreationParams,
cfg configmodels.Receiver,
consumer consumer.MetricsConsumerV2,
) (receiver.MetricsReceiver, error) {
Expand Down