Skip to content

Commit

Permalink
Pass processor params to the detectors (#1745)
Browse files Browse the repository at this point in the history
Fix ecs detector to not use the default golang logger.

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
  • Loading branch information
bogdandrutu authored Dec 3, 2020
1 parent 124933a commit 471b7c6
Show file tree
Hide file tree
Showing 18 changed files with 71 additions and 46 deletions.
15 changes: 7 additions & 8 deletions processor/resourcedetectionprocessor/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"go.opentelemetry.io/collector/config/configmodels"
"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/processor/processorhelper"
"go.uber.org/zap"

"github.com/open-telemetry/opentelemetry-collector-contrib/processor/resourcedetectionprocessor/internal"
"github.com/open-telemetry/opentelemetry-collector-contrib/processor/resourcedetectionprocessor/internal/aws/ec2"
Expand Down Expand Up @@ -98,7 +97,7 @@ func (f *factory) createTraceProcessor(
cfg configmodels.Processor,
nextConsumer consumer.TracesConsumer,
) (component.TracesProcessor, error) {
rdp, err := f.getResourceDetectionProcessor(params.Logger, cfg)
rdp, err := f.getResourceDetectionProcessor(params, cfg)
if err != nil {
return nil, err
}
Expand All @@ -117,7 +116,7 @@ func (f *factory) createMetricsProcessor(
cfg configmodels.Processor,
nextConsumer consumer.MetricsConsumer,
) (component.MetricsProcessor, error) {
rdp, err := f.getResourceDetectionProcessor(params.Logger, cfg)
rdp, err := f.getResourceDetectionProcessor(params, cfg)
if err != nil {
return nil, err
}
Expand All @@ -136,7 +135,7 @@ func (f *factory) createLogsProcessor(
cfg configmodels.Processor,
nextConsumer consumer.LogsConsumer,
) (component.LogsProcessor, error) {
rdp, err := f.getResourceDetectionProcessor(params.Logger, cfg)
rdp, err := f.getResourceDetectionProcessor(params, cfg)
if err != nil {
return nil, err
}
Expand All @@ -150,12 +149,12 @@ func (f *factory) createLogsProcessor(
}

func (f *factory) getResourceDetectionProcessor(
logger *zap.Logger,
params component.ProcessorCreateParams,
cfg configmodels.Processor,
) (*resourceDetectionProcessor, error) {
oCfg := cfg.(*Config)

provider, err := f.getResourceProvider(logger, cfg.Name(), oCfg.Timeout, oCfg.Detectors)
provider, err := f.getResourceProvider(params, cfg.Name(), oCfg.Timeout, oCfg.Detectors)
if err != nil {
return nil, err
}
Expand All @@ -167,7 +166,7 @@ func (f *factory) getResourceDetectionProcessor(
}

func (f *factory) getResourceProvider(
logger *zap.Logger,
params component.ProcessorCreateParams,
processorName string,
timeout time.Duration,
configuredDetectors []string,
Expand All @@ -184,7 +183,7 @@ func (f *factory) getResourceProvider(
detectorTypes = append(detectorTypes, internal.DetectorType(strings.TrimSpace(key)))
}

provider, err := f.resourceProviderFactory.CreateResourceProvider(logger, timeout, detectorTypes...)
provider, err := f.resourceProviderFactory.CreateResourceProvider(params, timeout, detectorTypes...)
if err != nil {
return nil, err
}
Expand Down
3 changes: 2 additions & 1 deletion processor/resourcedetectionprocessor/internal/aws/ec2/ec2.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"fmt"

"github.com/aws/aws-sdk-go/aws/session"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/consumer/pdata"
"go.opentelemetry.io/collector/translator/conventions"

Expand All @@ -35,7 +36,7 @@ type Detector struct {
metadataProvider metadataProvider
}

func NewDetector() (internal.Detector, error) {
func NewDetector(component.ProcessorCreateParams) (internal.Detector, error) {
sess, err := session.NewSession()
if err != nil {
return nil, err
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ import (
"github.com/aws/aws-sdk-go/aws/ec2metadata"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/consumer/pdata"
"go.uber.org/zap"

"github.com/open-telemetry/opentelemetry-collector-contrib/processor/resourcedetectionprocessor/internal"
)
Expand Down Expand Up @@ -58,7 +60,7 @@ func (mm mockMetadata) hostname(ctx context.Context) (string, error) {
}

func TestNewDetector(t *testing.T) {
detector, err := NewDetector()
detector, err := NewDetector(component.ProcessorCreateParams{Logger: zap.NewNop()})
assert.NotNil(t, detector)
assert.NoError(t, err)
}
Expand Down
5 changes: 3 additions & 2 deletions processor/resourcedetectionprocessor/internal/aws/ecs/ecs.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"os"
"strings"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/consumer/pdata"
"go.opentelemetry.io/collector/translator/conventions"

Expand All @@ -40,8 +41,8 @@ type Detector struct {
provider ecsMetadataProvider
}

func NewDetector() (internal.Detector, error) {
return &Detector{provider: &ecsMetadataProviderImpl{client: &http.Client{}}}, nil
func NewDetector(params component.ProcessorCreateParams) (internal.Detector, error) {
return &Detector{provider: &ecsMetadataProviderImpl{logger: params.Logger, client: &http.Client{}}}, nil
}

// Records metadata retrieved from the ECS Task Metadata Endpoint (TMDE) as resource attributes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/consumer/pdata"
"go.uber.org/zap"

"github.com/open-telemetry/opentelemetry-collector-contrib/processor/resourcedetectionprocessor/internal"
)
Expand Down Expand Up @@ -50,21 +52,21 @@ func (md *mockMetaDataProvider) fetchTaskMetaData(tmde string) (*TaskMetaData, e
return tmd, nil
}

func (md *mockMetaDataProvider) fetchContainerMetaData(tmde string) (*Container, error) {
func (md *mockMetaDataProvider) fetchContainerMetaData(string) (*Container, error) {
c := createTestContainer(md.isV4)
return &c, nil
}

func Test_ecsNewDetector(t *testing.T) {
d, err := NewDetector()
d, err := NewDetector(component.ProcessorCreateParams{Logger: zap.NewNop()})

assert.NotNil(t, d)
assert.Nil(t, err)
}

func Test_detectorReturnsIfNoEnvVars(t *testing.T) {
os.Clearenv()
d, _ := NewDetector()
d, _ := NewDetector(component.ProcessorCreateParams{Logger: zap.NewNop()})
res, err := d.Detect(context.TODO())

assert.Nil(t, err)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ package ecs

import (
"encoding/json"
"log"
"net/http"

"go.uber.org/zap"
)

type TaskMetaData struct {
Expand Down Expand Up @@ -45,14 +46,15 @@ type LogData struct {
}

type ecsMetadataProviderImpl struct {
logger *zap.Logger
client HTTPClient
}

var _ ecsMetadataProvider = &ecsMetadataProviderImpl{}

// Retrieves the metadata for a task running on Amazon ECS
func (md *ecsMetadataProviderImpl) fetchTaskMetaData(tmde string) (*TaskMetaData, error) {
ret, err := fetch(tmde+"/task", md, true)
ret, err := fetch(md.logger, tmde+"/task", md, true)
if ret == nil {
return nil, err
}
Expand All @@ -62,26 +64,26 @@ func (md *ecsMetadataProviderImpl) fetchTaskMetaData(tmde string) (*TaskMetaData

// Retrieves the metadata for the Amazon ECS Container the collector is running on
func (md *ecsMetadataProviderImpl) fetchContainerMetaData(tmde string) (*Container, error) {
ret, err := fetch(tmde, md, false)
ret, err := fetch(md.logger, tmde, md, false)
if ret == nil {
return nil, err
}

return ret.(*Container), err
}

func fetch(tmde string, md *ecsMetadataProviderImpl, task bool) (tmdeResp interface{}, err error) {
func fetch(logger *zap.Logger, tmde string, md *ecsMetadataProviderImpl, task bool) (tmdeResp interface{}, err error) {
req, err := http.NewRequest(http.MethodGet, tmde, nil)

if err != nil {
log.Printf("Received error constructing request to ECS Task Metadata Endpoint: %v", err)
logger.Error("Received error constructing request to ECS Task Metadata Endpoint", zap.Error(err))
return nil, err
}

resp, err := md.client.Do(req)

if err != nil {
log.Printf("Received error from ECS Task Metadata Endpoint: %v", err)
logger.Error("Received error from ECS Task Metadata Endpoint", zap.Error(err))
return nil, err
}

Expand All @@ -95,7 +97,7 @@ func fetch(tmde string, md *ecsMetadataProviderImpl, task bool) (tmdeResp interf
defer resp.Body.Close()

if err != nil {
log.Printf("Encountered unexpected error reading response from ECS Task Metadata Endpoint: %v", err)
logger.Error("Encountered unexpected error reading response from ECS Task Metadata Endpoint", zap.Error(err))
return nil, err
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"go.uber.org/zap"
)

const (
Expand Down Expand Up @@ -52,7 +53,7 @@ type mockClient struct {
retErr bool
}

func (mc *mockClient) Do(req *http.Request) (*http.Response, error) {
func (mc *mockClient) Do(*http.Request) (*http.Response, error) {
if mc.retErr {
return nil, errors.New("fake error")
}
Expand All @@ -65,7 +66,7 @@ func (mc *mockClient) Do(req *http.Request) (*http.Response, error) {
}

func Test_ecsMetadata_fetchTask(t *testing.T) {
md := ecsMetadataProviderImpl{client: &mockClient{response: taskMeta, retErr: false}}
md := ecsMetadataProviderImpl{logger: zap.NewNop(), client: &mockClient{response: taskMeta, retErr: false}}
fetchResp, err := md.fetchTaskMetaData("url")

assert.Nil(t, err)
Expand All @@ -78,7 +79,7 @@ func Test_ecsMetadata_fetchTask(t *testing.T) {
}

func Test_ecsMetadata_fetchContainer(t *testing.T) {
md := ecsMetadataProviderImpl{client: &mockClient{response: containerMeta, retErr: false}}
md := ecsMetadataProviderImpl{logger: zap.NewNop(), client: &mockClient{response: containerMeta, retErr: false}}
fetchResp, err := md.fetchContainerMetaData("url")

assert.Nil(t, err)
Expand All @@ -93,7 +94,7 @@ func Test_ecsMetadata_fetchContainer(t *testing.T) {
}

func Test_ecsMetadata_returnsError(t *testing.T) {
md := ecsMetadataProviderImpl{client: &mockClient{response: "{}", retErr: true}}
md := ecsMetadataProviderImpl{logger: zap.NewNop(), client: &mockClient{response: "{}", retErr: true}}
fetchResp, err := md.fetchContainerMetaData("url")

assert.Nil(t, fetchResp)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"io"
"strconv"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/consumer/pdata"
"go.opentelemetry.io/collector/translator/conventions"

Expand All @@ -45,7 +46,7 @@ type EbMetaData struct {
VersionLabel string `json:"version_label"`
}

func NewDetector() (internal.Detector, error) {
func NewDetector(component.ProcessorCreateParams) (internal.Detector, error) {
return &Detector{fs: &ebFileSystem{}}, nil
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/consumer/pdata"
"go.uber.org/zap"

"github.com/open-telemetry/opentelemetry-collector-contrib/processor/resourcedetectionprocessor/internal"
)
Expand Down Expand Up @@ -51,7 +53,7 @@ func (mfs *mockFileSystem) IsWindows() bool {
}

func Test_newDetector(t *testing.T) {
d, err := NewDetector()
d, err := NewDetector(component.ProcessorCreateParams{Logger: zap.NewNop()})

assert.Nil(t, err)
assert.NotNil(t, d)
Expand Down
3 changes: 2 additions & 1 deletion processor/resourcedetectionprocessor/internal/env/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"regexp"
"strings"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/consumer/pdata"

"github.com/open-telemetry/opentelemetry-collector-contrib/processor/resourcedetectionprocessor/internal"
Expand All @@ -40,7 +41,7 @@ var _ internal.Detector = (*Detector)(nil)

type Detector struct{}

func NewDetector() (internal.Detector, error) {
func NewDetector(component.ProcessorCreateParams) (internal.Detector, error) {
return &Detector{}, nil
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,15 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/consumer/pdata"
"go.uber.org/zap"

"github.com/open-telemetry/opentelemetry-collector-contrib/processor/resourcedetectionprocessor/internal"
)

func TestNewDetector(t *testing.T) {
d, err := NewDetector()
d, err := NewDetector(component.ProcessorCreateParams{Logger: zap.NewNop()})
assert.NotNil(t, d)
assert.NoError(t, err)
}
Expand Down
3 changes: 2 additions & 1 deletion processor/resourcedetectionprocessor/internal/gcp/gce/gce.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package gce // import "cloud.google.com/go/compute/metadata"
import (
"context"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/component/componenterror"
"go.opentelemetry.io/collector/consumer/pdata"
"go.opentelemetry.io/collector/translator/conventions"
Expand All @@ -36,7 +37,7 @@ type Detector struct {
metadata gceMetadata
}

func NewDetector() (internal.Detector, error) {
func NewDetector(component.ProcessorCreateParams) (internal.Detector, error) {
return &Detector{metadata: &gceMetadataImpl{}}, nil
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/translator/conventions"
"go.uber.org/zap"

"github.com/open-telemetry/opentelemetry-collector-contrib/processor/resourcedetectionprocessor/internal"
)
Expand Down Expand Up @@ -66,7 +68,7 @@ func (m *mockMetadata) Get(suffix string) (string, error) {
}

func TestNewDetector(t *testing.T) {
d, err := NewDetector()
d, err := NewDetector(component.ProcessorCreateParams{Logger: zap.NewNop()})
assert.NotNil(t, d)
assert.NoError(t, err)
}
Expand Down
Loading

0 comments on commit 471b7c6

Please sign in to comment.