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

pkg/loop: plugins report health to host [BCF-2709] #206

Merged
merged 3 commits into from
Jan 16, 2025

Conversation

jmank88
Copy link
Collaborator

@jmank88 jmank88 commented Oct 31, 2023

https://smartcontract-it.atlassian.net/browse/BCF-2709

This change makes the top level plugin types services.Services in order to report plugin health back through the host.
It also introduces testing helpers for verifying service names from health reports and log lines.

Requires:

Supports:

@jmank88 jmank88 temporarily deployed to integration October 31, 2023 00:26 — with GitHub Actions Inactive
@jmank88 jmank88 force-pushed the BCF-2709-loop-plugin-health branch from db7e8af to 51cc19d Compare October 31, 2023 00:40
@jmank88 jmank88 temporarily deployed to integration October 31, 2023 00:40 — with GitHub Actions Inactive
@jmank88 jmank88 force-pushed the BCF-2709-loop-plugin-health branch from 51cc19d to f22f854 Compare October 31, 2023 12:04
@jmank88 jmank88 temporarily deployed to integration October 31, 2023 12:04 — with GitHub Actions Inactive
@jmank88 jmank88 changed the title pkg/services/srvctest: add helper for testing HealthReport names pkg/loop: plugins report health to host [BCF-2709] Oct 31, 2023
@jmank88 jmank88 force-pushed the BCF-2709-loop-plugin-health branch from f22f854 to c2c5209 Compare November 27, 2023 01:17
@jmank88 jmank88 force-pushed the BCF-2709-loop-plugin-health branch from c2c5209 to 6ef2e4a Compare November 27, 2023 01:37
@jmank88 jmank88 force-pushed the BCF-2709-loop-plugin-health branch from 6ef2e4a to 1484a99 Compare December 22, 2023 12:58
@jmank88 jmank88 force-pushed the BCF-2709-loop-plugin-health branch from 1484a99 to e443f8f Compare December 22, 2023 14:14
@jmank88 jmank88 force-pushed the BCF-2709-loop-plugin-health branch from e443f8f to 86072cd Compare January 10, 2024 14:22

hr := map[string]error{s.Name(): s.Healthy()}

// wait until service is ready, which also triggers the deferred construction to ensure a complete HealthReport
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This works, but it feels hacky 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

seems reasonable to me. we either have to pull or push; changing to push would be a lot of work for little gain afaict

@jmank88 jmank88 force-pushed the BCF-2709-loop-plugin-health branch from 6098284 to 22774c5 Compare January 14, 2025 17:17
@jmank88 jmank88 force-pushed the BCF-2709-loop-plugin-health branch from 22774c5 to 7938700 Compare January 14, 2025 18:11
@jmank88 jmank88 marked this pull request as ready for review January 15, 2025 15:00
@jmank88 jmank88 requested a review from a team as a code owner January 15, 2025 15:00
serviceCh chan struct{} // closed when service is available
Service S
serviceCh chan struct{} // closed when service is available
Service S
Copy link
Contributor

Choose a reason for hiding this comment

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

Do S and HealthReport needed to be synchronously closed when PluginService is closed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It shouldn't really matter 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Of course if we have log noise or something then that is worth addressing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or log after a test close

@jmank88 jmank88 requested a review from a team January 15, 2025 22:38
@jmank88 jmank88 merged commit f49c5c2 into main Jan 16, 2025
9 of 10 checks passed
@jmank88 jmank88 deleted the BCF-2709-loop-plugin-health branch January 16, 2025 21:48
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.

3 participants