diff --git a/.claude/skills/add-gc-test/SKILL.md b/.claude/skills/add-gc-test/SKILL.md new file mode 100644 index 00000000..f070e3e8 --- /dev/null +++ b/.claude/skills/add-gc-test/SKILL.md @@ -0,0 +1,92 @@ +--- +name: add-gc-test +description: Add integration tests to TestGCMetrics.java for new GC collector support added to new-gc-default-jmx-metrics.yaml. Use when a PR adds a new garbage collector to the metrics YAML without a corresponding test. +allowed-tools: Read Edit Grep Bash(git diff *) Bash(git log *) Bash(./mvnw *) +argument-hint: [branch-or-commit] +--- + +You are helping add an integration test to `TestGCMetrics.java` for new GC collector support. + +## Changes Introducing New GC Collectors + +```diff +!`git diff master -- src/main/resources/org/datadog/jmxfetch/new-gc-default-jmx-metrics.yaml 2>/dev/null || git diff HEAD~1 -- src/main/resources/org/datadog/jmxfetch/new-gc-default-jmx-metrics.yaml 2>/dev/null || echo "(no diff found — check the branch or commit manually)"` +``` + +$ARGUMENTS + +## Your Task + +1. **Read** `src/main/resources/org/datadog/jmxfetch/new-gc-default-jmx-metrics.yaml` — identify which new collector `name:` entries were added and what `alias:` metrics they map to. + +2. **Read** `src/test/java/org/datadog/jmxfetch/TestGCMetrics.java` — understand existing test patterns and pick up on the `startAndGetMetrics` helper and both `assertGCMetric` overloads. + +3. **Read** `src/test/java/org/datadog/jmxfetch/util/server/JDKImage.java` — confirm available JDK image constants (`JDK_8`, `JDK_11`, `JDK_11_OPENJ9`, `JDK_17`, `JDK_21`). + +4. **Add** a new `@Test` method to `TestGCMetrics.java` following the patterns below. + +## Patterns + +### Standard GC with paired minor + major collectors +```java +@Test +public void testDefaultNewGCMetricsUse() throws IOException { + try (final MisbehavingJMXServer server = new MisbehavingJMXServer.Builder().withJDKImage( + ).appendJavaOpts("<-XX:+UseXxxGC>").build()) { + final List> actualMetrics = startAndGetMetrics(server, true); + assertThat(actualMetrics, hasSize(13)); + assertGCMetric(actualMetrics, + "jvm.gc.minor_collection_count", "", "counter"); + assertGCMetric(actualMetrics, + "jvm.gc.minor_collection_time", "", "counter"); + assertGCMetric(actualMetrics, + "jvm.gc.major_collection_count", "", "counter"); + assertGCMetric(actualMetrics, + "jvm.gc.major_collection_time", "", "counter"); + } +} +``` + +### GC with more than 2 active collectors (e.g., Generational ZGC with 4) +Use the `assertGCMetric(actualMetrics, metric, List gcGenerations)` overload: +```java +assertGCMetric(actualMetrics, "jvm.gc.major_collection_count", + Arrays.asList("", "")); +``` + +### Metric count formula +`hasSize(9 + N*2)` where N = number of distinct GC collector names active with that JVM flag. +- 9 = base JVM metrics (heap, threads, classes, etc.) emitted when `collect_default_jvm_metrics: true` + `new_gc_metrics: true` +- Each collector emits 2 metrics: count + time +- Examples: 2 collectors → `hasSize(13)`, 4 collectors → `hasSize(17)` + +### JDK image guidance +| GC flag | JDK image | +|---|---| +| `-XX:+UseSerialGC`, `-XX:+UseParallelGC`, `-XX:+UseConcMarkSweepGC`, `-XX:+UseG1GC` | `JDK_11` | +| `-XX:+UseZGC` (non-generational) | `JDK_17` | +| `-XX:+UseZGC -XX:+ZGenerational` | `JDK_21` | +| `-XX:+UseShenandoahGC` | `JDK_17` or `JDK_21` | +| `-Xgcpolicy:gencon`, `-Xgcpolicy:balanced` | `JDK_11_OPENJ9` | +| GraalVM Native | no `MisbehavingJMXServer` support yet — note this limitation | + +### Metric alias mapping +Look at the alias values in the YAML diff to determine which metric names to assert: +- `jvm.gc.minor_collection_count` / `jvm.gc.minor_collection_time` — young gen collectors +- `jvm.gc.major_collection_count` / `jvm.gc.major_collection_time` — old gen collectors +- Custom aliases (e.g., `jvm.gc.zgc_cycles_collection_count`) — assert those directly + +## Code Style Requirements +- 4-space indentation, no tabs +- Lines ≤ 100 characters — wrap `appendJavaOpts(...)` onto a second line if needed +- Method name in camelCase: `testDefaultNewGCMetrics` +- No Javadoc required on test methods +- Imports: add any missing ones in the correct group (static → special → third-party → java → javax) + +## After Writing the Test +Run Checkstyle to verify formatting: +```bash +./mvnw checkstyle:check 2>&1 | grep -E "ERROR|WARNING|BUILD" | tail -20 +``` + +If the new GC requires a JDK image not in `JDKImage.java`, note that it needs to be added there first. diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 00000000..2bd0247f --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,219 @@ +# JMXFetch Development Guide for AI Agents + +This guide provides essential information for AI coding agents working on the JMXFetch codebase. + +## Build & Test Commands + +### Building +```bash +# Clean build with JAR assembly +./mvnw clean compile assembly:single + +# Quick build without tests (for JDK 8) +./mvnw clean package -DskipTests + +# Full build with Docker (for compatibility with all JARs) +docker run -it --rm -v "$(pwd)":/usr/src/app -w /usr/src/app eclipse-temurin:8-jdk ./mvnw -DskipTests clean package +``` + +### Testing +```bash +# Run all tests +./mvnw test + +# Run a single test class +./mvnw test -Dtest=TestApp + +# Run a specific test method +./mvnw test -Dtest=TestApp#testBeanTags + +# Run tests with verbose logging +./mvnw test -Dtests.log_level=info + +# Run tests on macOS/Windows with Docker (for TestContainers compatibility) +docker run -it --rm -e TESTCONTAINERS_HOST_OVERRIDE=host.docker.internal \ + -v $PWD:$PWD -w $PWD -v /var/run/docker.sock:/var/run/docker.sock \ + eclipse-temurin:8-jdk ./mvnw test +``` + +### Linting & Code Quality +```bash +# Run Checkstyle analysis +./mvnw checkstyle:check + +# Verify without running tests +./mvnw verify -DskipTests + +# Skip Checkstyle (not recommended) +./mvnw test -Dcheckstyle.skip=true +``` + +## Code Style Guidelines + +JMXFetch follows **Google Java Style** with customizations defined in `style.xml`. + +### Formatting Rules + +- **Indentation**: 4 spaces (NO tabs) +- **Line length**: 100 characters max (exceptions for imports and URLs) +- **Braces**: Always required, even for single-line if/for/while statements +- **One statement per line**: No multiple statements on the same line +- **Empty blocks**: Use `{}` or provide a TEXT comment + +### Import Organization + +Imports MUST be organized in this order (enforced by Checkstyle): +1. **Static imports** (e.g., `import static org.junit.Assert.assertEquals;`) +2. **Special imports** (e.g., `com.google`) +3. **Third-party packages** (e.g., `org.datadog`, `lombok`, `org.yaml`) +4. **Standard Java packages** (e.g., `java.io`, `java.util`, `javax.management`) + +Within each group, imports are **alphabetically sorted**. + +**NO star imports** - always use specific imports (e.g., `import java.util.List;` not `import java.util.*;`) + +Example import block: +```java +package org.datadog.jmxfetch; + +import static org.junit.Assert.assertEquals; + +import lombok.extern.slf4j.Slf4j; + +import org.datadog.jmxfetch.reporter.Reporter; +import org.datadog.jmxfetch.util.CustomLogger; + +import java.io.File; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + +import javax.management.ObjectName; +``` + +### Naming Conventions + +- **Classes/Interfaces**: PascalCase (e.g., `AppConfig`, `TaskProcessor`) +- **Methods**: camelCase, minimum 3 characters (e.g., `addInstanceStats`, `flush`) +- **Variables/Parameters**: camelCase, minimum 3 characters (e.g., `instanceStats`, `checkName`) +- **Constants**: UPPER_SNAKE_CASE (e.g., `STATUS_WARNING`, `MAX_RETURNED_METRICS`) +- **Packages**: lowercase, no underscores (e.g., `org.datadog.jmxfetch.tasks`) +- **Type parameters**: Single capital letter or PascalCase ending in T (e.g., `T`, `ValueT`) + +### Types & Annotations + +- **Lombok**: Used extensively for reducing boilerplate + - `@Slf4j` for logging (provides `log` field) + - `@Builder` for builder pattern (use in AppConfig and similar) + - Enable annotation processors in your IDE +- **Suppress Warnings**: Use `@SuppressWarnings("unchecked")` when necessary for type casts +- **Nullability**: No explicit annotations; use defensive null checks + +### Documentation + +- **Public methods**: Should have Javadoc (minimum 2 lines to require Javadoc) +- **Javadoc style**: + - Start with `/**`, end with `*/` + - Order: `@param`, `@return`, `@throws`, `@deprecated` + - Summary sentence should not start with "This method returns" or "@return the" +- **Test methods**: Use `@Test` annotation, Javadoc not required + +### Logging + +Use SLF4J via Lombok's `@Slf4j` annotation: + +```java +@Slf4j +public class MyClass { + public void myMethod() { + log.debug("Debug message: {}", value); + log.info("Info message"); + log.warn("Warning message: {}", exception.getMessage()); + log.error("Error occurred", exception); + } +} +``` + +**Logging levels**: +- `log.debug()`: Detailed debugging information +- `log.info()`: General informational messages +- `log.warn()`: Warning conditions, recoverable errors +- `log.error()`: Error conditions, exceptions + +### Error Handling + +- **Custom Exceptions**: Extend `Exception` (e.g., `TaskProcessException`) +- **Exception naming**: End with "Exception" (e.g., `JsonException`) +- **Catch blocks**: Empty catch blocks ONLY allowed with variable name `expected` +- **Logging exceptions**: Include exception object in log calls for stack traces + +Example: +```java +try { + processData(); +} catch (IOException e) { + log.error("Failed to process data: {}", e.getMessage(), e); + throw new TaskProcessException("Data processing failed"); +} +``` + +## Testing Patterns + +- **Framework**: JUnit 4 (not JUnit 5) +- **Test class naming**: Prefix with `Test` (e.g., `TestApp`, `TestCommon`) +- **Test method naming**: Prefix with `test` (e.g., `testBeanTags`, `testBeanRegexTags`) +- **Assertions**: Use static imports from `org.junit.Assert` +- **Mocking**: Use Mockito (`spy()`, `when()`, `verify()`) +- **Test base**: Extend `TestCommon` for JMX-related tests +- **Setup**: Use `@BeforeClass` for class-level setup, `@After` for cleanup +- **TestContainers**: Some tests use Docker containers (requires Docker daemon) + +## Common Patterns + +### File References +When referencing code locations in messages or logs, use the format: +``` +src/main/java/org/datadog/jmxfetch/Instance.java:142 +``` + +### Constants +Define constants at the top of the class: +```java +public class MyClass { + private static final String STATUS_OK = "OK"; + private static final int MAX_RETRIES = 3; +``` + +### Thread Safety +Use `ConcurrentHashMap` for shared mutable state across threads. + +## Project Structure + +``` +src/main/java/org/datadog/jmxfetch/ +├── App.java # Main application entry point +├── Instance.java # JMX instance management +├── Configuration.java # Configuration parsing +├── reporter/ # Metric reporters +├── tasks/ # Task processing framework +├── util/ # Utility classes +└── validator/ # Validation logic + +src/test/java/org/datadog/jmxfetch/ +└── Test*.java # Test classes +``` + +## Development Environment + +- **JDK**: Use JDK 8 for development (JDK 7-24 supported for runtime) +- **JDK Management**: Use `sdkman` - run `sdk env` to activate project JDK +- **IDE Setup**: Enable annotation processors for Lombok support +- **Maven Wrapper**: Always use `./mvnw` instead of system Maven + +## Important Notes + +- Target compatibility is Java 1.7, so avoid Java 8+ language features +- Checkstyle runs automatically before compilation +- All PRs must pass CI tests on multiple JDK versions (8, 11, 17, 21, 24) +- Status file location: Test output goes to `/tmp/jmxfetch_test.log` diff --git a/CLAUDE.md b/CLAUDE.md new file mode 120000 index 00000000..47dc3e3d --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +AGENTS.md \ No newline at end of file diff --git a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java index a180c35e..9a4acca0 100644 --- a/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java +++ b/src/test/java/org/datadog/jmxfetch/TestGCMetrics.java @@ -77,6 +77,23 @@ public void testDefaultNewGCMetricsUseParallelGC() throws IOException { } } + @Test + public void testDefaultNewGCMetricsUseSerialGC() throws IOException { + try (final MisbehavingJMXServer server = new MisbehavingJMXServer.Builder().withJDKImage( + JDK_11).appendJavaOpts("-XX:+UseSerialGC").build()) { + final List> actualMetrics = startAndGetMetrics(server, true); + assertThat(actualMetrics, hasSize(13)); + assertGCMetric(actualMetrics, + "jvm.gc.minor_collection_count", "Copy", "counter"); + assertGCMetric(actualMetrics, + "jvm.gc.minor_collection_time", "Copy", "counter"); + assertGCMetric(actualMetrics, + "jvm.gc.major_collection_count", "MarkSweepCompact", "counter"); + assertGCMetric(actualMetrics, + "jvm.gc.major_collection_time", "MarkSweepCompact", "counter"); + } + } + @Test public void testDefaultNewGCMetricsUseConcMarkSweepGC() throws IOException { try (final MisbehavingJMXServer server = new MisbehavingJMXServer.Builder().withJDKImage( @@ -128,6 +145,19 @@ public void testDefaultNewGCMetricsUseZGC() throws IOException { } } + @Test + public void testDefaultNewGCMetricsUseShenandoahGC() throws IOException { + try (final MisbehavingJMXServer server = new MisbehavingJMXServer.Builder().withJDKImage( + JDK_17).appendJavaOpts("-XX:+UseShenandoahGC").build()) { + final List> actualMetrics = startAndGetMetrics(server, true); + assertThat(actualMetrics, hasSize(11)); + assertGCMetric(actualMetrics, + "jvm.gc.major_collection_count", "Shenandoah Cycles", "counter"); + assertGCMetric(actualMetrics, + "jvm.gc.major_collection_time", "Shenandoah Cycles", "counter"); + } + } + @Test public void testDefaultNewGCMetricsUseGenerationalZGC() throws IOException { try (final MisbehavingJMXServer server = new MisbehavingJMXServer.Builder().withJDKImage(