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

feat: add metrics with msg size histogram #1697

Merged
merged 2 commits into from
Apr 20, 2023
Merged

Conversation

alrevuelta
Copy link
Contributor

closes #1626

  • Add a new metric waku_histogram_message_size containing the message size histogram.
  • This allows to have better observability on the message distribution size.

Suggested usage:

image

  • 98% of the messages are between 5 and 15 kBytes
  • 0.7% of the messages are between 50 and 100 kBytes.
  • 99% of the messages are lower than 37.6 kBytes.

Copy link
Contributor

@jm-clius jm-clius left a comment

Choose a reason for hiding this comment

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

LGTM

declarePublicCounter waku_node_messages, "number of messages received", ["type"]
declarePublicHistogram waku_histogram_message_size, "message size histogram in kB", buckets = [0.0, 5.0, 15.0, 50.0, 100.0, 300.0, 700.0, 1000.0]
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps I'm missing something about how nim-metrics work, but should the Inf bucket be declared?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, added in e27702c to be explicit.

But looks like its automatically added. Have seen examples that both provide Inf and not, but seems that even without it its automatically added. Example without having Inf in bucket.

$ curl localhost:8004/metrics | grep histogram
...
waku_histogram_message_size_sum 0.0
waku_histogram_message_size_count 0.0
waku_histogram_message_size_created 1681987077.0
waku_histogram_message_size_bucket{le="0.0"} 0.0
waku_histogram_message_size_bucket{le="5.0"} 0.0
waku_histogram_message_size_bucket{le="15.0"} 0.0
waku_histogram_message_size_bucket{le="50.0"} 0.0
waku_histogram_message_size_bucket{le="100.0"} 0.0
waku_histogram_message_size_bucket{le="300.0"} 0.0
waku_histogram_message_size_bucket{le="700.0"} 0.0
waku_histogram_message_size_bucket{le="1000.0"} 0.0
waku_histogram_message_size_bucket{le="+Inf"} 0.0   <---

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! I thought it may be added automatically, but wasn't sure.

Copy link
Member

@vpavlin vpavlin left a comment

Choose a reason for hiding this comment

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

Awesome! LGTM

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.

feat: add metric with average message size
3 participants