Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
May 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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