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

chore: add telemetry realworld app #26896

Merged
merged 12 commits into from
Jun 1, 2023

Conversation

mjhenkes
Copy link
Member

  • Closes

Additional details

This PR enables telemetry for the test-binary-against-cypress-realworld-app job in CI. We will use this job as a performance baseline.

This change also exposed an issue when having telemetry enabled during a 'cypress verify' workflow. Since the project id and record key were not, nor would ever be specified the exporter would time out waiting to send the spans. Unfortunately that timeout is longer than the verify timeout and so verify would fail.

The fix is to fail sending spans fast if encryption is required and we will not get a record key or a project id.

Steps to test

How has the user experience changed?

PR Tasks

@cypress
Copy link

cypress bot commented May 31, 2023

5 flaky tests on run #46912 ↗︎

0 680 26 0 Flakiness 5

Details:

Merge branch 'develop' into matth/chore/add-telemetry-realworld-app
Project: cypress Commit: 6a8a5bb8b0
Status: Passed Duration: 13:32 💡
Started: Jun 1, 2023 4:05 PM Ended: Jun 1, 2023 4:18 PM
Flakiness  runner/reporter-ct-webpack.errors.cy.ts • 1 flaky test • app-e2e

View Output Video

Test Artifacts
Webpack - errors ui > cy.readFile Output Screenshots Video
Flakiness  runs.cy.ts • 1 flaky test • app-e2e

View Output Video

Test Artifacts
App: Runs > Runs - Create Project > when a project is created, injects new projectId into the config file, and sends expected UTM params Output Screenshots Video
Flakiness  specs_list_latest_runs.cy.ts • 1 flaky test • app-e2e

View Output Video

Test Artifacts
App/Cloud Integration - Latest runs and Average duration > when no runs are recorded > shows placeholders for all visible specs Output Screenshots Video
Flakiness  cypress-in-cypress-component.cy.ts • 1 flaky test • app-e2e

View Output Video

Test Artifacts
Cypress In Cypress CT > default config > redirects to the specs list with error if a spec is not found Output Screenshots Video
Flakiness  cypress-origin-communicator.cy.ts • 1 flaky test • app-e2e

View Output Video

Test Artifacts
Cypress In Cypress Origin Communicator > cy.origin passivity with app interactions > passes upon test reload mid test execution Output Screenshots Video

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

Comment on lines +57 to +58
if (this.requirementsToExport === 'unknown') {
this.requirementsToExport = 'unmet'
Copy link
Member

Choose a reason for hiding this comment

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

why do we need unknown and unmet? can we assume 'unmet' until met?

Copy link
Member

Choose a reason for hiding this comment

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

any reason this wasn't a bool?

Copy link
Member Author

Choose a reason for hiding this comment

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

when encryption is on, we can't assume unmet until we get a null project id or record key. The send function will behave differently. If unmet we error instantly, if unknown we will delay the request.

Copy link
Member Author

Choose a reason for hiding this comment

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

Otherwise we could incorrectly fail any spans sent prior to getting the project id/record key, since we capture spans prior to them being available.

Copy link
Member

@emilyrohrbough emilyrohrbough Jun 1, 2023

Choose a reason for hiding this comment

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

makes sense, maybe we throw a comment in there about this? unless I missed it per the limited diff view 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

sure

@@ -822,7 +822,7 @@ commands:
name: Run tests using browser "<<parameters.browser>>"
working_directory: /tmp/<<parameters.repo>>/<<parameters.folder>>
command: |
<<parameters.command>> -- --browser <<parameters.browser>>
<<parameters.command>> --browser <<parameters.browser>> --record false
Copy link
Member

Choose a reason for hiding this comment

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

was recording disabled to test this? or are you intentionally disabling?

Also not in this scope per-say, but do you know whats different between the test-binary-against-rwa and test-binary-against-repo jobs? test-binary-against-rwa seems like a copy paste?

Copy link
Member Author

Choose a reason for hiding this comment

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

Recording was never enabled, but i needed to override the project ID and record key to send telemetry. I disabled recording to silence a warning thrown when record is not enabled.

I don't know the different between test-binary-against-repo, I didn't notice they were the same.

Copy link
Member

Choose a reason for hiding this comment

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

ohh I see, passing CYPRESS_PROJECT_ID prob caused that error

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we no longer need the double -- since the command is executed with yarn?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, yarn kept yelling at me about it.

@mjhenkes mjhenkes requested a review from AtofStryker June 1, 2023 15:16
@@ -822,7 +822,7 @@ commands:
name: Run tests using browser "<<parameters.browser>>"
working_directory: /tmp/<<parameters.repo>>/<<parameters.folder>>
command: |
<<parameters.command>> -- --browser <<parameters.browser>>
<<parameters.command>> --browser <<parameters.browser>> --record false
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we no longer need the double -- since the command is executed with yarn?

@mjhenkes mjhenkes merged commit 3d1ec09 into develop Jun 1, 2023
@mjhenkes mjhenkes deleted the matth/chore/add-telemetry-realworld-app branch June 1, 2023 16:35
@cypress cypress bot mentioned this pull request Jun 1, 2023
3 tasks
mjhenkes added a commit that referenced this pull request Jun 2, 2023
* chore: add Nx Cloud (#26712)

* chore: add empty nx.json [run ci]

* chore: add nx cloud runner [run ci]

* chore: add nx-cloud dep [run ci]

* chore: update local nx cloud accessToken to be read-only

* feat: update git related messages for runs and debug (#26758)

* feat(app): update DebugError copy

* feat: set isUsingGit project flag and consume in DebugContainer

* feat(app): update not using git icon for DebugError

* feat(app): displays alert on runs page when not using git

* feat(app): add component for when no runs for current branch

* feat(app): add warning for no runs for branch on runs container

* chore: add feat to CHANGELOG

* chore: remove logged status value

* chore: resolve import from merge conflict

* test(app): stub branch name for e2e runs spec

* chore: add line break in changelog entry

* chore: cleanup PR

* chore: fix i18n import for DebugBranchError

* chore: add utm and update Warning links to inline

* chore: capitalize Git in i18n

* ref: warning liink

* test: add i18n to tests

* test(app): change utm_source assertions

* chore: cleanup pr

* chore: remove unused prop

* test(app): remove no git warning when moving to runs page in e2e

* chore: change template logic

* chore: remove duplicate RUNS_TAB_MEDIUM const

* Changed Debug test assertion and reordered new components for Debug

---------

Co-authored-by: Stokes Player <[email protected]>

* chore: rename video processing events to capture/compress (#26800)

* chore: change processing nomenclature to compressing when printing the run.

* chore: rename 'capturing of' to 'capturing'

* chore: rename upload results to upload screenshots & videos (#26811)

* chore: rename upload results to upload screenshots & videos

* run ci

* chore: capture versions of relevant dependencies with `x-dependencies` header (#26814)

* chore: update changlelog script to handle revert pr ref (#26801)

* fix: Correct typescript scaffold dependency (#26815)

* fix: correct typescript scaffold dependency (#26204)

* add changelog

* Update change log for PR comment

Co-authored-by: Mike Plummer <[email protected]>

---------

Co-authored-by: Mike Plummer <[email protected]>
Co-authored-by: Mark Noonan <[email protected]>

* chore: 12.13.0 prep (#26833)

* chore: 12.13.0 release (#26834)

* chore: 12.13.0 changelog fix

* remove pending, bump version

* fix typo

* chore: release @cypress/vite-plugin-cypress-esm-v1.0.1

[skip ci]

* chore: Implement runSpec mutation (#26782)

* chore: replace gitter badge with discord on readme (#26771)

* chore: add GraphQL mutation for sending system notifications via Electron (#26773)

* fix: upgrade typescript from 4.7.4 to 4.9.5 (#26826)

Snyk has created this PR to upgrade typescript from 4.7.4 to 4.9.5.

See this package in npm:


See this project in Snyk:
https://app.snyk.io/org/cypress-opensource/project/d5b36925-e6ee-455d-9649-6560a9aca413?utm_source=github&utm_medium=referral&page=upgrade-pr

* test: fix 2 broken tests for Windows (#26854)

* Update stale_issues_and_pr_cleanup.yml

Upped the number of operations per run.  Have been manually doing that so this job can get through all the issues in the repo with no problems.

* chore(dep): [Snyk] Upgrade vite from 2.9.13 to 2.9.15 (#26830)

Co-authored-by: Ben M <[email protected]>

* Update triage_add_to_project.yml

* chore: fix minor background color styling in debug results component (#26887)

* Update stale_issues_and_pr_cleanup.yml

stalebot was incorrectly configured to run in debug mode.  I have updated the default to run in normal mode when running scheduled

* chore: Deprecate @cypress/xpath package (#26893)

* chore: add telemetry realworld app (#26896)

* chore: capture telemetry for realworld app maybe

* idk what i was doing

* setup record key and telemetry

* testing

* override project id

* some times we just need a little context.

* Adding tests

* Adding comment

* fix yarn lock

* Trying this....

* fix tests?

---------

Co-authored-by: Jordan <[email protected]>
Co-authored-by: Stokes Player <[email protected]>
Co-authored-by: Bill Glesias <[email protected]>
Co-authored-by: Adam Stone-Lord <[email protected]>
Co-authored-by: Emily Rohrbough <[email protected]>
Co-authored-by: Dave Kasper <[email protected]>
Co-authored-by: Mike Plummer <[email protected]>
Co-authored-by: Mark Noonan <[email protected]>
Co-authored-by: Chris Breiding <[email protected]>
Co-authored-by: semantic-release-bot <[email protected]>
Co-authored-by: Ely Lucas <[email protected]>
Co-authored-by: Snyk bot <[email protected]>
Co-authored-by: Stokes Player <[email protected]>
Co-authored-by: Ben M <[email protected]>
Co-authored-by: Jennifer Shehane <[email protected]>
mjhenkes added a commit that referenced this pull request Jun 2, 2023
* chore: add Nx Cloud (#26712)

* chore: add empty nx.json [run ci]

* chore: add nx cloud runner [run ci]

* chore: add nx-cloud dep [run ci]

* chore: update local nx cloud accessToken to be read-only

* feat: update git related messages for runs and debug (#26758)

* feat(app): update DebugError copy

* feat: set isUsingGit project flag and consume in DebugContainer

* feat(app): update not using git icon for DebugError

* feat(app): displays alert on runs page when not using git

* feat(app): add component for when no runs for current branch

* feat(app): add warning for no runs for branch on runs container

* chore: add feat to CHANGELOG

* chore: remove logged status value

* chore: resolve import from merge conflict

* test(app): stub branch name for e2e runs spec

* chore: add line break in changelog entry

* chore: cleanup PR

* chore: fix i18n import for DebugBranchError

* chore: add utm and update Warning links to inline

* chore: capitalize Git in i18n

* ref: warning liink

* test: add i18n to tests

* test(app): change utm_source assertions

* chore: cleanup pr

* chore: remove unused prop

* test(app): remove no git warning when moving to runs page in e2e

* chore: change template logic

* chore: remove duplicate RUNS_TAB_MEDIUM const

* Changed Debug test assertion and reordered new components for Debug

---------

Co-authored-by: Stokes Player <[email protected]>

* chore: rename video processing events to capture/compress (#26800)

* chore: change processing nomenclature to compressing when printing the run.

* chore: rename 'capturing of' to 'capturing'

* chore: rename upload results to upload screenshots & videos (#26811)

* chore: rename upload results to upload screenshots & videos

* run ci

* chore: capture versions of relevant dependencies with `x-dependencies` header (#26814)

* chore: update changlelog script to handle revert pr ref (#26801)

* fix: Correct typescript scaffold dependency (#26815)

* fix: correct typescript scaffold dependency (#26204)

* add changelog

* Update change log for PR comment

Co-authored-by: Mike Plummer <[email protected]>

---------

Co-authored-by: Mike Plummer <[email protected]>
Co-authored-by: Mark Noonan <[email protected]>

* chore: 12.13.0 prep (#26833)

* chore: 12.13.0 release (#26834)

* chore: 12.13.0 changelog fix

* remove pending, bump version

* fix typo

* chore: release @cypress/vite-plugin-cypress-esm-v1.0.1

[skip ci]

* chore: Implement runSpec mutation (#26782)

* chore: replace gitter badge with discord on readme (#26771)

* chore: add GraphQL mutation for sending system notifications via Electron (#26773)

* fix: upgrade typescript from 4.7.4 to 4.9.5 (#26826)

Snyk has created this PR to upgrade typescript from 4.7.4 to 4.9.5.

See this package in npm:


See this project in Snyk:
https://app.snyk.io/org/cypress-opensource/project/d5b36925-e6ee-455d-9649-6560a9aca413?utm_source=github&utm_medium=referral&page=upgrade-pr

* test: fix 2 broken tests for Windows (#26854)

* Update stale_issues_and_pr_cleanup.yml

Upped the number of operations per run.  Have been manually doing that so this job can get through all the issues in the repo with no problems.

* chore(dep): [Snyk] Upgrade vite from 2.9.13 to 2.9.15 (#26830)

Co-authored-by: Ben M <[email protected]>

* Update triage_add_to_project.yml

* chore: fix minor background color styling in debug results component (#26887)

* Update stale_issues_and_pr_cleanup.yml

stalebot was incorrectly configured to run in debug mode.  I have updated the default to run in normal mode when running scheduled

* chore: Deprecate @cypress/xpath package (#26893)

* chore: add telemetry realworld app (#26896)

* chore: capture telemetry for realworld app maybe

* idk what i was doing

* setup record key and telemetry

* testing

* override project id

* some times we just need a little context.

* Adding tests

* Adding comment

* chore(deps): update dependency find-process to v1.4.7 🌟 (#26906)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Jennifer Shehane <[email protected]>

* chore(deps): update dependency firefox-profile to v4.3.2 🌟 (#26912)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Jennifer Shehane <[email protected]>

---------

Co-authored-by: Jordan <[email protected]>
Co-authored-by: Stokes Player <[email protected]>
Co-authored-by: Bill Glesias <[email protected]>
Co-authored-by: Adam Stone-Lord <[email protected]>
Co-authored-by: Emily Rohrbough <[email protected]>
Co-authored-by: Dave Kasper <[email protected]>
Co-authored-by: Mike Plummer <[email protected]>
Co-authored-by: Mark Noonan <[email protected]>
Co-authored-by: Chris Breiding <[email protected]>
Co-authored-by: semantic-release-bot <[email protected]>
Co-authored-by: Ely Lucas <[email protected]>
Co-authored-by: Snyk bot <[email protected]>
Co-authored-by: Stokes Player <[email protected]>
Co-authored-by: Ben M <[email protected]>
Co-authored-by: Jennifer Shehane <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jun 7, 2023

Released in 12.14.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v12.14.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Jun 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants