Skip to content

Commit 03513b7

Browse files
StarosdevStarosdev
committed
fix(config): use structured logging for config file messages
Replace log.Printf() calls in config loading with logrus structured logging. This ensures "No configuration file found" and similar messages use the proper log level format instead of printing directly to stderr. External log collectors (Fluent Bit, Loki) previously misclassified these messages as errors due to stderr output and lack of level prefix. Changes: - Add logger parameter to ReadConfig() interface method - Create bootstrap logger in main() before config loading - Replace log.Printf with logger.Infof/Errorf in config packages - Update mock files and tests for new signature Fixes #22 Upstream: AnalogJ#814
1 parent c072119 commit 03513b7

File tree

9 files changed

+75
-56
lines changed

9 files changed

+75
-56
lines changed

‎collector/cmd/collector-metrics/collector-metrics.go‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,19 @@ package main
33
import (
44
"encoding/json"
55
"fmt"
6-
"github.com/analogj/scrutiny/collector/pkg/collector"
7-
"github.com/analogj/scrutiny/collector/pkg/config"
8-
"github.com/analogj/scrutiny/collector/pkg/errors"
9-
"github.com/analogj/scrutiny/webapp/backend/pkg/version"
10-
"github.com/sirupsen/logrus"
116
"io"
127
"log"
138
"os"
149
"strings"
1510
"time"
1611

1712
utils "github.com/analogj/go-util/utils"
13+
"github.com/analogj/scrutiny/collector/pkg/collector"
14+
"github.com/analogj/scrutiny/collector/pkg/config"
15+
"github.com/analogj/scrutiny/collector/pkg/errors"
16+
"github.com/analogj/scrutiny/webapp/backend/pkg/version"
1817
"github.com/fatih/color"
18+
"github.com/sirupsen/logrus"
1919
"github.com/urfave/cli/v2"
2020
)
2121

@@ -36,9 +36,13 @@ func main() {
3636
configFilePath = configFilePathAlternative
3737
}
3838

