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 open telemetry to cypress to allow us to monitor the performance of the app overtime #26305

Merged
merged 96 commits into from
Apr 9, 2023

Conversation

mjhenkes
Copy link
Member

@mjhenkes mjhenkes commented Mar 30, 2023

Additional details

Telemetry in cypress is off by default. To enable telemetry in cypress set CYPRESS_INTERNAL_ENABLE_TELEMETRY="true".

Telemetry data is sent to the cloud /telemetry endpoint.

For the cypress cloud project only we forward the telemetry data to honeycomb. For all other projects telemetry data is currently not stored.

Staging: https://ui.honeycomb.io/cypress/environments/cypress-app-staging/datasets/cypress-app/home

Prod: https://ui.honeycomb.io/cypress/environments/cypress-app/datasets/cypress-app/home

Steps to test

How has the user experience changed?

PR Tasks

mjhenkes and others added 30 commits February 15, 2023 14:00
…S code for always inserting the wrong dependency
@AtofStryker AtofStryker self-requested a review April 5, 2023 20:50
packages/server/lib/modes/index.ts Outdated Show resolved Hide resolved
packages/server/lib/plugins/child/ts_node.js Outdated Show resolved Hide resolved
packages/server/lib/plugins/child/ts_node.js Outdated Show resolved Hide resolved
packages/server/lib/socket-base.ts Outdated Show resolved Hide resolved
packages/telemetry/README.md Show resolved Hide resolved
@AtofStryker AtofStryker self-requested a review April 7, 2023 14:16
Copy link
Contributor

@AtofStryker AtofStryker left a comment

Choose a reason for hiding this comment

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

:shipit: nice work on this!

packages/data-context/src/data/ProjectConfigIpc.ts Outdated Show resolved Hide resolved
Comment on lines 383 to 393
try {
await this.initializeConfig()

span?.end()

return true
} catch (error) {
span?.end()

return false
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
try {
await this.initializeConfig()
span?.end()
return true
} catch (error) {
span?.end()
return false
}
try {
await this.initializeConfig()
return true
} catch (error) {
return false
} finally {
span?.end()
}

Copy link
Contributor

Choose a reason for hiding this comment

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

the return statements I don't think get executed if you have a finally

Copy link
Member Author

Choose a reason for hiding this comment

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

finallys have burned me in the past 🧯

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't make this change

Copy link
Contributor

Choose a reason for hiding this comment

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

I tried this and it seemed to work? false is returned and the log message is executed. Not sure if that happens in all cases though. I just gave it a run in the browser 🤷🏻

(() => {
  try {
      throw 'a'
      return true
  } catch(e){
      return false
  } finally {
      console.log('foo')
  }
})()

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be curious when a finally doesn't execute (other than the process terminating).

packages/driver/cypress/e2e/commands/screenshot.cy.js Outdated Show resolved Hide resolved
packages/server/index.js Outdated Show resolved Hide resolved
packages/server/start-cypress.js Outdated Show resolved Hide resolved
@mjhenkes mjhenkes merged commit 62f58e0 into develop Apr 9, 2023
@mjhenkes mjhenkes deleted the matth/misc/telemetry branch April 9, 2023 02:18
astone123 pushed a commit to kgroat/cypress that referenced this pull request Apr 19, 2023
…rmance of the app overtime (cypress-io#26305)

* initial commit, a kinda working prototype

* Ready to test in CI

* "SyntaxError: Cannot use import statement outside a module" I blame VS code for always inserting the wrong dependency

* try using built js instead of the ts file

* typescript fixes?

* get version correctly, don't use optional chaining in child process.

* trying this, idk

* try running telemetry for driver-integration-tests-chrome

* fix missing spans, add more attributes for some spans

* fix missing spans and add suite spans

* types

* Remove un-used require

* remove spans for describe blocks in favor of the full title for tests

* migrate to sync resource discovery, start new custom exporters for spans

* encrypted

* localhost

* don't do things on child process

* latest changes

* update server start span time / add v8 snapshot span & update command span names

* prepare for sync

* don't send blank key

* make telemetry work again for sending directly to honeycomb

* web-socket exporter

* Add in IPC exporter and message the child process before disconnecting

* Use the cloud api by default

* move cloud span exporter into telemetry package

* shutdown fixes

* fix enabled

* improve types

* run in ci

* yml is the worst

* type!

* add spans for timing insights for visible areas of improvement

* type errors

* lets try sending data to staging

* types

* types again

* remove problematic attributes

* clean up exporters

* i like this better even though it doesn't seem to matter much

* some self review cleanup

* Update comment

* add debug messages

* mocha tests

* actually exit with the right code... oops

* simple mistake... have to look into how to fix with ts...

* try this i guess

* don't return undefined

* read me diagram

* color?

* no rect

* moar diagram

* docs!

* formatting

* build more binaries

* Supposedly fix cypress in cypress test failures

* test 'fixes'

* try this instead

* do not transpile cypress packages dir

* lets try escaping our regex string

* Add some diagnostics to help test the built binary....

* try a more complex solution

* fix tests probably

* just ignore the specific file

* fix unit tests

* cr updates

* Apply suggestions from code review

Co-authored-by: Matt Schile <mschile@cypress.io>
Co-authored-by: Bill Glesias <bglesias@gmail.com>

* Pr updates

* don't change the command queue

* move encoding and decoding telemetry context for ipc to the telemetry package

* build darn it

* plead for mercy from the testing gods, i merely wished to have named test reports, but clearly i have overreached.

* pr updates, send record key

* pr review

* optional chaining fails tests

* Apply suggestions from code review

Co-authored-by: Bill Glesias <bglesias@gmail.com>

* tests for cloud-span-exporter

* bad merge

* adding tests for the remaining exporters

* note

* docs

* Correctly set test under the current run span for component testing

* gate sending the message.

* pr updates

* finally, fingers crossed.

---------

Co-authored-by: Emily Rohrbough <emilyrohrbough@yahoo.com>
Co-authored-by: Matt Schile <mschile@cypress.io>
Co-authored-by: Bill Glesias <bglesias@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants