Skip to content
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.

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

Restore the test assertion that agent monitoring logs should not contain errors #3562

Closed
cmacknz opened this issue Oct 6, 2023 · 1 comment · Fixed by #3616
Closed

Restore the test assertion that agent monitoring logs should not contain errors #3562

cmacknz opened this issue Oct 6, 2023 · 1 comment · Fixed by #3616
Assignees
Labels
Team:Elastic-Agent Label for the Agent team

Comments

@cmacknz
Copy link
Member

cmacknz commented Oct 6, 2023

We have a test to ensure that agent monitoring logs are shipped to Fleet:

func TestMonitoringLogsShipped(t *testing.T) {

This test used to contain an assertion that the monitoring logs did not contain any error level logs. This was added because there are several problems that can hide in the logs due to the fact that failing processes will automatically be restarted by the agent. Panics in inputs and processors are recent example.

This test was removed because it was causing the test to be flaky, since there are several error level log messages that can occur as part of normal operation (retries connecting to Fleet or Elasticsearch for example). Rather than eliminating this check completely, we should add the ability to whitelist these expected errors or convert them to the warning level if they are truly expected.

Having a test looking for unexpected errors in our logs is extremely valuable and would have caught bugs that were released in the past. We should eventually expand this to all of our tests but we can start by introducing it in a single test.

The function to check for errors in the monitoring logs in Elasticsearch still exists:

// CheckForErrorsInLogs checks to see if any error-level lines exist
// excludeStrings can be used to remove any particular error strings from logs
func CheckForErrorsInLogs(client elastictransport.Interface, namespace string, excludeStrings []string) (Documents, error) {
return CheckForErrorsInLogsWithContext(context.Background(), client, namespace, excludeStrings)
}

It was removed in 908c912 which shows how it was called:

t.Log("Making sure there are no error logs")
	docs = findESDocs(t, func() (tools.Documents, error) {
		return tools.CheckForErrorsInLogs(info.ESClient, info.Namespace, []string{})
	})
	t.Logf("errors: Got %d documents", len(docs.Hits.Hits))
	for _, doc := range docs.Hits.Hits {
		t.Logf("%#v", doc.Source)
	}
@cmacknz cmacknz added the Team:Elastic-Agent Label for the Agent team label Oct 6, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/elastic-agent (Team:Elastic-Agent)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Elastic-Agent Label for the Agent team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants