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

Adding required instrumentation #14

Merged
merged 14 commits into from
May 15, 2020
Merged

Adding required instrumentation #14

merged 14 commits into from
May 15, 2020

Conversation

nagpalkaran95
Copy link
Collaborator

@nagpalkaran95 nagpalkaran95 commented May 5, 2020

User facing changelog

  • Colorised logging
    • info will be green
    • error will be red

PR Tasks

  • Collect anonymized usage data including the command-line arguments
    used, system details and errors that users get so that we can improve the way
    users run their Cypress tests on BrowserStack.
  • Update package imports as per nodejs best practices
  • Migrate to winston logger from custom logger
  • Handle case when browserstack.json is an invalid json

}

function isUsageReportingEnabled() {
return process.env.DISABLE_USAGE_REPORTING;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's document how to opt-out of the usage reporting, please.

Also, it's worth mentioning what all will be collected, and everything is anonymous and cannot be used for identifying a run.

bin/helpers/usageReporting.js Outdated Show resolved Hide resolved
bin/helpers/logger.js Outdated Show resolved Hide resolved
@@ -11,5 +11,7 @@ config.rails_host = hosts[config.env].rails_host;
config.cypress_v1 = `${config.rails_host}/automate/cypress/v1`;
config.buildUrl = `${config.cypress_v1}/builds/`;
config.buildStopUrl = `${config.cypress_v1}/builds/stop/`;
config.usageReportingUrl = `http://127.0.0.1:8000/send_event_cy_internal`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

What http://127.0.0.1:8000 points to?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will point to the endpoint where data will be sent. Will update this before merging

bin/commands/info.js Outdated Show resolved Hide resolved
}

exports.getUserAgent = () => {
return `BStack-Cypress-CLI/1.x (${os.arch()}/${os.platform()}/${os.release()})`;
Copy link

Choose a reason for hiding this comment

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

@nagpalkaran95 Can we get the exact version of the CLI?

Like BStack-Cypress-CLI/<full_version> (${os.arch()}/${os.platform()}/${os.release()})?

@@ -1,6 +1,6 @@
{
"name": "browserstack-cypress-cli",
"version": "1.1.0",
"version": "1.1.3",
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is handled during the package publishing, not needed.

suryart
suryart previously approved these changes May 15, 2020
@suryart suryart merged commit 11a63ce into master May 15, 2020
@suryart suryart deleted the CYP_123_instrumentation branch May 15, 2020 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants