Addressed review comments

extension/solarwindsapmsettingsextension/config.go

@@ -3,10 +3,68 @@
 
 package solarwindsapmsettingsextension // import "github.com/open-telemetry/opentelemetry-collector-contrib/extension/solarwindsapmsettingsextension"
 
-import "time"
+import (
+	"os"
+	"regexp"
+	"strings"
+	"time"
+
+	"go.opentelemetry.io/collector/component"
+)
 
 type Config struct {
 	Endpoint string        `mapstructure:"endpoint"`
 	Key      string        `mapstructure:"key"`
 	Interval time.Duration `mapstructure:"interval"`
 }
+
+const (
+	DefaultEndpoint = "apm.collector.na-01.cloud.solarwinds.com:443"
+	DefaultInterval = time.Duration(10) * time.Second
+	MinimumInterval = time.Duration(5) * time.Second
+	MaximumInterval = time.Duration(60) * time.Second
+)
+
+func createDefaultConfig() component.Config {
+	return &Config{
+		Endpoint: DefaultEndpoint,
+		Interval: DefaultInterval,
+	}
+}
+
+func (cfg *Config) Validate() error {
+	// Endpoint
+	matched, _ := regexp.MatchString(`apm.collector.[a-z]{2,3}-[0-9]{2}.[a-z\-]*.solarwinds.com:443`, cfg.Endpoint)
+	if !matched {
+		// Replaced by the default
+		cfg.Endpoint = DefaultEndpoint
+	}
+	// Key
+	keyArr := strings.Split(cfg.Key, ":")
+	if len(keyArr) == 2 && len(keyArr[1]) == 0 {
+		/**
+		 * Service name is empty. We are trying our best effort to resolve the service name
+		 */
+		serviceName := resolveServiceNameBestEffort()
+		if len(serviceName) > 0 {
+			cfg.Key = keyArr[0] + ":" + serviceName
+		}
+	}
+	// Interval
+	if cfg.Interval.Seconds() < MinimumInterval.Seconds() {
+		cfg.Interval = MinimumInterval
+	}
+	if cfg.Interval.Seconds() > MaximumInterval.Seconds() {
+		cfg.Interval = MaximumInterval
+	}
+	return nil
+}
+
+func resolveServiceNameBestEffort() string {
+	if otelServiceName, otelServiceNameDefined := os.LookupEnv("OTEL_SERVICE_NAME"); otelServiceNameDefined && len(otelServiceName) > 0 {
+		return otelServiceName
+	} else if awsLambdaFunctionName, awsLambdaFunctionNameDefined := os.LookupEnv("AWS_LAMBDA_FUNCTION_NAME"); awsLambdaFunctionNameDefined && len(awsLambdaFunctionName) > 0 {
+		return awsLambdaFunctionName
+	}
+	return ""
+}

extension/solarwindsapmsettingsextension/config_test.go

@@ -4,6 +4,7 @@
 package solarwindsapmsettingsextension
 
 import (
+	"os"
 	"path/filepath"
 	"testing"
 	"time"
@@ -30,17 +31,25 @@ func TestLoadConfig(t *testing.T) {
 		{
 			id: component.NewIDWithName(metadata.Type, "1"),
 			expected: &Config{
-				Endpoint: "0.0.0.0:1234",
-				Key:      "something",
+				Endpoint: "apm.collector.apj-01.cloud.solarwinds.com:443",
+				Key:      "something:name",
 				Interval: time.Duration(10) * time.Second,
 			},
 		},
 		{
 			id: component.NewIDWithName(metadata.Type, "2"),
 			expected: &Config{
-				Endpoint: "0.0.0.0:1234",
+				Endpoint: "apm.collector.na-01.cloud.solarwinds.com:443",
 				Key:      "something",
-				Interval: time.Duration(10) * time.Second,
+				Interval: time.Duration(5) * time.Second,
+			},
+		},
+		{
+			id: component.NewIDWithName(metadata.Type, "3"),
+			expected: &Config{
+				Endpoint: "apm.collector.na-01.cloud.solarwinds.com:443",
+				Key:      "something:name",
+				Interval: time.Duration(60) * time.Second,
 			},
 		},
 	}
@@ -58,3 +67,22 @@ func TestLoadConfig(t *testing.T) {
 		})
 	}
 }
