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

Fix metric listings for activemq / activemq_xml #2487

Merged
merged 3 commits into from
Nov 7, 2018
Merged

Conversation

ruthnaebeck
Copy link
Contributor

@ruthnaebeck ruthnaebeck commented Oct 29, 2018

What does this PR do?

Fix metric listings for activemq / activemq_xml

Motivation

Trello request

Review checklist

  • PR has a meaningful title or PR has the no-changelog label attached
    - [ ] Feature or bugfix has tests
  • Git history is clean
  • If PR impacts documentation, docs team has been notified or an issue has been opened on the documentation repo

l0k0ms
l0k0ms previously approved these changes Oct 30, 2018
Copy link
Contributor

@l0k0ms l0k0ms left a comment

Choose a reason for hiding this comment

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

LGTM 👌

Copy link
Contributor

@zippolyte zippolyte left a comment

Choose a reason for hiding this comment

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

Two comments about the unit names, otherwise 👍

activemq.queue.max_enqueue_time,gauge,10,millisecond,,The max the amount of time (ms) that messages remained enqueued.,0,activemq,max enq time
activemq.queue.min_enqueue_time,gauge,10,millisecond,,The min the amount of time (ms) that messages remained enqueued.,0,activemq,min enq time
activemq.queue.memory_pct,gauge,10,percent,,The percentage of memory currently in use,0,activemq,mem pct
activemq.queue.size,gauge,10,,,The amount of messages that remained queued.,0,activemq,enq size
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add the message unit for this metric and the ones below please ?

activemq.broker.memory_pct,gauge,10,percent,,The percentage of memory in use.,0,activemq,mem pct
activemq.topic.count,gauge,,,,The number of topics.,0,activemq_xml,topic count
activemq.topic.consumer_count,gauge,10,,,The number of consumers.,0,activemq_xml,topic cnsmr count
activemq.topic.dequeue_count,gauge,10,,,The total number of messages sent to the queue since the last restart.,0,activemq_xml,topic deq count
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, we could addd the message unit to all the metrics related to number of message

@ruthnaebeck
Copy link
Contributor Author

@zippolyte - Just following up on this one.

@zippolyte
Copy link
Contributor

Sorry, lost track of it.
There is just the activemq metadata file left to update base on my comment for the unit name and then it's 👍

@ruthnaebeck
Copy link
Contributor Author

Thanks @zippolyte! I've updated the activemq metadata file.

Copy link
Contributor

@zippolyte zippolyte 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

@l0k0ms l0k0ms merged commit 0135beb into master Nov 7, 2018
@l0k0ms l0k0ms deleted the ruth/activemq-docs branch November 7, 2018 15:54
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