Skip to content
Open
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
133 changes: 133 additions & 0 deletions .claude/CLAUDE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
# Similar Products Service

Spring Boot 3.3 reactive application that exposes `GET /product/{productId}/similar` on port 5000.

## Architecture

Aggregates two upstream APIs on port 3001 (Simulado mock):
1. `GET /product/{id}/similarids` → list of product IDs ordered by similarity
2. `GET /product/{id}` → product detail

All detail requests run in **parallel** via `flatMapSequential` (preserves similarity order while fetching concurrently).

```
Client → SimilarProductsController
→ SimilarProductsService
→ ProductClient.getSimilarIds() (sequential first)
→ ProductClient.getProductDetail() (all in parallel)
```

## Tech Stack

- Java 21, Spring Boot 3.3, Spring WebFlux (Project Reactor + Netty)
- Maven

## Build & Run

### Locally (requires Java 21 + Maven)
```bash
cd app
mvn spring-boot:run
```

### Docker Compose (builds and runs everything)
```bash
docker-compose up -d simulado influxdb grafana yourapp
```

## Testing

```bash
cd app && mvn test # 31 tests, ~25s — nothing needs to be running
```

The upstream API is stubbed in-process with MockWebServer, so the suite needs neither Docker nor Simulado.
Maven runs on JDK 21 (`~/.jdks/jdk-21.0.6`) even though `java` on the PATH is 17.

| Test class | Covers |
|------------|--------|
| `ProductClientTest` | Timeouts, 404/500 skipping, JSON id coercion, malformed bodies |
| `SimilarProductsServiceTest` | Similarity order, parallel fetching, dropping unresolvable products |
| `SimilarProductsControllerTest` | 200 / `[]` / 404 / 5xx mapping |
| `SimilarProductsIntegrationTest` | The five mock scenarios end-to-end over a real socket |

See the **write-tests** skill for conventions and the timing pitfalls (cold-start flakiness, virtual-time hangs).

## Load Testing

```bash
# Run k6 test (app + infra must be up)
docker-compose run --rm k6 run scripts/test.js

# View results in Grafana
# http://localhost:3000/d/Le2Ku9NMk/k6-performance-test
```

## Key Design Decisions

| Decision | Reason |
|----------|--------|
| WebClient (non-blocking) | 200 concurrent VUs; reactive I/O avoids thread-per-request overhead |
| `flatMapSequential` | Parallel HTTP calls, results emitted in original similarity order |
| 2s per-product timeout | Mocks have 5s/50s delays that must be bounded for acceptable p99 |
| Skip on 404/500/timeout | Individual product failures should not fail the whole request |
| 404 from similarids → 404 response | Contract requirement; means the base product has no similar IDs |
| `similar-ids-timeout-ms` (2s) | Entry-point call was unbounded; could hang on connection contention |
| No cache | Throughput is capped by k6 client pacing + timeout, not upstream calls — a cache can't beat that ceiling here |
| `max-connections: 50` (balanced) | See performance note below — the main tuning lever |

## Performance — the connection pool is the key lever

Load testing (see `app/README.md` for the full data) established:
- **The single-process mock is the bottleneck, not our pool.** Smaller pool = higher throughput
(pool 8 → ~289 req/s; pool 128 → ~77 req/s). Oversizing overloads the mock.
- **A small pool is fast because it silently drops available products under load** (the doomed 5s/50s
products monopolise connections). Completeness rises with pool size (pool 8 → 12%; pool 100+ → 100%).
- Throughput vs completeness is a **Pareto trade-off**; `max-connections: 50` is the chosen balance
(fast + correct in normal use, graceful degradation under extreme load).
- Presets (via `PRODUCT_API_MAX_CONNECTIONS`): max throughput = 8–16; max correctness = 150–200.

When investigating performance, do NOT reflexively enlarge the pool — measure first.

## Logging

Rule: **WARN is for what makes a request fail; DEBUG is for what the design deliberately tolerates.**
Skipping a product is designed behaviour that happens on nearly every request under load, so it must
never be logged above DEBUG — it would bury the failures that matter.

| Level | What | Frequency |
|-------|------|-----------|
| INFO | Effective client config + timeouts | 2 lines, once at startup |
| WARN | `/similarids` returned 5xx or timed out; request resolved to a 5xx | Only on a request that fails |
| DEBUG | IDs received, each skipped product + reason, products resolved + elapsed ms | ~2 lines/request + 1 per skip |

At INFO the whole k6 scenario set produces **zero** per-request lines. Set `LOG_LEVEL=DEBUG` to trace a
request end to end — never during a load test.

A 404 from `/similarids` is a client outcome, not a fault: DEBUG, not WARN. `ProductClientLoggingTest`
pins these levels so they can't be relaxed by accident.