+
+func TestResolveServiceNameBestEffort(t *testing.T) {
+	// Without any environment variables
+	require.Empty(t, resolveServiceNameBestEffort())
+	// With OTEL_SERVICE_NAME only
+	require.NoError(t, os.Setenv("OTEL_SERVICE_NAME", "otel_ser1"))
+	require.Equal(t, "otel_ser1", resolveServiceNameBestEffort())
+	require.NoError(t, os.Unsetenv("OTEL_SERVICE_NAME"))
+	// With AWS_LAMBDA_FUNCTION_NAME only
+	require.NoError(t, os.Setenv("AWS_LAMBDA_FUNCTION_NAME", "lambda"))
+	require.Equal(t, "lambda", resolveServiceNameBestEffort())
+	require.NoError(t, os.Unsetenv("AWS_LAMBDA_FUNCTION_NAME"))
+	// With both
+	require.NoError(t, os.Setenv("OTEL_SERVICE_NAME", "otel_ser1"))
+	require.NoError(t, os.Setenv("AWS_LAMBDA_FUNCTION_NAME", "lambda"))
+	require.Equal(t, "otel_ser1", resolveServiceNameBestEffort())
+	require.NoError(t, os.Unsetenv("AWS_LAMBDA_FUNCTION_NAME"))
+	require.NoError(t, os.Unsetenv("OTEL_SERVICE_NAME"))
+}

extension/solarwindsapmsettingsextension/extension.go

@@ -33,39 +33,39 @@ func newSolarwindsApmSettingsExtension(extensionCfg *Config, logger *zap.Logger)
 }
 
 func (extension *solarwindsapmSettingsExtension) Start(_ context.Context, _ component.Host) error {
-	extension.logger.Debug("Starting up solarwinds apm settings extension")
+	extension.logger.Info("Starting up solarwinds apm settings extension")
 	ctx := context.Background()
 	ctx, extension.cancel = context.WithCancel(ctx)
-	configOk := validateSolarwindsApmSettingsExtensionConfiguration(extension.config, extension.logger)
-	if configOk {
-		var err error
-		extension.conn, err = grpc.Dial(extension.config.Endpoint, grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{})))
-		if err != nil {
-			return err
-		}
-		extension.logger.Info("Dailed to endpoint", zap.String("endpoint", extension.config.Endpoint))
-		extension.client = collectorpb.NewTraceCollectorClient(extension.conn)
-		go func() {
-			ticker := newTicker(extension.config.Interval)
-			defer ticker.Stop()
-			for {
-				select {
-				case <-ticker.C:
-					refresh(extension)
-				case <-ctx.Done():
-					extension.logger.Debug("Received ctx.Done() from ticker")
-					return
-				}
-			}
-		}()
-	} else {
-		extension.logger.Warn("solarwindsapmsettingsextension is in noop. Please check config")
+	var err error
+	extension.conn, err = grpc.Dial(extension.config.Endpoint, grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{})))
+	if err != nil {
+		return err
 	}
+	extension.logger.Info("Dailed to endpoint", zap.String("endpoint", extension.config.Endpoint))
+	extension.client = collectorpb.NewTraceCollectorClient(extension.conn)
+
+	// initial refresh
+	refresh(extension)
+
+	go func() {
+		ticker := time.NewTicker(extension.config.Interval)
+		defer ticker.Stop()
+		for {
+			select {
+			case <-ticker.C:
+				refresh(extension)
+			case <-ctx.Done():
+				extension.logger.Info("Received ctx.Done() from ticker")
+				return
+			}
+		}
+	}()
+
 	return nil
 }
 
 func (extension *solarwindsapmSettingsExtension) Shutdown(_ context.Context) error {
-	extension.logger.Debug("Shutting down solarwinds apm settings extension")
+	extension.logger.Info("Shutting down solarwinds apm settings extension")
 	if extension.conn != nil {
 		return extension.conn.Close()
 	}
@@ -75,28 +75,7 @@ func (extension *solarwindsapmSettingsExtension) Shutdown(_ context.Context) err
 	return nil
 }
 
