Skip to content

SEC-1601: jvm metrics#14

Merged
lutaylor merged 9 commits into
mainfrom
SEC-1601
Nov 25, 2025
Merged

SEC-1601: jvm metrics#14
lutaylor merged 9 commits into
mainfrom
SEC-1601

Conversation

@Alexander-Cairns

@Alexander-Cairns Alexander-Cairns commented Nov 14, 2025

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features

    • Prometheus JMX exporter integrated into the Docker image.
    • ActiveMQ metrics exposed on port 3001 for monitoring.
    • New default metrics configuration providing comprehensive rules for queues, topics, and broker health.
  • Bug Fixes

    • Health check updated to avoid running the JMX exporter options during liveness/readiness probes.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Nov 14, 2025

Copy link
Copy Markdown

Walkthrough

Integrates Prometheus JMX exporter into the ActiveMQ Docker image: adds agent JAR download with checksum, adds jmx.yml for metric rules, sets environment variable to enable the Java agent on port 3001 at startup, and updates the healthcheck to exclude the JMX agent from its probe.

Changes

Cohort / File(s) Summary
JMX Exporter Integration
Dockerfile
Adds build ARGs JMX_EXPORTER_VERSION and JMX_EXPORTER_DIGEST; creates /jmx workdir; downloads jmx_prometheus_javaagent-$JMX_EXPORTER_VERSION.jar using ADD with checksum; copies jmx.yml into /jmx; sets JMX_OPT to enable the Java agent on port 3001 and updates ACTIVEMQ_OPTS; switches to activemq user and ensures /opt/activemq context.
JMX Exporter Integration
healthcheck.sh
Comments that healthcheck should not run JMX exporter; updates health-check command to strip $JMX_OPT from ACTIVEMQ_OPTS before executing the probe.
JMX Exporter Integration
jmx.yml
New JMX exporter config: enables lowercase metric/label output; defines whitelist/blacklist and regex-based metric rules for queues, topics, broker connection counts, various *Count metrics (COUNTER), and PercentUsage (GAUGE with valueFactor 0.01); maps attributes to Prometheus-like metric names and labels.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Builder as Build stage
  participant Image as Docker image (final)
  participant Init as Container init/start
  participant JVM as ActiveMQ JVM
  participant JMX as jmx_prometheus_javaagent
  participant Health as healthcheck.sh

  rect rgb(230, 245, 255)
    Builder->>Image: ADD jmx_prometheus_javaagent.jar (with checksum)\nCOPY jmx.yml\nSet ENV JMX_OPT
  end

  rect rgb(245, 250, 230)
    Init->>Image: container start
    Init->>JVM: launch JVM with ACTIVEMQ_OPTS (includes $JMX_OPT)
    JVM->>JMX: attach javaagent on port 3001
    JMX-->>JVM: expose metrics endpoint :3001/metrics
  end

  rect rgb(255, 243, 230)
    Health->>Image: run healthcheck.sh
    Health->>Health: strip $JMX_OPT from ACTIVEMQ_OPTS
    Health->>JVM: perform probe (HTTP/management endpoint)
    JVM-->>Health: return health status
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Pay special attention to:
    • Dockerfile ADD with checksum: verify JMX_EXPORTER_DIGEST correctness and ADD checksum syntax.
    • jmx.yml regex rules, metric naming, types (GAUGE/COUNTER), and valueFactor handling.
    • Environment composition: ensure JMX_OPT + ACTIVEMQ_OPTS interplay doesn't break other startup paths or permission contexts.
    • healthcheck.sh modification: confirm the stripping approach correctly removes only the JMX agent args.

Poem

🐇 In a burrow of bits I hop and sing,
I added a jar and a YAML ring,
Metrics now bloom on port three-thousand-one,
Healthcheck skips what the agent has spun —
Prometheus peeks as queues hum along!

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'SEC-1601: jvm metrics' is vague and overly broad. While the PR does add JMX metrics (Java Management Extensions), the title fails to convey that this is specifically about Prometheus JMX exporter integration into ActiveMQ. Consider a more descriptive title such as 'Add Prometheus JMX exporter integration to ActiveMQ' or 'Enable JMX metrics export via Prometheus exporter' to clearly indicate the main change.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch SEC-1601

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • JIRA integration encountered authorization issues. Please disconnect and reconnect the integration in the CodeRabbit UI.

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Alexander-Cairns Alexander-Cairns added the minor Added functionality that is backwards compatible. label Nov 14, 2025
@Alexander-Cairns
Alexander-Cairns marked this pull request as ready for review November 14, 2025 20:30

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Dockerfile (1)

10-10: Port 3001 (JMX metrics) is not exposed.

The JMX exporter is configured to run on port 3001 (line 46) but the EXPOSE statement does not include it. Metrics will be unreachable from outside the container.

Add port 3001 to the EXPOSE statement:

-EXPOSE 8161 61613 61616
+EXPOSE 8161 61613 61616 3001
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3ba3bb1 and b2e1ee7.

