[CORE-668,CORE-669] - Add health monitor flags #802

clemire · 2023-11-23T00:47:04Z

Changelist

Add 2 flags:

configure # of seconds before unhealthy daemon causes a panic
enable panicking behavior or log error when daemon health check fails

Test Plan

unit tests for daemonFlags
tested both flags with localnet.

Author/Reviewer Checklist

If this PR has changes that result in a different app state given the same prior state and transaction list, manually add the state-breaking label.
If this PR isn't state-breaking but has changes that modify behavior in PrepareProposal or ProcessProposal, manually add the label proposal-breaking.
If this PR is one of many that implement a specific feature, manually label them all feature:[feature-name].
If you wish to for mergify-bot to automatically create a PR to backport your change to a release branch, manually add the label backport/[branch-name].
Manually add any of the following labels: refactor, chore, bug.

linear · 2023-11-23T00:47:06Z

CORE-668 Add flag to reduce health monitor panic to error log on daemon health check failure

AI: https://www.notion.so/dydx/Add-flag-to-error-instead-of-panic-on-failure-of-the-daemon-health-check-d7e66c18271e4248b3ca4eb424e17902

Notion ddoc: https://www.notion.so/dydx/Daemon-Health-Check-Re-design-1-pager-477a0b31ce4f47b99b9b67679d01efc7

coderabbitai · 2023-11-23T00:47:08Z

Important

Auto Review Skipped

Auto reviews are disabled on base/target branches other than the default branch. Please add the base/target branch pattern to the list of additional branches to be reviewed in the settings.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository.

To trigger a single review, invoke the @coderabbitai review command.

Tips

Chat with CodeRabbit Bot (`@coderabbitai`)

If you reply to a review comment from CodeRabbit, the bot will automatically respond.
To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
Note: Review comments are made on code diffs or files, not on the PR overview.
Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
The JSON schema for the configuration file is available here.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

clemire · 2023-11-23T00:56:48Z

protocol/app/app.go

