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

chore(docs): developer docs on plugin system communication #728

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

j-lanson
Copy link
Collaborator

To start the hipcheck developer docs, I added a document and accompanying graphic explaining how Hipcheck core queries are conducted over gRPC.

@alilleybrinker I could use advice on where the .png / .drawio files should be stored in the site. I also need to set up to do local site dev to wire up this Markdown file to read the .png and be part of the Contributing page links.

@devin-b-lake @aamohd @ninaagrawal as the newer members on the team, I would appreciate if you read through the document and the graphic, and pointed out anything that was particularly confusing. The expectation is that someone would read it while looking through the functions it references.

@j-lanson j-lanson added type: chore Clean up or management task. product: website Relates to the Hipcheck website labels Dec 17, 2024
@j-lanson j-lanson added this to the 3.10.0 milestone Dec 17, 2024
@j-lanson j-lanson self-assigned this Dec 17, 2024
@j-lanson j-lanson force-pushed the jlanson/dev-docs branch 2 times, most recently from aaf2cd8 to c062616 Compare December 19, 2024 19:48
@j-lanson j-lanson requested a review from ninaagrawal January 7, 2025 16:38
Comment on lines 39 to 48
In Rust, the gRPC channel is accessed with using a `mpsc::{Sender, Receiver}`
pair. The `Sender` can be cloned many times, meaning that many threads can
`send` messages on the channel without needing exclusive access to any resource.
However, a `Receiver` cannot be shared. The Rust SDK addresses this restriction
and the above multiple-session, single-channel requirement by having a
`HcSessionSocket` object that has the exclusive `Receiver` to the plugin's gRPC
channel with Hipcheck core. The `HcSessionSocket` is responsible for tracking
the set of live sessions, and detecting whether a new message from the gRPC
channel should be forwarded to a live session, or constitues an entirely new
session. Each session is represented by a `PluginEngine` instance. The
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a little confusing. There's explanation for why and functionality for there being multiple Sender instances (if I'm understanding correctly), but I'm a little confused on how the HcSessionSocket is handling the situation. Is it that the HcSessionSocket forwards the session to the Receiver when it's free? And I might be misunderstanding the wording, but is the session within the send messages as a PluginEngine instance?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We don't send "sessions," we send "messages". A session is made up of a series of messages with the same ID. The HcSessionSocket forwards a message it receives on the Receiver tied to the gRPC channel, and then forwards it on one of the many Senders it has to different PluginEngine instances. I will add a graphic and update the explanation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added a graphic and expanded the description. @aamohd let me know if this is more clear.

Copy link
Contributor

@aamohd aamohd left a comment

Choose a reason for hiding this comment

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

I left comments on plugin-query-system.md

@alilleybrinker
Copy link
Collaborator

Regarding where to put images, here's the guide from the Zola docs on "asset colocation": https://www.getzola.org/documentation/content/overview/#asset-colocation

The basic idea is that you turn the relevant page into a folder with an index.md file in it, and with the images you want to include on the page inside of that folder.

site/content/docs/contributing/_index.md Show resolved Hide resolved
@@ -0,0 +1,83 @@
---
title: Repo Structure
Copy link
Collaborator

Choose a reason for hiding this comment

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

This page is one that we'll want to make sure we keep up-to-date. In general, I think we probably ought to add a review checklist for approvals on PRs, which would include a documentation review to make sure documentation is updated to reflect PR changes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added a PR Review Checklist page under the Contributing section. Let me know if you have any suggested changes.

@j-lanson
Copy link
Collaborator Author

Regarding where to put images, here's the guide from the Zola docs on "asset colocation": https://www.getzola.org/documentation/content/overview/#asset-colocation

The basic idea is that you turn the relevant page into a folder with an index.md file in it, and with the images you want to include on the page inside of that folder.

@alilleybrinker this doesn't actually show how to display co-located images in a markdown file. The way I found to work with a custom shortcut only works if the images are in static/

Initial documentation oriented towards developers, describing the repo
structure, Hipcheck distributed architecture, and plugin query control
flow.

Signed-off-by: jlanson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product: website Relates to the Hipcheck website type: chore Clean up or management task.
Projects
Status: In Progress
Status: No status
Development

Successfully merging this pull request may close these issues.

4 participants