Skip to content

Conversation

@mrodm
Copy link
Contributor

@mrodm mrodm commented Jun 7, 2023

Closes #1296

When running elastic-package along with docker-compose v1.* , the first line of the docker-compose logs command is always like: Attaching to <container>. For instance:

Attaching to elastic-package-stack_elastic-agent_1

This message is not actual log and elastic-package test system was failing due to that. Expected messages should be like:

elastic-package-stack-elastic-agent-1  | {"log.level":"info","@timestamp":"2023-06-07T13:27:48.548Z","message":"Non-zero metrics in the last 30s","component":{"binary":"metricbeat","dataset":"elastic_agent.metricbeat","id":"beat/metrics-monitoring","type":"beat/metrics"},"log":{"source":"beat/metrics-monitoring"},"log.logger":"monitoring","log.origin":{"file.line":187,"file.name":"log/log.go"},"service.name":"metricbeat",...,"ecs.version":"1.6.0"}

This PR changes this behaviour to avoid raising an error and instead it logs a debug message. Those log lines that are not log-like messages are skipped.

@mrodm mrodm self-assigned this Jun 7, 2023
@mrodm mrodm marked this pull request as ready for review June 7, 2023 14:20
@mrodm mrodm requested review from a team and efd6 June 7, 2023 14:21
Comment on lines 44 to +45
if len(messageSlice) != 2 {
return fmt.Errorf("malformed docker-compose log line")
logger.Debugf("skipped malformed docker-compose log line: %s", line)
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.

Copy link
Contributor

@efd6 efd6 left a comment

Choose a reason for hiding this comment

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

Thanks

@mrodm
Copy link
Contributor Author

mrodm commented Jun 8, 2023

/test

1 similar comment
@mrodm
Copy link
Contributor Author

mrodm commented Jun 8, 2023

/test

@mrodm
Copy link
Contributor Author

mrodm commented Jun 8, 2023

buildkite test this

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @mrodm

@mrodm mrodm merged commit a58c8ba into elastic:main Jun 8, 2023
@mrodm mrodm deleted the elastic_agent_logs_compose_v1 branch June 8, 2023 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

f1106b9b causes build failures due to valid log lines

5 participants