Address feedback from upstream PR we did not merge (#1)

* Backport changes from upstream PR

Remove `err` from MapMetrics

* Remove usage of pdatautil

* Fix tests

* Use TCPAddr

* Review which functions should be private

exporter/datadogexporter/config.go

@@ -21,6 +21,7 @@ import (
 	"strings"
 
 	"go.opentelemetry.io/collector/config/configmodels"
+	"go.opentelemetry.io/collector/config/confignet"
 )
 
 var (
@@ -54,20 +55,21 @@ func (api *APIConfig) GetCensoredKey() string {
 
 // DogStatsDConfig defines the DogStatsd related configuration
 type DogStatsDConfig struct {
+	// FIXME Use confignet.NetAddr
 	// Endpoint is the DogStatsD address.
 	// The default value is 127.0.0.1:8125
 	// A Unix address is supported
 	Endpoint string `mapstructure:"endpoint"`
 
-	// Telemetry states whether to send metrics
+	// Telemetry states whether to send internal telemetry metrics from the statsd client
 	Telemetry bool `mapstructure:"telemetry"`
 }
 
 // AgentlessConfig defines the Agentless related configuration
 type AgentlessConfig struct {
 	// Endpoint is the host of the Datadog intake server to send metrics to.
 	// If unset, the value is obtained from the Site.
-	Endpoint string `mapstructure:"endpoint"`
+	confignet.TCPAddr `mapstructure:",squash"`
 }
 
 // MetricsConfig defines the metrics exporter specific configuration options
@@ -180,7 +182,7 @@ type Config struct {
 func (c *Config) Sanitize() error {
 
 	if c.Metrics.Mode != AgentlessMode && c.Metrics.Mode != DogStatsDMode {
-		return fmt.Errorf("Metrics mode '%s' is not recognized", c.Metrics.Mode)
+		return fmt.Errorf("metrics mode '%s' is not recognized", c.Metrics.Mode)
 	}
 
 	// Get info from environment variables

exporter/datadogexporter/config_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"go.opentelemetry.io/collector/component/componenttest"
 	"go.opentelemetry.io/collector/config/configmodels"
+	"go.opentelemetry.io/collector/config/confignet"
 	"go.opentelemetry.io/collector/config/configtest"
 )
 
@@ -41,10 +42,10 @@ func TestLoadConfig(t *testing.T) {
 	err = apiConfig.Sanitize()
 
 	require.NoError(t, err)
-	assert.Equal(t, apiConfig, &Config{
+	assert.Equal(t, &Config{
 		ExporterSettings: configmodels.ExporterSettings{
 			NameVal: "datadog/api",
-			TypeVal: "datadog",
+			TypeVal: typeStr,
 		},
 
 		TagsConfig: TagsConfig{
@@ -71,19 +72,21 @@ func TestLoadConfig(t *testing.T) {
 			},
 
 			Agentless: AgentlessConfig{
-				Endpoint: "https://api.datadoghq.eu",
+				confignet.TCPAddr{
+					Endpoint: "https://api.datadoghq.eu",
+				},
 			},
 		},
-	})
+	}, apiConfig)
 
 	dogstatsdConfig := cfg.Exporters["datadog/dogstatsd"].(*Config)
 	err = dogstatsdConfig.Sanitize()
 
 	require.NoError(t, err)
-	assert.Equal(t, dogstatsdConfig, &Config{
+	assert.Equal(t, &Config{
 		ExporterSettings: configmodels.ExporterSettings{
 			NameVal: "datadog/dogstatsd",
-			TypeVal: "datadog",
+			TypeVal: typeStr,
 		},
 
 		TagsConfig: TagsConfig{},
@@ -96,10 +99,8 @@ func TestLoadConfig(t *testing.T) {
 				Endpoint:  "127.0.0.1:8125",
 				Telemetry: true,
 			},
-
-			Agentless: AgentlessConfig{},
 		},
-	})
+	}, dogstatsdConfig)
 
 	invalidConfig := cfg.Exporters["datadog/invalid"].(*Config)
 	err = invalidConfig.Sanitize()
@@ -111,6 +112,28 @@ func TestLoadConfig(t *testing.T) {
 
 }
 
+func TestTags(t *testing.T) {
+	tc := TagsConfig{
+		Hostname: "customhost",
+		Env:      "customenv",
+		Service:  "customservice",
+		Version:  "customversion",
+		Tags:     []string{"key1:val1", "key2:val2"},
+	}
+
+	assert.ElementsMatch(t,
+		[]string{
+			"host:customhost",
+			"env:customenv",
+			"service:customservice",
+			"version:customversion",
+			"key1:val1",
+			"key2:val2",
+		},
+		tc.GetTags(true), // get host
+	)
+}
+
 // TestOverrideMetricsURL tests that the metrics URL is overridden
 // correctly when set manually.
 func TestOverrideMetricsURL(t *testing.T) {
@@ -121,7 +144,7 @@ func TestOverrideMetricsURL(t *testing.T) {
 		API: APIConfig{Key: "notnull", Site: DefaultSite},
 		Metrics: MetricsConfig{
 			Mode:      AgentlessMode,
-			Agentless: AgentlessConfig{Endpoint: DebugEndpoint},
+			Agentless: AgentlessConfig{confignet.TCPAddr{Endpoint: DebugEndpoint}},
 		},
 	}
 

exporter/datadogexporter/dogstatd_test.go

@@ -0,0 +1,69 @@
+// Copyright The OpenTelemetry Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package datadogexporter
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/zap"
+)
+
+func TestDogStatsDExporter(t *testing.T) {
+	var (
+		testNamespace = "test."
+		testTags      = []string{"key1:val1", "key2:val2"}
+	)
+
+	logger := zap.NewNop()
+	cfg := &Config{
+		TagsConfig: TagsConfig{
+			Tags: testTags,
+		},
+
+		Metrics: MetricsConfig{
+			Namespace: testNamespace,
+			DogStatsD: DogStatsDConfig{
+				Endpoint: "localhost:5000",
+			},
+		},
+	}
+
+	exp, err := newDogStatsDExporter(logger, cfg)
+	require.NoError(t, err)
+
+	assert.Equal(t, cfg, exp.GetConfig())
+	assert.Equal(t, logger, exp.GetLogger())
+	assert.Equal(t, testNamespace, exp.client.Namespace)
+	assert.Equal(t, testTags, exp.client.Tags)
+}
+
+func TestInvalidDogStatsDExporter(t *testing.T) {
+	logger := zap.NewNop()
+
+	// The configuration is invalid if no
+	// endpoint is set
+	cfg := &Config{
+		Metrics: MetricsConfig{
+			DogStatsD: DogStatsDConfig{
+				Endpoint: "",
+			},
+		},
+	}
+
+	_, err := newDogStatsDExporter(logger, cfg)
+	require.Error(t, err)
+}

exporter/datadogexporter/dogstatsd.go

@@ -21,6 +21,7 @@ import (
 	"github.com/DataDog/datadog-go/statsd"
 	"go.opentelemetry.io/collector/consumer/pdata"
 	"go.opentelemetry.io/collector/exporter/exporterhelper"
+	"go.opentelemetry.io/collector/translator/internaldata"
 	"go.uber.org/zap"
 )
 
@@ -47,14 +48,15 @@ func newDogStatsDExporter(logger *zap.Logger, cfg *Config) (*dogStatsDExporter,
 	)
 
 	if err != nil {
-		return nil, fmt.Errorf("Failed to initialize DogStatsD client: %s", err)
+		return nil, fmt.Errorf("failed to initialize DogStatsD client: %s", err)
 	}
 
 	return &dogStatsDExporter{logger, cfg, client}, nil
 }
 
 func (exp *dogStatsDExporter) PushMetricsData(_ context.Context, md pdata.Metrics) (int, error) {
-	series, droppedTimeSeries := MapMetrics(exp, md)
+	data := internaldata.MetricsToOC(md)
+	series, droppedTimeSeries := MapMetrics(exp, data)
 
 	for _, metric := range series.metrics {
 
@@ -72,7 +74,7 @@ func (exp *dogStatsDExporter) PushMetricsData(_ context.Context, md pdata.Metric
 		}
 
 		if err != nil {
-			return droppedTimeSeries, err
+			exp.GetLogger().Warn("could not send metric to statsd", zap.String("metric", *metric.Metric), zap.Error(err))
 		}
 	}
 

exporter/datadogexporter/factory.go

@@ -19,6 +19,7 @@ import (
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/config/configerror"
 	"go.opentelemetry.io/collector/config/configmodels"
+	"go.opentelemetry.io/collector/config/confignet"
 	"go.opentelemetry.io/collector/exporter/exporterhelper"
 )
 
@@ -28,31 +29,26 @@ const (
 
 	// DefaultSite is the default site of the Datadog intake to send data to
 	DefaultSite = "datadoghq.com"
-
-	// List the different sending methods
-	AgentMode   = "agent"
-	APIMode     = "api"
-	DefaultMode = APIMode
-)
-
-var (
-	// DefaultTags is the default set of tags to add to every metric or trace
-	DefaultTags = []string{}
 )
 
 // NewFactory creates a Datadog exporter factory
 func NewFactory() component.ExporterFactory {
 	return exporterhelper.NewFactory(
 		typeStr,
 		createDefaultConfig,
-		exporterhelper.WithMetrics(CreateMetricsExporter),
-		exporterhelper.WithTraces(CreateTracesExporter),
+		exporterhelper.WithMetrics(createMetricsExporter),
+		exporterhelper.WithTraces(createTracesExporter),
 	)
 }
 
 // createDefaultConfig creates the default exporter configuration
 func createDefaultConfig() configmodels.Exporter {
 	return &Config{
+		ExporterSettings: configmodels.ExporterSettings{
+			TypeVal: configmodels.Type(typeStr),
+			NameVal: typeStr,
+		},
+
 		API: APIConfig{
 			Key:  "", // must be set if using API
 			Site: "datadoghq.com",
@@ -68,14 +64,16 @@ func createDefaultConfig() configmodels.Exporter {
 			},
 
 			Agentless: AgentlessConfig{
-				Endpoint: "", // set during config sanitization
+				confignet.TCPAddr{
+					Endpoint: "", // set during config sanitization
+				},
 			},
 		},
 	}
 }
 
 // CreateMetricsExporter creates a metrics exporter based on this config.
-func CreateMetricsExporter(
+func createMetricsExporter(
 	_ context.Context,
 	params component.ExporterCreateParams,
 	c configmodels.Exporter,
@@ -102,7 +100,7 @@ func CreateMetricsExporter(
 }
 
 // CreateTracesExporter creates a traces exporter based on this config.
-func CreateTracesExporter(
+func createTracesExporter(
 	_ context.Context,
 	params component.ExporterCreateParams,
 	c configmodels.Exporter) (component.TraceExporter, error) {

exporter/datadogexporter/factory_test.go

@@ -33,7 +33,11 @@ func TestCreateDefaultConfig(t *testing.T) {
 	factory := NewFactory()
 	cfg := factory.CreateDefaultConfig()
 
-	assert.Equal(t, cfg, &Config{
+	assert.Equal(t, &Config{
+		ExporterSettings: configmodels.ExporterSettings{
+			TypeVal: configmodels.Type(typeStr),
+			NameVal: typeStr,
+		},
 		API: APIConfig{Site: "datadoghq.com"},
 		Metrics: MetricsConfig{
 			Mode:        DogStatsDMode,
@@ -43,7 +47,7 @@ func TestCreateDefaultConfig(t *testing.T) {
 				Telemetry: true,
 			},
 		},
-	}, "failed to create default config")
+	}, cfg, "failed to create default config")
 
 	assert.NoError(t, configcheck.ValidateConfig(cfg))
 }
@@ -96,6 +100,31 @@ func TestCreateAPIMetricsExporter(t *testing.T) {
 	assert.Nil(t, exp)
 }
 
+func TestCreateInvalidMetricsExporter(t *testing.T) {
+	logger := zap.NewNop()
+
+	factories, err := componenttest.ExampleComponents()
+	assert.NoError(t, err)
+
+	factory := NewFactory()
+	factories.Exporters[configmodels.Type(typeStr)] = factory
+	cfg, err := configtest.LoadConfigFile(t, path.Join(".", "testdata", "config.yaml"), factories)
+
+	require.NoError(t, err)
+	require.NotNil(t, cfg)
+
+	ctx := context.Background()
+	exp, err := factory.CreateMetricsExporter(
+		ctx,
+		component.ExporterCreateParams{Logger: logger},
+		cfg.Exporters["datadog/dogstatsd/invalid"],
+	)
+
+	// The address is invalid
+	assert.NotNil(t, err)
+	assert.Nil(t, exp)
+}
+
 func TestCreateAPITraceExporter(t *testing.T) {
 	logger := zap.NewNop()
 

exporter/datadogexporter/go.mod

@@ -6,8 +6,8 @@ require (
 	github.com/DataDog/datadog-go v4.0.0+incompatible
 	github.com/census-instrumentation/opencensus-proto v0.3.0
 	github.com/stretchr/testify v1.6.1
+	github.com/zorkian/go-datadog-api v2.29.0+incompatible // indirect
 	go.opentelemetry.io/collector v0.10.1-0.20200917170114-639b9a80ed46
-	go.uber.org/zap v1.15.0
-	google.golang.org/protobuf v1.25.0
+	go.uber.org/zap v1.16.0
 	gopkg.in/zorkian/go-datadog-api.v2 v2.29.0
 )