Skip to content

Commit

Permalink
chore: Increase some logging levels to Info (#2064)
Browse files Browse the repository at this point in the history
Increase debug level to Info
  • Loading branch information
shazlehu authored Dec 17, 2024
1 parent e9cb5fe commit 10318a1
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 13 deletions.
7 changes: 4 additions & 3 deletions opamp/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ func (c Config) GetSecretKey() string {

// CmpUpdatableFields compares updatable fields for equality
func (c Config) CmpUpdatableFields(o Config) (equal bool) {
if !cmpStringPtr(c.AgentName, o.AgentName) {
if !CmpStringPtr(c.AgentName, o.AgentName) {
return false
}

Expand All @@ -385,10 +385,11 @@ func (c Config) CmpUpdatableFields(o Config) (equal bool) {
return false
}

return cmpStringPtr(c.Labels, o.Labels)
return CmpStringPtr(c.Labels, o.Labels)
}

func cmpStringPtr(p1, p2 *string) bool {
// CmpStringPtr compares two string pointers for equality
func CmpStringPtr(p1, p2 *string) bool {
switch {
case p1 == nil && p2 == nil:
return true
Expand Down
72 changes: 72 additions & 0 deletions opamp/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1195,3 +1195,75 @@ func TestAgentID_UnmarshalYaml(t *testing.T) {
require.Equal(t, EmptyAgentID, emptyID)
})
}

func Test_cmpStringPtr(t *testing.T) {
strPtr := func(s string) *string {
return &s
}
type args struct {
p1 *string
p2 *string
}
tests := []struct {
name string
args args
want bool
}{

{
name: "Both nil",
args: args{
p1: nil,
p2: nil,
},
want: true,
},
{
name: "p1 nil",
args: args{
p1: nil,
p2: strPtr("foo"),
},
want: false,
},
{
name: "p2 nil",
args: args{
p1: strPtr("foo"),
p2: nil,
},
want: false,
},
{
name: "Both empty",
args: args{
p1: strPtr(""),
p2: strPtr(""),
},
want: true,
},
{
name: "Both equal",
args: args{
p1: strPtr("foo"),
p2: strPtr("foo"),
},
want: true,
},
{
name: "Different",
args: args{
p1: strPtr("foo"),
p2: strPtr("bar"),
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := CmpStringPtr(tt.args.p1, tt.args.p2); got != tt.want {
t.Errorf("cmpStringPtr() = %v, want %v", got, tt.want)
}
})
}
}
4 changes: 2 additions & 2 deletions opamp/observiq/observiq_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -370,15 +370,15 @@ func (c *Client) onRemoteConfigHandler(ctx context.Context, remoteConfig *protob
remoteCfgStatus.ErrorMessage = fmt.Sprintf("Failed to apply config changes: %s", err.Error())
}

c.logger.Debug("Setting remote config status", zap.String("status", remoteCfgStatus.Status.String()))
c.logger.Info("Setting remote config status", zap.String("status", remoteCfgStatus.Status.String()))
// Set the remote config status
if err := c.opampClient.SetRemoteConfigStatus(remoteCfgStatus); err != nil {
return fmt.Errorf("failed to set remote config status: %w", err)
}

// If we changed the config call UpdateEffectiveConfig
if changed {
c.logger.Debug("Updating effective config")
c.logger.Info("Updating effective config")
if err := c.opampClient.UpdateEffectiveConfig(ctx); err != nil {
return fmt.Errorf("failed to update effective config: %w", err)
}
Expand Down
27 changes: 21 additions & 6 deletions opamp/observiq/reload_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package observiq
import (
"context"
"fmt"
"maps"
"os"
"path/filepath"

Expand All @@ -41,6 +42,20 @@ func managerReload(client *Client, managerConfigPath string) opamp.ReloadFunc {
return false, nil
}

updatedKeys := []string{}
if !opamp.CmpStringPtr(client.currentConfig.AgentName, newConfig.AgentName) {
updatedKeys = append(updatedKeys, "agent_name")
}

if client.currentConfig.MeasurementsInterval != newConfig.MeasurementsInterval {
updatedKeys = append(updatedKeys, "measurements_interval")
}

if !maps.Equal(client.currentConfig.ExtraMeasurementsAttributes, newConfig.ExtraMeasurementsAttributes) {
updatedKeys = append(updatedKeys, "extra_measurements_attributes")
}

client.logger.Info("Manager config update detected", zap.Strings("updated_keys", updatedKeys))
// Going to do an update prep a rollback
rollbackFunc, cleanupFunc, err := prepRollback(managerConfigPath)
if err != nil {
Expand Down Expand Up @@ -114,7 +129,7 @@ func managerReload(client *Client, managerConfigPath string) opamp.ReloadFunc {
func collectorReload(client *Client, collectorConfigPath string) opamp.ReloadFunc {
return func(contents []byte) (bool, error) {
rollbackFunc, cleanupFunc, err := prepRollback(collectorConfigPath)
client.logger.Debug("Rollback prepped", zap.String("collectorConfigPath", collectorConfigPath))
client.logger.Info("Rollback prepped", zap.String("collectorConfigPath", collectorConfigPath))
if err != nil {
return false, fmt.Errorf("failed to prep for rollback: %w", err)
}
Expand All @@ -130,17 +145,17 @@ func collectorReload(client *Client, collectorConfigPath string) opamp.ReloadFun
if err := updateConfigFile(CollectorConfigName, collectorConfigPath, contents); err != nil {
return false, err
}
client.logger.Debug("Config file updated", zap.String("name", CollectorConfigName), zap.String("collectorConfigPath", collectorConfigPath))
client.logger.Info("Config file updated", zap.String("name", CollectorConfigName), zap.String("collectorConfigPath", collectorConfigPath))
// Stop collector monitoring as we are going to restart it
client.stopCollectorMonitoring()

client.logger.Debug("Collector monitoring stopped")
client.logger.Info("Collector monitoring stopped")
// Setup new monitoring after collector has been restarted
defer client.startCollectorMonitoring(context.Background())

// Reload collector
if err := client.collector.Restart(context.Background()); err != nil {
client.logger.Debug("OTEL Collector restart error", zap.Error(err))
client.logger.Info("OTEL Collector restart error", zap.Error(err))

// Rollback file
if rollbackErr := rollbackFunc(); rollbackErr != nil {
Expand All @@ -154,10 +169,10 @@ func collectorReload(client *Client, collectorConfigPath string) opamp.ReloadFun

return false, fmt.Errorf("collector failed to restart: %w", err)
}
client.logger.Debug("OTEL Collector restarted")
client.logger.Info("OTEL Collector restarted")
// Reset Snapshot Reporter
report.GetSnapshotReporter().Reset()
client.logger.Debug("Snapshot reporter reset")
client.logger.Info("Snapshot reporter reset")

return true, nil
}
Expand Down
10 changes: 8 additions & 2 deletions opamp/observiq/reload_funcs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ func Test_managerReload(t *testing.T) {
{
desc: "Invalid new config contents",
testFunc: func(*testing.T) {
client := &Client{}
client := &Client{
logger: zap.NewNop(),
}
reloadFunc := managerReload(client, ManagerConfigName)

badContents := []byte(`\t\t\t`)
Expand All @@ -57,6 +59,7 @@ func Test_managerReload(t *testing.T) {

managerFilePath := filepath.Join(tmpDir, ManagerConfigName)
client := &Client{
logger: zap.NewNop(),
currentConfig: opamp.Config{
Endpoint: "ws://localhost:1234",
AgentID: testAgentID,
Expand Down Expand Up @@ -92,6 +95,7 @@ func Test_managerReload(t *testing.T) {
mockOpAmpClient.On("SetAgentDescription", mock.Anything).Return(nil)

client := &Client{
logger: zap.NewNop(),
opampClient: mockOpAmpClient,
ident: newIdentity(zap.NewNop(), *currConfig, "0.0.0"),
currentConfig: *currConfig,
Expand Down Expand Up @@ -148,6 +152,7 @@ func Test_managerReload(t *testing.T) {
mockOpAmpClient.On("SetAgentDescription", mock.Anything).Return(expectedErr)

client := &Client{
logger: zap.NewNop(),
opampClient: mockOpAmpClient,
ident: newIdentity(zap.NewNop(), *currConfig, "0.0.0"),
currentConfig: *currConfig,
Expand Down Expand Up @@ -221,8 +226,8 @@ func Test_collectorReload(t *testing.T) {
assert.NoError(t, err)

client := &Client{
collector: mockCollector,
logger: zap.NewNop(),
collector: mockCollector,
}

// Setup Context to mock out already running collector monitor
Expand Down Expand Up @@ -324,6 +329,7 @@ func Test_loggerReload(t *testing.T) {
mockCol.On("Restart", mock.Anything).Return(nil)

client := &Client{
logger: zap.NewNop(),
collector: mockCol,
}

Expand Down

0 comments on commit 10318a1

Please sign in to comment.