Allow pushing single paths only [+ fix flaky tests] #526

lucasfcosta · 2022-06-09T17:06:57Z

Summary

This PR solves #523 by allowing users to only upload the current path of monitors.

Because now users could only point to a single target file, I made sure to implement the --match and --tags flags so that users could filter journeys. Otherwise, the ergonomics of the command could become inconvenient.

Implementation Details

The implementation of the changes described above was quite simple: I basically pass the path to the CWD when we require the journeys so that we can still reuse the same code we had before to load them.

Then, I went on and used the same matching function to make sure we'd only bundle monitors matching the tags or names specified.

How to test this PR

Ensure you have the latest version of synthetics
Generate a new project with init
cd into the new project's folder (IMPORTANT!)
Push its monitors without passing a path

Please also try to use --match and --tags to filter different journeys that you upload.

Flaky tests

The flaky tests were a funny one.

The reason they were flaky is that one of the tests would sometime take longer to return the output, and, therefore, by the time it returned it, there was more than one event in it.

  {"type":"journey/start","@timestamp":1654791216548408.8,"journey":{"name":"inline","id":"inline"},"root_fields":{"os":{"platform":"darwin"},"package":{"name":"@elastic/synthetics","version":"1.0.0-beta.27"}},"payload":{"params":{"url":"non-dev"},"source":"({ page, context, browser, params }) => {\n        scriptFn.apply(null, [core_1.step, page, context, browser, params, expect_1.expect]);\n    }"},"package_version":"1.0.0-beta.27"}
    {"type":"step/end","@timestamp":1654791216551033.8,"journey":{"name":"inline","id":"inline"},"step":{"name":"fake step","index":1,"status":"succeeded","duration":{"us":599}},"root_fields":{"os":{"platform":"darwin"},"package":{"name":"@elastic/synthetics","version":"1.0.0-beta.27"}},"payload":{"source":"async () => {}","url":"about:blank","status":"succeeded"},"url":"about:blank","package_version":"1.0.0-beta.27"}
    {"type":"journey/end","@timestamp":1654791216552023.8,"journey":{"name":"inline","id":"inline","status":"succeeded"},"root_fields":{"os":{"platform":"darwin"},"package":{"name":"@elastic/synthetics","version":"1.0.0-beta.27"}},"payload":{"start":288041.905244907,"end":288041.909911118,"status":"succeeded"},"package_version":"1.0.0-beta.27"}

As you can see above, one of these events is opening a blank page.

So what was happening was: sometimes there would be enough time to open a blank page and return that in the output, and sometimes there wouldn't be, in which case we'd get a single line:

  {"type":"journey/start","@timestamp":1654791216548408.8,"journey":{"name":"inline","id":"inline"},"root_fields":{"os":{"platform":"darwin"},"package":{"name":"@elastic/synthetics","version":"1.0.0-beta.27"}},"payload":{"params":{"url":"non-dev"},"source":"({ page, context, browser, params }) => {\n        scriptFn.apply(null, [core_1.step, page, context, browser, params, expect_1.expect]);\n    }"},"package_version":"1.0.0-beta.27"}

Now, JSON.parse could parse a single line, but not multiple ones. Therefore, we'd see an error.

Then, because cleanup (the assignment to NODE_ENV and its restoration) was being done within the test, it was skipped when the test failed, thus causing other tests to fail.

I fixed that by parsing the output line-by-line and picking the correct event + moving cleanup to the top of the file on a hook.

apmmachine · 2022-06-09T17:15:11Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2022-06-13T10:22:01.931+0000
Duration: 14 min 12 sec

Test stats 🧪

Test	Results
Failed	0
Passed	166
Skipped	2
Total	168

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.

vigneshshanmugam · 2022-06-09T17:25:06Z

__tests__/generator/index.test.ts

@@ -42,6 +42,9 @@ describe('Generator', () => {
    });
  });
  it('generate synthetics project - NPM', async () => {
+    // In a few development environments, this test may take
+    // a few millisseconds more than the default 15000ms timeout
+    jest.setTimeout(30000);


Weird why this timeouts even after pw download is skipped. Its already this way -

synthetics/__tests__/generator/index.test.ts

Line 89 in de8feee

}, 30000);

Maybe add retries for this particular test?

vigneshshanmugam

Great catch on the NODE_ENV stuff.

vigneshshanmugam · 2022-06-09T17:28:09Z

