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

Activemq #188

Merged
merged 12 commits into from
Jan 19, 2022
Merged

Activemq #188

merged 12 commits into from
Jan 19, 2022

Conversation

armstrmi
Copy link
Contributor

@armstrmi armstrmi commented Jan 13, 2022

Description:

  • added in Metric receiver for activemq

Existing Issue(s):

Testing:

Added integration tests for activemq metrics

Documentation:

  • Documentation for activemq metrics added

@armstrmi armstrmi requested a review from a team January 13, 2022 19:17
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jan 13, 2022

CLA Signed

The committers are authorized under a signed CLA.

@@ -0,0 +1,79 @@
# ActiveMQ Metrics

The JMX Metric Gatherer provides built in Tomcat metric gathering capabilities.
Copy link
Contributor

Choose a reason for hiding this comment

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

Update this to say ActiveMQ

Suggested change
The JMX Metric Gatherer provides built in Tomcat metric gathering capabilities.
The JMX Metric Gatherer provides built in ActiveMQ metric gathering capabilities.

@trask
Copy link
Member

trask commented Jan 13, 2022

hi @armstrmi! can you fetch upstream and merge upstream/main` into your PR? that should fix the "Component Owners" build failure until we can fix the underlying github action (dyladan/component-owners#10)

* Description: The number of consumers currently reading from the broker.
* Unit: `{consumers}`
* Labels: `destination`
* Instrument Type: LongUpDownCounterCallback
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to standardize these names throughout our docs, but should these be something like ObservableLongUpDownCounter? I'm not sure where the spec has landed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious about this one, can't seem to find any reference to how we should be naming it, personally, to me, it seems to make sense to match what's actually being used in the script. Is there a reason we're adding "observable"? just wasn't sure about this one. Thanks for any clarification!

Copy link
Contributor

Choose a reason for hiding this comment

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

* Instrument Type: LongCounterCallback


* Name: `activemq.wait_time.avg`
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like this name is wrong compared to the script activemq.message.wait_time.avg

Copy link
Contributor

@Mrod1598 Mrod1598 left a comment

Choose a reason for hiding this comment

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

LGTM
@rmfitzpatrick I believe all of your feedback has been addressed as well

@armstrmi
Copy link
Contributor Author

@rmfitzpatrick could you please take a look at this when you get the chance

@rmfitzpatrick
Copy link
Contributor

@rmfitzpatrick could you please take a look at this when you get the chance

Thanks for the addition!

@rmfitzpatrick rmfitzpatrick merged commit 89aa698 into open-telemetry:main Jan 19, 2022
@Mrod1598 Mrod1598 deleted the activemq branch January 19, 2022 18:12
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.

5 participants