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][Endpoint] Cypress test improvements to capture Agent diagnostics file when test fails #202965

Merged
merged 17 commits into from
Dec 12, 2024

Conversation

paul-tavares
Copy link
Contributor

@paul-tavares paul-tavares commented Dec 4, 2024

Summary

  • the Cypress parallel runner was updated to set tooling logging level first from Env. variables before falling back to the value defined in the Cypress configuration file
    • The env. value to set, if wanting to enable a specific logging level, is TOOLING_LOG_LEVEL. The values supported are the same as those used with ToolingLog (here): silent, error, warning, success, info, debug, verbose
    • This change makes it easier to run Cypress tests locally with (for example) a logging level of verbose for our tooling without having to modify the Cypress configuration file. Example: export TOOLING_LOG_LEVEL=verbose && yarn cypress:dw:open
  • Added two new methods to our scripting VM service clients (for Vagrant and Multipass):
    • download: allow you to pull files out of the VM and save them locally
    • upload: uploads a local file to the VM. (upload already existed as transfer - which has now been marked as deprecated).
  • Added new service function on our Fleet scripting module to enable us to set the logging level on a Fleet Agent
    • Cypress tests were adjusted to automatically set the agent logging to debug when running in CI
  • A new Cypress task that allows for an Agent Diagnostic file (which includes the Endpoint Log) to be retrieved from the host VM and stored with the CI job (under the artifacts tab)
    • A few tests were updated to include this step for failed test

Screen capture showing Agent Diagnostics zip file captured for failing test:

image

@paul-tavares paul-tavares added release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) v8.18.0 labels Dec 4, 2024
@paul-tavares paul-tavares self-assigned this Dec 4, 2024
@paul-tavares
Copy link
Contributor Author

/ci

@paul-tavares
Copy link
Contributor Author

/ci

@paul-tavares paul-tavares marked this pull request as ready for review December 5, 2024 14:30
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-defend-workflows (Team:Defend Workflows)

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.

@paul-tavares Thanks for improving tests logging 🙏 I reviewed changed to x-pack/plugins/security_solution/scripts/run_cypress/parallel.ts and left comments.

@@ -54,8 +54,6 @@ export const cli = () => {
)
.boolean('inspect');

const USE_CHROME_BETA = process.env.USE_CHROME_BETA?.match(/(1|true)/i);
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't an ability to test in Chrome beta version required anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh.... no no. this is mistake. I did not meant to remove this. Will add it back. Thanks for catching it.

