Skip to content

Commit

Permalink
Avoid using type casting on Config, otel core guarantees access to in…
Browse files Browse the repository at this point in the history
…stances (#368)

* Avoid using type casting on Config, otel core guarantees access to instances

OpenTelemetry collector core in the Host interface does not guarantee access to the Config as a Key in the map (which is NamedInstance and not config.Extension).

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>

* Adopt extension as SmartAgentConfigProvider in tests

Co-authored-by: Ryan Fitzpatrick <rmfitzpatrick@signalfx.com>
  • Loading branch information
Bogdan Drutu and Ryan Fitzpatrick authored May 5, 2021
1 parent 00fb4e0 commit 1eb516e
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 36 deletions.
10 changes: 0 additions & 10 deletions internal/extension/smartagentextension/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,6 @@ import (
"github.com/signalfx/splunk-otel-collector/internal/utils"
)

// SmartAgentConfigProvider exposes global saconfig.Config to other components
type SmartAgentConfigProvider interface {
SmartAgentConfig() *saconfig.Config
}

var _ SmartAgentConfigProvider = (*Config)(nil)
var _ config.CustomUnmarshable = (*Config)(nil)

type Config struct {
Expand All @@ -42,10 +36,6 @@ type Config struct {
saconfig.Config `mapstructure:"-,squash"`
}

func (cfg Config) SmartAgentConfig() *saconfig.Config {
return &cfg.Config
}

func (cfg *Config) Unmarshal(componentParser *config.Parser) error {
allSettings := componentParser.Viper().AllSettings()

Expand Down
8 changes: 7 additions & 1 deletion internal/extension/smartagentextension/config_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@
package smartagentextension

import (
"context"
"path"
"testing"

"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/component/componenttest"
"go.opentelemetry.io/collector/config/configtest"
)
Expand All @@ -42,7 +44,11 @@ func TestBundleDirDefault(t *testing.T) {
allSettingsConfig := cfg.Extensions["smartagent/default_settings"]
require.NotNil(t, allSettingsConfig)

saConfigProvider, ok := allSettingsConfig.(SmartAgentConfigProvider)
ext, err := factory.CreateExtension(context.Background(), component.ExtensionCreateParams{}, allSettingsConfig)
require.NoError(t, err)
require.NotNil(t, ext)

saConfigProvider, ok := ext.(SmartAgentConfigProvider)
require.True(t, ok)

require.Equal(t, "/usr/lib/splunk-otel-collector/agent-bundle", saConfigProvider.SmartAgentConfig().BundleDir)
Expand Down
8 changes: 7 additions & 1 deletion internal/extension/smartagentextension/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package smartagentextension

import (
"context"
"path"
"path/filepath"
"testing"
Expand All @@ -25,6 +26,7 @@ import (
"github.com/signalfx/signalfx-agent/pkg/core/config/sources/file"
"github.com/signalfx/signalfx-agent/pkg/utils/timeutil"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/component/componenttest"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/config/configcheck"
Expand Down Expand Up @@ -120,7 +122,11 @@ func TestSmartAgentConfigProvider(t *testing.T) {
allSettingsConfig := cfg.Extensions["smartagent/all_settings"]
require.NotNil(t, allSettingsConfig)

saConfigProvider, ok := allSettingsConfig.(SmartAgentConfigProvider)
ext, err := factory.CreateExtension(context.Background(), component.ExtensionCreateParams{}, allSettingsConfig)
require.NoError(t, err)
require.NotNil(t, ext)

saConfigProvider, ok := ext.(SmartAgentConfigProvider)
require.True(t, ok)

require.Equal(t, func() saconfig.CollectdConfig {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@
package smartagentextension

import (
"context"
"path"
"testing"

"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/component/componenttest"
"go.opentelemetry.io/collector/config/configtest"
)
Expand All @@ -42,7 +44,11 @@ func TestBundleDirDefault(t *testing.T) {
allSettingsConfig := cfg.Extensions["smartagent/default_settings"]
require.NotNil(t, allSettingsConfig)

saConfigProvider, ok := allSettingsConfig.(SmartAgentConfigProvider)
ext, err := factory.CreateExtension(context.Background(), component.ExtensionCreateParams{}, allSettingsConfig)
require.NoError(t, err)
require.NotNil(t, ext)

saConfigProvider, ok := ext.(SmartAgentConfigProvider)
require.True(t, ok)

require.Equal(t, "C:\\Program Files\\Splunk\\OpenTelemetry Collector\\agent-bundle", saConfigProvider.SmartAgentConfig().BundleDir)
Expand Down
18 changes: 14 additions & 4 deletions internal/extension/smartagentextension/extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,20 @@ package smartagentextension
import (
"context"

saconfig "github.com/signalfx/signalfx-agent/pkg/core/config"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
)

// SmartAgentConfigProvider exposes global saconfig.Config to other components
type SmartAgentConfigProvider interface {
SmartAgentConfig() *saconfig.Config
}

type smartAgentConfigExtension struct {
saCfg *saconfig.Config
}

var _ component.Extension = (*smartAgentConfigExtension)(nil)
var _ SmartAgentConfigProvider = (*smartAgentConfigExtension)(nil)

func (sae *smartAgentConfigExtension) Start(_ context.Context, _ component.Host) error {
return nil
Expand All @@ -34,6 +40,10 @@ func (sae *smartAgentConfigExtension) Shutdown(_ context.Context) error {
return nil
}

func newSmartAgentConfigExtension(_ config.Extension) (*smartAgentConfigExtension, error) {
return &smartAgentConfigExtension{}, nil
func (sae *smartAgentConfigExtension) SmartAgentConfig() *saconfig.Config {
return sae.saCfg
}

func newSmartAgentConfigExtension(cfg *Config) (component.Extension, error) {
return &smartAgentConfigExtension{saCfg: &cfg.Config}, nil
}
2 changes: 1 addition & 1 deletion internal/extension/smartagentextension/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,5 +88,5 @@ func createExtension(
_ component.ExtensionCreateParams,
cfg config.Extension,
) (component.Extension, error) {
return newSmartAgentConfigExtension(cfg)
return newSmartAgentConfigExtension(cfg.(*Config))
}
30 changes: 14 additions & 16 deletions internal/receiver/smartagentreceiver/receiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ var (
rusToZap *logrusToZap
configureCollectdOnce sync.Once
configureEnvironmentOnce sync.Once
saConfigProvider smartagentextension.SmartAgentConfigProvider
saConfig *saconfig.Config
configureRusToZapOnce sync.Once
nonWordCharacters = regexp.MustCompile(`[^\w]+`)
)
Expand Down Expand Up @@ -113,7 +113,7 @@ func (r *Receiver) Start(_ context.Context, host component.Host) error {
return fmt.Errorf("failed creating monitor %q: %w", monitorType, err)
}

configCore.ProcPath = saConfigProvider.SmartAgentConfig().ProcPath
configCore.ProcPath = saConfig.ProcPath

return saconfig.CallConfigure(r.monitor, r.config.monitorConfig)
}
Expand Down Expand Up @@ -185,7 +185,7 @@ func (r *Receiver) createMonitor(monitorType string, host component.Host) (monit
if r.config.monitorConfig.MonitorConfigCore().IsCollectdBased() {
configureCollectdOnce.Do(func() {
r.logger.Info("Configuring collectd")
err = collectd.ConfigureMainCollectd(&saConfigProvider.SmartAgentConfig().Collectd)
err = collectd.ConfigureMainCollectd(&saConfig.Collectd)
})
}

Expand All @@ -195,15 +195,14 @@ func (r *Receiver) createMonitor(monitorType string, host component.Host) (monit
func (r *Receiver) setUpSmartAgentConfigProvider(extensions map[config.NamedEntity]component.Extension) {
// If smartagent extension is not configured, use the default config.
f := smartagentextension.NewFactory()
defaultCfg := f.CreateDefaultConfig().(smartagentextension.SmartAgentConfigProvider)
saConfigProvider = defaultCfg
saConfig = &f.CreateDefaultConfig().(*smartagentextension.Config).Config

// Do a lookup for any smartagent extensions to pick up common collectd options
// to be applied across instances of the receiver.
var foundAtLeastOne bool
var multipleSAExtensions bool
var chosenExtension string
for c := range extensions {
for c, ext := range extensions {
if c.Type() != f.Type() {
continue
}
Expand All @@ -214,11 +213,11 @@ func (r *Receiver) setUpSmartAgentConfigProvider(extensions map[config.NamedEnti
}

var cfgProvider smartagentextension.SmartAgentConfigProvider
cfgProvider, foundAtLeastOne = c.(smartagentextension.SmartAgentConfigProvider)
cfgProvider, foundAtLeastOne = ext.(smartagentextension.SmartAgentConfigProvider)
if !foundAtLeastOne {
continue
}
saConfigProvider = cfgProvider
saConfig = cfgProvider.SmartAgentConfig()
chosenExtension = c.Name()
r.logger.Info("Smart Agent Config provider configured", zap.String("extension_name", chosenExtension))
}
Expand All @@ -229,15 +228,14 @@ func (r *Receiver) setUpSmartAgentConfigProvider(extensions map[config.NamedEnti
}

func setUpEnvironment() {
cfg := saConfigProvider.SmartAgentConfig()
os.Setenv(constants.BundleDirEnvVar, cfg.BundleDir)
os.Setenv(constants.BundleDirEnvVar, saConfig.BundleDir)
if runtime.GOOS != "windows" { // Agent bundle doesn't include jre for Windows
os.Setenv("JAVA_HOME", filepath.Join(cfg.BundleDir, "jre"))
os.Setenv("JAVA_HOME", filepath.Join(saConfig.BundleDir, "jre"))
}

os.Setenv(hostfs.HostProcVar, cfg.ProcPath)
os.Setenv(hostfs.HostEtcVar, cfg.EtcPath)
os.Setenv(hostfs.HostVarVar, cfg.VarPath)
os.Setenv(hostfs.HostRunVar, cfg.RunPath)
os.Setenv(hostfs.HostSysVar, cfg.SysPath)
os.Setenv(hostfs.HostProcVar, saConfig.ProcPath)
os.Setenv(hostfs.HostEtcVar, saConfig.EtcPath)
os.Setenv(hostfs.HostVarVar, saConfig.VarPath)
os.Setenv(hostfs.HostRunVar, saConfig.RunPath)
os.Setenv(hostfs.HostSysVar, saConfig.SysPath)
}
4 changes: 2 additions & 2 deletions internal/receiver/smartagentreceiver/receiver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ func TestSmartAgentConfigProviderOverrides(t *testing.T) {
HasGenericJMXMonitor: false,
InstanceName: "",
WriteServerQuery: "",
}, saConfigProvider.SmartAgentConfig().Collectd)
}, saConfig.Collectd)

// Ensure envs are setup.
require.Equal(t, "/opt/", os.Getenv("SIGNALFX_BUNDLE_DIR"))
Expand Down Expand Up @@ -384,7 +384,7 @@ func (m *mockHost) GetExtensions() map[config.NamedEntity]component.Extension {
return map[config.NamedEntity]component.Extension{
m.smartagentextensionConfig: getExtension(smartagentextension.NewFactory(), m.smartagentextensionConfig),
randomExtensionConfig: getExtension(exampleFactory, randomExtensionConfig),
m.smartagentextensionConfigExtra: nil,
m.smartagentextensionConfigExtra: getExtension(smartagentextension.NewFactory(), m.smartagentextensionConfigExtra),
}
}

Expand Down

0 comments on commit 1eb516e

Please sign in to comment.