39+
// Create a bootstrap logger for config loading (before config-based logger is available)
40+
bootstrapLogger := logrus.WithFields(logrus.Fields{"type": "metrics"})
41+
bootstrapLogger.Logger.SetLevel(logrus.InfoLevel)
42+
3943
//we're going to load the config file manually, since we need to validate it.
40-
err = config.ReadConfig(configFilePath) // Find and read the config file
41-
if _, ok := err.(errors.ConfigFileMissingError); ok { // Handle errors reading the config file
44+
err = config.ReadConfig(configFilePath, bootstrapLogger) // Find and read the config file
45+
if _, ok := err.(errors.ConfigFileMissingError); ok { // Handle errors reading the config file
4246
//ignore "could not find config file"
4347
} else if err != nil {
4448
os.Exit(1)
@@ -100,8 +104,8 @@ OPTIONS:
100104
Usage: "Run the scrutiny smartctl metrics collector",
101105
Action: func(c *cli.Context) error {
102106
if c.IsSet("config") {
103-
err = config.ReadConfig(c.String("config")) // Find and read the config file
104-
if err != nil { // Handle errors reading the config file
107+
err = config.ReadConfig(c.String("config"), bootstrapLogger) // Find and read the config file
108+
if err != nil { // Handle errors reading the config file
105109
//ignore "could not find config file"
106110
fmt.Printf("Could not find config file at specified path: %s", c.String("config"))
107111
return err

‎collector/pkg/config/config.go‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,16 @@ package config
22

33
import (
44
"fmt"
5+
"os"
6+
"sort"
7+
"strings"
8+
59
"github.com/analogj/go-util/utils"
610
"github.com/analogj/scrutiny/collector/pkg/errors"
711
"github.com/analogj/scrutiny/collector/pkg/models"
812
"github.com/mitchellh/mapstructure"
13+
"github.com/sirupsen/logrus"
914
"github.com/spf13/viper"
10-
"log"
11-
"os"
12-
"sort"
13-
"strings"
1415
)
1516

1617
// When initializing this class the following methods must be called:
@@ -67,29 +68,29 @@ func (c *configuration) Init() error {
6768
return nil
6869
}
6970

70-
func (c *configuration) ReadConfig(configFilePath string) error {
71+
func (c *configuration) ReadConfig(configFilePath string, logger *logrus.Entry) error {
7172
configFilePath, err := utils.ExpandPath(configFilePath)
7273
if err != nil {
7374
return err
7475
}
7576

7677
if !utils.FileExists(configFilePath) {
77-
log.Printf("No configuration file found at %v. Using Defaults.", configFilePath)
78+
logger.Infof("No configuration file found at %v. Using Defaults.", configFilePath)
7879
return errors.ConfigFileMissingError("The configuration file could not be found.")
7980
}
8081

8182
//validate config file contents
8283
//err = c.ValidateConfigFile(configFilePath)
8384
//if err != nil {
84-
// log.Printf("Config file at `%v` is invalid: %s", configFilePath, err)
85+
// logger.Errorf("Config file at `%v` is invalid: %s", configFilePath, err)
8586
// return err
8687
//}
8788

88-
log.Printf("Loading configuration file: %s", configFilePath)
89+
logger.Infof("Loading configuration file: %s", configFilePath)
8990

9091
config_data, err := os.Open(configFilePath)
9192
if err != nil {
92-
log.Printf("Error reading configuration file: %s", err)
93+
logger.Errorf("Error reading configuration file: %s", err)
9394
return err
9495
}
9596

‎collector/pkg/config/config_test.go‎

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,27 @@
11
package config_test
22

33
import (
4+
"path"
5+
"testing"
6+
47
"github.com/analogj/scrutiny/collector/pkg/config"
58
"github.com/analogj/scrutiny/collector/pkg/models"
9+
"github.com/sirupsen/logrus"
610
"github.com/stretchr/testify/require"
7-
"path"
8-
"testing"
911
)
1012

13+
func testLogger() *logrus.Entry {
14+
return logrus.WithFields(logrus.Fields{"type": "test"})
15+
}
16+
1117
func TestConfiguration_InvalidConfigPath(t *testing.T) {
1218
t.Parallel()
1319

1420
//setup
1521
testConfig, _ := config.Create()
1622

1723
//test
18-
err := testConfig.ReadConfig("does_not_exist.yaml")
24+
err := testConfig.ReadConfig("does_not_exist.yaml", testLogger())
1925

2026
//assert
2127
require.Error(t, err, "should return an error")
@@ -28,7 +34,7 @@ func TestConfiguration_GetScanOverrides_Simple(t *testing.T) {
2834
testConfig, _ := config.Create()
2935

3036
//test
31-
err := testConfig.ReadConfig(path.Join("testdata", "simple_device.yaml"))
37+
err := testConfig.ReadConfig(path.Join("testdata", "simple_device.yaml"), testLogger())
3238
require.NoError(t, err, "should correctly load simple device config")
3339
scanOverrides := testConfig.GetDeviceOverrides()
3440

@@ -44,7 +50,7 @@ func TestConfiguration_GetScanOverrides_DeviceTypeComma(t *testing.T) {
4450
testConfig, _ := config.Create()
4551

4652
//test
47-
err := testConfig.ReadConfig(path.Join("testdata", "device_type_comma.yaml"))
53+
err := testConfig.ReadConfig(path.Join("testdata", "device_type_comma.yaml"), testLogger())
4854
require.NoError(t, err, "should correctly load simple device config")
4955
scanOverrides := testConfig.GetDeviceOverrides()
5056

@@ -62,7 +68,7 @@ func TestConfiguration_GetScanOverrides_Ignore(t *testing.T) {
6268
testConfig, _ := config.Create()
6369

6470
//test
65-
err := testConfig.ReadConfig(path.Join("testdata", "ignore_device.yaml"))
71+
err := testConfig.ReadConfig(path.Join("testdata", "ignore_device.yaml"), testLogger())
6672
require.NoError(t, err, "should correctly load ignore device config")
6773
scanOverrides := testConfig.GetDeviceOverrides()
6874

@@ -77,7 +83,7 @@ func TestConfiguration_GetScanOverrides_Raid(t *testing.T) {
7783
testConfig, _ := config.Create()
7884

7985
//test
80-
err := testConfig.ReadConfig(path.Join("testdata", "raid_device.yaml"))
86+
err := testConfig.ReadConfig(path.Join("testdata", "raid_device.yaml"), testLogger())
8187
require.NoError(t, err, "should correctly load ignore device config")
8288
scanOverrides := testConfig.GetDeviceOverrides()
8389

@@ -102,7 +108,7 @@ func TestConfiguration_InvalidCommands_MissingJson(t *testing.T) {
102108
testConfig, _ := config.Create()
103109

104110
//test
105-
err := testConfig.ReadConfig(path.Join("testdata", "invalid_commands_missing_json.yaml"))
111+
err := testConfig.ReadConfig(path.Join("testdata", "invalid_commands_missing_json.yaml"), testLogger())
106112
require.EqualError(t, err, `ConfigValidationError: "configuration key 'commands.metrics_scan_args' is missing '--json' flag"`, "should throw an error because json flag is missing")
107113
}
108114

@@ -113,7 +119,7 @@ func TestConfiguration_InvalidCommands_IncludesDevice(t *testing.T) {
113119
testConfig, _ := config.Create()
114120

115121
//test
116-
err := testConfig.ReadConfig(path.Join("testdata", "invalid_commands_includes_device.yaml"))
122+
err := testConfig.ReadConfig(path.Join("testdata", "invalid_commands_includes_device.yaml"), testLogger())
117123
require.EqualError(t, err, `ConfigValidationError: "configuration key 'commands.metrics_info_args' must not contain '--device' or '-d' flag, configuration key 'commands.metrics_smart_args' must not contain '--device' or '-d' flag"`, "should throw an error because device flags detected")
118124
}
119125

@@ -124,7 +130,7 @@ func TestConfiguration_OverrideCommands(t *testing.T) {
124130
testConfig, _ := config.Create()
125131

126132
//test
127-
err := testConfig.ReadConfig(path.Join("testdata", "override_commands.yaml"))
133+
err := testConfig.ReadConfig(path.Join("testdata", "override_commands.yaml"), testLogger())
128134
require.NoError(t, err, "should not throw an error")
129135
require.Equal(t, "--xall --json -T permissive", testConfig.GetString("commands.metrics_smart_args"))
130136
}
@@ -136,7 +142,7 @@ func TestConfiguration_OverrideDeviceCommands_MetricsInfoArgs(t *testing.T) {
136142
testConfig, _ := config.Create()
137143

138144
//test
139-
err := testConfig.ReadConfig(path.Join("testdata", "override_device_commands.yaml"))
145+
err := testConfig.ReadConfig(path.Join("testdata", "override_device_commands.yaml"), testLogger())
140146
require.NoError(t, err, "should correctly override device command")
141147

142148
//assert
@@ -152,7 +158,7 @@ func TestConfiguration_DeviceAllowList(t *testing.T) {
152158
testConfig, err := config.Create()
153159
require.NoError(t, err)
154160

155-
require.NoError(t, testConfig.ReadConfig(path.Join("testdata", "allow_listed_devices_present.yaml")))
161+
require.NoError(t, testConfig.ReadConfig(path.Join("testdata", "allow_listed_devices_present.yaml"), testLogger()))
156162

157163
require.True(t, testConfig.IsAllowlistedDevice("/dev/sda"), "/dev/sda should be allow listed")
158164
require.False(t, testConfig.IsAllowlistedDevice("/dev/sdc"), "/dev/sda should not be allow listed")
@@ -163,7 +169,7 @@ func TestConfiguration_DeviceAllowList(t *testing.T) {
163169
require.NoError(t, err)
164170

165171
// Really just any other config where the key is full missing
166-
require.NoError(t, testConfig.ReadConfig(path.Join("testdata", "override_device_commands.yaml")))
172+
require.NoError(t, testConfig.ReadConfig(path.Join("testdata", "override_device_commands.yaml"), testLogger()))
167173

168174
// Anything should be allow listed if the key isnt there
169175
require.True(t, testConfig.IsAllowlistedDevice("/dev/sda"), "/dev/sda should be allow listed")

‎collector/pkg/config/interface.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,15 @@ package config
22

33
import (
44
"github.com/analogj/scrutiny/collector/pkg/models"
5+
"github.com/sirupsen/logrus"
56
"github.com/spf13/viper"
67
)
78

89
// Create mock using:
910
// mockgen -source=collector/pkg/config/interface.go -destination=collector/pkg/config/mock/mock_config.go
1011
type Interface interface {
1112
Init() error
12-
ReadConfig(configFilePath string) error
13+
ReadConfig(configFilePath string, logger *logrus.Entry) error
1314
Set(key string, value interface{})
1415
SetDefault(key string, value interface{})
1516

‎collector/pkg/config/mock/mock_config.go‎

Lines changed: 5 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎webapp/backend/cmd/scrutiny/scrutiny.go‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,18 @@ package main
33
import (
44
"encoding/json"
55
"fmt"
6-
"github.com/analogj/scrutiny/webapp/backend/pkg/config"
7-
"github.com/analogj/scrutiny/webapp/backend/pkg/errors"
8-
"github.com/analogj/scrutiny/webapp/backend/pkg/version"
9-
"github.com/analogj/scrutiny/webapp/backend/pkg/web"
10-
"github.com/sirupsen/logrus"
116
"io"
127
"log"
138
"os"
149
"time"
1510

1611
utils "github.com/analogj/go-util/utils"
12+
"github.com/analogj/scrutiny/webapp/backend/pkg/config"
13+
"github.com/analogj/scrutiny/webapp/backend/pkg/errors"
14+
"github.com/analogj/scrutiny/webapp/backend/pkg/version"
15+
"github.com/analogj/scrutiny/webapp/backend/pkg/web"
1716
"github.com/fatih/color"
17+
"github.com/sirupsen/logrus"
1818
"github.com/urfave/cli/v2"
1919
)
2020

@@ -35,9 +35,13 @@ func main() {
3535
configFilePath = configFilePathAlternative
3636
}
3737

38+
// Create a bootstrap logger for config loading (before config-based logger is available)
39+
bootstrapLogger := logrus.WithFields(logrus.Fields{"type": "web"})
40+
bootstrapLogger.Logger.SetLevel(logrus.InfoLevel)
41+
3842
//we're going to load the config file manually, since we need to validate it.
39-
err = config.ReadConfig(configFilePath) // Find and read the config file
40-
if _, ok := err.(errors.ConfigFileMissingError); ok { // Handle errors reading the config file
43+
err = config.ReadConfig(configFilePath, bootstrapLogger) // Find and read the config file
44+
if _, ok := err.(errors.ConfigFileMissingError); ok { // Handle errors reading the config file
4145
//ignore "could not find config file"
4246
} else if err != nil {
4347
log.Print(color.HiRedString("CONFIG ERROR: %v", err))
@@ -101,8 +105,8 @@ OPTIONS:
101105
Action: func(c *cli.Context) error {
102106
fmt.Fprintln(c.App.Writer, c.Command.Usage)
103107
if c.IsSet("config") {
104-
err = config.ReadConfig(c.String("config")) // Find and read the config file
105-
if err != nil { // Handle errors reading the config file
108+
err = config.ReadConfig(c.String("config"), bootstrapLogger) // Find and read the config file
109+
if err != nil { // Handle errors reading the config file
106110
//ignore "could not find config file"
107111
fmt.Printf("Could not find config file at specified path: %s", c.String("config"))
108112
return err

‎webapp/backend/pkg/config/config.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
package config
22

33
import (
4-
"log"
54
"os"
65
"strings"
76

87
"github.com/analogj/go-util/utils"
98
"github.com/analogj/scrutiny/webapp/backend/pkg/errors"
9+
"github.com/sirupsen/logrus"
1010
"github.com/spf13/viper"
1111
)
1212

@@ -86,7 +86,7 @@ func (c *configuration) Sub(key string) Interface {
8686
return &config
8787
}
8888

89-
func (c *configuration) ReadConfig(configFilePath string) error {
89+
func (c *configuration) ReadConfig(configFilePath string, logger *logrus.Entry) error {
9090
//make sure that we specify that this is the correct config path (for eventual WriteConfig() calls)
9191
c.SetConfigFile(configFilePath)
9292

@@ -96,22 +96,22 @@ func (c *configuration) ReadConfig(configFilePath string) error {
9696
}
9797

9898
if !utils.FileExists(configFilePath) {
99-
log.Printf("No configuration file found at %v. Using Defaults.", configFilePath)
99+
logger.Infof("No configuration file found at %v. Using Defaults.", configFilePath)
100100
return errors.ConfigFileMissingError("The configuration file could not be found.")
101101
}
102102

103103
//validate config file contents
104104
//err = c.ValidateConfigFile(configFilePath)
105105
//if err != nil {
106-
// log.Printf("Config file at `%v` is invalid: %s", configFilePath, err)
106+
// logger.Errorf("Config file at `%v` is invalid: %s", configFilePath, err)
107107
// return err
108108
//}
109109

110-
log.Printf("Loading configuration file: %s", configFilePath)
110+
logger.Infof("Loading configuration file: %s", configFilePath)
111111

112112
config_data, err := os.Open(configFilePath)
113113
if err != nil {
114-
log.Printf("Error reading configuration file: %s", err)
114+
logger.Errorf("Error reading configuration file: %s", err)
115115
return err
116116
}
117117

‎webapp/backend/pkg/config/interface.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
package config
22

33
import (
4+
"github.com/sirupsen/logrus"
45
"github.com/spf13/viper"
56
)
67

78
// Create mock using:
89
// mockgen -source=webapp/backend/pkg/config/interface.go -destination=webapp/backend/pkg/config/mock/mock_config.go
910
type Interface interface {
1011
Init() error
11-
ReadConfig(configFilePath string) error
12+
ReadConfig(configFilePath string, logger *logrus.Entry) error
1213
WriteConfig() error
1314
Set(key string, value interface{})
1415
SetDefault(key string, value interface{})

0 commit comments

Comments
 (0)