-func validateSolarwindsApmSettingsExtensionConfiguration(_ *Config, _ *zap.Logger) bool {
-	// Concrete implementation will be available in later PR
-	return true
-}
-
 func refresh(extension *solarwindsapmSettingsExtension) {
 	// Concrete implementation will be available in later PR
 	extension.logger.Info("refresh task")
 }
-
-// Start ticking immediately.
-// Ref: https://stackoverflow.com/questions/32705582/how-to-get-time-tick-to-tick-immediately
-func newTicker(repeat time.Duration) *time.Ticker {
-	ticker := time.NewTicker(repeat)
-	oc := ticker.C
-	nc := make(chan time.Time, 1)
-	go func() {
-		nc <- time.Now()
-		for tm := range oc {
-			nc <- tm
-		}
-	}()
-	ticker.C = nc
-	return ticker
-}

extension/solarwindsapmsettingsextension/extension_test.go

@@ -23,15 +23,15 @@ func TestCreateExtension(t *testing.T) {
 		{
 			name: "default",
 			cfg: &Config{
-				Endpoint: "0.0.0.0:1234",
-				Interval: time.Duration(10) * time.Second,
+				Endpoint: DefaultEndpoint,
+				Interval: DefaultInterval,
 			},
 		},
 		{
 			name: "anything",
 			cfg: &Config{
-				Endpoint: "0.0.0.0:1234",
-				Key:      "something",
+				Endpoint: "apm.collector.na-02.cloud.solarwinds.com:443",
+				Key:      "something:name",
 				Interval: time.Duration(10) * time.Second,
 			},
 		},

extension/solarwindsapmsettingsextension/factory.go

@@ -5,24 +5,13 @@ package solarwindsapmsettingsextension // import "github.com/open-telemetry/open
 
 import (
 	"context"
-	"time"
 
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/extension"
 
 	"github.com/open-telemetry/opentelemetry-collector-contrib/extension/solarwindsapmsettingsextension/internal/metadata"
 )
 
-const (
-	DefaultInterval = time.Duration(10) * time.Second
-)
-
-func createDefaultConfig() component.Config {
-	return &Config{
-		Interval: DefaultInterval,
-	}
-}
-
 func createExtension(_ context.Context, settings extension.CreateSettings, cfg component.Config) (extension.Extension, error) {
 	return newSolarwindsApmSettingsExtension(cfg.(*Config), settings.Logger)
 }

extension/solarwindsapmsettingsextension/factory_test.go

@@ -17,7 +17,7 @@ func TestCreateDefaultConfig(t *testing.T) {
 	assert.NoError(t, componenttest.CheckConfigStruct(cfg))
 	ocfg, ok := factory.CreateDefaultConfig().(*Config)
 	assert.True(t, ok)
-	assert.Empty(t, ocfg.Endpoint, "There is no default endpoint")
+	assert.Equal(t, ocfg.Endpoint, DefaultEndpoint, "Wrong default endpoint")
 	assert.Empty(t, ocfg.Key, "There is no default key")
 	assert.Equal(t, ocfg.Interval, DefaultInterval, "Wrong default interval")
 }

extension/solarwindsapmsettingsextension/testdata/config.yaml

@@ -1,9 +1,13 @@
 solarwindsapmsettings:
 solarwindsapmsettings/1:
-  endpoint: "0.0.0.0:1234"
-  key: "something"
+  endpoint: "apm.collector.apj-01.cloud.solarwinds.com:443"
+  key: "something:name"
   interval: 10s
 solarwindsapmsettings/2:
-  endpoint: "0.0.0.0:1234"
+  endpoint: "apm.collector.eu-01.cloud.solarwinds.com"
   key: "something"
-  interval: "10s"
+  interval: "4s"
+solarwindsapmsettings/3:
+  endpoint: ""
+  key: "something:name"
+  interval: 61s