-
Notifications
You must be signed in to change notification settings - Fork 1k
improve and align activemq classic JMX metrics with semconv #14996
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
base: main
Are you sure you want to change the base?
Conversation
…nstrumentation into jmx-activemq
…nstrumentation into jmx-activemq
|
🔧 The result from spotlessApply was committed to the PR branch. |
|
|
Just for clarification, description above:
shoud be: right? |
instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/activemq.yaml
Show resolved
Hide resolved
| - bean: org.apache.activemq:type=Broker,brokerName=* | ||
| metricAttribute: | ||
| messaging.system: const(activemq) | ||
| activemq.broker.name: param(brokerName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider messaging.activemq.broker.name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or even messaging.brooker.name so it can be reused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is messaging.broker.name a part of semconv?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not currently but it could be added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to generalize activemq.broker.name by adding messaging.broker.name to semconv we need to have more than one implementation to serve as an example (this PR provides one).
From experience, it usually takes a while as name-related discussions always take time to settle, I don't think we should make this PR depend on it. We can always iterate on it later and change that once a stable semconv attribute is available.
instrumentation/jmx-metrics/library/src/main/resources/jmx/rules/activemq.yaml
Show resolved
Hide resolved
Good catch, I've updated the PR description, should be good now. |
|
Looking at https://opentelemetry.io/docs/specs/semconv/general/naming/#client-and-server-metrics should alot of these metrics actually become activemq.server.* or perhaps activemq.brooker.* as Brooker would indicate that it is the server, just like process does indicate that it is client. Also shouldn't |
|
constant |
In the client and server metrics, I think that ActiveMQ fits the last example where Example about the "kestrel" http server:
I haven't seen anything explicitly written in semconv, but I think there is a different strategy to name attributes and metrics, maybe it's because I'm not a native english speaker:
|
|
@SylvainJuge for me the key part of that link is:
In the case here, ActiveMQ would be the area and in fact it would actually make it consistant with the client metrics which is messaging.client.sent.messages etc as well as the third example. Note I don't think the kestrel example applies here as kestrel can only be server side app which is not the case for ActiveMQ. The way I look at it the last segment of the metric name should be the unit when without the name wouldn't be clear. This matches:
But yes I agree the documentation can be streamlined. Perhaps @trask can clarify what the naming intention is for these scenarios. |
Fixes #14278
Only covers ActiveMQ Classic, ActiveMQ Artemis will be covered with #15006.
Some of the ActiveMQ metrics are provided as percentages with integer values in
[0,100]range, this PR adds a%to1unit conversion to translate this to[0.0,1.0]range.For the resource-related metrics, this follows the same strategy we have on GPU hardware metrics (semconv)
hw.gpu.memory.usageto measure usage in byteshw.gpu.memory.limitto measure limit in byteshw.gpu.memory.utilizationas usage ratio between 0.0 and 1.0Summary of changes
Metric attributes
messaging.systemmetric attribute added to all metrics withactivemqconstant valuebrokermetric attribute is renamed toactivemq.broker.namedestinationmetric attribute is renamed tomessaging.destination.nameNew metrics
Destination (queue/topic) resource usage and configured limits
activemq.destination.memory.usage, value in bytes (see related item to discuss below)activemq.destination.memory.limit, value in bytesactivemq.destination.temp.utilization, value in [0.0,1.0] converted from [0,100]activemq.destination.temp.limit, value in bytesBroker resource usage and configured limits
activemq.memory.utilization, value in[0.0,1.0]converted from[0,100]activemq.memory.limit, value in bytesactivemq.store.utilization, value in[0.0,1.0]converted from[0,100]activemq.store.limit, value in bytesactivemq.temp.utilization, value in[0.0,1.0]converted from[0,100]activemq.temp.limit, value in bytesRemoved metrics
activemq.memory.MemoryPercentUsagemetric removed, replaced byactivemq.destination.memory.usageandactivemq.destination.memory.limitRenamed metrics
activemq.ProducerCountmetric renamed toactivemq.producer.countactivemq.ConsumerCountmetric renamed toactivemq.consumer.countactivemq.message.QueueSizemetric renamed toactivemq.message.queue.sizeactivemq.message.ExpiredCountmetric renamed toactivemq.message.expiredactivemq.message.EnqueueCountmetric renamed toactivemq.message.enqueued(with an extradfor consistency withexpired.activemq.message.DequeueCountmetric renamed toactivemq.message.dequeued(with an extradfor consistency withexpired.activemq.message.AverageEnqueueTimemetric renamed toactivemq.message.enqueue.average_duration(see related item to discuss below)Things to potentially discuss/clarify
messaging.systemmetric attribute to constant valueactivemqwhen we monitor activemq itself or is it only meant on the application side ? Because metric names are already part ofactivemq.*namespace it might be a bit redundant, but could help with correlation with traces from application. Also, themessaging.systemis about to be renamed tomessaging.system.name, removing this attribute would avoid one known breaking change.activemq.message.enqueue.average_duration, does using the same convention ask8s.hpa.metric.target.cpu.average_utilizationis a good fit ?destination"sub-namespace"activemq.{memory,temp}.destination.*, the broker-level equivalents were not previously captured, they are expected to be more global and thus in theactivemq.{memory,temp,store}.*namespace.activemq.destination.memory.usage, but we can't do the same with non-persistent storage which is always captured as a ratio between 0.0 and 1.0, should we favor consistency between similar metrics or give priority to accuracy ? Ratio is provided through an int percentage, thus max 1% accuracy. We could also provide bothactivemq.destination.memory.usageandactivemq.destination.memory.utilizationhere. On the monitoring side, having an exact measurement is probably not required.