Use new heartbeat package #16285

leogermani · 2020-06-25T14:33:26Z

This PR makes jetpack uses the newly created Heartbeat package to handle the Heartbeat. It also creates a new CLI command to inspect the heartbeat

The approach here was to touch as little as possible the current heartbeat class, so everything still works as usual. Jetpack_Heartbeat class is now acting as a proxy class to the package, so you still can initialize and call its methods.

Important - There are some uses of the generate_stats_array method that are not related to the heartbeat itself. That's why we kept it as is and static. It's used, for example, by wp jetpack status CLI command.

That's why these stats are always returned by the static method. On the other hand, they are only added to the heartbeat stats batch if the connection is active. (Originally, the whole heartbeat was only initialized if the connection was active)

This PR depends on the new Heartbeat package -> #16276

Changes proposed in this Pull Request:

Uses Heartbeat package to handle heartbeat

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

1146297891914257-as-1146297891914257
p9dueE-1tz-p2

Does this pull request change what data or activity we track or use?

no

Testing instructions:

Nothing should change and heartbeat should work as usual, you have to manually debug the heartbeat to see if it's still working by adding debug functions in its cron_execmethod.

Here's how to manually trigger the heart beat:

wp jetpack options update last_heartbeat 0 - to trick Jetpack into running the heartbeat on command.
wp cron event run jetpack_v2_heartbeat - to run the heartbeat on command.
Test the WP CLI command: wp jetpack-heartbeat and confirm it outputs all the blog details that are sent via heartbeat

matticbot · 2020-06-25T14:33:32Z

Caution: This PR has changes that must be merged to WordPress.com
Hello leogermani! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D45492-code before merging this PR. Thank you!
This revision will be updated with each commit to this PR

sergeymitr

Everything looks good and works fine as far as I can tell.

packages/heartbeat/src/class-heartbeat.php

jetpackbot · 2020-07-30T20:16:04Z

	Warnings
⚠️	"Proposed changelog entry" is missing for this PR. Please include any meaningful changes

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 🤖

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-16285

Generated by 🚫 dangerJS against 1addc07

sergeymitr

LGTM 🙂

sergeymitr · 2020-07-31T15:51:21Z

It looks like more reviews are needed, so I'm keeping the "Needs Review" label.

jeherve

I have some questions as I am not sure I understand the inner workings here.

class.jetpack.php

class.jetpack-heartbeat.php

Co-authored-by: Jeremy Herve <[email protected]>

leogermani · 2020-08-04T15:23:50Z

@jeherve As I feared tests have broken after adding the deprecated notices...

I took a path of letting the class fully backward compatible and not follow the rabbit hole of replacing all calls it... What do you think? Should we remove the notices or is it better to clean all the usage of these methods?

jeherve · 2020-08-10T13:22:16Z

What do you think? Should we remove the notices or is it better to clean all the usage of these methods?

Maybe now is a good time to clean things up then?

leogermani · 2020-08-10T17:29:04Z

@jeherve Fixed tests. Check 47dac49 and 1addc07

I think we can do some further cleaning, for example making sync use the new class directly and drop the usage of deprecated code, but I'd rather do this in a separated PR. I've created an issue for that: #16789

leogermani · 2020-08-18T13:20:35Z

@jeherve quick ping to move this forward

jeherve

Looking good now 🚢

* master: (23 commits) Premium Blocks: set blocks availability (#16898) Compat Package: Fix method declaration compatibility (#16900) Jetpack Dashboard: More meaningful error notices. (#16883) Connection REST API: Unit test for the `remote_authorize` request. (#16879) use blog token to request jetpack.updateBlog (#16698) Improve Story block media loading (#16663) Simplify error notices for broken connections (#16655) Use new heartbeat package (#16285) wrap-paid-block: remove component. deprecated. (#16895) Social Previews: improve preview description handling (#16889) Stats module use blog token (#16727) Form Block: add a new Consent Field, a new Newsletter setting, and a new newsletter variation (#16808) AAG: Backup card, fall back to VP content in case of /rewind API error. (#16867) Donations: Fix dependencies (#16892) Creative Mail: update option to lowercase (#16861) Premium Blocks: Implement the new design (#16611) Requests to Stats CSV use the blog token (#16716) Update spacing around sharing buttons to avoid no bottom margin below the customize link. (#16811) Jetpack SSO: Cleaning up the `requestNonce` API request. (#16830) Donations: Update plans when currency changes (#16844) ...

* add heartbeat as a dependecy for jetpack * Replace internal calls to use Heartbeat package * do not deactivate the heartbeat upon disconnect We are only adding jetpack stats to the heartbeat when jetpack is connected We dont need, and dont want, to disable all heartbeat when disconnecting since that may affect other plugins/packages using Heartbeat * adds heartbeat wp cli command * bump version in docblocks * Update class.jetpack-heartbeat.php Co-authored-by: Jeremy Herve <[email protected]> * add deprecation notices * Fix test Jetpack_Heartbeat class * add deprecated notice to cron_exec Co-authored-by: Jeremy Herve <[email protected]>

leogermani added the [Status] Needs Review This PR is ready for review. label Jun 25, 2020

leogermani self-assigned this Jun 25, 2020

matticbot added the Touches WP.com Files label Jun 25, 2020

This was referenced Jun 25, 2020

Add/use heartbeat package #16012

Closed

Creates a new Heartbeat package #16276

Merged

jeherve added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Focus] Jetpack DNA labels Jun 25, 2020

leogermani force-pushed the add/new-heartbeat-package branch from 1678866 to 2451002 Compare July 20, 2020 21:24

github-actions bot added the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Jul 20, 2020

leogermani force-pushed the use-new-heartbeat-package branch from 9474367 to 9fb747b Compare July 21, 2020 17:52

leogermani requested review from a team, georgestephanis and kraftbj as code owners July 21, 2020 17:52

leogermani force-pushed the add/new-heartbeat-package branch 2 times, most recently from 2d3f84b to de4253b Compare July 21, 2020 18:58

leogermani force-pushed the use-new-heartbeat-package branch from c5a89bc to 40c098c Compare July 21, 2020 19:13

leogermani mentioned this pull request Jul 23, 2020

Tracks missing primary user via heartbeat #16565

Closed

leogermani force-pushed the use-new-heartbeat-package branch from f7fab8d to 0d4ba32 Compare July 27, 2020 14:42

leogermani force-pushed the add/new-heartbeat-package branch from e0b6f19 to bf97efb Compare July 27, 2020 15:00

leogermani removed the [Status] Needs Review This PR is ready for review. label Jul 27, 2020

leogermani force-pushed the use-new-heartbeat-package branch from 0d4ba32 to 82dc320 Compare July 27, 2020 16:09

leogermani added the [Status] Needs Review This PR is ready for review. label Jul 27, 2020

Base automatically changed from add/new-heartbeat-package to master July 27, 2020 19:46

leogermani added this to the 8.9 milestone Jul 28, 2020

sergeymitr requested changes Jul 30, 2020

View reviewed changes

packages/heartbeat/src/class-heartbeat.php Outdated Show resolved Hide resolved

packages/heartbeat/src/class-heartbeat.php Outdated Show resolved Hide resolved

sergeymitr added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Jul 30, 2020

add heartbeat as a dependecy for jetpack

4318b06

leogermani mentioned this pull request Jul 31, 2020

Connection: use heartbeat to send connected plugins info #16286

Merged

sergeymitr previously approved these changes Jul 31, 2020

View reviewed changes

jeherve reviewed Aug 3, 2020

View reviewed changes

class.jetpack.php Show resolved Hide resolved

class.jetpack-heartbeat.php Outdated Show resolved Hide resolved

class.jetpack-heartbeat.php Show resolved Hide resolved

Update class.jetpack-heartbeat.php

a354e50

Co-authored-by: Jeremy Herve <[email protected]>

leogermani dismissed sergeymitr’s stale review via a354e50 August 3, 2020 22:27

add deprecation notices

c171e76

jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Aug 10, 2020

leogermani added 2 commits August 10, 2020 12:58

Fix test Jetpack_Heartbeat class

47dac49

add deprecated notice to cron_exec

1addc07

leogermani mentioned this pull request Aug 10, 2020

Drop usage of the old Heartbeat #16789

Open

leogermani added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Aug 10, 2020

jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Aug 19, 2020

jeherve approved these changes Aug 19, 2020

View reviewed changes

leogermani merged commit 367fbcc into master Aug 19, 2020

leogermani deleted the use-new-heartbeat-package branch August 19, 2020 17:00

matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Aug 19, 2020

jeherve removed the [Status] Needs Changelog label Aug 24, 2020

kbrown9 mentioned this pull request Feb 11, 2021

Sync: Use the Heartbeat package to generate the stats array #18494

Merged

Use new heartbeat package #16285

Use new heartbeat package #16285

Uh oh!

Conversation

leogermani commented Jun 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes proposed in this Pull Request:

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

Does this pull request change what data or activity we track or use?

Testing instructions:

Uh oh!

matticbot commented Jun 25, 2020

Uh oh!

sergeymitr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jetpackbot commented Jul 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sergeymitr left a comment

Choose a reason for hiding this comment

Uh oh!

sergeymitr commented Jul 31, 2020

Uh oh!

jeherve left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

leogermani commented Aug 4, 2020

Uh oh!

jeherve commented Aug 10, 2020

Uh oh!

leogermani commented Aug 10, 2020

Uh oh!

leogermani commented Aug 18, 2020

Uh oh!

jeherve left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

leogermani commented Jun 25, 2020 •

edited

Loading

jetpackbot commented Jul 30, 2020 •

edited

Loading