Skip to content

Commit

Permalink
[chore] Simplify context with EnvKey usage in hostmetrics receiver (o…
Browse files Browse the repository at this point in the history
…pen-telemetry#36907)

Signed-off-by: Bogdan Drutu <[email protected]>
  • Loading branch information
bogdandrutu authored Dec 20, 2024
1 parent b600350 commit 9959e34
Show file tree
Hide file tree
Showing 26 changed files with 82 additions and 138 deletions.
2 changes: 0 additions & 2 deletions receiver/hostmetricsreceiver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,6 @@ func (cfg *Config) Unmarshal(componentParser *confmap.Conf) error {
}

scraperCfg.SetRootPath(cfg.RootPath)
envMap := setGoPsutilEnvVars(cfg.RootPath, &osEnv{})
scraperCfg.SetEnvMap(envMap)

cfg.Scrapers[key] = scraperCfg
}
Expand Down
12 changes: 0 additions & 12 deletions receiver/hostmetricsreceiver/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"testing"
"time"

"github.com/shirou/gopsutil/v4/common"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/component"
Expand Down Expand Up @@ -47,7 +46,6 @@ func TestLoadConfig(t *testing.T) {
cfg.Scrapers = map[component.Type]internal.Config{
cpuscraper.Type: func() internal.Config {
cfg := (&cpuscraper.Factory{}).CreateDefaultConfig()
cfg.SetEnvMap(common.EnvMap{})
return cfg
}(),
}
Expand All @@ -65,28 +63,23 @@ func TestLoadConfig(t *testing.T) {
Scrapers: map[component.Type]internal.Config{
cpuscraper.Type: func() internal.Config {
cfg := (&cpuscraper.Factory{}).CreateDefaultConfig()
cfg.SetEnvMap(common.EnvMap{})
return cfg
}(),
diskscraper.Type: func() internal.Config {
cfg := (&diskscraper.Factory{}).CreateDefaultConfig()
cfg.SetEnvMap(common.EnvMap{})
return cfg
}(),
loadscraper.Type: (func() internal.Config {
cfg := (&loadscraper.Factory{}).CreateDefaultConfig()
cfg.(*loadscraper.Config).CPUAverage = true
cfg.SetEnvMap(common.EnvMap{})
return cfg
})(),
filesystemscraper.Type: func() internal.Config {
cfg := (&filesystemscraper.Factory{}).CreateDefaultConfig()
cfg.SetEnvMap(common.EnvMap{})
return cfg
}(),
memoryscraper.Type: func() internal.Config {
cfg := (&memoryscraper.Factory{}).CreateDefaultConfig()
cfg.SetEnvMap(common.EnvMap{})
return cfg
}(),
networkscraper.Type: (func() internal.Config {
Expand All @@ -95,17 +88,14 @@ func TestLoadConfig(t *testing.T) {
Interfaces: []string{"test1"},
Config: filterset.Config{MatchType: "strict"},
}
cfg.SetEnvMap(common.EnvMap{})
return cfg
})(),
processesscraper.Type: func() internal.Config {
cfg := (&processesscraper.Factory{}).CreateDefaultConfig()
cfg.SetEnvMap(common.EnvMap{})
return cfg
}(),
pagingscraper.Type: func() internal.Config {
cfg := (&pagingscraper.Factory{}).CreateDefaultConfig()
cfg.SetEnvMap(common.EnvMap{})
return cfg
}(),
processscraper.Type: (func() internal.Config {
Expand All @@ -114,12 +104,10 @@ func TestLoadConfig(t *testing.T) {
Names: []string{"test2", "test3"},
Config: filterset.Config{MatchType: "regexp"},
}
cfg.SetEnvMap(common.EnvMap{})
return cfg
})(),
systemscraper.Type: (func() internal.Config {
cfg := (&systemscraper.Factory{}).CreateDefaultConfig()
cfg.SetEnvMap(common.EnvMap{})
return cfg
})(),
},
Expand Down
26 changes: 8 additions & 18 deletions receiver/hostmetricsreceiver/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package hostmetricsreceiver // import "github.com/open-telemetry/opentelemetry-c
import (
"context"
"fmt"
"os"
"time"

"github.com/shirou/gopsutil/v4/host"
Expand Down Expand Up @@ -106,19 +105,22 @@ func createLogsReceiver(
func createAddScraperOptions(
ctx context.Context,
set receiver.Settings,
config *Config,
cfg *Config,
factories map[component.Type]internal.ScraperFactory,
) ([]scraperhelper.ScraperControllerOption, error) {
scraperControllerOptions := make([]scraperhelper.ScraperControllerOption, 0, len(config.Scrapers))
scraperControllerOptions := make([]scraperhelper.ScraperControllerOption, 0, len(cfg.Scrapers))

for key, cfg := range config.Scrapers {
hostMetricsScraper, ok, err := createHostMetricsScraper(ctx, set, key, cfg, factories)
envMap := setGoPsutilEnvVars(cfg.RootPath)

for key, cfg := range cfg.Scrapers {
scrp, ok, err := createHostMetricsScraper(ctx, set, key, cfg, factories)
if err != nil {
return nil, fmt.Errorf("failed to create scraper for key %q: %w", key, err)
}

if ok {
scraperControllerOptions = append(scraperControllerOptions, scraperhelper.AddScraper(metadata.Type, hostMetricsScraper))
scrp = internal.NewEnvVarScraper(scrp, envMap)
scraperControllerOptions = append(scraperControllerOptions, scraperhelper.AddScraper(metadata.Type, scrp))
continue
}

Expand All @@ -139,15 +141,3 @@ func createHostMetricsScraper(ctx context.Context, set receiver.Settings, key co
s, err = factory.CreateMetricsScraper(ctx, set, cfg)
return
}

type environment interface {
Lookup(k string) (string, bool)
}

type osEnv struct{}

var _ environment = (*osEnv)(nil)

func (e *osEnv) Lookup(k string) (string, bool) {
return os.LookupEnv(k)
}
4 changes: 2 additions & 2 deletions receiver/hostmetricsreceiver/hostmetrics_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,14 @@ func validateRootPath(rootPath string) error {
return nil
}

func setGoPsutilEnvVars(rootPath string, env environment) common.EnvMap {
func setGoPsutilEnvVars(rootPath string) common.EnvMap {
m := common.EnvMap{}
if rootPath == "" || rootPath == "/" {
return m
}

for envVarKey, defaultValue := range gopsutilEnvVars {
_, ok := env.Lookup(string(envVarKey))
_, ok := os.LookupEnv(string(envVarKey))
if ok {
continue // don't override if existing env var is set
}
Expand Down
10 changes: 5 additions & 5 deletions receiver/hostmetricsreceiver/hostmetrics_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,16 @@ func TestLoadConfigRootPath(t *testing.T) {
expectedConfig.RootPath = "testdata"
cpuScraperCfg := (&cpuscraper.Factory{}).CreateDefaultConfig()
cpuScraperCfg.SetRootPath("testdata")
cpuScraperCfg.SetEnvMap(common.EnvMap{
expectedConfig.Scrapers = map[component.Type]internal.Config{cpuscraper.Type: cpuScraperCfg}
assert.Equal(t, expectedConfig, cfg)
expectedEnvMap := common.EnvMap{
common.HostDevEnvKey: "testdata/dev",
common.HostEtcEnvKey: "testdata/etc",
common.HostRunEnvKey: "testdata/run",
common.HostSysEnvKey: "testdata/sys",
common.HostVarEnvKey: "testdata/var",
})
expectedConfig.Scrapers = map[component.Type]internal.Config{cpuscraper.Type: cpuScraperCfg}

assert.Equal(t, expectedConfig, cfg)
}
assert.Equal(t, expectedEnvMap, setGoPsutilEnvVars("testdata"))
}

func TestLoadInvalidConfig_RootPathNotExist(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion receiver/hostmetricsreceiver/hostmetrics_others.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@ func validateRootPath(rootPath string) error {
return fmt.Errorf("root_path is supported on linux only")
}

func setGoPsutilEnvVars(_ string, _ environment) common.EnvMap {
func setGoPsutilEnvVars(_ string) common.EnvMap {
return common.EnvMap{}
}
25 changes: 2 additions & 23 deletions receiver/hostmetricsreceiver/hostmetrics_receiver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"testing"
"time"

"github.com/shirou/gopsutil/v4/common"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/component"
Expand Down Expand Up @@ -72,22 +71,6 @@ var systemSpecificMetrics = map[string][]string{
"solaris": {"system.filesystem.inodes.usage", "system.paging.faults"},
}

type testEnv struct {
env map[string]string
}

var _ environment = (*testEnv)(nil)

func (e *testEnv) Lookup(k string) (string, bool) {
v, ok := e.env[k]
return v, ok
}

func (e *testEnv) Set(k, v string) error {
e.env[k] = v
return nil
}

func TestGatherMetrics_EndToEnd(t *testing.T) {
sink := new(consumertest.MetricsSink)

Expand Down Expand Up @@ -205,8 +188,6 @@ type mockConfig struct{}

func (m *mockConfig) SetRootPath(_ string) {}

func (m *mockConfig) SetEnvMap(_ common.EnvMap) {}

type errFactory struct{}

func (m *errFactory) CreateDefaultConfig() internal.Config { return &mockConfig{} }
Expand All @@ -221,9 +202,8 @@ func TestGatherMetrics_ScraperKeyConfigError(t *testing.T) {
scraperFactories = tmp
}()

sink := new(consumertest.MetricsSink)
cfg := &Config{Scrapers: map[component.Type]internal.Config{component.MustNewType("error"): &mockConfig{}}}
_, err := NewFactory().CreateMetrics(context.Background(), creationSet, cfg, sink)
_, err := NewFactory().CreateMetrics(context.Background(), creationSet, cfg, consumertest.NewNop())
require.Error(t, err)
}

Expand All @@ -235,9 +215,8 @@ func TestGatherMetrics_CreateMetricsScraperError(t *testing.T) {
scraperFactories = tmp
}()

sink := new(consumertest.MetricsSink)
cfg := &Config{Scrapers: map[component.Type]internal.Config{mockType: &mockConfig{}}}
_, err := NewFactory().CreateMetrics(context.Background(), creationSet, cfg, sink)
_, err := NewFactory().CreateMetrics(context.Background(), creationSet, cfg, consumertest.NewNop())
require.Error(t, err)
}

Expand Down
2 changes: 0 additions & 2 deletions receiver/hostmetricsreceiver/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ func Test_ProcessScrapeWithCustomRootPath(t *testing.T) {
rCfg.RootPath = rootPath
pCfg := (&processscraper.Factory{}).CreateDefaultConfig().(*processscraper.Config)
pCfg.SetRootPath(rootPath)
pCfg.SetEnvMap(setGoPsutilEnvVars(rootPath, &osEnv{}))
rCfg.Scrapers = map[component.Type]internal.Config{
processscraper.Type: pCfg,
}
Expand Down Expand Up @@ -102,7 +101,6 @@ func Test_ProcessScrapeWithBadRootPathAndEnvVar(t *testing.T) {
rCfg.CollectionInterval = time.Second
pCfg := (&processscraper.Factory{}).CreateDefaultConfig().(*processscraper.Config)
pCfg.SetRootPath(badRootPath)
pCfg.SetEnvMap(setGoPsutilEnvVars(badRootPath, &osEnv{}))
rCfg.Scrapers = map[component.Type]internal.Config{
processscraper.Type: pCfg,
}
Expand Down
30 changes: 25 additions & 5 deletions receiver/hostmetricsreceiver/internal/scraper.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
"context"

"github.com/shirou/gopsutil/v4/common"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/pdata/pmetric"
"go.opentelemetry.io/collector/receiver"
"go.opentelemetry.io/collector/scraper"
)
Expand All @@ -24,18 +26,36 @@ type ScraperFactory interface {
// Config is the configuration of a scraper.
type Config interface {
SetRootPath(rootPath string)
SetEnvMap(envMap common.EnvMap)
}

type ScraperConfig struct {
RootPath string `mapstructure:"-"`
EnvMap common.EnvMap `mapstructure:"-"`
RootPath string `mapstructure:"-"`
}

func (p *ScraperConfig) SetRootPath(rootPath string) {
p.RootPath = rootPath
}

func (p *ScraperConfig) SetEnvMap(envMap common.EnvMap) {
p.EnvMap = envMap
type EnvVarScraper struct {
delegate scraper.Metrics
envMap common.EnvMap
}

func NewEnvVarScraper(delegate scraper.Metrics, envMap common.EnvMap) scraper.Metrics {
return &EnvVarScraper{delegate: delegate, envMap: envMap}
}

func (evs *EnvVarScraper) Start(ctx context.Context, host component.Host) error {
ctx = context.WithValue(ctx, common.EnvKey, evs.envMap)
return evs.delegate.Start(ctx, host)
}

func (evs *EnvVarScraper) ScrapeMetrics(ctx context.Context) (pmetric.Metrics, error) {
ctx = context.WithValue(ctx, common.EnvKey, evs.envMap)
return evs.delegate.ScrapeMetrics(ctx)
}

func (evs *EnvVarScraper) Shutdown(ctx context.Context) error {
ctx = context.WithValue(ctx, common.EnvKey, evs.envMap)
return evs.delegate.Shutdown(ctx)
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"fmt"
"time"

"github.com/shirou/gopsutil/v4/common"
"github.com/shirou/gopsutil/v4/cpu"
"github.com/shirou/gopsutil/v4/host"
"go.opentelemetry.io/collector/component"
Expand Down Expand Up @@ -50,7 +49,6 @@ func newCPUScraper(_ context.Context, settings receiver.Settings, cfg *Config) *
}

func (s *cpuScraper) start(ctx context.Context, _ component.Host) error {
ctx = context.WithValue(ctx, common.EnvKey, s.config.EnvMap)
bootTime, err := s.bootTime(ctx)
if err != nil {
return err
Expand All @@ -60,7 +58,6 @@ func (s *cpuScraper) start(ctx context.Context, _ component.Host) error {
}

func (s *cpuScraper) scrape(ctx context.Context) (pmetric.Metrics, error) {
ctx = context.WithValue(ctx, common.EnvKey, s.config.EnvMap)
now := pcommon.NewTimestampFromTime(s.now())
cpuTimes, err := s.times(ctx, true /*percpu=*/)
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"fmt"
"time"

"github.com/shirou/gopsutil/v4/common"
"github.com/shirou/gopsutil/v4/disk"
"github.com/shirou/gopsutil/v4/host"
"go.opentelemetry.io/collector/component"
Expand Down Expand Up @@ -66,7 +65,6 @@ func newDiskScraper(_ context.Context, settings receiver.Settings, cfg *Config)
}

func (s *diskScraper) start(ctx context.Context, _ component.Host) error {
ctx = context.WithValue(ctx, common.EnvKey, s.config.EnvMap)
bootTime, err := s.bootTime(ctx)
if err != nil {
return err
Expand All @@ -78,8 +76,6 @@ func (s *diskScraper) start(ctx context.Context, _ component.Host) error {
}

func (s *diskScraper) scrape(ctx context.Context) (pmetric.Metrics, error) {
ctx = context.WithValue(ctx, common.EnvKey, s.config.EnvMap)

now := pcommon.NewTimestampFromTime(time.Now())
ioCounters, err := s.ioCounters(ctx)
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ func newFileSystemScraper(_ context.Context, settings receiver.Settings, cfg *Co
}

func (s *filesystemsScraper) start(ctx context.Context, _ component.Host) error {
ctx = context.WithValue(ctx, common.EnvKey, s.config.EnvMap)
bootTime, err := s.bootTime(ctx)
if err != nil {
return err
Expand All @@ -69,7 +68,6 @@ func (s *filesystemsScraper) start(ctx context.Context, _ component.Host) error
}

func (s *filesystemsScraper) scrape(ctx context.Context) (pmetric.Metrics, error) {
ctx = context.WithValue(ctx, common.EnvKey, s.config.EnvMap)
now := pcommon.NewTimestampFromTime(time.Now())

var errors scrapererror.ScrapeErrors
Expand Down
Loading

0 comments on commit 9959e34

Please sign in to comment.