## Mock Scenarios (port 3001)

| Our endpoint | Similar IDs | What happens |
|-------------|-------------|--------------|
| /product/1/similar | [2,3,4] | Fast — all 3 products return quickly |
| /product/2/similar | [3,100,1000] | Product 100 (1s) OK; product 1000 (5s) → timeout → skipped |
| /product/3/similar | [100,1000,10000] | Product 100 OK; 1000 and 10000 → timeout → skipped |
| /product/4/similar | [1,2,5] | Product 5 returns 404 → skipped; returns [1,2] |
| /product/5/similar | [1,2,6] | Product 6 returns 500 → skipped; returns [1,2] |

## Configuration

All properties live under `product-api` in `application.yaml` and map to an env var (Spring relaxed
binding), so they can be overridden without a rebuild. See `app/README.md` for the full reference.

| Property | Env var | Default | Description |
|----------|---------|---------|-------------|
| `product-api.base-url` | `PRODUCT_API_BASE_URL` | `http://localhost:3001` | Upstream API base URL (`http://simulado` in Docker) |
| `product-api.connect-timeout-ms` | `PRODUCT_API_CONNECT_TIMEOUT_MS` | 1000 | TCP connect timeout |
| `product-api.detail-timeout-ms` | `PRODUCT_API_DETAIL_TIMEOUT_MS` | 2000 | Per-product detail request timeout |
| `product-api.similar-ids-timeout-ms` | `PRODUCT_API_SIMILAR_IDS_TIMEOUT_MS` | 2000 | Entry-point (similarids) call timeout |
| `product-api.max-connections` | `PRODUCT_API_MAX_CONNECTIONS` | 50 | Outbound connection pool size — the main perf lever |
| `product-api.pending-acquire-timeout-ms` | `PRODUCT_API_PENDING_ACQUIRE_TIMEOUT_MS` | 2000 | Max wait for a pooled connection |
| `logging.level.com.inditex.similarproducts` | `LOG_LEVEL` | `INFO` | `DEBUG` traces every request; keep at INFO under load |
54 changes: 54 additions & 0 deletions .claude/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# .claude

Project-specific configuration for Claude Code for the **Similar Products** service.

## Structure

```
.claude/
├── agents/ Subagents (name + description frontmatter → the system prompt)
│ ├── spring-boot-dev.md Reactive Spring Boot developer with full project context
│ ├── test-engineer.md Writes/runs/diagnoses the automated test suite
│ └── performance-analyst.md Interprets k6 results and proposes improvements
├── commands/ Slash commands (user-triggered operational shortcuts)
│ ├── run-app.md → /run-app Build & start the app locally on port 5000
│ ├── test.md → /test Run the k6 load test
│ ├── unit-test.md → /unit-test Run the JUnit suite (mvn test; nothing needs to be running)
│ └── infra.md → /infra Start/stop/check the Docker infrastructure
├── skills/ Skills (each is a DIRECTORY with a SKILL.md; model-invocable)
│ ├── check-endpoints/SKILL.md Smoke-test the 5 scenarios and validate responses
│ ├── write-tests/SKILL.md Conventions & pitfalls for adding tests
│ └── analyze-performance/SKILL.md Run/interpret the load test and recommend fixes
├── hooks/ Shell scripts wired from settings.json (JSON on stdin)
│ ├── report-infra-status.sh SessionStart: reports whether mock + app are up
│ ├── guard-blocking-calls.sh PreToolUse(Write|Edit): blocks .block()/Thread.sleep in main sources
│ └── verify-app-running.sh PreToolUse(Bash): blocks the k6 test if the app is down
├── CLAUDE.md Project context, loaded automatically each session
├── README.md This file
└── settings.json Permissions + hook wiring
```

## Format notes (why it's laid out this way)

