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

feat: add optional InstanceID field to log output #46

Merged
merged 1 commit into from
Apr 15, 2024
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
1 change: 1 addition & 0 deletions .devcontainer/host-metering.conf
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@ write_retry_max_int_sec=2
metrics_wal_path=./mocks/cpumetrics
metrics_max_age_sec=30
log_level=DEBUG
instance_id=
10 changes: 10 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const (
DefaultMetricsWALPath = "/var/run/host-metering/metrics"
DefaultLogLevel = "INFO"
DefaultLogPath = "" //Default to stderr, will be logged in journal.
DefaultInstanceID = ""
)

type Config struct {
Expand All @@ -49,6 +50,7 @@ type Config struct {
MetricsWALPath string
LogLevel string // one of "ERROR", "WARN", "INFO", "DEBUG", "TRACE"
LogPath string
InstanceID string
}

func NewConfig() *Config {
Expand All @@ -68,6 +70,7 @@ func NewConfig() *Config {
MetricsWALPath: DefaultMetricsWALPath,
LogLevel: DefaultLogLevel,
LogPath: DefaultLogPath,
InstanceID: DefaultInstanceID,
}
}

Expand All @@ -90,6 +93,7 @@ func (c *Config) String() string {
fmt.Sprintf("| MetricsWALPath: %s", c.MetricsWALPath),
fmt.Sprintf("| LogLevel: %s", c.LogLevel),
fmt.Sprintf("| LogPath: %s", c.LogPath),
fmt.Sprintf("| InstanceID: %s", c.InstanceID),
}, "\n")
}

Expand Down Expand Up @@ -150,6 +154,9 @@ func (c *Config) UpdateFromEnvVars() error {
if v := os.Getenv("HOST_METERING_LOG_PATH"); v != "" {
c.LogPath = v
}
if v := os.Getenv("HOST_METERING_INSTANCE_ID"); v != "" {
c.InstanceID = v
}
return multiError.ErrorOrNil()
}

