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

[Security Solution][Serverless] Logging - Fix explore test issue #195941

Merged
merged 12 commits into from
Oct 15, 2024

Conversation

dkirchan
Copy link
Contributor

Summary

This PR addresses two points:

  • First it introduces the project information logging (not any sensitive data like pwd etc) for better troubleshooting. This will allow teams to be able to get the project ID and the organisation ID in order to search for project logs etc in the overview consoles.
  • Explore test issue: A specific spec file was crashing causing Buildkite timeout which was taking 5 hours to be reached. Something seems to be going wrong with the order of the tests within the specific spec suite, specifically in CI. Potentially the configuration of the machines where the test run. After a lot of investigation the order is changed and the
    Copy value test was moved to the top of the spec file. This allows the proper execution of all the tests. Pending further investigation. Me and @MadameSheema tested this locally and it always passes without any issues.

Also I tried to increase the resources of the agent assigned in Buildkite to run the tests but this still does not seem to be resolving the issue.

@dkirchan dkirchan added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.16.0 labels Oct 11, 2024
@dkirchan dkirchan self-assigned this Oct 11, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

Copy link
Member

@MadameSheema MadameSheema left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

@dkirchan
Copy link
Contributor Author

/ci

Copy link
Contributor

@maximpn maximpn left a comment

Choose a reason for hiding this comment

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

LGTM

await cloudHandler.deleteSecurityProject(project.id, PROJECT_NAME);
} catch (error) {
// False positive
// eslint-disable-next-line require-atomic-updates
result = error;
failedSpecFilePaths.push(filePath);
log.error(`Something went wrong with the cypress execution: ${error}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Could we make error message clearer? Something like Cypress runner failed: ${error} looks better.

@dkirchan dkirchan requested a review from a team as a code owner October 14, 2024 14:22
@elasticmachine
Copy link
Contributor

elasticmachine commented Oct 14, 2024

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #6 / threatMatchRowRenderer #renderRow rendered when indicator matches are more than MAX rendered

Metrics [docs]

✅ unchanged

History

cc @dkirchan

@dkirchan dkirchan merged commit 90b4ba5 into main Oct 15, 2024
43 checks passed
@dkirchan dkirchan deleted the sec-eng-prod-project-log-mki branch October 15, 2024 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants