[carbonreceiver] Hide unnecessary public API (open-telemetry#29895)

Signed-off-by: Bogdan Drutu <[email protected]>

.chloggen/carbonreceiver.yaml

@@ -0,0 +1,22 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: 'breaking'
+
+# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
+component: "carbonreceiver"
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Hide unnecessary public API
+
+# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
+issues: [29895]
+
+# If your change doesn't affect end users or the exported elements of any package,
+# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: [api]

receiver/carbonreceiver/config.go

@@ -4,23 +4,16 @@
 package carbonreceiver // import "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/carbonreceiver"
 
 import (
-	"fmt"
+	"errors"
 	"time"
 
+	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/config/confignet"
-	"go.opentelemetry.io/collector/confmap"
 
 	"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/carbonreceiver/protocol"
 )
 
-const (
-	// parserConfigSection is the name that must be used for the parser settings
-	// in the configuration struct. The metadata mapstructure for the parser
-	// should use the same string.
-	parserConfigSection = "parser"
-)
-
-var _ confmap.Unmarshaler = (*Config)(nil)
+var _ component.ConfigValidator = (*Config)(nil)
 
 // Config defines configuration for the Carbon receiver.
 type Config struct {
@@ -35,29 +28,9 @@ type Config struct {
 	Parser *protocol.Config `mapstructure:"parser"`
 }
 
-func (cfg *Config) Unmarshal(componentParser *confmap.Conf) error {
-	if componentParser == nil {
-		// The section is empty nothing to do, using the default config.
-		return nil
+func (cfg *Config) Validate() error {
+	if cfg.TCPIdleTimeout < 0 {
+		return errors.New("'tcp_idle_timeout' must be non-negative")
 	}
-
-	// Unmarshal but not exact yet so the different keys under config do not
-	// trigger errors, this is needed so that the types of protocol and transport
-	// are read.
-	if err := componentParser.Unmarshal(cfg); err != nil {
-		return err
-	}
-
-	// Unmarshal the protocol, so the type of config can be properly set.
-	vParserCfg, errSub := componentParser.Sub(parserConfigSection)
-	if errSub != nil {
-		return errSub
-	}
-
-	if err := protocol.LoadParserConfig(vParserCfg, cfg.Parser); err != nil {
-		return fmt.Errorf("error on %q section: %w", parserConfigSection, err)
-	}
-
-	// Unmarshal exact to validate the config keys.
-	return componentParser.Unmarshal(cfg, confmap.WithErrorUnused())
+	return nil
 }

receiver/carbonreceiver/config_test.go

@@ -92,3 +92,18 @@ func TestLoadConfig(t *testing.T) {
 		})
 	}
 }
+
+func TestConfigValidate(t *testing.T) {
+	cfg := &Config{
+		NetAddr: confignet.NetAddr{
+			Endpoint:  "localhost:2003",
+			Transport: "tcp",
+		},
+		TCPIdleTimeout: -1 * time.Second,
+		Parser: &protocol.Config{
+			Type:   "plaintext",
+			Config: &protocol.PlaintextConfig{},
+		},
+	}
+	assert.Error(t, cfg.Validate())
+}

receiver/carbonreceiver/factory.go

@@ -5,6 +5,7 @@ package carbonreceiver // import "github.com/open-telemetry/opentelemetry-collec
 
 import (
 	"context"
+	"time"
 
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/config/confignet"
@@ -13,7 +14,11 @@ import (
 
 	"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/carbonreceiver/internal/metadata"
 	"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/carbonreceiver/protocol"
-	"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/carbonreceiver/transport"
+)
+
+const (
+	// tcpIdleTimeoutDefault is the default timeout for idle TCP connections.
+	tcpIdleTimeoutDefault = 30 * time.Second
 )
 
 // This file implements factory for Carbon receiver.
@@ -32,7 +37,7 @@ func createDefaultConfig() component.Config {
 			Endpoint:  "localhost:2003",
 			Transport: "tcp",
 		},
-		TCPIdleTimeout: transport.TCPIdleTimeoutDefault,
+		TCPIdleTimeout: tcpIdleTimeoutDefault,
 		Parser: &protocol.Config{
 			Type:   "plaintext",
 			Config: &protocol.PlaintextConfig{},

receiver/carbonreceiver/transport/client/plaintext_client.go → receiver/carbonreceiver/internal/client/plaintext_client.go

@@ -1,7 +1,7 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
-package client // import "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/carbonreceiver/transport/client"
+package client // import "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/carbonreceiver/internal/client"
 
 import (
 	"fmt"

receiver/carbonreceiver/transport/server.go → receiver/carbonreceiver/internal/transport/server.go

@@ -1,7 +1,7 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
-package transport // import "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/carbonreceiver/transport"
+package transport // import "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/carbonreceiver/internal/transport"
 
 import (
 	"context"

receiver/carbonreceiver/transport/server_test.go → receiver/carbonreceiver/internal/transport/server_test.go

@@ -4,6 +4,7 @@
 package transport
 
 import (
+	"context"
 	"runtime"
 	"sync"
 	"testing"
@@ -14,8 +15,8 @@ import (
 	"go.opentelemetry.io/collector/consumer/consumertest"
 
 	"github.com/open-telemetry/opentelemetry-collector-contrib/internal/common/testutil"
+	"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/carbonreceiver/internal/client"
 	"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/carbonreceiver/protocol"
-	"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/carbonreceiver/transport/client"
 )
 
 func Test_Server_ListenAndServe(t *testing.T) {
@@ -52,7 +53,8 @@ func Test_Server_ListenAndServe(t *testing.T) {
 			mc := new(consumertest.MetricsSink)
 			p, err := (&protocol.PlaintextConfig{}).BuildParser()
 			require.NoError(t, err)
-			mr := NewMockReporter(1)
+			mr := &mockReporter{}
+			mr.wgMetricsProcessed.Add(1)
 
 			wgListenAndServe := sync.WaitGroup{}
 			wgListenAndServe.Add(1)
@@ -76,7 +78,7 @@ func Test_Server_ListenAndServe(t *testing.T) {
 			err = gc.Disconnect()
 			assert.NoError(t, err)
 
-			mr.WaitAllOnMetricsProcessedCalls()
+			mr.wgMetricsProcessed.Wait()
 
 			// Keep trying until we're timed out or got a result
 			assert.Eventually(t, func() bool {
@@ -96,3 +98,21 @@ func Test_Server_ListenAndServe(t *testing.T) {
 		})
 	}
 }
+
+// mockReporter provides a Reporter that provides some useful functionalities for
+// tests (eg.: wait for certain number of messages).
+type mockReporter struct {
+	wgMetricsProcessed sync.WaitGroup
+}
+
+func (m *mockReporter) OnDataReceived(ctx context.Context) context.Context {
+	return ctx
+}
+
+func (m *mockReporter) OnTranslationError(context.Context, error) {}
+
+func (m *mockReporter) OnMetricsProcessed(context.Context, int, error) {
+	m.wgMetricsProcessed.Done()
+}
+
+func (m *mockReporter) OnDebugf(string, ...any) {}

receiver/carbonreceiver/transport/tcp_server.go → receiver/carbonreceiver/internal/transport/tcp_server.go

@@ -1,13 +1,12 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
-package transport // import "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/carbonreceiver/transport"
+package transport // import "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/carbonreceiver/internal/transport"
 
 import (
 	"bufio"
 	"context"
 	"errors"
-	"fmt"
 	"io"
 	"net"
 	"strings"
@@ -20,11 +19,6 @@ import (
 	"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/carbonreceiver/protocol"
 )
 
-const (
-	// TCPIdleTimeoutDefault is the default timeout for idle TCP connections.
-	TCPIdleTimeoutDefault = 30 * time.Second
-)
-
 type tcpServer struct {
 	ln          net.Listener
 	wg          sync.WaitGroup
@@ -39,14 +33,6 @@ func NewTCPServer(
 	addr string,
 	idleTimeout time.Duration,
 ) (Server, error) {
-	if idleTimeout < 0 {
-		return nil, fmt.Errorf("invalid idle timeout: %v", idleTimeout)
-	}
-
-	if idleTimeout == 0 {
-		idleTimeout = TCPIdleTimeoutDefault
-	}
-
 	ln, err := net.Listen("tcp", addr)
 	if err != nil {
 		return nil, err

receiver/carbonreceiver/transport/udp_server.go → receiver/carbonreceiver/internal/transport/udp_server.go

@@ -1,7 +1,7 @@
 // Copyright The OpenTelemetry Authors
 // SPDX-License-Identifier: Apache-2.0
 
-package transport // import "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/carbonreceiver/transport"
+package transport // import "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/carbonreceiver/internal/transport"
 
 import (
 	"bytes"

receiver/carbonreceiver/protocol/config.go

@@ -35,12 +35,7 @@ func init() {
 	})
 }
 
-const (
-	// configSection is the name that must be used for the config settings
-	// in the configuration struct. The metadata mapstructure for the parser
-	// should use the same string.
-	configSection = "config"
-)
+var _ confmap.Unmarshaler = (*Config)(nil)
 
 // Config is the general configuration for the parser to be used.
 type Config struct {
@@ -57,10 +52,14 @@ type ParserConfig interface {
 	BuildParser() (Parser, error)
 }
 
-// LoadParserConfig is used to load the parser configuration according to the
-// specified parser type. It expects the passed viper to be pointing at the level
-// of the Config reference.
-func LoadParserConfig(cp *confmap.Conf, cfg *Config) error {
+// Unmarshal is used to load the parser configuration according to the
+// specified parser type.
+func (cfg *Config) Unmarshal(cp *confmap.Conf) error {
+	// If type is configured then use that, otherwise use default.
+	if configuredType, ok := cp.Get("type").(string); ok {
+		cfg.Type = configuredType
+	}
+
 	defaultCfgFn, ok := parserMap[cfg.Type]
 	if !ok {
 		return fmt.Errorf(
@@ -71,10 +70,5 @@ func LoadParserConfig(cp *confmap.Conf, cfg *Config) error {
 
 	cfg.Config = defaultCfgFn()
 
-	vParserCfg, errSub := cp.Sub(configSection)
-	if errSub != nil {
-		return errSub
-	}
-
-	return vParserCfg.Unmarshal(cfg.Config, confmap.WithErrorUnused())
+	return cp.Unmarshal(cfg, confmap.WithErrorUnused())
 }

receiver/carbonreceiver/protocol/config_test.go

@@ -14,14 +14,12 @@ func TestLoadParserConfig(t *testing.T) {
 	tests := []struct {
 		name    string
 		cfgMap  map[string]any
-		cfg     Config
 		want    Config
 		wantErr bool
 	}{
 		{
 			name:    "unknow_type",
-			cfgMap:  map[string]any{"type": "unknow"},
-			cfg:     Config{Type: "unknown"},
+			cfgMap:  map[string]any{"type": "unknown"},
 			want:    Config{Type: "unknown"},
 			wantErr: true,
 		},
@@ -35,7 +33,6 @@ func TestLoadParserConfig(t *testing.T) {
 					"rules": []any{map[string]any{"regexp": "(?<key_test>.*test)"}},
 				},
 			},
-			cfg: Config{Type: "regex"},
 			want: Config{
 				Type: "regex",
 				Config: &RegexParserConfig{
@@ -47,19 +44,26 @@ func TestLoadParserConfig(t *testing.T) {
 		{
 			name:   "default_regex",
 			cfgMap: map[string]any{"type": "regex"},
-			cfg:    Config{Type: "regex"},
 			want: Config{
 				Type:   "regex",
 				Config: &RegexParserConfig{},
 			},
 		},
+		{
+			name:   "plaintext",
+			cfgMap: map[string]any{"type": "plaintext"},
+			want: Config{
+				Type:   "plaintext",
+				Config: &PlaintextConfig{},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			v := confmap.NewFromStringMap(tt.cfgMap)
 
-			got := tt.cfg // Not strictly necessary but it makes easier to debug issues.
-			err := LoadParserConfig(v, &got)
+			got := Config{}
+			err := got.Unmarshal(v)
 			assert.Equal(t, tt.want, got)
 			assert.Equal(t, tt.wantErr, err != nil)
 		})

receiver/carbonreceiver/receiver.go

@@ -13,8 +13,8 @@ import (
 	"go.opentelemetry.io/collector/consumer"
 	"go.opentelemetry.io/collector/receiver"
 
+	"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/carbonreceiver/internal/transport"
 	"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/carbonreceiver/protocol"
-	"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/carbonreceiver/transport"
 )
 
 var (