Expand Down Expand Up @@ -267,6 +274,9 @@ func (c *Config) UpdateFromConfigFile(path string) error {
if v, ok := config[section]["log_path"]; ok {
c.LogPath = v
}
if v, ok := config[section]["instance_id"]; ok {
c.InstanceID = v
}

return multiError.ErrorOrNil()
}
Expand Down
15 changes: 11 additions & 4 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ func TestDefaultConfig(t *testing.T) {
"| MetricsMaxAgeSec: 5400\n" +
"| MetricsWALPath: /var/run/host-metering/metrics\n" +
"| LogLevel: INFO\n" +
"| LogPath: \n"
"| LogPath: \n" +
"| InstanceID: \n"

// Create the default configuration.
c := NewConfig()
Expand Down Expand Up @@ -73,7 +74,8 @@ func TestConfigFile(t *testing.T) {
"| MetricsMaxAgeSec: 700\n" +
"| MetricsWALPath: /tmp/metrics\n" +
"| LogLevel: ERROR\n" +
"| LogPath: /tmp/log\n"
"| LogPath: /tmp/log\n" +
"| InstanceID: test-instance\n"

// Update the configuration from a valid config file.
fileContent := "[host-metering]\n" +
Expand All @@ -93,7 +95,9 @@ func TestConfigFile(t *testing.T) {
"metrics_max_age_sec = 700\n" +
"metrics_wal_path = /tmp/metrics\n" +
"log_level = ERROR\n" +
"log_path = /tmp/log\n"
"log_path = /tmp/log\n" +
"instance_id = test-instance\n"

c := NewConfig()

createConfigFile(t, path, fileContent)
Expand Down Expand Up @@ -151,7 +155,8 @@ func TestEnvVariables(t *testing.T) {
"| MetricsMaxAgeSec: 700\n" +
"| MetricsWALPath: /tmp/metrics\n" +
"| LogLevel: ERROR\n" +
"| LogPath: /tmp/log\n"
"| LogPath: /tmp/log\n" +
"| InstanceID: test-instance\n"

// Set valid environment variables.
t.Setenv("HOST_METERING_WRITE_URL", "http://test/url")
Expand All @@ -169,6 +174,7 @@ func TestEnvVariables(t *testing.T) {
t.Setenv("HOST_METERING_METRICS_WAL_PATH", "/tmp/metrics")
t.Setenv("HOST_METERING_LOG_LEVEL", "ERROR")
t.Setenv("HOST_METERING_LOG_PATH", "/tmp/log")
t.Setenv("HOST_METERING_INSTANCE_ID", "test-instance")

// Environment variables are set. Change the defaults.
c := NewConfig()
Expand Down Expand Up @@ -223,6 +229,7 @@ func clearEnvironment() {
_ = os.Unsetenv("HOST_METERING_METRICS_WAL_PATH")
_ = os.Unsetenv("HOST_METERING_LOG_LEVEL")
_ = os.Unsetenv("HOST_METERING_LOG_PATH")
_ = os.Unsetenv("HOST_METERING_INSTANCE_ID")
}

func checkError(t *testing.T, err error, message string) {
Expand Down
3 changes: 3 additions & 0 deletions contrib/man/host-metering.1
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ Log level. Possible values are: DEBUG, INFO, WARN, ERROR, TRACE.
\fBHOST_METERING_LOG_PATH\fR
Path to log file. Default is empty - stderr.

\fBHOST_METERING_INSTANCE_ID\fR
Instance id. Default is empty.

.SH "FILES"
.PP
\fI/etc/host-metering.conf\fR
Expand Down
5 changes: 5 additions & 0 deletions contrib/man/host-metering.conf.5
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,11 @@ log_path (string)
Path to log file. Default is empty - stderr.
.RE

.PP
instance_id (string)
.RS 4
Instance id. Default is empty.

.SH "EXAMPLES"
.PP
1\&. The following example shows how to switch the logging to DEBUG level\&.
Expand Down
17 changes: 15 additions & 2 deletions logger/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,20 @@ type Logger logrus.FieldLogger
type CustomFormatter struct{}

func (f *CustomFormatter) Format(entry *logrus.Entry) ([]byte, error) {
var msg string

instanceID, ok := entry.Data["instance_id"].(string)
if ok {
msg = fmt.Sprintf("[%s] ", instanceID)
}

message := entry.Message
if !strings.HasSuffix(message, "\n") {
message += "\n"
}
msg := fmt.Sprintf("%s %s", entry.Time.Format("2006/01/02 15:04:05"), message)

msg += fmt.Sprintf("%s %s", entry.Time.Format("2006/01/02 15:04:05"), message)

return []byte(msg), nil
}

Expand All @@ -35,7 +44,7 @@ func InitDefaultLogger() Logger {
return logger
}

func InitLogger(file string, level string) error {
func InitLogger(file string, level string, instanceID string) error {
logLevel, err := logrus.ParseLevel(level)

if err != nil {
Expand All @@ -56,6 +65,10 @@ func InitLogger(file string, level string) error {
Level: logLevel,
}

if instanceID != "" {
log = log.WithField("instance_id", instanceID)
}

return nil
}

Expand Down
9 changes: 6 additions & 3 deletions logger/logger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestLoggerGlobalFunctions(t *testing.T) {

// Test initialization of logger with only log level
func TestInitLogger(t *testing.T) {
InitLogger("", DebugLevel.String())
InitLogger("", DebugLevel.String(), "")
if log == nil {
t.Fatalf("logger is not initialized")
}
Expand All @@ -51,7 +51,7 @@ func TestInitLogger(t *testing.T) {
func TestInitLoggerFile(t *testing.T) {
dir := t.TempDir()
path := dir + "/test.log"
InitLogger(path, DebugLevel.String())
InitLogger(path, DebugLevel.String(), "test_instance")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Currently we only test whether the logger is correctly initialized.
Consider adding a test for the prefix as well, e.g.

	if !strings.HasPrefix(string(data), "[test_instance]") {
		t.Errorf("Expected the output to start with '[test_instance]', but got: %s", string(data))
	}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the feedback and for testing the code. Will make the change!

if log == nil {
t.Fatalf("logger is not initialized")
}
Expand All @@ -64,14 +64,17 @@ func TestInitLoggerFile(t *testing.T) {
t.Fatalf("log file is not created")
}

// Check that the file contains the logger message
// Check that the file contains the instanceID and logger message
data, err := os.ReadFile(path)
if err != nil {
t.Fatalf("cannot read the log file")
}
if !strings.Contains(string(data), testMsg) {
t.Fatalf("log file does not contain the logged message")
}
if !strings.HasPrefix(string(data), "[test_instance]") {
t.Errorf("Expected the output to start with '[test_instance]', but got: %s", string(data))
}
}

// Test that logger can be raplaced by other implementation of Logger interface.
Expand Down
2 changes: 1 addition & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func main() {
}

// initialize the logger according to the given configuration
err = logger.InitLogger(cfg.LogPath, cfg.LogLevel)
err = logger.InitLogger(cfg.LogPath, cfg.LogLevel, cfg.InstanceID)

if err != nil {
logger.Debugf("Error initializing logger: %s\n", err.Error())
Expand Down
Loading