- **Skills are directories**, not flat files: `.claude/skills/<name>/SKILL.md`. The directory name is what
becomes invocable. Frontmatter uses `name`, `description`, and `allowed-tools`.
- **Commands** are flat `.md` files under `commands/`; the filename becomes the `/command`. `run-app` is named
to avoid colliding with Claude Code's built-in `run` skill.
- **Hooks** receive the tool call as **JSON on stdin** (there is no `$CLAUDE_TOOL_EXIT_CODE`). In `settings.json`,
`matcher` filters by **tool name** only (regex, e.g. `Write|Edit`). There is **no field that filters on command
content** — a `Bash` matcher fires on *every* command, so any command filter has to live inside the script
(see `verify-app-running.sh`, which exits 0 for anything that isn't a k6 run). Exit code `2` blocks the action
and sends stderr back as feedback.

## Quick reference

| You want to… | Do |
|--------------|-----|
| Start the app locally | `/run-app` |
| Start / stop Docker infra | `/infra` |
| Run the k6 load test | `/test` |
| Run the JUnit test suite | `/unit-test` |
| Smoke-test all 5 scenarios | ask for the **check-endpoints** skill |
| Analyze load-test performance | ask for the **analyze-performance** skill |
| Add or fix tests | ask for the **write-tests** skill, or the **test-engineer** agent |
| Implement / debug a feature | delegate to the **spring-boot-dev** agent |
72 changes: 72 additions & 0 deletions .claude/agents/performance-analyst.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
---
name: performance-analyst
description: Analyzes k6 load test results and Spring Boot metrics to identify bottlenecks and suggest improvements for the Similar Products service. Knows the measured performance characteristics of this specific system.
tools: Read, Grep, Glob, Bash
model: inherit
color: orange
---

You are a performance analyst for the **Similar Products Service** load tests.

## Test setup

- **Tool:** k6 (loadimpact/k6:0.28.0 — note: predates the `http_req_failed` metric)
- **VUs:** 200 concurrent virtual users per scenario, each with `sleep(0.5)` between iterations
- **Duration:** 10s per scenario
- **Results:** InfluxDB → Grafana at `http://localhost:3000/d/Le2Ku9NMk/k6-performance-test`, plus the
k6 stdout summary (the most reliable source in this k6 version)

## Scenarios and expected behaviour

| Scenario | Endpoint | Expected latency | Notes |
|----------|----------|------------------|-------|
| normal | /product/1/similar | tens of ms | all 3 upstreams fast |
| notFound | /product/4/similar | tens of ms | product 5 (404) skipped |
| error | /product/5/similar | tens of ms | product 6 (500) skipped |
| slow | /product/2/similar | ~2s (timeout-bound) | product 1000 (5s) hits the 2s timeout |
| verySlow | /product/3/similar | ~2s (timeout-bound) | products 1000 + 10000 hit the 2s timeout |

p90/p95 ≈ 2s is EXPECTED, not a bug — it is the intentional timeout on the slow scenarios.

## What this system's performance actually looks like (measured — do not re-derive from scratch)

1. **The single-process mock (simulado) is the bottleneck, not our connection pool.** Throughput is
*inversely* related to pool size: pool 8 → ~289 req/s, pool 32 → ~206 req/s, pool 128 → ~77 req/s.
Oversizing floods the mock and degrades every response. **Never reflexively enlarge the pool.**
2. **A small pool inflates throughput by silently dropping valid products.** The doomed slow products
(1000/10000) hold connections for the full 2s timeout and starve the available 1s product (100).
Completeness of `/product/2/similar` under load: pool 8 → ~12%, pool 50 → ~25–56%, pool 100+ → 100%.
k6 does not validate response bodies, so this is invisible on the dashboard — verify it separately.
3. **Throughput vs completeness is a Pareto trade-off.** Default is `max-connections: 50` (balanced).
Presets via `PRODUCT_API_MAX_CONNECTIONS`: max throughput = 8–16; max correctness = 150–200.

## How to measure

- **Throughput / latency:** `docker-compose run --rm k6 run scripts/test.js` and read the stdout summary
(`http_req_duration`, `http_reqs`, `iterations`).
- **Errors / resilience:** check app logs (`docker-compose logs yourapp`) for exceptions; there is no
`http_req_failed` in this k6 version, so confirm 0 errors via logs + all requests completing.
- **Result completeness under load:** fire N concurrent requests to a slow endpoint and count how many
responses contain the available slow product, e.g.:
```bash
for i in $(seq 1 80); do (curl -s http://localhost:5000/product/2/similar > /tmp/r$i.json) & done; wait
grep -l '"id":"100"' /tmp/r*.json | wc -l # want 80/80
```
- **Sweep a parameter without rebuilding:** every property maps to an env var, so restart the
container with a different `PRODUCT_API_MAX_CONNECTIONS`/`PRODUCT_API_DETAIL_TIMEOUT_MS` and re-run.
Run configs back-to-back to cancel host-load noise.

## Levers, ranked by real impact here

1. **`max-connections`** — the dominant lever (see the Pareto trade-off above). Pick the point on the
frontier that matches the goal; do not just make it bigger.
2. **Timeouts** (`detail-timeout-ms`, `similar-ids-timeout-ms`) — bound tail latency. Shortening the
detail timeout was tested and does NOT improve completeness (that is demand-bound, not hold-bound).
3. **Do NOT propose caching for the current setup.** It was measured and rejected: throughput is capped
by the k6 client `sleep(0.5)` and the 2s timeout, so a cache cannot raise the measured numbers — it
would only cut upstream volume (not the bottleneck) while adding memory + staleness.
*Only* revisit caching if the upstream becomes a real, scalable API rather than this single-process
mock — then a short-TTL Caffeine `AsyncCache` with request coalescing becomes the top optimisation.

Always report findings as: measured numbers → gap vs target → concrete change (with file), and state
which side of the throughput/completeness trade-off the change moves.
74 changes: 74 additions & 0 deletions .claude/agents/spring-boot-dev.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
---
name: spring-boot-dev
description: Spring Boot reactive developer with full context of the Similar Products service. Use for implementing features, debugging reactive chains, and reviewing code changes.
tools: Read, Edit, Write, Grep, Glob, Bash
model: inherit
color: green
---

You are working on the **Similar Products Service** — a Spring Boot 3.3 + Java 21 + WebFlux reactive microservice located in the `app/` directory.

## Your context

**What it does:** Exposes `GET /product/{productId}/similar` on port 5000 by aggregating two upstream APIs on port 3001:
1. `/product/{id}/similarids` → list of IDs
2. `/product/{id}` → product detail (called in parallel for all IDs)

**Package:** `com.inditex.similarproducts`

**Key files:**
- `app/src/main/java/com/inditex/similarproducts/client/ProductClient.java` — HTTP calls with timeout and error handling
- `app/src/main/java/com/inditex/similarproducts/service/SimilarProductsService.java` — `flatMapSequential` for parallel+ordered fetching
- `app/src/main/java/com/inditex/similarproducts/controller/SimilarProductsController.java` — 404 propagation
- `app/src/main/java/com/inditex/similarproducts/config/WebClientConfig.java` — connection pool (the main perf lever)
- `app/src/main/resources/application.yaml` — port 5000, timeouts, pool, base URL
- `app/README.md` — architecture + the measured performance analysis (read before touching perf)

## Rules for this codebase

- **Never block inside a reactive chain.** No `block()`, no `Thread.sleep()`, no synchronous I/O.
(A PreToolUse hook enforces this on `src/main`.)
- **Use `flatMapSequential`** when fetching product details — parallelism with order preservation.
- **Timeouts are intentional.** Products 1000 (5s) and 10000 (50s) must be skipped; the 2s timeout does this.
Both the detail call AND the `similarids` entry-point call are timeout-bounded.
- **Skip, don't fail.** A 404/500/timeout on an individual product detail must resolve to `Mono.empty()`, not an error.
- **`ProductNotFoundException`** is only thrown when the `/similarids` endpoint itself returns 404.
- **Do NOT add a cache.** It was measured and rejected — throughput is capped by the k6 client pacing and
the timeout, not by upstream call count, so a cache adds memory + staleness for no measurable gain here.
- **Do NOT reflexively grow the connection pool.** `max-connections` is a throughput↔completeness Pareto
trade-off (the single-process mock is the bottleneck); measure before changing it. Default is 50.
- **`PRODUCT_API_BASE_URL`** env var controls the upstream: `http://localhost:3001` locally, `http://simulado` in Docker.
- **Logging: WARN only for what fails a request; DEBUG for what the design tolerates.** A skipped product
(404/500/timeout) runs on nearly every request under load — logging it above DEBUG buries real failures.
Use parameterised SLF4J (`log.debug("... {}", id)`), never string concatenation, and never log per product
at INFO. `LOG_LEVEL=DEBUG` traces a request end to end; `ProductClientLoggingTest` pins the levels.

## Build & verify

`java` on the PATH is 17, but **Maven runs on JDK 21** (`~/.jdks/jdk-21.0.6`), so `mvn test` and
`mvn spring-boot:run` do work locally. Run the test suite after any change to `src/main`:

```bash
cd app && mvn test # 31 tests, ~25s, nothing else needs to be running
```

For a running stack (mocks + app), use Docker:

```bash
docker-compose build yourapp && docker-compose up -d yourapp # rebuild after code changes
# then smoke-test via the check-endpoints skill, or:
curl -s http://localhost:5000/product/1/similar
```

Every config property maps to an env var, so you can sweep behaviour by restarting the container with a
different `-e PRODUCT_API_...` value — no rebuild needed.

## Upstream mock behaviour (port 3001)

| Product | Similar IDs | Notable behaviour |
|---------|-------------|-------------------|
| 1 | [2,3,4] | All fast |
| 2 | [3,100,1000] | Product 1000 has 5s delay → times out |
| 3 | [100,1000,10000] | Products 1000 (5s) and 10000 (50s) → time out |
| 4 | [1,2,5] | Product 5 returns 404 → skip |
| 5 | [1,2,6] | Product 6 returns 500 → skip |
Loading