SEC-1601: Java memory percentage #26
Conversation
WalkthroughReplaces CANTALOUPE_MEM/JAVA_OPTS with a single JAVA_MEMORY env, adds Prometheus JMX exporter (build ARGs, downloaded jar, Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Operator
participant Container
participant Entrypoint
participant JVM as JVM/Cantaloupe
participant JMX as JMX Agent
participant Prometheus
Operator->>Container: docker run ...
activate Container
Container->>Entrypoint: start entrypoint script
Entrypoint->>JVM: exec java $JAVA_MEMORY -javaagent:/jmx/jmx_prometheus_javaagent.jar=3001:/jmx/jmx.yml -server -D... -jar cantaloupe.jar
activate JVM
Note over JVM,JMX: JMX agent attached at JVM startup (port 3001)
Prometheus->>JMX: HTTP GET /metrics
JMX-->>Prometheus: metrics payload
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
jmx.yml (1)
1-2: Scope JMX metrics to avoid high-cardinality/overexposure.A catch-all rule will export every MBean/attribute, which can bloat metrics and leak internals. Narrow to memory/GC and normalize labels.
Apply:
-rules: -- pattern: ".*" +lowercaseOutputName: true +lowercaseOutputLabelNames: true +whitelistObjectNames: + - 'java.lang:type=Memory,*' + - 'java.lang:type=MemoryPool,*' + - 'java.lang:type=GarbageCollector,*' +rules: + - pattern: 'java.lang<type=(Memory|MemoryPool|GarbageCollector),.*>.*'Dockerfile (4)
130-130: Prefer container-aware heap sizing by default.Percent-based flags adapt better to cgroup limits while remaining overrideable via env.
-ENV JAVA_MEMORY="-Xms1g -Xmx1g" +ENV JAVA_MEMORY="-XX:InitialRAMPercentage=25 -XX:MaxRAMPercentage=75"
178-178: Make agent bind/port and extra JVM flags configurable; keep JAVA_OPTS compatibility.This helps different deployments and preserves a common extension point.
-exec java $JAVA_MEMORY -javaagent:/jmx/jmx_prometheus_javaagent.jar=3001:/jmx/jmx.yml -server -Djava.awt.headless=true -Dcantaloupe.config=${CANTALOUPE_PROPERTIES} -jar cantaloupe.jar +exec java ${JAVA_OPTS:-} ${JAVA_MEMORY:-} \ + -javaagent:/jmx/jmx_prometheus_javaagent.jar=${JMX_EXPORTER_BIND_HOST:-0.0.0.0}:${JMX_EXPORTER_PORT:-3001}:${JMX_EXPORTER_CONFIG:-/jmx/jmx.yml} \ + -server -Djava.awt.headless=true -Dcantaloupe.config="${CANTALOUPE_PROPERTIES}" -jar cantaloupe.jarOptional: allow disabling by guarding with JMX_EXPORTER_ENABLED and omitting the flag when set to "false".
134-135: Advertise metrics port.Expose 3001 (or document it) so it’s discoverable to orchestrators.
-EXPOSE 8080 +EXPOSE 8080 3001
10-13: Base image OS is Ubuntu 20.04 (focal), now ESM-only. Consider jammy.Moving to a jammy-based Temurin image improves security updates and longevity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
Dockerfile(2 hunks)jmx.yml(1 hunks)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Dockerfile (1)
172-173: Implement remote ADD with integrity verification and explicit ownership.Remote ADD lacks integrity checking and can be flaky. Additionally, after switching to the cantaloupe user (line 159), the COPY on line 173 should explicitly set ownership. Previous review feedback on this commit already suggested these improvements.
Apply this diff to add SHA256 verification and proper ownership:
-WORKDIR /jmx -ADD --link --chmod=644 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 ./ +WORKDIR /jmx +ARG JMX_EXPORTER_SHA256=<fill-in-sha256-hash> +RUN set -eux; \ + curl -fSL -o jmx_prometheus_javaagent.jar \ + "https://github.com/prometheus/jmx_exporter/releases/download/${JMX_EXPORTER_VERSION}/jmx_prometheus_javaagent-${JMX_EXPORTER_VERSION}.jar"; \ + echo "${JMX_EXPORTER_SHA256} jmx_prometheus_javaagent.jar" | sha256sum -c - +COPY --chmod=644 --chown=$CANTALOUPE_UID:$CANTALOUPE_GID jmx.yml ./You'll also need to ensure curl and ca-certificates are available in the base image. Update line 141:
- ffmpeg libopenjp2-tools imagemagick + curl ca-certificates ffmpeg libopenjp2-tools imagemagickTo find the correct SHA256 hash for version 1.4.0, download the
.jarand runsha256sumon it, or check the GitHub release page for a checksum file.
🧹 Nitpick comments (1)
Dockerfile (1)
180-180: Expose JMX metrics port for operational clarity.The JMX agent exports metrics on port 3001, but the Dockerfile only exposes port 8080 (line 134). While users can map the port at runtime, explicitly documenting it here improves discoverability.
Add the JMX port to the EXPOSE directive:
-EXPOSE 8080 +EXPOSE 8080 3001Alternatively, add a comment near line 180 documenting that port 3001 must be mapped for JMX metrics:
+# JMX metrics available on port 3001 (must be exposed separately if needed) exec java $JAVA_MEMORY -javaagent:/jmx/jmx_prometheus_javaagent.jar=3001:/jmx/jmx.yml ...
📜 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.
📒 Files selected for processing (1)
Dockerfile(2 hunks)
⏰ 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 (1)
Dockerfile (1)
130-130: JAVA_MEMORY environment variable is well implemented.The new environment variable allows full customization of Java heap settings at runtime while providing sensible defaults. This satisfies the PR objective of making memory settings customizable from environment variables.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
Dockerfile (1)
178-178: Add explicit ownership to the jmx.yml file.Line 178 should include
--chown=$CANTALOUPE_UID:$CANTALOUPE_GIDto ensure the file is owned by the cantaloupe user, maintaining consistency with other copied artifacts and avoiding potential access issues.-COPY --chmod=644 jmx.yml ./ +COPY --chmod=644 --chown=$CANTALOUPE_UID:$CANTALOUPE_GID jmx.yml ./
🧹 Nitpick comments (1)
Dockerfile (1)
134-134: Consider exposing the JMX monitoring port.The JMX agent runs on port 3001 (from the entrypoint configuration on line 185), but this port is not documented via an
EXPOSEstatement. AddEXPOSE 3001for clarity and to support port discovery in container orchestration platforms.EXPOSE 8080 +EXPOSE 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.
📒 Files selected for processing (1)
Dockerfile(2 hunks)
⏰ 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 (1)
Dockerfile (1)
169-178: Verify Docker version compatibility for ADD --checksum syntax.The
--checksumflag on line 176 requires Docker 1.4.0 or later (released 2024). Confirm that the project's Docker build environment and documentation specify a compatible version, or document the minimum required version.
Summary by CodeRabbit
New Features
Chores