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 additional health and performance metrics #266

Merged
merged 2 commits into from
Sep 20, 2022
Merged

Conversation

Neverlord
Copy link
Member

This PR adds a couple of new metrics to Broker that keep track of message and store command throughput as well as buffer slots. It also introduces a new metric factory to allow us to have all the strings (names and help texts) in a single file.

We've also talked about having metrics that estimate how many bytes are bound up in Broker messages. I've tried that and I couldn't find a good approach. We consider a message as buffered (received but not processed) when it is in any of the flow stages in the core. There are three stages we care about here: data inputs, command inputs and the central merge point. While messages travel through the pipeline, we convert them from data and command messages to node messages.

For data and command messages, there's no easy way of estimating the size of the payload other than applying a serializer to the messages. Even if that serializer doesn't actually write any data but only keeps track of how many bytes would be written: this would potentially add quite a bit of runtime overhead. Ideally, the instrumentation shouldn't add any measurable overhead to the system. If we still want that buffer size estimation in bytes, maybe we can benchmark that overhead in a real system (Zeek instance) to see whether the extra CPU load is negligible or not.

Relates #254.

@Neverlord Neverlord force-pushed the topic/neverlord/gh-254 branch from bbc7da2 to ec295bd Compare August 15, 2022 15:50
Copy link
Contributor

@awelzel awelzel left a comment

Choose a reason for hiding this comment

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

Approving based on skimming through chatting with @timwoj .

@timwoj timwoj merged commit 4f1667f into master Sep 20, 2022
@timwoj timwoj deleted the topic/neverlord/gh-254 branch September 20, 2022 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants