Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 92 additions & 0 deletions .claude/skills/add-gc-test/SKILL.md
Original file line number Diff line number Diff line change
@@ -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<GCName>() throws IOException {
try (final MisbehavingJMXServer server = new MisbehavingJMXServer.Builder().withJDKImage(
<JDK_IMAGE>).appendJavaOpts("<-XX:+UseXxxGC>").build()) {
final List<Map<String, Object>> actualMetrics = startAndGetMetrics(server, true);
assertThat(actualMetrics, hasSize(13));
assertGCMetric(actualMetrics,
"jvm.gc.minor_collection_count", "<YoungCollectorName>", "counter");
assertGCMetric(actualMetrics,
"jvm.gc.minor_collection_time", "<YoungCollectorName>", "counter");
assertGCMetric(actualMetrics,
"jvm.gc.major_collection_count", "<OldCollectorName>", "counter");
assertGCMetric(actualMetrics,
"jvm.gc.major_collection_time", "<OldCollectorName>", "counter");
}
}
```

### GC with more than 2 active collectors (e.g., Generational ZGC with 4)
Use the `assertGCMetric(actualMetrics, metric, List<String> gcGenerations)` overload:
```java
assertGCMetric(actualMetrics, "jvm.gc.major_collection_count",
Arrays.asList("<Collector1>", "<Collector2>"));
```

### 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<Descriptive Suffix>`
- 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.
219 changes: 219 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
@@ -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`
1 change: 1 addition & 0 deletions CLAUDE.md
30 changes: 30 additions & 0 deletions src/test/java/org/datadog/jmxfetch/TestGCMetrics.java
Original file line number Diff line number Diff line change
Expand Up @@ -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<Map<String, Object>> 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(
Expand Down Expand Up @@ -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<Map<String, Object>> 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(
Expand Down
Loading