Skip to content

Skipped malformed docker-compose log lines #1297

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jun 8, 2023
Merged

Skipped malformed docker-compose log lines #1297

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b32a3e8
Skipped malformed docker-compose log lines
mrodm Jun 7, 2023
76a357e
Add more context to comment
mrodm Jun 8, 2023
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
8 changes: 6 additions & 2 deletions internal/stack/parselogs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ package stack
import (
"bufio"
"encoding/json"
"fmt"
"os"
"strings"
"time"

"github.com/elastic/elastic-package/internal/logger"
)

type ParseLogsOptions struct {
Expand Down Expand Up @@ -41,7 +42,8 @@ func ParseLogs(options ParseLogsOptions, process func(log LogLine) error) error
messageSlice := strings.SplitN(line, "|", 2)

if len(messageSlice) != 2 {
return fmt.Errorf("malformed docker-compose log line")
logger.Debugf("skipped malformed docker-compose log line: %s", line)
Comment on lines 44 to +45
Copy link
Member

Choose a reason for hiding this comment

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

Should we check if the malformed message is actually "Attaching to..." to avoid silently ignoring messages here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could be checked that to narrow the skipped messages.
So for any other unexpected message, should we return error as it was before? @jsoriano

Copy link
Contributor

Choose a reason for hiding this comment

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

What is the concern with missing unknown log format lines? ISTM that given that the vetting done later depends on knowing what we are looking for, unknown things will always be at risk of being missed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The messages that should be interesting for this purpose should be the ones with this format:

  • <container_name> | [<json> | string ]
    • Sometimes I've seen messages directly without being a JSON.
elastic-package-stack-elastic-agent-1  | {"log.level":"info", .. }
elastic-package-stack-elastic-agent-1  | Message

So, it looks reasonably to me skipping those other messages that do not have that format.

continue
}

messageLog := messageSlice[1]
Expand All @@ -54,6 +56,8 @@ func ParseLogs(options ParseLogsOptions, process func(log LogLine) error) error
}

// There could be valid messages with just plain text without timestamp
// and therefore not processed, cannot be ensured in which timestamp they
// were generated
if !startProcessing && log.Timestamp.Before(options.StartTime) {
continue
}
Expand Down