createToolingLogger.defaultLogLevel = cypressConfigFile.env.TOOLING_LOG_LEVEL;
// Adjust tooling log level based on the `TOOLING_LOG_LEVEL` property, which can be
// defined in the cypress config file or set in the `env`
{
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be useful to have the same functionality for Serverless i.e. the same changes are required in x-pack/plugins/security_solution/scripts/run_cypress/parallel_serverless.ts.

Have you considered creation a separate file/function like createLogger() reading env variable under the hood?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohhhh. Did not realize there was a serverless parallel runner. I will break this out into a utility and will call it from both parallel runners. Thanks for the feedback


if (logLevel) {
_cliLogger.info(`Setting tooling log level to [${logLevel}]`);
createToolingLogger.defaultLogLevel = logLevel;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it expected behavior to affect all the instances created below in the execution flow?

As a safer option it might better to create a ToolingLog instance and pass it to prefixedOutputLogger.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah... its expected. The goal is to ensure that all intances of ToolingLog (created by createToolingLogger()) uses by default the log level set via the env. variable or the cypress config file.

@paul-tavares paul-tavares requested a review from maximpn December 10, 2024 16:17
* It will first check the NodeJs `process.env` to see if an Environment Variable was set
* and then, if provided, it will use the value defined in the Cypress Config. file.
*/
export const setDefaultToolingLoggingLevel = (cypressConfigFile?: any) => {
Copy link
Contributor

@maximpn maximpn Dec 10, 2024

Choose a reason for hiding this comment

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

There is no point using any type here. cypressConfigFile in upstream parallel and parallel_serverless files already has type any. We could provide a more specific type for cypressConfigFile here like

interface CypressConfigFile {
  env?: Record<string, string>;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used any here because that matches the type I am seeing for the cypressConfigFile in the parallel modules:

image

And I set this type here to be any more specific I'll likely have to cast the value when I attempt to pass it to this function. I'll instead change the signature of the function to not require the entire config file and instead just pass in the value for TOOLING_LOG_LEVEL instead.

@paul-tavares paul-tavares requested a review from maximpn December 10, 2024 19:32
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id before after diff
securitySolution 560 561 +1

Total ESLint disabled count

id before after diff
securitySolution 644 645 +1

History

cc @paul-tavares

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.

@paul-tavares Thanks for addressing my comments 🙏

Changes to x-pack/plugins/security_solution/scripts/run_cypress/parallel.ts and x-pack/plugins/security_solution/scripts/run_cypress/parallel_serverless.ts LGTM

process.env.TOOLING_LOG_LEVEL ||
process.env.CYPRESS_TOOLING_LOG_LEVEL ||
defaultFallbackLoggingLevel ||
'';
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: It looks like empty string isn't necessary here since if condition below is gonna check for all "falsable" values.

@paul-tavares paul-tavares merged commit 2ab8a5c into elastic:main Dec 12, 2024
8 checks passed
@paul-tavares paul-tavares deleted the task/cypress-test-improvements branch December 12, 2024 15:21
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12299431737

@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.x Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 202965

Questions ?

Please refer to the Backport tool documentation

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Dec 13, 2024
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 202965 locally

1 similar comment
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 202965 locally

@paul-tavares
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

paul-tavares added a commit to paul-tavares/kibana that referenced this pull request Dec 16, 2024
…ent diagnostics file when test fails (elastic#202965)

## Summary

- the Cypress `parallel` runner was updated to set tooling logging level
first from Env. variables before falling back to the value defined in
the Cypress configuration file
- The env. value to set, if wanting to enable a specific logging level,
is `TOOLING_LOG_LEVEL`. The values supported are the same as those used
with `ToolingLog`
([here](https://github.com/elastic/kibana/blob/b6287708f687d4e3288851052c0c6ae4ade8ce60/packages/kbn-tooling-log/src/log_levels.ts#L10)):
`silent`, `error`, `warning`, `success`, `info`, `debug`, `verbose`
- This change makes it easier to run Cypress tests locally with (for
example) a logging level of `verbose` for our tooling without having to
modify the Cypress configuration file. Example: `export
TOOLING_LOG_LEVEL=verbose && yarn cypress:dw:open`
- Added two new methods to our scripting VM service clients (for Vagrant
and Multipass):
- `download`: allow you to pull files out of the VM and save them
locally
- `upload`: uploads a local file to the VM. (upload already existed as
`transfer` - which has now been marked as deprecated).
- Added new service function on our Fleet scripting module to enable us
to set the logging level on a Fleet Agent
- Cypress tests were adjusted to automatically set the agent logging to
debug when running in CI
- A new Cypress task that allows for an Agent Diagnostic file (which
includes the Endpoint Log) to be retrieved from the host VM and stored
with the CI job (under the artifacts tab)
    - A few tests were updated to include this step for failed test

(cherry picked from commit 2ab8a5c)

# Conflicts:
#	x-pack/plugins/security_solution/scripts/endpoint/common/fleet_services.ts
paul-tavares added a commit that referenced this pull request Dec 17, 2024
…ure Agent diagnostics file when test fails (#202965) (#204485)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Security Solution][Endpoint] Cypress test improvements to capture
Agent diagnostics file when test fails
(#202965)](#202965)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Paul
Tavares","email":"56442535+paul-tavares@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-12-12T15:21:23Z","message":"[Security
Solution][Endpoint] Cypress test improvements to capture Agent
diagnostics file when test fails (#202965)\n\n## Summary\r\n\r\n- the
Cypress `parallel` runner was updated to set tooling logging
level\r\nfirst from Env. variables before falling back to the value
defined in\r\nthe Cypress configuration file\r\n- The env. value to set,
if wanting to enable a specific logging level,\r\nis
`TOOLING_LOG_LEVEL`. The values supported are the same as those
used\r\nwith
`ToolingLog`\r\n([here](https://github.com/elastic/kibana/blob/b6287708f687d4e3288851052c0c6ae4ade8ce60/packages/kbn-tooling-log/src/log_levels.ts#L10)):\r\n`silent`,
`error`, `warning`, `success`, `info`, `debug`, `verbose`\r\n- This
change makes it easier to run Cypress tests locally with
(for\r\nexample) a logging level of `verbose` for our tooling without
having to\r\nmodify the Cypress configuration file. Example:
`export\r\nTOOLING_LOG_LEVEL=verbose && yarn cypress:dw:open`\r\n- Added
two new methods to our scripting VM service clients (for Vagrant\r\nand
Multipass):\r\n- `download`: allow you to pull files out of the VM and
save them\r\nlocally\r\n- `upload`: uploads a local file to the VM.
(upload already existed as\r\n`transfer` - which has now been marked as
deprecated).\r\n- Added new service function on our Fleet scripting
module to enable us\r\nto set the logging level on a Fleet Agent\r\n-
Cypress tests were adjusted to automatically set the agent logging
to\r\ndebug when running in CI\r\n- A new Cypress task that allows for
an Agent Diagnostic file (which\r\nincludes the Endpoint Log) to be
retrieved from the host VM and stored\r\nwith the CI job (under the
artifacts tab)\r\n - A few tests were updated to include this step for
failed
test","sha":"2ab8a5ced075da08f3aeb9526a76b60b1a964602","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport
missing","v9.0.0","Team:Defend
Workflows","backport:prev-minor","v8.18.0"],"number":202965,"url":"https://github.com/elastic/kibana/pull/202965","mergeCommit":{"message":"[Security
Solution][Endpoint] Cypress test improvements to capture Agent
diagnostics file when test fails (#202965)\n\n## Summary\r\n\r\n- the
Cypress `parallel` runner was updated to set tooling logging
level\r\nfirst from Env. variables before falling back to the value
defined in\r\nthe Cypress configuration file\r\n- The env. value to set,
if wanting to enable a specific logging level,\r\nis
`TOOLING_LOG_LEVEL`. The values supported are the same as those
used\r\nwith
`ToolingLog`\r\n([here](https://github.com/elastic/kibana/blob/b6287708f687d4e3288851052c0c6ae4ade8ce60/packages/kbn-tooling-log/src/log_levels.ts#L10)):\r\n`silent`,
`error`, `warning`, `success`, `info`, `debug`, `verbose`\r\n- This
change makes it easier to run Cypress tests locally with
(for\r\nexample) a logging level of `verbose` for our tooling without
having to\r\nmodify the Cypress configuration file. Example:
`export\r\nTOOLING_LOG_LEVEL=verbose && yarn cypress:dw:open`\r\n- Added
two new methods to our scripting VM service clients (for Vagrant\r\nand
Multipass):\r\n- `download`: allow you to pull files out of the VM and
save them\r\nlocally\r\n- `upload`: uploads a local file to the VM.
(upload already existed as\r\n`transfer` - which has now been marked as
deprecated).\r\n- Added new service function on our Fleet scripting
module to enable us\r\nto set the logging level on a Fleet Agent\r\n-
Cypress tests were adjusted to automatically set the agent logging
to\r\ndebug when running in CI\r\n- A new Cypress task that allows for
an Agent Diagnostic file (which\r\nincludes the Endpoint Log) to be
retrieved from the host VM and stored\r\nwith the CI job (under the
artifacts tab)\r\n - A few tests were updated to include this step for
failed
test","sha":"2ab8a5ced075da08f3aeb9526a76b60b1a964602"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/202965","number":202965,"mergeCommit":{"message":"[Security
Solution][Endpoint] Cypress test improvements to capture Agent
diagnostics file when test fails (#202965)\n\n## Summary\r\n\r\n- the
Cypress `parallel` runner was updated to set tooling logging
level\r\nfirst from Env. variables before falling back to the value
defined in\r\nthe Cypress configuration file\r\n- The env. value to set,
if wanting to enable a specific logging level,\r\nis
`TOOLING_LOG_LEVEL`. The values supported are the same as those
used\r\nwith
`ToolingLog`\r\n([here](https://github.com/elastic/kibana/blob/b6287708f687d4e3288851052c0c6ae4ade8ce60/packages/kbn-tooling-log/src/log_levels.ts#L10)):\r\n`silent`,
`error`, `warning`, `success`, `info`, `debug`, `verbose`\r\n- This
change makes it easier to run Cypress tests locally with
(for\r\nexample) a logging level of `verbose` for our tooling without
having to\r\nmodify the Cypress configuration file. Example:
`export\r\nTOOLING_LOG_LEVEL=verbose && yarn cypress:dw:open`\r\n- Added
two new methods to our scripting VM service clients (for Vagrant\r\nand
Multipass):\r\n- `download`: allow you to pull files out of the VM and
save them\r\nlocally\r\n- `upload`: uploads a local file to the VM.
(upload already existed as\r\n`transfer` - which has now been marked as
deprecated).\r\n- Added new service function on our Fleet scripting
module to enable us\r\nto set the logging level on a Fleet Agent\r\n-
Cypress tests were adjusted to automatically set the agent logging
to\r\ndebug when running in CI\r\n- A new Cypress task that allows for
an Agent Diagnostic file (which\r\nincludes the Endpoint Log) to be
retrieved from the host VM and stored\r\nwith the CI job (under the
artifacts tab)\r\n - A few tests were updated to include this step for
failed
test","sha":"2ab8a5ced075da08f3aeb9526a76b60b1a964602"}},{"branch":"8.x","label":"v8.18.0","labelRegex":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Dec 17, 2024
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Jan 13, 2025
…ent diagnostics file when test fails (elastic#202965)

## Summary

- the Cypress `parallel` runner was updated to set tooling logging level
first from Env. variables before falling back to the value defined in
the Cypress configuration file
- The env. value to set, if wanting to enable a specific logging level,
is `TOOLING_LOG_LEVEL`. The values supported are the same as those used
with `ToolingLog`
([here](https://github.com/elastic/kibana/blob/b6287708f687d4e3288851052c0c6ae4ade8ce60/packages/kbn-tooling-log/src/log_levels.ts#L10)):
`silent`, `error`, `warning`, `success`, `info`, `debug`, `verbose`
- This change makes it easier to run Cypress tests locally with (for
example) a logging level of `verbose` for our tooling without having to
modify the Cypress configuration file. Example: `export
TOOLING_LOG_LEVEL=verbose && yarn cypress:dw:open`
- Added two new methods to our scripting VM service clients (for Vagrant
and Multipass):
- `download`: allow you to pull files out of the VM and save them
locally
- `upload`: uploads a local file to the VM. (upload already existed as
`transfer` - which has now been marked as deprecated).
- Added new service function on our Fleet scripting module to enable us
to set the logging level on a Fleet Agent
- Cypress tests were adjusted to automatically set the agent logging to
debug when running in CI
- A new Cypress task that allows for an Agent Diagnostic file (which
includes the Endpoint Log) to be retrieved from the host VM and stored
with the CI job (under the artifacts tab)
    - A few tests were updated to include this step for failed test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants