Skip to content
Open
Show file tree
Hide file tree
Changes from 6 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
1 change: 1 addition & 0 deletions pkg/collector/corechecks/system/disk/diskv2/disk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1360,6 +1360,7 @@ func TestDiskCheckWithoutCoreLoader(t *testing.T) {

cfg := configmock.New(t)
cfg.Set("disk_check.use_core_loader", false, configmodel.SourceAgentRuntime)
cfg.Set("use_diskv2_check", false, configmodel.SourceAgentRuntime)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we are in diskv2 folder, don't we want to set it to true ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use_diskv2_check takes precedence over disk_check.use_core_loader in the code
Ie. if use_diskv2_check: true then we actually use the Go v2 check instead of the Python check, no matter the value of disk_check.use_core_loader
Which is quite counter intuitive though 😄

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the details 🙇🏻

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think your comment still reveals an issue, we're testing something which can't happen here: if use_diskv2_check is false then the v2 code is unreachable, so there is no point in checking (and testing) that here

I think we can just remove the test and the whole condition at the beginning of Configure in diskv2 (btw there is no equivalent in networkv2)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is testing that an error is thrown when trying to initialize the diskv2 check when it shouldn't be used. It maybe should just be deleted instead


diskFactory := diskv2.Factory()
diskCheckFunc, ok := diskFactory.Get()
Expand Down
24 changes: 14 additions & 10 deletions pkg/config/setup/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,20 +333,24 @@ func InitConfig(config pkgconfigmodel.Setup) {
// Otherwise, Python is loaded when the collector is initialized.
config.BindEnvAndSetDefault("python_lazy_loading", true)

// If false, the core check will be skipped.
config.BindEnvAndSetDefault("disk_check.use_core_loader", false)
config.BindEnvAndSetDefault("network_check.use_core_loader", false)

// If true, then the go loader will be prioritized over the python loader.
config.BindEnvAndSetDefault("prioritize_go_check_loader", true)

// If true, then new version of disk v2 check will be used.
// Otherwise, the old version of disk check will be used (maintaining backward compatibility).
config.BindEnvAndSetDefault("use_diskv2_check", false)

// If true, then new version of network v2 check will be used.
// Otherwise, the old version of network check will be used (maintaining backward compatibility).
config.BindEnvAndSetDefault("use_networkv2_check", false)
// Otherwise, the python version of disk check will be used.
config.BindEnvAndSetDefault("use_diskv2_check", true)
config.BindEnvAndSetDefault("disk_check.use_core_loader", true)

// the darwin network and check has not been ported from python
if runtime.GOOS == "linux" || runtime.GOOS == "windows" {
// If true, then new version of network v2 check will be used.
// Otherwise, the python version of network check will be used.
config.BindEnvAndSetDefault("use_networkv2_check", true)
config.BindEnvAndSetDefault("network_check.use_core_loader", true)
} else {
config.BindEnvAndSetDefault("use_networkv2_check", false)
config.BindEnvAndSetDefault("network_check.use_core_loader", false)
}

// if/when the default is changed to true, make the default platform
// dependent; default should remain false on Windows to maintain backward
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Each section from every release note are combined when the
# CHANGELOG.rst is rendered. So the text needs to be worded so that
# it does not depend on any information only available in another
# section. This may mean repeating some details, but each section
# must be readable independently of the other.
#
# Each section note must be formatted as reStructuredText.
---
enhancements:
- |
Enable the Go disk and network core checks by default for Windows and Linux.
These are direct ports of the existing Python disk and network checks and allow
the Python runtime to be lazy loaded when other integrations are enabled. It
can be disabled with setting ``use_diskv2_check`` and ``use_networkv2_check``
respectively along with the loader in your configuration to use the Python
version.