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

Include msgsin msgsout fields #109

Merged
merged 4 commits into from
Jan 20, 2020

Conversation

renanberto
Copy link
Contributor

@renanberto renanberto commented Nov 21, 2019

Hi guys.

This change is intended to include the new streaming-server API metrics msgs_in and msgs_out.
Thanks!

[Warning] this metric is only collected after this nats-streaming-server commit : nats-io/nats-streaming-server@b95714e

@kozlovic
Copy link
Member

Just be noted that the PR in the NATS Streaming server is not yet merged.

@renanberto renanberto changed the title Include msgsin msgsout fields [WIP] - Include msgsin msgsout fields Nov 21, 2019
@kozlovic
Copy link
Member

I have merged PR (nats-io/nats-streaming-server#976) in the Streaming server. Note that the PR adds the in/out msgs (count) but also bytes. However this PR deals only with the count. For completeness, I wonder if we should include bytes too?

@renanberto
Copy link
Contributor Author

I have merged PR (nats-io/nats-streaming-server#976) in the Streaming server. Note that the PR adds the in/out msgs (count) but also bytes. However this PR deals only with the count. For completeness, I wonder if we should include bytes too?

I think it's worth Ivan, I'll add these fields to the collect.

@renanberto renanberto changed the title [WIP] - Include msgsin msgsout fields Include msgsin msgsout fields Nov 25, 2019
Copy link
Member

@wallyqs wallyqs left a comment

Choose a reason for hiding this comment

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

LGTM

@wallyqs wallyqs merged commit 3e49ee5 into nats-io:master Jan 20, 2020
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