@@ -183,10 +183,6 @@ import (
 var (
 	// DefaultNodeHome default home directories for the application daemon
 	DefaultNodeHome string
-


This constant is now defined within daemon flags, not needed here.

linear · 2023-11-23T01:40:50Z

CORE-669 Add flag to disable daemon health check monitor altogether

AI: https://www.notion.so/dydx/Add-a-flag-to-disable-the-health-check-f7e78c87d7f341248b95de8b0806acfa

protocol/daemons/flags/flags.go

protocol/app/app.go

protocol/daemons/server/types/health_checker.go

ttl33 · 2023-11-29T23:24:08Z

protocol/daemons/server/types/health_monitor.go

 // checker is lazily created using the provided function if needed. This method is synchronized. It returns an error if
 // the service was already registered.
 func (ms *healthMonitorMutableState) RegisterHealthChecker(
 	checkable types.HealthCheckable,
 	lazyHealthCheckerCreator func() *healthChecker,
 ) error {
+	stopService := false


hmm, is this change intended to be part of this PR? thought this PR was strictly adding the flags.

Ah, looks like I mixed up some changes I meant to add to the other health monitoring PR.

…PC query. Update error for a more specific test. PR comments. Revert liquidations health reporting to occur only at task loop boundary. Checkpoint. checkpoint - implementation, moved methods to app. need to migrate / add testing. Update test app. Checkpoint for tests. Finish test cases. App unit test + cleanup. Cleanup, rename. cleanup. Move monitoring call to before client start. Update placement of monitor code for pricefeed client. Revert ServiceName changes. Checkpoint. Revert bridge failure. Rename flag. Disable panics for localnet due to bridge daemon flakiness. Rename again after rebase. Rename flag. Fix unaddressed merge lines.

github-actions bot added the protocol label Nov 23, 2023

clemire commented Nov 23, 2023

View reviewed changes

clemire force-pushed the crystal/CORE-666-monitor-update branch from aa7e502 to db8d8d0 Compare November 23, 2023 01:39

clemire force-pushed the crystal/CORE-668-health-monitor-flags branch from 96b6c25 to 12fe89e Compare November 23, 2023 01:39

clemire changed the title ~~[CORE-668] - Add health monitor flags~~ [CORE-668,CORE-669] - Add health monitor flags Nov 23, 2023

lcwik approved these changes Nov 27, 2023

View reviewed changes

clemire force-pushed the crystal/CORE-666-monitor-update branch from 5fe6028 to bc6fe09 Compare November 28, 2023 00:52

clemire force-pushed the crystal/CORE-668-health-monitor-flags branch from 12fe89e to a1cc295 Compare November 28, 2023 00:59

ttl33 reviewed Nov 28, 2023

View reviewed changes

protocol/daemons/flags/flags.go Show resolved Hide resolved

protocol/daemons/flags/flags.go Outdated Show resolved Hide resolved

clemire force-pushed the crystal/CORE-668-health-monitor-flags branch 4 times, most recently from 96af5ff to f0fc022 Compare November 29, 2023 00:53

ttl33 reviewed Nov 29, 2023

View reviewed changes

protocol/daemons/flags/flags.go Outdated Show resolved Hide resolved

protocol/daemons/flags/flags.go Outdated Show resolved Hide resolved

clemire force-pushed the crystal/CORE-668-health-monitor-flags branch 2 times, most recently from 059b00d to 3281291 Compare November 29, 2023 21:01

ttl33 reviewed Nov 29, 2023

View reviewed changes

clemire force-pushed the crystal/CORE-668-health-monitor-flags branch 2 times, most recently from d4a3839 to 27b4c4e Compare November 29, 2023 23:49

clemire changed the base branch from crystal/CORE-666-monitor-update to main November 29, 2023 23:58

clemire changed the base branch from main to crystal/CORE-666-monitor-update November 29, 2023 23:59

clemire force-pushed the crystal/CORE-666-monitor-update branch from a71a5b4 to 7b8f83f Compare November 30, 2023 00:01

clemire force-pushed the crystal/CORE-668-health-monitor-flags branch from 1538cc4 to 6bedc57 Compare November 30, 2023 00:02

ttl33 approved these changes Nov 30, 2023

View reviewed changes

ttl33 mentioned this pull request Dec 4, 2023

[CORE-666] - Migrate daemon monitoring to use health checks #783

Merged

5 tasks

clemire force-pushed the crystal/CORE-666-monitor-update branch from cd77cee to f996947 Compare December 7, 2023 18:30

clemire force-pushed the crystal/CORE-668-health-monitor-flags branch from 6bedc57 to 21bd7cd Compare December 7, 2023 18:46

Crystal Lemire added 7 commits December 7, 2023 13:29

Re-add flag, update test case comment.

179d52a

PR comments.

ecf688f

Add docker-compose comment.

276d30d

PR comments.

8e6bb13

Disable daemon panic for clob cli tests.

1ba1721

Fix docker-compose flag.

3e20131

clemire force-pushed the crystal/CORE-668-health-monitor-flags branch from ed839cf to 3e20131 Compare December 7, 2023 21:29

Fix rebase.

8c8c3f9

clemire merged commit 4336388 into crystal/CORE-666-monitor-update Dec 7, 2023
16 of 17 checks passed

clemire deleted the crystal/CORE-668-health-monitor-flags branch December 7, 2023 23:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CORE-668,CORE-669] - Add health monitor flags #802

[CORE-668,CORE-669] - Add health monitor flags #802

clemire commented Nov 23, 2023 •

edited

Loading

linear bot commented Nov 23, 2023

coderabbitai bot commented Nov 23, 2023

Auto Review Skipped

Chat with CodeRabbit Bot (`@coderabbitai`)

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (`.coderabbit.yaml`)

clemire Nov 23, 2023

linear bot commented Nov 23, 2023

ttl33 Nov 29, 2023

clemire Nov 29, 2023

[CORE-668,CORE-669] - Add health monitor flags #802

[CORE-668,CORE-669] - Add health monitor flags #802

Conversation

clemire commented Nov 23, 2023 • edited Loading

Changelist

Test Plan

Author/Reviewer Checklist

linear bot commented Nov 23, 2023

coderabbitai bot commented Nov 23, 2023

Auto Review Skipped

Chat with CodeRabbit Bot (@coderabbitai)

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (.coderabbit.yaml)

clemire Nov 23, 2023

Choose a reason for hiding this comment

linear bot commented Nov 23, 2023

ttl33 Nov 29, 2023

Choose a reason for hiding this comment

clemire Nov 29, 2023

Choose a reason for hiding this comment

clemire commented Nov 23, 2023 •

edited

Loading

Chat with CodeRabbit Bot (`@coderabbitai`)

CodeRabbit Configration File (`.coderabbit.yaml`)