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

Add Stats to XML-RPC Errors #13335

Merged
merged 2 commits into from
Aug 27, 2019
Merged

Add Stats to XML-RPC Errors #13335

merged 2 commits into from
Aug 27, 2019

Conversation

mdawaffe
Copy link
Member

Changes proposed in this Pull Request:

  • Track unique XML-RPC authentication error codes in a new Jetpack Option.
  • Send those error codes as part of the heartbeat.

Sending a stat for each failed XML-RPC request might not scale for sites with too many errors. Sending a stat for each failed XML-RPC request might not scale for the stats system :)

Using the heartbeat makes it impossible to gauge how many XML-RPC errors occur, but it does allow us to determine how many sites experience XML-RPC errors, which is probably more useful for us.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

No: improved telemetry.

Testing instructions:

  1. Break your XML-RPC authentication :) For example, with the patch below.
  2. Do something that will generate XML-RPC requests to your site. (API Dev Console, Calypso, …)
  3. wp jetpack options get xmlrpc_errors - see the error code in the option
  4. wp jetpack options update last_heartbeat 0 - to trick Jetpack into running the heartbeat on command.
  5. wp cron event run jetpack_v2_heartbeat - to run the heartbeat on command.
  6. wp jetpack options get xmlrpc_errors - see the option has been removed.
  7. Wait a minute, then check that the stat has bumped. (If multiple people happen to be testing this around the same time, you may have to wait up to 5 minutes to see the stat bump results.)
diff --git a/packages/connection/src/Manager.php b/packages/connection/src/Manager.php
index b3d9b4ea9..dd7ea6eab 100644
--- a/packages/connection/src/Manager.php
+++ b/packages/connection/src/Manager.php
@@ -350,6 +350,7 @@ class Manager implements Manager_Interface {
 			$user_id = (int) $user_id;
 
 			$user = new \WP_User( $user_id );
+$user = false;
 			if ( ! $user || ! $user->exists() ) {
 				return new \WP_Error(
 					'unknown_user',

Proposed changelog entry for your changes:

  • Improved error reporting for sites having connection trouble.

Track unique error codes in `xmlrpc_errors` option.
@mdawaffe mdawaffe added the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label Aug 27, 2019
@mdawaffe mdawaffe added this to the 7.7 milestone Aug 27, 2019
@mdawaffe mdawaffe requested a review from a team August 27, 2019 04:47
@mdawaffe mdawaffe self-assigned this Aug 27, 2019
@jetpackbot
Copy link
Collaborator

Warnings
⚠️

pre-commit hook was skipped for one or more commits

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against a36cb4e

@jeherve jeherve added General [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Aug 27, 2019
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

This works well for me. 👍 Merging.

@jeherve jeherve merged commit 4e1a49a into master Aug 27, 2019
@jeherve jeherve deleted the add/stats-to-xmlrpc-errors branch August 27, 2019 08:13
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Aug 27, 2019
jeherve added a commit that referenced this pull request Aug 27, 2019
jeherve added a commit that referenced this pull request Aug 27, 2019
* 7.7 changelog: initial set of changes

* Changelog: add #13154

* Changelog: add #13134

* Changelog: add #12699 and many others

* Changelog: add #13127

* Changelog: add #13167

* Changelog: add #13225

* Changelog: add #13179

* Changelog: add #13173

* Changelog: add #13232

* Changelog: add #13137

* Changelog: add #13172

* Changelog: add #13182

* Changelog: add #13200

* Changelog: add #13244

* Changelog: add #13267

* Changelog: add #13204

* changelog: add #13205

* Changelog: add #13183

* Changelog: add #13278

* Changelog: add #13162

* Changelog: add #13268

* Changelog: add #13286

* Changelog: add #13273

* Changelog: add #12474

* Changelog: add #13085

* Changelog: add #13266

* Changelog: add #13306

* Changelog: add #13311

* Changelog: add #13302

* Changelog: add #13196

* Changelog: add #12733

* Changelog: add #13261

* Changelog: add #13322

* Changelog: add #13333

* Changelog: add #13335
@dereksmart
Copy link
Member

Beautiful, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants