From 36bc5aadb12e70ef7949fa87dca595b47ba6d1db Mon Sep 17 00:00:00 2001
From: robsunday <rniedziela@splunk.com>
Date: Fri, 18 Oct 2024 10:22:56 +0200
Subject: [PATCH] Code review changes

---
 .../target_systems/ActiveMqIntegrationTest.java    |  2 ++
 jmx-scraper/src/main/resources/activemq.yaml       | 14 ++++++++++++++
 jmx-scraper/src/main/resources/activemq/env        |  4 +++-
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/ActiveMqIntegrationTest.java b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/ActiveMqIntegrationTest.java
index 1a79469ee..2648ddc60 100644
--- a/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/ActiveMqIntegrationTest.java
+++ b/jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/target_systems/ActiveMqIntegrationTest.java
@@ -26,6 +26,8 @@ protected GenericContainer<?> createTargetContainer(int jmxPort) {
                     builder -> builder.from("apache/activemq-classic:5.18.6").build()))
         .withEnv("LOCAL_JMX", "no")
         .withCopyFileToContainer(
+            // Overwrite default ActiveMQ configuration in order to let ActiveMQ use JMX options
+            // stored in $ACTIVEMQ_JMX_OPTS env variable defined below
             MountableFile.forClasspathResource("activemq/env"), "/opt/apache-activemq/bin/env")
         .withEnv(
             "ACTIVEMQ_JMX_OPTS",
diff --git a/jmx-scraper/src/main/resources/activemq.yaml b/jmx-scraper/src/main/resources/activemq.yaml
index 6300ab8cb..8387cf5b6 100644
--- a/jmx-scraper/src/main/resources/activemq.yaml
+++ b/jmx-scraper/src/main/resources/activemq.yaml
@@ -10,11 +10,15 @@ rules:
     mapping:
       ProducerCount:
         metric: producer.count
+        # Unit name inherited from activemq.groovy file.
+        # Will be updated to {} semconv notation when we switch to use original files from JMX Insights
         unit: "producers"
         type: updowncounter
         desc: The number of producers currently attached to the broker.
       ConsumerCount:
         metric: consumer.count
+        # Unit name inherited from activemq.groovy file.
+        # Will be updated to {} semconv notation when we switch to use original files from JMX Insights
         unit: "consumers"
         type: updowncounter
         desc: The number of consumers currently reading from the broker.
@@ -25,21 +29,29 @@ rules:
         desc: The percentage of configured memory used.
       QueueSize:
         metric: message.current
+        # Unit name inherited from activemq.groovy file.
+        # Will be updated to {} semconv notation when we switch to use original files from JMX Insights
         unit: "messages"
         type: updowncounter
         desc: The current number of messages waiting to be consumed.
       ExpiredCount:
         metric: message.expired
+        # Unit name inherited from activemq.groovy file.
+        # Will be updated to {} semconv notation when we switch to use original files from JMX Insights
         unit: "messages"
         type: counter
         desc: The total number of messages not delivered because they expired.
       EnqueueCount:
         metric: message.enqueued
+        # Unit name inherited from activemq.groovy file.
+        # Will be updated to {} semconv notation when we switch to use original files from JMX Insights
         unit: "messages"
         type: counter
         desc: The total number of messages received by the broker.
       DequeueCount:
         metric: message.dequeued
+        # Unit name inherited from activemq.groovy file.
+        # Will be updated to {} semconv notation when we switch to use original files from JMX Insights
         unit: "messages"
         type: counter
         desc: The total number of messages delivered to consumers.
@@ -61,6 +73,8 @@ rules:
       CurrentConnectionsCount:
         metric: connection.count
         type: updowncounter
+        # Unit name inherited from activemq.groovy file.
+        # Will be updated to {} semconv notation when we switch to use original files from JMX Insights
         unit: "connections"
         desc: The total number of current connections.
       StorePercentUsage:
diff --git a/jmx-scraper/src/main/resources/activemq/env b/jmx-scraper/src/main/resources/activemq/env
index bfcc87c6f..23d70bf4f 100644
--- a/jmx-scraper/src/main/resources/activemq/env
+++ b/jmx-scraper/src/main/resources/activemq/env
@@ -114,4 +114,6 @@
  # (set JAVACMD to "auto" for automatic detection)
  #JAVA_HOME=""
  JAVACMD="auto"
- ACTIVEMQ_OPTS="$ACTIVEMQ_OPTS $ACTIVEMQ_JMX_OPTS -Dhawtio.authenticationEnabled=false -Dhawtio.realm=activemq -Dhawtio.role=admins -Dhawtio.rolePrincipalClasses=org.apache.activemq.jaas.GroupPrincipal"
+
+ # Use JMX options defined in environment variable
+ ACTIVEMQ_OPTS="$ACTIVEMQ_OPTS $ACTIVEMQ_JMX_OPTS"