Skip to content

Commit

Permalink
address comments and add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
soberpeach committed Oct 4, 2024
1 parent 2b84780 commit 306c6c8
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 12 deletions.
10 changes: 7 additions & 3 deletions pkg/logs/launchers/integration/launcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ func NewLauncher(sources *sources.LogSources, integrationsLogsComp integrations.
ddLog.Warn("Unable to compute integrations logs max disk usage, falling back to integrations_logs_total_usage setting:", err)
maxDiskUsage = logsTotalUsageSetting
}

if maxDiskUsage == 0 {
ddLog.Warn("No space available to store logs. Logs from integrations will be dropped. Please allocate space for logs to be stored.")
}
}

return &Launcher{
Expand Down Expand Up @@ -197,14 +201,14 @@ func (s *Launcher) receiveLogs(log integrations.IntegrationLog) {
for s.combinedUsageSize+logSize > s.combinedUsageMax {
leastRecentlyModifiedFile := s.getLeastRecentlyModifiedFile()
if leastRecentlyModifiedFile == nil {
ddLog.Warn("Could not determine least recently modified file, skipping writing log to file.")
ddLog.Error("Could not determine least recently modified file, skipping writing log to file.")
return
}

err := s.deleteFile(leastRecentlyModifiedFile)
if err != nil {
ddLog.Error("Error deleting log file:", err)
continue
return
}

file, err := os.Create(leastRecentlyModifiedFile.filename)
Expand Down Expand Up @@ -387,7 +391,7 @@ func (s *Launcher) scanInitialFiles(dir string) error {
leastRecentlyModifiedFile := s.getLeastRecentlyModifiedFile()

if leastRecentlyModifiedFile == nil {
ddLog.Warn("getLeastRecentlyModifiedFile returned nil.")
ddLog.Error("Could not determine least recently modified file")
return errors.New("getLeastRecentlyModifiedFile returned nil when trying to delete files")
}

Expand Down
93 changes: 84 additions & 9 deletions pkg/logs/launchers/integration/launcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ func (suite *LauncherTestSuite) TestFileCreation() {
}

func (suite *LauncherTestSuite) TestSendLog() {

mockConf := &integration.Config{}
mockConf.Provider = "container"
mockConf.LogsConfig = integration.Data(`[{"type": "integration", "source": "foo", "service": "bar"}]`)
Expand Down Expand Up @@ -99,6 +98,90 @@ func (suite *LauncherTestSuite) TestSendLog() {
assert.Equal(suite.T(), expectedPath, <-filepathChan)
}

// TestNegativeCombinedUsageMax ensures errors in combinedUsageMax don't result
// in panics from `deleteFile`
func (suite *LauncherTestSuite) TestNegativeCombinedUsageMax() {
suite.s.combinedUsageMax = -1
err := suite.s.scanInitialFiles(suite.s.runPath)
assert.NotNil(suite.T(), err)
}

// TestZeroCombinedUsageMax ensures the launcher won't panic when
// combinedUsageMax is zero. Realistically the launcher would never run receiveLogs since there is a check for
func (suite *LauncherTestSuite) TestZeroCombinedUsageMaxFileCreated() {
suite.s.combinedUsageMax = 0

filename := "sample_integration_123.log"
filepath := filepath.Join(suite.s.runPath, filename)
file, err := os.Create(filepath)
assert.Nil(suite.T(), err)

file.Close()

suite.s.Start(nil, nil, nil, nil)

integrationLog := integrations.IntegrationLog{
Log: "sample log",
IntegrationID: "sample_integration:123",
}

suite.s.receiveLogs(integrationLog)
}

func (suite *LauncherTestSuite) TestZeroCombinedUsageMaxFileNotCreated() {
suite.s.combinedUsageMax = 0

suite.s.Start(nil, nil, nil, nil)

integrationLog := integrations.IntegrationLog{
Log: "sample log",
IntegrationID: "sample_integration:123",
}

suite.s.receiveLogs(integrationLog)
}

func (suite *LauncherTestSuite) TestSmallCombinedUsageMax() {
suite.s.combinedUsageMax = 10

filename := "sample_integration_123.log"
filepath := filepath.Join(suite.s.runPath, filename)
file, err := os.Create(filepath)
assert.Nil(suite.T(), err)

file.Close()

suite.s.Start(nil, nil, nil, nil)

// Launcher should write this log
writtenLog := "sample"
integrationLog := integrations.IntegrationLog{
Log: writtenLog,
IntegrationID: "sample_integration:123",
}
suite.s.receiveLogs(integrationLog)
fileStat, err := os.Stat(filepath)
assert.Nil(suite.T(), err)
assert.Equal(suite.T(), fileStat.Size(), int64(len(writtenLog)+1))

// Launcher should delete file for this log
unwrittenLog := "sample log two"
integrationLogTwo := integrations.IntegrationLog{
Log: unwrittenLog,
IntegrationID: "sample_integration:123",
}
suite.s.receiveLogs(integrationLogTwo)

_, err = os.Stat(filepath)
assert.True(suite.T(), os.IsNotExist(err))

// Remake the file
suite.s.receiveLogs(integrationLog)
fileStat, err = os.Stat(filepath)
assert.Nil(suite.T(), err)
assert.Equal(suite.T(), fileStat.Size(), int64(len(writtenLog)+1))
}

func (suite *LauncherTestSuite) TestWriteLogToFile() {
logText := "hello world"
err := suite.s.writeLogToFileFunction(suite.testPath, logText)
Expand Down Expand Up @@ -156,14 +239,6 @@ func (suite *LauncherTestSuite) TestDeleteFile() {
assert.True(suite.T(), os.IsNotExist(err))
}

// TestNegativeCombinedUsageMax ensures errors in combinedUsageMax don't result
// in panics from `deleteFile`
func (suite *LauncherTestSuite) TestNegativeCombinedUsageMax() {
suite.s.combinedUsageMax = -1
err := suite.s.scanInitialFiles(suite.s.runPath)
assert.NotNil(suite.T(), err)
}

// TestIntegrationLogFilePath ensures the filepath for the logs files are correct
func (suite *LauncherTestSuite) TestIntegrationLogFilePath() {
id := "123456789"
Expand Down

0 comments on commit 306c6c8

Please sign in to comment.