📒 Files selected for processing (3)
  • Dockerfile (1 hunks)
  • healthcheck.sh (1 hunks)
  • jmx.yml (1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
healthcheck.sh

[warning] 4-4: In POSIX sh, string replacement is undefined.

(SC3060)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build / build-push
🔇 Additional comments (2)
jmx.yml (1)

1-36: Verify jmx_exporter configuration against actual ActiveMQ MBean names.

The configuration looks structurally sound for Prometheus JMX exporter, but the regex patterns should be validated against actual ActiveMQ broker MBean attribute names to ensure they correctly match and extract metrics.

To verify the patterns work correctly, confirm:

  • ActiveMQ broker exposes MBeans matching org.apache.activemq:type=Broker,brokerName=*
  • Queue/topic destination names match the pattern separators (spaces after commas in line 12, 18)
  • The valueFactor: 0.01 correctly converts percent usage to 0–1 range

If you have access to a running ActiveMQ instance, you can verify the MBean names by inspecting JMX metrics. Alternatively, consult the jmx_prometheus_exporter documentation to confirm the pattern syntax (particularly the angle bracket format <type=Broker, ...><> on lines 12, 18, etc.).

Dockerfile (1)

35-43: Based on my verification:

Update Dockerfile to use the latest JMX exporter version 1.5.0 and verify the digest.

Version 1.4.0 was released Aug 25, 2025, but version 1.5.0 (published Oct 7, 2025) is the latest release. While 1.4.0 is technically valid, using an outdated version introduces maintenance risk. GitHub Release page hosts the JARs (collector jar no longer published to Maven Central).

Manually verify the SHA256 digest db1492e95a7ee95cd5e0a969875c0d4f0ef6413148d750351a41cc71d775f59a matches the official GitHub release artifact for version 1.4.0 before proceeding.

Comment thread Dockerfile
Comment thread healthcheck.sh
Comment thread healthcheck.sh

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
Dockerfile (1)

47-47: ACTIVEMQ_OPTS overwrite loses pre-existing environment settings.

Line 47 sets ENV ACTIVEMQ_OPTS="${JMX_OPT}" which overwrites any pre-existing ACTIVEMQ_OPTS value (e.g., from the base image or previous layers). This should append to the existing value instead.

Apply this diff to preserve existing options:

-ENV ACTIVEMQ_OPTS="${JMX_OPT}"
+ENV ACTIVEMQ_OPTS="${ACTIVEMQ_OPTS:-} ${JMX_OPT}"
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between b2e1ee7 and a2e23b0.

📒 Files selected for processing (1)
  • Dockerfile (1 hunks)

Comment thread Dockerfile
Comment on lines +35 to +44
# renovate: datasource=github-release-attachments depName=prometheus/jmx_exporter
ARG JMX_EXPORTER_VERSION=1.4.0
ARG JMX_EXPORTER_DIGEST=sha256:db1492e95a7ee95cd5e0a969875c0d4f0ef6413148d750351a41cc71d775f59a
WORKDIR /jmx
ADD \
--link \
--chmod=644 \
--checksum=$JMX_EXPORTER_DIGEST \
https://github.com/prometheus/jmx_exporter/releases/download/$JMX_EXPORTER_VERSION/jmx_prometheus_javaagent-$JMX_EXPORTER_VERSION.jar jmx_prometheus_javaagent.jar
COPY --chmod=644 jmx.yml ./

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

❓ Verification inconclusive

Solid security practices with checksummed artifact download.

The renovate comment, ARG declarations with SHA256 digest, and ADD command with --checksum verification provide strong supply-chain security. Permissions are properly set, and the working directory restructuring is clean.

Please verify that your Docker build environment supports the --link flag (Docker 23.0+). If using an earlier version, this build will fail. Run the following to confirm:


🏁 Script executed:

#!/bin/bash
# Check Docker version
docker --version

Length of output: 44


The web search reveals an important correction: the --link flag in Dockerfile (used in ADD/COPY commands) requires Dockerfile syntax version to be at least 1.4, not Docker version 23.0+. Add # syntax=docker/dockerfile:1.4 to the top of your Dockerfile to enable the --link flag.

The code snippet provided does not show a syntax declaration at the beginning of the Dockerfile. This is a critical issue that needs to be addressed.

Add Dockerfile syntax declaration for --link flag support.

The Dockerfile uses --link in the ADD command (line 40) but lacks the required syntax directive. To use the --link flag set Dockerfile version to at least 1.4 by adding # syntax=docker/dockerfile:1.4 to the top of the file.

🤖 Prompt for AI Agents
In Dockerfile around lines 35 to 44, the ADD uses the --link flag but the file
lacks the required Dockerfile syntax directive; add a syntax declaration as the
very first line of the Dockerfile such as specifying the Dockerfile frontend
version (e.g. # syntax=docker/dockerfile:1.4 or a newer compatible tag) so the
--link flag is recognized, then keep the rest of the file unchanged.

@lutaylor
lutaylor merged commit 256a1de into main Nov 25, 2025
3 checks passed
@lutaylor
lutaylor deleted the SEC-1601 branch November 25, 2025 15:29
@github-actions

Copy link
Copy Markdown

Tag generated by PR: v1.3.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

minor Added functionality that is backwards compatible.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants