From 38568b59b266b842c8ed3ed56a3f3f0bbe379b63 Mon Sep 17 00:00:00 2001 From: dkeysil Date: Tue, 6 Feb 2024 17:02:15 +0100 Subject: [PATCH 01/11] Use since instead of tail --- clients/docker/client.go | 4 ++-- clients/interfaces.go | 2 +- clients/mocks/mock_clients.go | 8 ++++---- services/components/lifecycle/bot_logger.go | 8 +++----- .../components/lifecycle/bot_logger_test.go | 20 +++++++++---------- services/supervisor/agent_logs.go | 2 +- services/supervisor/services_test.go | 8 ++++++-- 7 files changed, 27 insertions(+), 25 deletions(-) diff --git a/clients/docker/client.go b/clients/docker/client.go index 39271abf..f7c0042b 100644 --- a/clients/docker/client.go +++ b/clients/docker/client.go @@ -799,12 +799,12 @@ func (d *dockerClient) ListDigestReferences(ctx context.Context) (imgs []string, } // GetContainerLogs gets the container logs. -func (d *dockerClient) GetContainerLogs(ctx context.Context, containerID, tail string, truncate int) (string, error) { +func (d *dockerClient) GetContainerLogs(ctx context.Context, containerID, since string, truncate int) (string, error) { r, err := d.cli.ContainerLogs(ctx, containerID, types.ContainerLogsOptions{ ShowStdout: true, ShowStderr: true, Timestamps: true, - Tail: tail, + Since: since, }) if err != nil { return "", err diff --git a/clients/interfaces.go b/clients/interfaces.go index e23b2cec..68671499 100644 --- a/clients/interfaces.go +++ b/clients/interfaces.go @@ -43,7 +43,7 @@ type DockerClient interface { EnsureLocalImage(ctx context.Context, name, ref string) error EnsureLocalImages(ctx context.Context, timeoutPerPull time.Duration, imagePulls []docker.ImagePull) []error ListDigestReferences(ctx context.Context) ([]string, error) - GetContainerLogs(ctx context.Context, containerID, tail string, truncate int) (string, error) + GetContainerLogs(ctx context.Context, containerID, since string, truncate int) (string, error) GetContainerFromRemoteAddr(ctx context.Context, hostPort string) (*types.Container, error) SetImagePullCooldown(threshold int, cooldownDuration time.Duration) Events(ctx context.Context, since time.Time) (<-chan events.Message, <-chan error) diff --git a/clients/mocks/mock_clients.go b/clients/mocks/mock_clients.go index 7865ddc4..20daf87a 100644 --- a/clients/mocks/mock_clients.go +++ b/clients/mocks/mock_clients.go @@ -203,18 +203,18 @@ func (mr *MockDockerClientMockRecorder) GetContainerFromRemoteAddr(ctx, hostPort } // GetContainerLogs mocks base method. -func (m *MockDockerClient) GetContainerLogs(ctx context.Context, containerID, tail string, truncate int) (string, error) { +func (m *MockDockerClient) GetContainerLogs(ctx context.Context, containerID, since string, truncate int) (string, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetContainerLogs", ctx, containerID, tail, truncate) + ret := m.ctrl.Call(m, "GetContainerLogs", ctx, containerID, since, truncate) ret0, _ := ret[0].(string) ret1, _ := ret[1].(error) return ret0, ret1 } // GetContainerLogs indicates an expected call of GetContainerLogs. -func (mr *MockDockerClientMockRecorder) GetContainerLogs(ctx, containerID, tail, truncate interface{}) *gomock.Call { +func (mr *MockDockerClientMockRecorder) GetContainerLogs(ctx, containerID, since, truncate interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetContainerLogs", reflect.TypeOf((*MockDockerClient)(nil).GetContainerLogs), ctx, containerID, tail, truncate) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetContainerLogs", reflect.TypeOf((*MockDockerClient)(nil).GetContainerLogs), ctx, containerID, since, truncate) } // GetContainers mocks base method. diff --git a/services/components/lifecycle/bot_logger.go b/services/components/lifecycle/bot_logger.go index da7620ac..568f7a54 100644 --- a/services/components/lifecycle/bot_logger.go +++ b/services/components/lifecycle/bot_logger.go @@ -3,7 +3,6 @@ package lifecycle import ( "context" "fmt" - "strconv" "time" "github.com/ethereum/go-ethereum/accounts/keystore" @@ -17,7 +16,7 @@ import ( // BotLogger manages bots logging. type BotLogger interface { - SendBotLogs(ctx context.Context) error + SendBotLogs(ctx context.Context, snapshotInterval time.Duration) error } type botLogger struct { @@ -47,12 +46,11 @@ func NewBotLogger( // adjust these better with auto-upgrade later const ( - defaultAgentLogSendInterval = time.Minute defaultAgentLogTailLines = 50 defaultAgentLogAvgMaxCharsPerLine = 200 ) -func (bl *botLogger) SendBotLogs(ctx context.Context) error { +func (bl *botLogger) SendBotLogs(ctx context.Context, snapshotInterval time.Duration) error { var ( sendLogs agentlogs.Agents keepLogs agentlogs.Agents @@ -69,7 +67,7 @@ func (bl *botLogger) SendBotLogs(ctx context.Context) error { } logs, err := bl.dockerClient.GetContainerLogs( ctx, container.ID, - strconv.Itoa(defaultAgentLogTailLines), + fmt.Sprintf("%ds", int64(snapshotInterval.Seconds())), defaultAgentLogAvgMaxCharsPerLine*defaultAgentLogTailLines, ) if err != nil { diff --git a/services/components/lifecycle/bot_logger_test.go b/services/components/lifecycle/bot_logger_test.go index 55e84946..e5cd3865 100644 --- a/services/components/lifecycle/bot_logger_test.go +++ b/services/components/lifecycle/bot_logger_test.go @@ -3,8 +3,8 @@ package lifecycle import ( "context" "errors" - "strconv" "testing" + "time" "github.com/docker/docker/api/types" "github.com/ethereum/go-ethereum/accounts/keystore" @@ -87,18 +87,18 @@ func (s *BotLoggerSuite) TestSendBotLogs() { } s.dockerClient.EXPECT().GetContainerLogs( ctx, "bot1", - strconv.Itoa(defaultAgentLogTailLines), + "60s", defaultAgentLogAvgMaxCharsPerLine*defaultAgentLogTailLines, ).Return("some log", nil).Times(1) s.dockerClient.EXPECT().GetContainerLogs( ctx, "bot2", - strconv.Itoa(defaultAgentLogTailLines), + "60s", defaultAgentLogAvgMaxCharsPerLine*defaultAgentLogTailLines, ).Return("some log", nil).Times(1) s.botClient.EXPECT().LoadBotContainers(ctx).Return(mockContainers, nil) - s.r.NoError(botLogger.SendBotLogs(ctx)) + s.r.NoError(botLogger.SendBotLogs(ctx, time.Minute)) } // should fail if there is an error loading @@ -115,7 +115,7 @@ func (s *BotLoggerSuite) TestLoadBotContainersError() { mockContainers := []types.Container{} s.botClient.EXPECT().LoadBotContainers(ctx).Return(mockContainers, errors.New("test")) - s.r.EqualError(botLogger.SendBotLogs(ctx), "failed to load the bot containers: test") + s.r.EqualError(botLogger.SendBotLogs(ctx, time.Minute), "failed to load the bot containers: test") } // Should not send agent logs if fails @@ -154,17 +154,17 @@ func (s *BotLoggerSuite) TestGetContainerLogsError() { s.dockerClient.EXPECT().GetContainerLogs( ctx, "bot1", - strconv.Itoa(defaultAgentLogTailLines), + "60s", defaultAgentLogAvgMaxCharsPerLine*defaultAgentLogTailLines, ).Return("", errors.New("test")).Times(1) s.dockerClient.EXPECT().GetContainerLogs( ctx, "bot2", - strconv.Itoa(defaultAgentLogTailLines), + "60s", defaultAgentLogAvgMaxCharsPerLine*defaultAgentLogTailLines, ).Return("some log", nil).Times(1) - s.r.NoError(botLogger.SendBotLogs(ctx)) + s.r.NoError(botLogger.SendBotLogs(ctx, time.Minute)) } // Fails sending agent logs @@ -192,9 +192,9 @@ func (s *BotLoggerSuite) TestFailsToSendLogs() { s.dockerClient.EXPECT().GetContainerLogs( ctx, "bot1", - strconv.Itoa(defaultAgentLogTailLines), + "60s", defaultAgentLogAvgMaxCharsPerLine*defaultAgentLogTailLines, ).Return("some log", nil).Times(1) - s.r.EqualError(botLogger.SendBotLogs(ctx), "failed to send agent logs: test") + s.r.EqualError(botLogger.SendBotLogs(ctx, time.Minute), "failed to send agent logs: test") } diff --git a/services/supervisor/agent_logs.go b/services/supervisor/agent_logs.go index 27352884..3498f989 100644 --- a/services/supervisor/agent_logs.go +++ b/services/supervisor/agent_logs.go @@ -10,7 +10,7 @@ func (sup *SupervisorService) syncAgentLogs() { interval := time.Duration(sup.botLifecycleConfig.Config.AgentLogsConfig.SendIntervalSeconds) * time.Second ticker := time.NewTicker(interval) for range ticker.C { - err := sup.botLifecycle.BotLogger.SendBotLogs(sup.ctx) + err := sup.botLifecycle.BotLogger.SendBotLogs(sup.ctx, interval) sup.lastAgentLogsRequest.Set() sup.lastAgentLogsRequestError.Set(err) if err != nil { diff --git a/services/supervisor/services_test.go b/services/supervisor/services_test.go index ebc923c6..0df9b12b 100644 --- a/services/supervisor/services_test.go +++ b/services/supervisor/services_test.go @@ -21,6 +21,7 @@ import ( "github.com/forta-network/forta-node/config" "github.com/forta-network/forta-node/services/components/containers" mock_containers "github.com/forta-network/forta-node/services/components/containers/mocks" + mock_lifecycle "github.com/forta-network/forta-node/services/components/lifecycle/mocks" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -53,8 +54,9 @@ type Suite struct { globalClient *mock_clients.MockDockerClient releaseClient *mrelease.MockClient - msgClient *mock_clients.MockMessageClient - botClient *mock_containers.MockBotClient + msgClient *mock_clients.MockMessageClient + botClient *mock_containers.MockBotClient + botLifeCycleManager *mock_lifecycle.MockBotLifecycleManager supervisor *SupervisorService @@ -104,6 +106,7 @@ func (s *Suite) SetupTest() { s.globalClient = mock_clients.NewMockDockerClient(ctrl) s.releaseClient = mrelease.NewMockClient(ctrl) s.botClient = mock_containers.NewMockBotClient(ctrl) + s.botLifeCycleManager = mock_lifecycle.NewMockBotLifecycleManager(ctrl) s.msgClient = mock_clients.NewMockMessageClient(ctrl) @@ -132,6 +135,7 @@ func (s *Suite) SetupTest() { supervisor.config.Config.AgentLogsConfig.SendIntervalSeconds = 1 supervisor.botLifecycleConfig.Config = supervisor.config.Config supervisor.botLifecycle.BotClient = s.botClient + supervisor.botLifecycle.BotManager = s.botLifeCycleManager s.supervisor = supervisor } From 3c37ca7e643370e55b1e31b6f674bacf3cc389a0 Mon Sep 17 00:00:00 2001 From: dkeysil Date: Wed, 7 Feb 2024 14:02:15 +0100 Subject: [PATCH 02/11] Use chain id from agetn config --- go.mod | 2 +- go.sum | 6 ++---- services/components/components.go | 1 + services/components/lifecycle/bot_logger.go | 13 +++++++++++- .../components/lifecycle/bot_logger_test.go | 20 +++++++++++-------- .../registry/mocks/mock_registry.go | 15 ++++++++++++++ services/components/registry/registry.go | 19 ++++++++++++++++++ 7 files changed, 62 insertions(+), 14 deletions(-) diff --git a/go.mod b/go.mod index 58de2f5a..d0100aee 100644 --- a/go.mod +++ b/go.mod @@ -41,7 +41,7 @@ replace github.com/docker/docker => github.com/moby/moby v20.10.25+incompatible require ( github.com/docker/docker v1.6.2 github.com/docker/go-connections v0.4.0 - github.com/forta-network/forta-core-go v0.0.0-20240129180226-af53540338f3 + github.com/forta-network/forta-core-go v0.0.0-20240207125602-ede00282c520 github.com/prometheus/client_golang v1.14.0 github.com/prometheus/client_model v0.3.0 github.com/prometheus/common v0.39.0 diff --git a/go.sum b/go.sum index d4c5c1a3..fff084b4 100644 --- a/go.sum +++ b/go.sum @@ -329,10 +329,8 @@ github.com/flynn/noise v0.0.0-20180327030543-2492fe189ae6/go.mod h1:1i71OnUq3iUe github.com/flynn/noise v1.0.0 h1:DlTHqmzmvcEiKj+4RYo/imoswx/4r6iBlCMfVtrMXpQ= github.com/flynn/noise v1.0.0/go.mod h1:xbMo+0i6+IGbYdJhF31t2eR1BIU0CYc12+BNAKwUTag= github.com/fogleman/gg v1.2.1-0.20190220221249-0403632d5b90/go.mod h1:R/bRT+9gY/C5z7JzPU0zXsXHKM4/ayA+zqcVNZzPa1k= -github.com/forta-network/forta-core-go v0.0.0-20240129095537-dad5459b7283 h1:MmvZ3so59eNLtsJgEnRS1cwy/uqI/PazAS0x9Xkl3+E= -github.com/forta-network/forta-core-go v0.0.0-20240129095537-dad5459b7283/go.mod h1:iNehCWOypwVeO8b1GKmsrEWReHTvO5qw8SsGvZsBINo= -github.com/forta-network/forta-core-go v0.0.0-20240129180226-af53540338f3 h1:tfuCghhFdyolM3CiapTxtdLVHcy7ssRUjo5JxwwJnGc= -github.com/forta-network/forta-core-go v0.0.0-20240129180226-af53540338f3/go.mod h1:iNehCWOypwVeO8b1GKmsrEWReHTvO5qw8SsGvZsBINo= +github.com/forta-network/forta-core-go v0.0.0-20240207125602-ede00282c520 h1:4RGJtf8/9K8nPCpIxcBWrXt7wE+pcJzwDl/tELuvb3c= +github.com/forta-network/forta-core-go v0.0.0-20240207125602-ede00282c520/go.mod h1:iNehCWOypwVeO8b1GKmsrEWReHTvO5qw8SsGvZsBINo= github.com/forta-network/go-multicall v0.0.0-20230609185354-1436386c6707 h1:f6I7K43i2m6AwHSsDxh0Mf3qFzYt8BKnabSl/zGFmh0= github.com/forta-network/go-multicall v0.0.0-20230609185354-1436386c6707/go.mod h1:nqTUF1REklpWLZ/M5HfzqhSHNz4dPVKzJvbLziqTZpw= github.com/fortytw2/leaktest v1.3.0/go.mod h1:jDsjWgpAGjm2CA7WthBh/CdZYEPF31XHquHwclZch5g= diff --git a/services/components/components.go b/services/components/components.go index 8c0e8a68..5cf8a572 100644 --- a/services/components/components.go +++ b/services/components/components.go @@ -130,6 +130,7 @@ func GetBotLifecycleComponents( botLogger := lifecycle.NewBotLogger( botClient, dockerClient, + botLifeConfig.BotRegistry, botLifeConfig.Key, agentlogs.NewClient(botLifeConfig.Config.AgentLogsConfig.URL).SendLogs, ) diff --git a/services/components/lifecycle/bot_logger.go b/services/components/lifecycle/bot_logger.go index 568f7a54..1ced64d9 100644 --- a/services/components/lifecycle/bot_logger.go +++ b/services/components/lifecycle/bot_logger.go @@ -11,6 +11,7 @@ import ( "github.com/forta-network/forta-node/clients" "github.com/forta-network/forta-node/clients/docker" "github.com/forta-network/forta-node/services/components/containers" + "github.com/forta-network/forta-node/services/components/registry" log "github.com/sirupsen/logrus" ) @@ -22,6 +23,7 @@ type BotLogger interface { type botLogger struct { botClient containers.BotClient dockerClient clients.DockerClient + agentRegistry registry.BotRegistry key *keystore.Key prevAgentLogs agentlogs.Agents @@ -33,12 +35,14 @@ var _ BotLogger = &botLogger{} func NewBotLogger( botClient containers.BotClient, dockerClient clients.DockerClient, + agentRegistry registry.BotRegistry, key *keystore.Key, sendAgentLogs func(agents agentlogs.Agents, authToken string) error, ) *botLogger { return &botLogger{ botClient: botClient, dockerClient: dockerClient, + agentRegistry: agentRegistry, key: key, sendAgentLogs: sendAgentLogs, } @@ -75,10 +79,17 @@ func (bl *botLogger) SendBotLogs(ctx context.Context, snapshotInterval time.Dura continue } + agentID := container.Labels[docker.LabelFortaBotID] agent := &agentlogs.Agent{ - ID: container.Labels[docker.LabelFortaBotID], + ID: agentID, Logs: logs, } + + agentConfig, err := bl.agentRegistry.GetConfigByID(agentID) + if err == nil { + agent.ChainID = int64(agentConfig.ChainID) + } + // don't send if it's the same with previous logs but keep it for next time // so we can check keepLogs = append(keepLogs, agent) diff --git a/services/components/lifecycle/bot_logger_test.go b/services/components/lifecycle/bot_logger_test.go index e5cd3865..42fa019e 100644 --- a/services/components/lifecycle/bot_logger_test.go +++ b/services/components/lifecycle/bot_logger_test.go @@ -13,6 +13,7 @@ import ( "github.com/forta-network/forta-node/clients/docker" mock_clients "github.com/forta-network/forta-node/clients/mocks" mock_containers "github.com/forta-network/forta-node/services/components/containers/mocks" + mock_registry "github.com/forta-network/forta-node/services/components/registry/mocks" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -25,10 +26,11 @@ func TestSendBotLogsSuite(t *testing.T) { type BotLoggerSuite struct { r *require.Assertions - botLogger *botLogger - botClient *mock_containers.MockBotClient - dockerClient *mock_clients.MockDockerClient - key *keystore.Key + botLogger *botLogger + botClient *mock_containers.MockBotClient + dockerClient *mock_clients.MockDockerClient + agentRegistry *mock_registry.MockBotRegistry + key *keystore.Key suite.Suite } @@ -39,6 +41,7 @@ func (s *BotLoggerSuite) SetupTest() { botClient := mock_containers.NewMockBotClient(ctrl) dockerClient := mock_clients.NewMockDockerClient(ctrl) + agentRegistry := mock_registry.NewMockBotRegistry(ctrl) dir := t.TempDir() ks := keystore.NewKeyStore(dir, keystore.StandardScryptN, keystore.StandardScryptP) @@ -51,13 +54,14 @@ func (s *BotLoggerSuite) SetupTest() { s.botClient = botClient s.dockerClient = dockerClient + s.agentRegistry = agentRegistry s.key = key s.r = r } func (s *BotLoggerSuite) TestSendBotLogs() { botLogger := NewBotLogger( - s.botClient, s.dockerClient, s.key, + s.botClient, s.dockerClient, s.agentRegistry, s.key, func(agents agentlogs.Agents, authToken string) error { s.r.Equal(2, len(agents)) s.r.Equal("bot1ID", agents[0].ID) @@ -105,7 +109,7 @@ func (s *BotLoggerSuite) TestSendBotLogs() { // bot containers func (s *BotLoggerSuite) TestLoadBotContainersError() { botLogger := NewBotLogger( - s.botClient, s.dockerClient, s.key, + s.botClient, s.dockerClient, s.agentRegistry,s.key, func(agents agentlogs.Agents, authToken string) error { return nil }, @@ -122,7 +126,7 @@ func (s *BotLoggerSuite) TestLoadBotContainersError() { // to get container logs but continue processing func (s *BotLoggerSuite) TestGetContainerLogsError() { botLogger := NewBotLogger( - s.botClient, s.dockerClient, s.key, + s.botClient, s.dockerClient,s.agentRegistry, s.key, func(agents agentlogs.Agents, authToken string) error { s.r.Equal(1, len(agents)) s.r.Equal("bot2ID", agents[0].ID) @@ -170,7 +174,7 @@ func (s *BotLoggerSuite) TestGetContainerLogsError() { // Fails sending agent logs func (s *BotLoggerSuite) TestFailsToSendLogs() { botLogger := NewBotLogger( - s.botClient, s.dockerClient, s.key, + s.botClient, s.dockerClient,s.agentRegistry, s.key, func(agents agentlogs.Agents, authToken string) error { return errors.New("test") }, diff --git a/services/components/registry/mocks/mock_registry.go b/services/components/registry/mocks/mock_registry.go index 7cac901f..f02a6c2b 100644 --- a/services/components/registry/mocks/mock_registry.go +++ b/services/components/registry/mocks/mock_registry.go @@ -35,6 +35,21 @@ func (m *MockBotRegistry) EXPECT() *MockBotRegistryMockRecorder { return m.recorder } +// GetConfigByID mocks base method. +func (m *MockBotRegistry) GetConfigByID(agentID string) (*config.AgentConfig, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetConfigByID", agentID) + ret0, _ := ret[0].(*config.AgentConfig) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetConfigByID indicates an expected call of GetConfigByID. +func (mr *MockBotRegistryMockRecorder) GetConfigByID(agentID interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetConfigByID", reflect.TypeOf((*MockBotRegistry)(nil).GetConfigByID), agentID) +} + // Health mocks base method. func (m *MockBotRegistry) Health() health.Reports { m.ctrl.T.Helper() diff --git a/services/components/registry/registry.go b/services/components/registry/registry.go index d809f5b0..ab61e9ee 100644 --- a/services/components/registry/registry.go +++ b/services/components/registry/registry.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "sync" "github.com/forta-network/forta-node/store" @@ -17,6 +18,7 @@ import ( type BotRegistry interface { LoadAssignedBots() ([]config.AgentConfig, error) LoadHeartbeatBot() (*config.AgentConfig, error) + GetConfigByID(agentID string) (*config.AgentConfig, error) health.Reporter } @@ -27,6 +29,7 @@ type botRegistry struct { registryStore store.RegistryStore + mu *sync.RWMutex botConfigs []config.AgentConfig lastChecked health.TimeTracker @@ -39,6 +42,7 @@ func New(cfg config.Config, scannerAddress common.Address) (BotRegistry, error) service := &botRegistry{ cfg: cfg, scannerAddress: scannerAddress, + mu: &sync.RWMutex{}, } var ( regStr store.RegistryStore @@ -69,6 +73,9 @@ func (br *botRegistry) LoadHeartbeatBot() (*config.AgentConfig, error) { // LoadAssignedBots returns the latest bot list for the running scanner. func (br *botRegistry) LoadAssignedBots() ([]config.AgentConfig, error) { + br.mu.Lock() + defer br.mu.Unlock() + br.lastChecked.Set() agts, changed, err := br.registryStore.GetAgentsIfChanged(br.scannerAddress.Hex()) if err != nil { @@ -87,6 +94,18 @@ func (br *botRegistry) LoadAssignedBots() ([]config.AgentConfig, error) { return br.botConfigs, nil } +func (br *botRegistry) GetConfigByID(agentID string) (*config.AgentConfig, error) { + br.mu.Lock() + defer br.mu.Unlock() + + for _, ac := range br.botConfigs { + if ac.ID == agentID { + return &ac, nil + } + } + return nil, fmt.Errorf("cannot find bot with ID %s", agentID) +} + // Name implements health.Reporter interface. func (br *botRegistry) Name() string { return "bot-registry" From 514e0eb6f1abc00b920e1a89ce0136b7a22fa0d1 Mon Sep 17 00:00:00 2001 From: dkeysil Date: Wed, 7 Feb 2024 14:06:42 +0100 Subject: [PATCH 03/11] Fix test --- .../components/lifecycle/bot_logger_test.go | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/services/components/lifecycle/bot_logger_test.go b/services/components/lifecycle/bot_logger_test.go index 42fa019e..13b0acbb 100644 --- a/services/components/lifecycle/bot_logger_test.go +++ b/services/components/lifecycle/bot_logger_test.go @@ -12,6 +12,7 @@ import ( "github.com/forta-network/forta-core-go/security" "github.com/forta-network/forta-node/clients/docker" mock_clients "github.com/forta-network/forta-node/clients/mocks" + "github.com/forta-network/forta-node/config" mock_containers "github.com/forta-network/forta-node/services/components/containers/mocks" mock_registry "github.com/forta-network/forta-node/services/components/registry/mocks" "github.com/golang/mock/gomock" @@ -26,7 +27,6 @@ func TestSendBotLogsSuite(t *testing.T) { type BotLoggerSuite struct { r *require.Assertions - botLogger *botLogger botClient *mock_containers.MockBotClient dockerClient *mock_clients.MockDockerClient agentRegistry *mock_registry.MockBotRegistry @@ -64,8 +64,12 @@ func (s *BotLoggerSuite) TestSendBotLogs() { s.botClient, s.dockerClient, s.agentRegistry, s.key, func(agents agentlogs.Agents, authToken string) error { s.r.Equal(2, len(agents)) + s.r.Equal("bot1ID", agents[0].ID) + s.r.EqualValues(1, agents[0].ChainID) + s.r.Equal("bot2ID", agents[1].ID) + s.r.EqualValues(2, agents[1].ChainID) return nil }, ) @@ -101,6 +105,9 @@ func (s *BotLoggerSuite) TestSendBotLogs() { defaultAgentLogAvgMaxCharsPerLine*defaultAgentLogTailLines, ).Return("some log", nil).Times(1) + s.agentRegistry.EXPECT().GetConfigByID("bot1ID").Return(&config.AgentConfig{ChainID: 1}, nil).Times(1) + s.agentRegistry.EXPECT().GetConfigByID("bot2ID").Return(&config.AgentConfig{ChainID: 2}, nil).Times(1) + s.botClient.EXPECT().LoadBotContainers(ctx).Return(mockContainers, nil) s.r.NoError(botLogger.SendBotLogs(ctx, time.Minute)) } @@ -109,7 +116,7 @@ func (s *BotLoggerSuite) TestSendBotLogs() { // bot containers func (s *BotLoggerSuite) TestLoadBotContainersError() { botLogger := NewBotLogger( - s.botClient, s.dockerClient, s.agentRegistry,s.key, + s.botClient, s.dockerClient, s.agentRegistry, s.key, func(agents agentlogs.Agents, authToken string) error { return nil }, @@ -126,7 +133,7 @@ func (s *BotLoggerSuite) TestLoadBotContainersError() { // to get container logs but continue processing func (s *BotLoggerSuite) TestGetContainerLogsError() { botLogger := NewBotLogger( - s.botClient, s.dockerClient,s.agentRegistry, s.key, + s.botClient, s.dockerClient, s.agentRegistry, s.key, func(agents agentlogs.Agents, authToken string) error { s.r.Equal(1, len(agents)) s.r.Equal("bot2ID", agents[0].ID) @@ -168,13 +175,15 @@ func (s *BotLoggerSuite) TestGetContainerLogsError() { defaultAgentLogAvgMaxCharsPerLine*defaultAgentLogTailLines, ).Return("some log", nil).Times(1) + s.agentRegistry.EXPECT().GetConfigByID("bot2ID").Return(&config.AgentConfig{ChainID: 2}, nil).Times(1) + s.r.NoError(botLogger.SendBotLogs(ctx, time.Minute)) } // Fails sending agent logs func (s *BotLoggerSuite) TestFailsToSendLogs() { botLogger := NewBotLogger( - s.botClient, s.dockerClient,s.agentRegistry, s.key, + s.botClient, s.dockerClient, s.agentRegistry, s.key, func(agents agentlogs.Agents, authToken string) error { return errors.New("test") }, @@ -200,5 +209,7 @@ func (s *BotLoggerSuite) TestFailsToSendLogs() { defaultAgentLogAvgMaxCharsPerLine*defaultAgentLogTailLines, ).Return("some log", nil).Times(1) + s.agentRegistry.EXPECT().GetConfigByID("bot1ID").Return(&config.AgentConfig{ChainID: 1}, nil).Times(1) + s.r.EqualError(botLogger.SendBotLogs(ctx, time.Minute), "failed to send agent logs: test") } From 13fcbddd06b081da3827b8ebc33caf823265a49b Mon Sep 17 00:00:00 2001 From: dkeysil Date: Wed, 7 Feb 2024 14:22:00 +0100 Subject: [PATCH 04/11] Use tail to define number of logs but preserve slice cutting with some limit --- clients/docker/client.go | 15 ++++++++++++--- services/components/lifecycle/bot_logger.go | 5 ++--- services/components/lifecycle/bot_logger_test.go | 10 +++++----- services/components/registry/registry_test.go | 2 ++ 4 files changed, 21 insertions(+), 11 deletions(-) diff --git a/clients/docker/client.go b/clients/docker/client.go index f7c0042b..5761da3e 100644 --- a/clients/docker/client.go +++ b/clients/docker/client.go @@ -9,6 +9,7 @@ import ( "fmt" "io" "path" + "strconv" "strings" "time" @@ -798,13 +799,18 @@ func (d *dockerClient) ListDigestReferences(ctx context.Context) (imgs []string, return } +const ( + defaultAgentLogAvgMaxCharsPerLine = 200 +) + // GetContainerLogs gets the container logs. -func (d *dockerClient) GetContainerLogs(ctx context.Context, containerID, since string, truncate int) (string, error) { +func (d *dockerClient) GetContainerLogs(ctx context.Context, containerID, since string, tail int) (string, error) { r, err := d.cli.ContainerLogs(ctx, containerID, types.ContainerLogsOptions{ ShowStdout: true, ShowStderr: true, Timestamps: true, Since: since, + Tail: strconv.Itoa(tail), }) if err != nil { return "", err @@ -813,9 +819,12 @@ func (d *dockerClient) GetContainerLogs(ctx context.Context, containerID, since if err != nil { return "", err } - if truncate >= 0 && len(b) > truncate { - b = b[:truncate] + + // limit the log size + if tail >= 0 && len(b) > defaultAgentLogAvgMaxCharsPerLine*tail { + b = b[:defaultAgentLogAvgMaxCharsPerLine*tail] } + // remove strange 8-byte prefix in each line lines := strings.Split(string(b), "\n") for i, line := range lines { diff --git a/services/components/lifecycle/bot_logger.go b/services/components/lifecycle/bot_logger.go index 1ced64d9..94119267 100644 --- a/services/components/lifecycle/bot_logger.go +++ b/services/components/lifecycle/bot_logger.go @@ -50,8 +50,7 @@ func NewBotLogger( // adjust these better with auto-upgrade later const ( - defaultAgentLogTailLines = 50 - defaultAgentLogAvgMaxCharsPerLine = 200 + defaultAgentLogTailLines = 600 ) func (bl *botLogger) SendBotLogs(ctx context.Context, snapshotInterval time.Duration) error { @@ -72,7 +71,7 @@ func (bl *botLogger) SendBotLogs(ctx context.Context, snapshotInterval time.Dura logs, err := bl.dockerClient.GetContainerLogs( ctx, container.ID, fmt.Sprintf("%ds", int64(snapshotInterval.Seconds())), - defaultAgentLogAvgMaxCharsPerLine*defaultAgentLogTailLines, + defaultAgentLogTailLines, ) if err != nil { log.WithError(err).Warn("failed to get agent container logs") diff --git a/services/components/lifecycle/bot_logger_test.go b/services/components/lifecycle/bot_logger_test.go index 13b0acbb..c28634b0 100644 --- a/services/components/lifecycle/bot_logger_test.go +++ b/services/components/lifecycle/bot_logger_test.go @@ -96,13 +96,13 @@ func (s *BotLoggerSuite) TestSendBotLogs() { s.dockerClient.EXPECT().GetContainerLogs( ctx, "bot1", "60s", - defaultAgentLogAvgMaxCharsPerLine*defaultAgentLogTailLines, + defaultAgentLogTailLines, ).Return("some log", nil).Times(1) s.dockerClient.EXPECT().GetContainerLogs( ctx, "bot2", "60s", - defaultAgentLogAvgMaxCharsPerLine*defaultAgentLogTailLines, + defaultAgentLogTailLines, ).Return("some log", nil).Times(1) s.agentRegistry.EXPECT().GetConfigByID("bot1ID").Return(&config.AgentConfig{ChainID: 1}, nil).Times(1) @@ -166,13 +166,13 @@ func (s *BotLoggerSuite) TestGetContainerLogsError() { s.dockerClient.EXPECT().GetContainerLogs( ctx, "bot1", "60s", - defaultAgentLogAvgMaxCharsPerLine*defaultAgentLogTailLines, + defaultAgentLogTailLines, ).Return("", errors.New("test")).Times(1) s.dockerClient.EXPECT().GetContainerLogs( ctx, "bot2", "60s", - defaultAgentLogAvgMaxCharsPerLine*defaultAgentLogTailLines, + defaultAgentLogTailLines, ).Return("some log", nil).Times(1) s.agentRegistry.EXPECT().GetConfigByID("bot2ID").Return(&config.AgentConfig{ChainID: 2}, nil).Times(1) @@ -206,7 +206,7 @@ func (s *BotLoggerSuite) TestFailsToSendLogs() { s.dockerClient.EXPECT().GetContainerLogs( ctx, "bot1", "60s", - defaultAgentLogAvgMaxCharsPerLine*defaultAgentLogTailLines, + defaultAgentLogTailLines, ).Return("some log", nil).Times(1) s.agentRegistry.EXPECT().GetConfigByID("bot1ID").Return(&config.AgentConfig{ChainID: 1}, nil).Times(1) diff --git a/services/components/registry/registry_test.go b/services/components/registry/registry_test.go index 1c987ac1..5d228b5f 100644 --- a/services/components/registry/registry_test.go +++ b/services/components/registry/registry_test.go @@ -2,6 +2,7 @@ package registry import ( "errors" + "sync" "testing" "github.com/ethereum/go-ethereum/common" @@ -20,6 +21,7 @@ func TestLoadAssignedBots(t *testing.T) { botReg := &botRegistry{ scannerAddress: common.HexToAddress(utils.ZeroAddress), registryStore: regStore, + mu: &sync.RWMutex{}, } cfgs := []config.AgentConfig{{}} From 773205b5d24c792f42c0670e8f1c7e95fbefa160 Mon Sep 17 00:00:00 2001 From: dkeysil Date: Wed, 7 Feb 2024 16:00:24 +0100 Subject: [PATCH 05/11] Fix bytes prefix in container logs fetching --- clients/docker/client.go | 18 +++++++----------- cmd/supervisor/main.go | 1 + 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/clients/docker/client.go b/clients/docker/client.go index 5761da3e..373d8078 100644 --- a/clients/docker/client.go +++ b/clients/docker/client.go @@ -825,19 +825,15 @@ func (d *dockerClient) GetContainerLogs(ctx context.Context, containerID, since b = b[:defaultAgentLogAvgMaxCharsPerLine*tail] } - // remove strange 8-byte prefix in each line - lines := strings.Split(string(b), "\n") - for i, line := range lines { - if len(line) == 0 { - continue - } - prefixEnd := strings.Index(line, "2") // timestamp beginning - if prefixEnd < 0 || prefixEnd > len(line) { - continue + // remove 8-byte prefix in each line + // https://github.com/moby/moby/issues/8223 + bs := bytes.Split(b, []byte("\n")) + for i, v := range bs { + if len(v) > 8 { + bs[i] = v[8:] } - lines[i] = line[prefixEnd:] } - return strings.Join(lines, "\n"), nil + return string(bytes.Join(bs, []byte("\n"))), nil } func (d *dockerClient) labelFilter() filters.Args { diff --git a/cmd/supervisor/main.go b/cmd/supervisor/main.go index 668cc574..e91b9224 100644 --- a/cmd/supervisor/main.go +++ b/cmd/supervisor/main.go @@ -38,6 +38,7 @@ func initServices(ctx context.Context, cfg config.Config) ([]services.Service, e Config: cfg, ScannerAddress: key.Address, BotRegistry: botRegistry, + Key: key, } svc, err := supervisor.NewSupervisorService(ctx, supervisor.SupervisorServiceConfig{ Config: cfg, From 826c282bb5642bfce5fce6358f0ec8e71da95d19 Mon Sep 17 00:00:00 2001 From: dkeysil Date: Thu, 8 Feb 2024 08:54:45 +0100 Subject: [PATCH 06/11] Proper usage of locks and limit increasement --- clients/docker/client.go | 2 +- services/components/registry/registry.go | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/clients/docker/client.go b/clients/docker/client.go index 373d8078..1062bc26 100644 --- a/clients/docker/client.go +++ b/clients/docker/client.go @@ -800,7 +800,7 @@ func (d *dockerClient) ListDigestReferences(ctx context.Context) (imgs []string, } const ( - defaultAgentLogAvgMaxCharsPerLine = 200 + defaultAgentLogAvgMaxCharsPerLine = 1000 ) // GetContainerLogs gets the container logs. diff --git a/services/components/registry/registry.go b/services/components/registry/registry.go index ab61e9ee..6fe021a4 100644 --- a/services/components/registry/registry.go +++ b/services/components/registry/registry.go @@ -73,8 +73,6 @@ func (br *botRegistry) LoadHeartbeatBot() (*config.AgentConfig, error) { // LoadAssignedBots returns the latest bot list for the running scanner. func (br *botRegistry) LoadAssignedBots() ([]config.AgentConfig, error) { - br.mu.Lock() - defer br.mu.Unlock() br.lastChecked.Set() agts, changed, err := br.registryStore.GetAgentsIfChanged(br.scannerAddress.Hex()) @@ -85,7 +83,11 @@ func (br *botRegistry) LoadAssignedBots() ([]config.AgentConfig, error) { logger := log.WithField("component", "bot-loader") if changed { br.lastChangeDetected.Set() + + br.mu.Lock() + defer br.mu.Unlock() br.botConfigs = agts + logger.WithField("count", len(agts)).Info("updated bot list") } else { logger.Debug("no bot list changes detected") @@ -95,8 +97,8 @@ func (br *botRegistry) LoadAssignedBots() ([]config.AgentConfig, error) { } func (br *botRegistry) GetConfigByID(agentID string) (*config.AgentConfig, error) { - br.mu.Lock() - defer br.mu.Unlock() + br.mu.RLock() + defer br.mu.RUnlock() for _, ac := range br.botConfigs { if ac.ID == agentID { From b7dc1c3d34694089536eeef225d7bcaa3a7b178d Mon Sep 17 00:00:00 2001 From: dkeysil Date: Thu, 8 Feb 2024 15:29:33 +0100 Subject: [PATCH 07/11] Decrease log lines to 300 --- services/components/lifecycle/bot_logger.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/components/lifecycle/bot_logger.go b/services/components/lifecycle/bot_logger.go index 94119267..b748307a 100644 --- a/services/components/lifecycle/bot_logger.go +++ b/services/components/lifecycle/bot_logger.go @@ -50,7 +50,7 @@ func NewBotLogger( // adjust these better with auto-upgrade later const ( - defaultAgentLogTailLines = 600 + defaultAgentLogTailLines = 300 ) func (bl *botLogger) SendBotLogs(ctx context.Context, snapshotInterval time.Duration) error { From 828076f9c70bec071fba145ffd515f20d7c626f0 Mon Sep 17 00:00:00 2001 From: dkeysil Date: Thu, 8 Feb 2024 17:43:49 +0100 Subject: [PATCH 08/11] Use correct access param for jwt key --- services/components/lifecycle/bot_logger.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/components/lifecycle/bot_logger.go b/services/components/lifecycle/bot_logger.go index b748307a..20445b00 100644 --- a/services/components/lifecycle/bot_logger.go +++ b/services/components/lifecycle/bot_logger.go @@ -102,7 +102,7 @@ func (bl *botLogger) SendBotLogs(ctx context.Context, snapshotInterval time.Dura if len(sendLogs) > 0 { scannerJwt, err := security.CreateScannerJWT(bl.key, map[string]interface{}{ - "access": "bot_logger", + "access": "agent_logs", }) if err != nil { return fmt.Errorf("failed to create scanner token: %v", err) From 18242c9a17add0f0f580edcb94b513ec27ceb718 Mon Sep 17 00:00:00 2001 From: dkeysil Date: Fri, 9 Feb 2024 13:05:41 +0100 Subject: [PATCH 09/11] Do not send empty bot logs --- services/components/lifecycle/bot_logger.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/services/components/lifecycle/bot_logger.go b/services/components/lifecycle/bot_logger.go index 20445b00..870d147d 100644 --- a/services/components/lifecycle/bot_logger.go +++ b/services/components/lifecycle/bot_logger.go @@ -78,6 +78,11 @@ func (bl *botLogger) SendBotLogs(ctx context.Context, snapshotInterval time.Dura continue } + if len(logs) == 0 { + log.WithField("agent", container.Labels[docker.LabelFortaBotID]).Debug("no logs found for agent") + continue + } + agentID := container.Labels[docker.LabelFortaBotID] agent := &agentlogs.Agent{ ID: agentID, From ca74353e89b4e66908e7cc858c0596db0f424904 Mon Sep 17 00:00:00 2001 From: dkeysil Date: Fri, 9 Feb 2024 13:21:51 +0100 Subject: [PATCH 10/11] Tweak get container logs params --- clients/docker/client.go | 2 +- services/components/lifecycle/bot_logger.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clients/docker/client.go b/clients/docker/client.go index 1062bc26..373d8078 100644 --- a/clients/docker/client.go +++ b/clients/docker/client.go @@ -800,7 +800,7 @@ func (d *dockerClient) ListDigestReferences(ctx context.Context) (imgs []string, } const ( - defaultAgentLogAvgMaxCharsPerLine = 1000 + defaultAgentLogAvgMaxCharsPerLine = 200 ) // GetContainerLogs gets the container logs. diff --git a/services/components/lifecycle/bot_logger.go b/services/components/lifecycle/bot_logger.go index 870d147d..0263b5a2 100644 --- a/services/components/lifecycle/bot_logger.go +++ b/services/components/lifecycle/bot_logger.go @@ -50,7 +50,7 @@ func NewBotLogger( // adjust these better with auto-upgrade later const ( - defaultAgentLogTailLines = 300 + defaultAgentLogTailLines = 120 ) func (bl *botLogger) SendBotLogs(ctx context.Context, snapshotInterval time.Duration) error { From 7145720395452de674605bd20f48260199cfe5e6 Mon Sep 17 00:00:00 2001 From: Dmitry <46797839+dkeysil@users.noreply.github.com> Date: Fri, 9 Feb 2024 16:55:57 +0100 Subject: [PATCH 11/11] Update services/components/registry/registry.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Caner Çıdam --- services/components/registry/registry.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/components/registry/registry.go b/services/components/registry/registry.go index 6fe021a4..529227b9 100644 --- a/services/components/registry/registry.go +++ b/services/components/registry/registry.go @@ -85,8 +85,8 @@ func (br *botRegistry) LoadAssignedBots() ([]config.AgentConfig, error) { br.lastChangeDetected.Set() br.mu.Lock() - defer br.mu.Unlock() br.botConfigs = agts + br.mu.Unlock() logger.WithField("count", len(agts)).Info("updated bot list") } else {