src/cli.ts

  .description(
-    'Push journeys to create montors within Kibana monitor management UI'
+    'Push journeys in the current directory to create montors within the Kibana monitor management UI'


typo: monitors

vigneshshanmugam · 2022-06-09T17:31:25Z

src/cli.ts

-    'Push journeys to create montors within Kibana monitor management UI'
+    'Push journeys in the current directory to create montors within the Kibana monitor management UI'
+  )
+  .option(


Not sure why we need these options here? Subcommands should be able to access these pattern, tags, match without any problem IIRC.

If you don't add these options here they don't appear in the --help info for the command.

See below, for example, what happens if I remove pattern and tags, for example:

➜ node ./dist/cli.js push --help Usage: npx @elastic/synthetics push [options] Push journeys in the current directory to create monitors within the Kibana monitor management UI Options: --match <name> push only journeys with a name or tag that matches the glob --schedule <time-in-minutes> schedule in minutes for the pushed monitors. Setting `10`, for example, configures monitors which don't have an interval defined to run every 10 minutes. --locations <locations...> default list of locations from which your monitors will run. (choices: "japan", "india", "singapore", "australia_east", "united_kingdom", "germany", "canada_east", "brazil", "us_east", "us_west") --project <project-id> id that will be used for logically grouping monitors --url <url> kibana URL to upload the monitors --auth <auth> API key used for Kibana authentication(https://www.elastic.co/guide/en/kibana/master/api-keys.html). --space <space> the target Kibana spaces for the pushed monitors — spaces help you organise pushed monitors. (default: "default") --delete automatically delete the stale monitors. -h, --help display help for command

Now, if I keep pattern and tags in, they appear in push again:

➜ node ./dist/cli.js push --help npm run build Usage: npx @elastic/synthetics push [options] Push journeys in the current directory to create monitors within the Kibana monitor management UI Options: --pattern <pattern> RegExp file patterns to push inside current directory --tags <name...> push only journeys with a tag that matches the glob --match <name> push only journeys with a name or tag that matches the glob --schedule <time-in-minutes> schedule in minutes for the pushed monitors. Setting `10`, for example, configures monitors which don't have an interval defined to run every 10 minutes. --locations <locations...> default list of locations from which your monitors will run. (choices: "japan", "india", "singapore", "australia_east", "united_kingdom", "germany", "canada_east", "brazil", "us_east", "us_west") --project <project-id> id that will be used for logically grouping monitors --url <url> kibana URL to upload the monitors --auth <auth> API key used for Kibana authentication(https://www.elastic.co/guide/en/kibana/master/api-keys.html). --space <space> the target Kibana spaces for the pushed monitors — spaces help you organise pushed monitors. (default: "default") --delete automatically delete the stale monitors. -h, --help display help for command

…ate-self-cert

andrewvc · 2022-06-09T23:06:08Z

I've merged in some changes that fix the remaining failures. ae66e63 solved a lot of the problems, with 989cd7d solving the rest. The cert stuff in retrospect was a red herring but it doesn't hurt to bump the cert

lucasfcosta

@andrewvc Just looked through the changes you added, thanks for those! They LGTM.

@vigneshshanmugam also fixed the typo and responded to your comment about the options.

I'll go ahead and merge this one once I get an approval from one of you if that's okay.

Please feel free to just give this a 👍 when you think we can merge it.

andrewvc · 2022-06-13T13:14:15Z

I'm ++ on merging this ASAP!

make push support a single path

de8feee

fix flaky tests

b8403d4

lucasfcosta force-pushed the push-single-paths branch from 09a409b to b8403d4 Compare June 9, 2022 17:19

vigneshshanmugam reviewed Jun 9, 2022

View reviewed changes

andrewvc added 4 commits June 9, 2022 13:38

Update self signed cert

8cacba7

Better JSON parsing

ae66e63

Merge remote-tracking branch 'lucasfcosta/push-single-paths' into upd…

ba5b0cb

…ate-self-cert

Test improvements

989cd7d

andrewvc mentioned this pull request Jun 10, 2022

Update self signed cert #527

Closed

fix monitor typo

ed27648

lucasfcosta commented Jun 13, 2022

View reviewed changes

lucasfcosta merged commit 9a71138 into elastic:main Jun 13, 2022

lucasfcosta deleted the push-single-paths branch June 13, 2022 13:42

lucasfcosta restored the push-single-paths branch June 13, 2022 13:42

lucasfcosta deleted the push-single-paths branch June 13, 2022 13:42

lucasfcosta restored the push-single-paths branch June 13, 2022 13:42

lucasfcosta mentioned this pull request Jun 13, 2022

Add advanced examples, github, README to init #525

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow pushing single paths only [+ fix flaky tests] #526

Allow pushing single paths only [+ fix flaky tests] #526

lucasfcosta commented Jun 9, 2022 •

edited

apmmachine commented Jun 9, 2022 •

edited

Build stats

Test stats 🧪

vigneshshanmugam Jun 9, 2022 •

edited

vigneshshanmugam left a comment

vigneshshanmugam Jun 9, 2022

vigneshshanmugam Jun 9, 2022

lucasfcosta Jun 13, 2022

andrewvc commented Jun 9, 2022 •

edited

lucasfcosta left a comment

andrewvc commented Jun 13, 2022

Allow pushing single paths only [+ fix flaky tests] #526

Allow pushing single paths only [+ fix flaky tests] #526

Conversation

lucasfcosta commented Jun 9, 2022 • edited

Summary

Implementation Details

How to test this PR

Flaky tests

apmmachine commented Jun 9, 2022 • edited

💚 Build Succeeded

Build stats

Test stats 🧪

🤖 GitHub comments

vigneshshanmugam Jun 9, 2022 • edited

Choose a reason for hiding this comment

vigneshshanmugam left a comment

Choose a reason for hiding this comment

vigneshshanmugam Jun 9, 2022

Choose a reason for hiding this comment

vigneshshanmugam Jun 9, 2022

Choose a reason for hiding this comment

lucasfcosta Jun 13, 2022

Choose a reason for hiding this comment

andrewvc commented Jun 9, 2022 • edited

lucasfcosta left a comment

Choose a reason for hiding this comment

andrewvc commented Jun 13, 2022

lucasfcosta commented Jun 9, 2022 •

edited

apmmachine commented Jun 9, 2022 •

edited

vigneshshanmugam Jun 9, 2022 •

edited

andrewvc commented Jun 9, 2022 •

edited