Skip to content

Commit

Permalink
Updates receiver creators to take a zap.Logger parameter. (#74)
Browse files Browse the repository at this point in the history
* Updates receiver creators to take a zap.Logger paramater.

* Updates prometheusreceviver CreateMetricReceiver to return nil error explicitly.

* Updates factory tests to use zap.NewNop() rather than nil.
  • Loading branch information
dinooliva authored and songy23 committed Jul 1, 2019
1 parent 07fc0f3 commit 5b9be2f
Show file tree
Hide file tree
Showing 13 changed files with 57 additions and 22 deletions.
4 changes: 2 additions & 2 deletions cmd/occollector/app/builder/receivers_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,11 +188,11 @@ func (rb *ReceiversBuilder) attachReceiverToPipelines(
junction := buildFanoutTraceConsumer(pipelineProcessors)

// Now create the receiver and tell it to send to the junction point.
receiver.trace, err = factory.CreateTraceReceiver(context.Background(), config, junction)
receiver.trace, err = factory.CreateTraceReceiver(context.Background(), rb.logger, config, junction)

case models.MetricsDataType:
junction := buildFanoutMetricConsumer(pipelineProcessors)
receiver.metrics, err = factory.CreateMetricsReceiver(config, junction)
receiver.metrics, err = factory.CreateMetricsReceiver(rb.logger, config, junction)
}

if err != nil {
Expand Down
4 changes: 4 additions & 0 deletions configv2/example_factories.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ func (f *ExampleReceiverFactory) CreateDefaultConfig() models.Receiver {
// CreateTraceReceiver creates a trace receiver based on this config.
func (f *ExampleReceiverFactory) CreateTraceReceiver(
ctx context.Context,
logger *zap.Logger,
cfg models.Receiver,
nextConsumer consumer.TraceConsumer,
) (receiver.TraceReceiver, error) {
Expand All @@ -79,6 +80,7 @@ func (f *ExampleReceiverFactory) CreateTraceReceiver(

// CreateMetricsReceiver creates a metrics receiver based on this config.
func (f *ExampleReceiverFactory) CreateMetricsReceiver(
logger *zap.Logger,
cfg models.Receiver,
nextConsumer consumer.MetricsConsumer,
) (receiver.MetricsReceiver, error) {
Expand Down Expand Up @@ -205,6 +207,7 @@ func (f *MultiProtoReceiverFactory) CreateDefaultConfig() models.Receiver {
// CreateTraceReceiver creates a trace receiver based on this config.
func (f *MultiProtoReceiverFactory) CreateTraceReceiver(
ctx context.Context,
logger *zap.Logger,
cfg models.Receiver,
nextConsumer consumer.TraceConsumer,
) (receiver.TraceReceiver, error) {
Expand All @@ -214,6 +217,7 @@ func (f *MultiProtoReceiverFactory) CreateTraceReceiver(

// CreateMetricsReceiver creates a metrics receiver based on this config.
func (f *MultiProtoReceiverFactory) CreateMetricsReceiver(
logger *zap.Logger,
cfg models.Receiver,
consumer consumer.MetricsConsumer,
) (receiver.MetricsReceiver, error) {
Expand Down
4 changes: 2 additions & 2 deletions factories/factories.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,13 @@ type ReceiverFactory interface {
// CreateTraceReceiver 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.
CreateTraceReceiver(ctx context.Context, cfg models.Receiver,
CreateTraceReceiver(ctx context.Context, logger *zap.Logger, cfg models.Receiver,
nextConsumer consumer.TraceConsumer) (receiver.TraceReceiver, error)

// CreateMetricsReceiver 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.
CreateMetricsReceiver(cfg models.Receiver,
CreateMetricsReceiver(logger *zap.Logger, cfg models.Receiver,
consumer consumer.MetricsConsumer) (receiver.MetricsReceiver, error)
}

Expand Down
2 changes: 2 additions & 0 deletions factories/factories_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ func (f *ExampleReceiverFactory) CreateDefaultConfig() models.Receiver {
// CreateTraceReceiver creates a trace receiver based on this config.
func (f *ExampleReceiverFactory) CreateTraceReceiver(
ctx context.Context,
logger *zap.Logger,
cfg models.Receiver,
nextConsumer consumer.TraceConsumer,
) (receiver.TraceReceiver, error) {
Expand All @@ -56,6 +57,7 @@ func (f *ExampleReceiverFactory) CreateTraceReceiver(

// CreateMetricsReceiver creates a metrics receiver based on this config.
func (f *ExampleReceiverFactory) CreateMetricsReceiver(
logger *zap.Logger,
cfg models.Receiver,
consumer consumer.MetricsConsumer,
) (receiver.MetricsReceiver, error) {
Expand Down
4 changes: 4 additions & 0 deletions receiver/jaegerreceiver/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import (
"net"
"strconv"

"go.uber.org/zap"

"github.com/open-telemetry/opentelemetry-service/consumer"
"github.com/open-telemetry/opentelemetry-service/factories"
"github.com/open-telemetry/opentelemetry-service/models"
Expand Down Expand Up @@ -79,6 +81,7 @@ func (f *receiverFactory) CreateDefaultConfig() models.Receiver {
// CreateTraceReceiver creates a trace receiver based on provided config.
func (f *receiverFactory) CreateTraceReceiver(
ctx context.Context,
logger *zap.Logger,
cfg models.Receiver,
nextConsumer consumer.TraceConsumer,
) (receiver.TraceReceiver, error) {
Expand Down Expand Up @@ -125,6 +128,7 @@ func (f *receiverFactory) CreateTraceReceiver(

// CreateMetricsReceiver creates a metrics receiver based on provided config.
func (f *receiverFactory) CreateMetricsReceiver(
logger *zap.Logger,
cfg models.Receiver,
consumer consumer.MetricsConsumer,
) (receiver.MetricsReceiver, error) {
Expand Down
16 changes: 9 additions & 7 deletions receiver/jaegerreceiver/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
"context"
"testing"

"go.uber.org/zap"

"github.com/stretchr/testify/assert"

"github.com/open-telemetry/opentelemetry-service/factories"
Expand All @@ -33,11 +35,11 @@ func TestCreateReceiver(t *testing.T) {
factory := factories.GetReceiverFactory(typeStr)
cfg := factory.CreateDefaultConfig()

tReceiver, err := factory.CreateTraceReceiver(context.Background(), cfg, nil)
tReceiver, err := factory.CreateTraceReceiver(context.Background(), zap.NewNop(), cfg, nil)
assert.NoError(t, err, "receiver creation failed")
assert.NotNil(t, tReceiver, "receiver creation failed")

mReceiver, err := factory.CreateMetricsReceiver(cfg, nil)
mReceiver, err := factory.CreateMetricsReceiver(zap.NewNop(), cfg, nil)
assert.Equal(t, err, factories.ErrDataTypeIsNotSupported)
assert.Nil(t, mReceiver)
}
Expand All @@ -49,7 +51,7 @@ func TestCreateNoEndpoints(t *testing.T) {

rCfg.Protocols[protoThriftHTTP].Endpoint = ""
rCfg.Protocols[protoThriftTChannel].Endpoint = ""
_, err := factory.CreateTraceReceiver(context.Background(), cfg, nil)
_, err := factory.CreateTraceReceiver(context.Background(), zap.NewNop(), cfg, nil)
assert.Error(t, err, "receiver creation with no endpoints must fail")
}

Expand All @@ -59,7 +61,7 @@ func TestCreateInvalidTChannelEndpoint(t *testing.T) {
rCfg := cfg.(*ConfigV2)

rCfg.Protocols[protoThriftTChannel].Endpoint = ""
_, err := factory.CreateTraceReceiver(context.Background(), cfg, nil)
_, err := factory.CreateTraceReceiver(context.Background(), zap.NewNop(), cfg, nil)
assert.Error(t, err, "receiver creation with invalid tchannel endpoint must fail")
}

Expand All @@ -69,7 +71,7 @@ func TestCreateNoPort(t *testing.T) {
rCfg := cfg.(*ConfigV2)

rCfg.Protocols[protoThriftHTTP].Endpoint = "127.0.0.1:"
_, err := factory.CreateTraceReceiver(context.Background(), cfg, nil)
_, err := factory.CreateTraceReceiver(context.Background(), zap.NewNop(), cfg, nil)
assert.Error(t, err, "receiver creation with no port number must fail")
}

Expand All @@ -79,7 +81,7 @@ func TestCreateLargePort(t *testing.T) {
rCfg := cfg.(*ConfigV2)

rCfg.Protocols[protoThriftHTTP].Endpoint = "127.0.0.1:65536"
_, err := factory.CreateTraceReceiver(context.Background(), cfg, nil)
_, err := factory.CreateTraceReceiver(context.Background(), zap.NewNop(), cfg, nil)
assert.Error(t, err, "receiver creation with too large port number must fail")
}

Expand All @@ -90,6 +92,6 @@ func TestCreateNoProtocols(t *testing.T) {

delete(rCfg.Protocols, protoThriftHTTP)
delete(rCfg.Protocols, protoThriftTChannel)
_, err := factory.CreateTraceReceiver(context.Background(), cfg, nil)
_, err := factory.CreateTraceReceiver(context.Background(), zap.NewNop(), cfg, nil)
assert.Error(t, err, "receiver creation with no protocols must fail")
}
4 changes: 4 additions & 0 deletions receiver/opencensusreceiver/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ package opencensusreceiver
import (
"context"

"go.uber.org/zap"

"github.com/open-telemetry/opentelemetry-service/factories"
"github.com/open-telemetry/opentelemetry-service/models"

Expand Down Expand Up @@ -60,6 +62,7 @@ func (f *receiverFactory) CreateDefaultConfig() models.Receiver {
// CreateTraceReceiver creates a trace receiver based on provided config.
func (f *receiverFactory) CreateTraceReceiver(
ctx context.Context,
logger *zap.Logger,
cfg models.Receiver,
nextConsumer consumer.TraceConsumer,
) (receiver.TraceReceiver, error) {
Expand All @@ -76,6 +79,7 @@ func (f *receiverFactory) CreateTraceReceiver(

// CreateMetricsReceiver creates a metrics receiver based on provided config.
func (f *receiverFactory) CreateMetricsReceiver(
logger *zap.Logger,
cfg models.Receiver,
consumer consumer.MetricsConsumer,
) (receiver.MetricsReceiver, error) {
Expand Down
6 changes: 4 additions & 2 deletions receiver/opencensusreceiver/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
"context"
"testing"

"go.uber.org/zap"

"github.com/stretchr/testify/assert"

"github.com/open-telemetry/opentelemetry-service/factories"
Expand All @@ -37,13 +39,13 @@ func TestCreateReceiver(t *testing.T) {
config := cfg.(*ConfigV2)
config.Endpoint = testutils.GetAvailableLocalAddress(t)

tReceiver, err := factory.CreateTraceReceiver(context.Background(), cfg, nil)
tReceiver, err := factory.CreateTraceReceiver(context.Background(), zap.NewNop(), cfg, nil)
assert.NotNil(t, tReceiver)
assert.Nil(t, err)

// The default config does not provide scrape_config so we expect that metrics receiver
// creation must also fail.
mReceiver, err := factory.CreateMetricsReceiver(cfg, nil)
mReceiver, err := factory.CreateMetricsReceiver(zap.NewNop(), cfg, nil)
assert.NotNil(t, mReceiver)
assert.Nil(t, err)
}
9 changes: 4 additions & 5 deletions receiver/prometheusreceiver/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"fmt"

"github.com/spf13/viper"
"go.uber.org/zap"
"gopkg.in/yaml.v2"

"github.com/open-telemetry/opentelemetry-service/consumer"
Expand Down Expand Up @@ -98,6 +99,7 @@ func (f *ReceiverFactory) CreateDefaultConfig() models.Receiver {
// CreateTraceReceiver creates a trace receiver based on provided config.
func (f *ReceiverFactory) CreateTraceReceiver(
ctx context.Context,
logger *zap.Logger,
cfg models.Receiver,
nextConsumer consumer.TraceConsumer,
) (receiver.TraceReceiver, error) {
Expand All @@ -107,6 +109,7 @@ func (f *ReceiverFactory) CreateTraceReceiver(

// CreateMetricsReceiver creates a metrics receiver based on provided config.
func (f *ReceiverFactory) CreateMetricsReceiver(
logger *zap.Logger,
cfg models.Receiver,
consumer consumer.MetricsConsumer,
) (receiver.MetricsReceiver, error) {
Expand All @@ -123,9 +126,5 @@ func (f *ReceiverFactory) CreateMetricsReceiver(
if config.ScrapeConfig == nil || len(config.ScrapeConfig.ScrapeConfigs) == 0 {
return nil, errNilScrapeConfig
}
pr := &Preceiver{
cfg: &config,
consumer: consumer,
}
return pr, nil
return newPrometheusReceiver(logger, &config, consumer), nil
}
6 changes: 4 additions & 2 deletions receiver/prometheusreceiver/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
"context"
"testing"

"go.uber.org/zap"

"github.com/stretchr/testify/assert"

"github.com/open-telemetry/opentelemetry-service/factories"
Expand All @@ -33,13 +35,13 @@ func TestCreateReceiver(t *testing.T) {
factory := factories.GetReceiverFactory(typeStr)
cfg := factory.CreateDefaultConfig()

tReceiver, err := factory.CreateTraceReceiver(context.Background(), cfg, nil)
tReceiver, err := factory.CreateTraceReceiver(context.Background(), zap.NewNop(), cfg, nil)
assert.Equal(t, err, factories.ErrDataTypeIsNotSupported)
assert.Nil(t, tReceiver)

// The default config does not provide scrape_config so we expect that metrics receiver
// creation must also fail.
mReceiver, err := factory.CreateMetricsReceiver(cfg, nil)
mReceiver, err := factory.CreateMetricsReceiver(zap.NewNop(), cfg, nil)
assert.Equal(t, err, errNilScrapeConfig)
assert.Nil(t, mReceiver)
}
10 changes: 10 additions & 0 deletions receiver/prometheusreceiver/metrics_receiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,16 @@ func New(logger *zap.Logger, v *viper.Viper, next consumer.MetricsConsumer) (*Pr
return pr, nil
}

// New creates a new prometheus.Receiver reference.
func newPrometheusReceiver(logger *zap.Logger, cfg *Configuration, next consumer.MetricsConsumer) *Preceiver {
pr := &Preceiver{
cfg: cfg,
consumer: next,
logger: logger,
}
return pr
}

const metricsSource string = "Prometheus"

// MetricsSource returns the name of the metrics data source.
Expand Down
4 changes: 4 additions & 0 deletions receiver/zipkinreceiver/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ package zipkinreceiver
import (
"context"

"go.uber.org/zap"

"github.com/open-telemetry/opentelemetry-service/consumer"
"github.com/open-telemetry/opentelemetry-service/factories"
"github.com/open-telemetry/opentelemetry-service/models"
Expand Down Expand Up @@ -62,6 +64,7 @@ func (f *ReceiverFactory) CreateDefaultConfig() models.Receiver {
// CreateTraceReceiver creates a trace receiver based on provided config.
func (f *ReceiverFactory) CreateTraceReceiver(
ctx context.Context,
logger *zap.Logger,
cfg models.Receiver,
nextConsumer consumer.TraceConsumer,
) (receiver.TraceReceiver, error) {
Expand All @@ -72,6 +75,7 @@ func (f *ReceiverFactory) CreateTraceReceiver(

// CreateMetricsReceiver creates a metrics receiver based on provided config.
func (f *ReceiverFactory) CreateMetricsReceiver(
logger *zap.Logger,
cfg models.Receiver,
consumer consumer.MetricsConsumer,
) (receiver.MetricsReceiver, error) {
Expand Down
6 changes: 4 additions & 2 deletions receiver/zipkinreceiver/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
"context"
"testing"

"go.uber.org/zap"

"github.com/stretchr/testify/assert"

"github.com/open-telemetry/opentelemetry-service/data"
Expand All @@ -39,11 +41,11 @@ func TestCreateReceiver(t *testing.T) {
factory := factories.GetReceiverFactory(typeStr)
cfg := factory.CreateDefaultConfig()

tReceiver, err := factory.CreateTraceReceiver(context.Background(), cfg, &mockTraceConsumer{})
tReceiver, err := factory.CreateTraceReceiver(context.Background(), zap.NewNop(), cfg, &mockTraceConsumer{})
assert.Nil(t, err, "receiver creation failed")
assert.NotNil(t, tReceiver, "receiver creation failed")

mReceiver, err := factory.CreateMetricsReceiver(cfg, nil)
mReceiver, err := factory.CreateMetricsReceiver(zap.NewNop(), cfg, nil)
assert.Equal(t, err, factories.ErrDataTypeIsNotSupported)
assert.Nil(t, mReceiver)
}

0 comments on commit 5b9be2f

Please sign in to comment.