[NH-81807] Use a default port (443) for collector address (#102)

internal/config/config.go

@@ -11,6 +11,7 @@
 // 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 config is responsible for loading the configuration from various
 // sources, e.g., environment variables, configuration files and user input.
 // It also accepts dynamic settings from the collector server.
@@ -25,6 +26,7 @@ package config
 
 import (
 	"fmt"
+	"net"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -750,7 +752,21 @@ func (c *Config) loadConfigFile() error {
 func (c *Config) GetCollector() string {
 	c.RLock()
 	defer c.RUnlock()
-	return c.Collector
+	target := c.Collector
+	// We have to detect whether the port is in the target string, however,
+	// accounting for ipv6 means we can't simply search for `:`. This is the
+	// second time we use SplitHostPort, the first is inside the validator. Since
+	// this is only ever called once on startup, I think this is a fair tradeoff.
+	if _, _, err := net.SplitHostPort(target); err != nil {
+		if strings.Contains(err.Error(), "missing port in address") {
+			target = target + ":443"
+		} else {
+			// We should never reach this because we've detected an invalid
+			// value in the validator
+			panic(err)
+		}
+	}
+	return target
 }
 
 // GetServiceKey returns the service key

internal/config/config_test.go

@@ -64,7 +64,7 @@ func TestLoadConfig(t *testing.T) {
 	c = NewConfig(
 		WithCollector("hello.world"),
 		WithServiceKey(key2))
-	assert.Equal(t, "hello.world", c.GetCollector())
+	assert.Equal(t, "hello.world:443", c.GetCollector())
 	assert.Equal(t, ToServiceKey(key2), c.GetServiceKey())
 
 	os.Setenv(envSolarWindsAPMServiceKey, key1)
@@ -111,12 +111,12 @@ func TestConfig_HasLocalSamplingConfig(t *testing.T) {
 
 func TestPrintDelta(t *testing.T) {
 	changed := newConfig().reset()
-	changed.Collector = "test.com:443"
+	changed.Collector = "test.com"
 	changed.PrependDomain = true
 	changed.ReporterProperties.EventFlushInterval = 100
 
 	assert.Equal(t,
-		` - Collector (SW_APM_COLLECTOR) = test.com:443 (default: apm.collector.na-01.cloud.solarwinds.com:443)
+		` - Collector (SW_APM_COLLECTOR) = test.com (default: apm.collector.na-01.cloud.solarwinds.com:443)
  - PrependDomain (SW_APM_PREPEND_DOMAIN) = true (default: false)
  - ReporterProperties.EventFlushInterval (SW_APM_EVENTS_FLUSH_INTERVAL) = 100 (default: 2)`,
 		getDelta(newConfig().reset(), changed, "").sanitize().String())

internal/config/validators.go

@@ -11,10 +11,13 @@
 // 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 config
 
 import (
 	"fmt"
+	"github.com/solarwinds/apm-go/internal/log"
+	"net"
 	"regexp"
 	"strconv"
 	"strings"
@@ -86,7 +89,14 @@ func ToServiceKey(s string) string {
 
 // IsValidHost verifies if the host is in a valid format
 func IsValidHost(host string) bool {
-	// TODO
+	if _, _, err := net.SplitHostPort(host); err != nil {
+		if strings.Contains(err.Error(), "missing port in address") {
+			// OK!
+		} else {
+			log.Warningf("Invalid host format: %s", host)
+			return false
+		}
+	}
 	return host != ""
 }
 

internal/config/validators_test.go

@@ -15,6 +15,7 @@ package config
 
 import (
 	"fmt"
+	"github.com/stretchr/testify/require"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -111,3 +112,14 @@ func TestToServiceKey(t *testing.T) {
 		assert.Equal(t, tc.after, ToServiceKey(tc.before), fmt.Sprintf("Case #%d", idx))
 	}
 }
+
+func TestIsValidHost(t *testing.T) {
+	require.True(t, IsValidHost("localhost"))
+	require.True(t, IsValidHost("localhost:321"))
+	require.True(t, IsValidHost("[2001:db8::ff00:42:8329]"))
+	require.True(t, IsValidHost("[2001:db8::ff00:42:8329]:1234"))
+	require.False(t, IsValidHost(""))
+	require.False(t, IsValidHost("localhost:321:321"))
+	require.False(t, IsValidHost("2001:db8::ff00:42:8329"))
+	require.False(t, IsValidHost("2001:db8::ff00:42:8329:1234"))
+}