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 durable_name to streaming subscription metrics #81

Merged
merged 2 commits into from
Jun 6, 2019

Conversation

Will2817
Copy link
Contributor

@Will2817 Will2817 commented Jun 6, 2019

image

@Will2817
Copy link
Contributor Author

Will2817 commented Jun 6, 2019

One question I wasn't sure about was: Since the durable_name is in the queue_name label when the subscription is a queue group and durable, should the queue_name label be split into durable_name and queue_name in this case?

@coveralls
Copy link

coveralls commented Jun 6, 2019

Coverage Status

Coverage increased (+0.1%) to 90.247% when pulling 432bba2 on Will2817:export-subscription-durable-name into 873a29a on nats-io:master.

collector/collector_test.go Outdated Show resolved Hide resolved
collector/collector_test.go Outdated Show resolved Hide resolved
@wallyqs
Copy link
Member

wallyqs commented Jun 6, 2019

@Will2817 thanks for PR! splitting into queue_name and durable_name sounds like a good idea to me, would be useful to be able to query using different fields

@Will2817
Copy link
Contributor Author

Will2817 commented Jun 6, 2019

@wallyqs great, I've now updated the PR to also split the durable_name out from the queue_name when appropriate.

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, thanks!

@wallyqs wallyqs merged commit b03c364 into nats-io:master Jun 6, 2019
@wallyqs wallyqs mentioned this pull request Jun 24, 2019
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.

None yet

3 participants