importing service results in importing std testing

[efd6]

Component(s)

service

Describe the issue you're reporting

If the service package is imported, due to the use of testing types in exported packages for testing, the std testing package is included in the final compiled binary. Due to the fact that types in testing carry methods, the linker is unable to elide inclusion of code from these types when reflect is linked (i.e. nearly always). The testing package's init functions are also all run at application start-up, resulting in unnecessary start lag.

This import path is illustrated so

To avoid this, the types that are exported for testing by other packages should be put in separate packages subordinate to the package that they serve, with only _test.go files importing those subordinate packages.

For example, extract builders.NewNopExporterConfigsAndFactories from "go.opentelemetry.io/collector/service/internal/builders" into a new "go.opentelemetry.io/collector/service/internal/builderstest" which is not exported by e.g. "go.opentelemetry.io/collector/service" except in its _test.go files (this is the only package of the three shown in the graph above that actually imports builders for that function). In fact, you already have a builders_test package where that could live without any issue.

diff --git a/service/internal/builders/builders_test/exporter.go b/service/internal/builders/builders_test/exporter.go
new file mode 100644
index 000000000..bacba4522
--- /dev/null
+++ b/service/internal/builders/builders_test/exporter.go
@@ -0,0 +1,24 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package builders
+
+import (
+	"go.opentelemetry.io/collector/component"
+	"go.opentelemetry.io/collector/exporter"
+	"go.opentelemetry.io/collector/exporter/exportertest"
+	"go.opentelemetry.io/collector/service/internal/builders"
+)
+
+// NewNopExporterConfigsAndFactories returns a configuration and factories that allows building a new nop exporter.
+func NewNopExporterConfigsAndFactories() (map[component.ID]component.Config, map[component.Type]exporter.Factory) {
+	nopFactory := exportertest.NewNopFactory()
+	configs := map[component.ID]component.Config{
+		component.NewID(builders.NopType): nopFactory.CreateDefaultConfig(),
+	}
+	factories := map[component.Type]exporter.Factory{
+		builders.NopType: nopFactory,
+	}
+
+	return configs, factories
+}
diff --git a/service/internal/builders/builders_test/exporter_test.go b/service/internal/builders/builders_test/exporter_test.go
index 4124a3e88..35fe10c59 100644
--- a/service/internal/builders/builders_test/exporter_test.go
+++ b/service/internal/builders/builders_test/exporter_test.go
@@ -151,7 +151,7 @@ func TestExporterBuilderFactory(t *testing.T) {
 }
 
 func TestNewNopExporterConfigsAndFactories(t *testing.T) {
-	configs, factories := builders.NewNopExporterConfigsAndFactories()
+	configs, factories := NewNopExporterConfigsAndFactories()
 	builder := builders.NewExporter(configs, factories)
 	require.NotNil(t, builder)
 
diff --git a/service/internal/builders/exporter.go b/service/internal/builders/exporter.go
index bb2367d22..589eb34e8 100644
--- a/service/internal/builders/exporter.go
+++ b/service/internal/builders/exporter.go
@@ -9,7 +9,6 @@ import (
 
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/exporter"
-	"go.opentelemetry.io/collector/exporter/exportertest"
 	"go.opentelemetry.io/collector/exporter/xexporter"
 	"go.opentelemetry.io/collector/pipeline"
 )
@@ -97,16 +96,3 @@ func (b *ExporterBuilder) CreateProfiles(ctx context.Context, set exporter.Setti
 func (b *ExporterBuilder) Factory(componentType component.Type) component.Factory {
 	return b.factories[componentType]
 }
-
-// NewNopExporterConfigsAndFactories returns a configuration and factories that allows building a new nop exporter.
-func NewNopExporterConfigsAndFactories() (map[component.ID]component.Config, map[component.Type]exporter.Factory) {
-	nopFactory := exportertest.NewNopFactory()
-	configs := map[component.ID]component.Config{
-		component.NewID(NopType): nopFactory.CreateDefaultConfig(),
-	}
-	factories := map[component.Type]exporter.Factory{
-		NopType: nopFactory,
-	}
-
-	return configs, factories
-}
diff --git a/service/service_test.go b/service/service_test.go
index a96f3b608..d2d63a40d 100644
--- a/service/service_test.go
+++ b/service/service_test.go
@@ -34,6 +34,7 @@ import (
 	"go.opentelemetry.io/collector/pipeline/xpipeline"
 	"go.opentelemetry.io/collector/service/extensions"
 	"go.opentelemetry.io/collector/service/internal/builders"
+	builders_test "go.opentelemetry.io/collector/service/internal/builders/builders_test"
 	"go.opentelemetry.io/collector/service/internal/promtest"
 	"go.opentelemetry.io/collector/service/pipelines"
 	"go.opentelemetry.io/collector/service/telemetry"
@@ -638,7 +639,7 @@ func newNopSettings() Settings {
 	receiversConfigs, receiversFactories := builders.NewNopReceiverConfigsAndFactories()
 	processorsConfigs, processorsFactories := builders.NewNopProcessorConfigsAndFactories()
 	connectorsConfigs, connectorsFactories := builders.NewNopConnectorConfigsAndFactories()
-	exportersConfigs, exportersFactories := builders.NewNopExporterConfigsAndFactories()
+	exportersConfigs, exportersFactories := builders_test.NewNopExporterConfigsAndFactories()
 	extensionsConfigs, extensionsFactories := builders.NewNopExtensionConfigsAndFactories()
 
 	return Settings{
@@ -881,7 +882,7 @@ func TestValidateGraph(t *testing.T) {
 
 	_, connectorsFactories := builders.NewNopConnectorConfigsAndFactories()
 	_, receiversFactories := builders.NewNopReceiverConfigsAndFactories()
-	_, exportersFactories := builders.NewNopExporterConfigsAndFactories()
+	_, exportersFactories := builders_test.NewNopExporterConfigsAndFactories()
 
 	for name, tc := range testCases {
 		t.Run(name, func(t *testing.T) {

The equivalent change would also be applied for builders.NewNopReceiverConfigsAndFactories, builders.NewNopProcessorConfigsAndFactories, builders.NewNopConnectorConfigsAndFactories, and builders.NewNopExtensionConfigsAndFactories.

I imagine that this pattern of putting testing code in consumed API exists in other package hierachies as well, so the same comments apply to those if they exist as well.