Repository navigation
feat: Micrometer/Brave B3 support — field-style baggage headers, HeaderStyle testkit API, Boot 3 E2E (v2.1.0) - #38
Open
baekchangjoon wants to merge 26 commits into
Open
baekchangjoon wants to merge 26 commits into
baekchangjoon wants to merge 26 commits into
Conversation
… reviewed) Design: tracer path verified working on Boot 3.3 + micrometer-tracing- bridge-brave + B3 by a live spike (S1-S4 PASS; S5 confirmed REQ-007 priority as designed). Scope: 3 built-in inbound baggage headers, testkit HeaderStyle (enable/baggageFilter overloads), MM E2E vectors in an isolated JDK-17 build, and the JDK-proxy default-exclusion fix for the Boot 3 boot crash the spike discovered. MicrometerTestIdSource was cut in review (no static accessor on the DI-based facade). Requirements: REQ-MM-001..015 with GWT criteria and a traceability matrix (12 Must + 3 Should). Review rounds (3 slots each, Sonnet fallbacks for unavailable external CLIs) produced 35 findings across both docs; all accepted, including one critical factual correction: the spike's '9/11 async attribution' was a misread of jacococli probe coverage — line-mapping re-verification showed zero attribution failures (constructor runs outside the trace; one unexecuted ternary branch), and the async acceptance criterion is now deterministic. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ
…on source The spike app lived only in session-scoped scratchpad storage (plan review finding): commit it now so Task 7 of the MM/B3 plan can promote it in any future session. Build wiring comes with that task. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ
…backprop) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ
…er fallback) Implements REQ-MM-001/002/004/005/007: ServletAdvice.fallbackTestId() resolves inbound test.id in priority order — W3C baggage header (unchanged parser) → Brave/Micrometer test.id field header → legacy Sleuth baggage-test.id field header — with empty/blank values falling through to the next header. Metrics gains three partition counters (testIdFromW3cBaggage/FieldHeader/LegacyFieldHeader) surfaced in the shutdown summary, satisfying the fallbackActivations invariant (REQ-019: sum of the three == fallbackActivations). Turns Task 1's MmBaggageHeadersE2E green (4/4): fieldHeaderProducesPerTestExec, legacyHeaderProducesPerTestExec, w3cBaggageUnchanged, conflictW3cWins. Store creation (ServletAdvice.java:83-87, reg.forCoverageKey) and the missing-id warning block are untouched — Task 3's scope. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ
…reate) REQ-MM-006: with traceKeyAutoCreate=true, ServletAdvice.activate() routed BOTH tracer-resolved and baggage-fallback keys through TestStoreRegistry.forCoverageKey (auto-create). Only the baggage-fallback branch now calls registry.active(key) instead, so an un-started fallback-derived testId is rejected under strict mode like any other unregistered key. Tracer-resolved keys and TraceScopeBridge's scope-enter path (async weave) are unchanged and keep auto-create. Adds StoreCreationRuleIT (red -> green on baggageKeySkipsAutoCreate; servletTracerKeyAutoCreates/scopeBridgeKeyAutoCreates prove the tracer and TraceScopeBridge paths are unaffected). :agent:test, :agent:integrationTest, and :agent:e2eTest all green; MmBaggageHeadersE2E stays 4/4 (it runs strict mode without traceKeyAutoCreate, so its behavior was already routed through active() and is unchanged by this fix). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ
Review finding: baggageKeySkipsAutoCreate only checked peek==null + CoverageContext==null, not REQ-MM-006's strict-rejection accounting clause. The registry and ServletAdvice.metrics held separate Metrics instances, so TestStoreRegistry.active()'s rejectedUnregistered counter was invisible to the test -- a future refactor that short-circuits "no store" without going through active() could pass silently while losing the mandated accounting. strictAutoCreateRegistry now takes a shared Metrics instance; the test passes the same instance to both the registry and ServletAdvice.metrics and asserts rejectedUnregistered==1. No production change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ
Unconditionally exclude jdk.proxy* and com.sun.proxy.* classes from instrumentation at the transform layer. These generated proxy classes are loaded by dynamic modules and instrumenting them injects a $jacocoInit that crosses JPMS read edges, throwing IllegalAccessError at boot (discovered by the 2026-08-03 spike). The exclusion is unconditional like the self-excludes above — user includes=/excludes= cannot re-enable it. ProxyExclusionTest#transformReturnsNullForJdkProxy validates the exclusion under default includes=*. ProxyExclusionTest#explicitIncludesCannotReenable confirms that explicit includes=jdk.proxy* still excludes (proving the exclusion is not settable by user configuration). REQ-MM-010 ✓ (integration level, transform observability). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ
…clusionTest Review feedback: specify why instrumentFailures doesn't catch dynamic proxy instrumentation failure — the failure is a runtime IllegalAccessError at class initialization (after instrument() returns bytecode), not during instrumentation, so the only reliable pre-check observable is transform()'s null return value. Enhanced javadoc in ProxyExclusionTest#transformReturnsNullForJdkProxy and mirrored a short clause in ProbeInstrumentation.java's comment. Also remove unused assertNotNull import. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ
Implements HeaderStyle enum (W3C_BAGGAGE, FIELD, BOTH) with comprehensive javadoc covering wire formats and usage scenarios. Adds field-header accessors to Pjacoco: - fieldHeaderName() returns "test.id" for Brave BaggagePropagation remote fields - fieldHeaderValue() returns current testId for field-header propagation Supports Brave/Micrometer configuration (management.tracing.baggage.remote-fields=test.id) and OpenTelemetry W3C Baggage standard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ
Adds PjacocoRestAssured.enable(HeaderStyle) and baggageFilter(HeaderStyle) alongside the existing no-arg overloads, which now delegate to HeaderStyle.W3C_BAGGAGE (source/binary compatible, no behavior change for existing callers). FIELD emits only the test.id field header, BOTH emits both, W3C_BAGGAGE only baggage. HeaderStyleTest reuses the loopback com.sun.net.httpserver.HttpServer fixture from PjacocoRestAssuredTest (no Mockito added to this module) and calls RestAssured.reset() in @AfterEach since noArgEnableKeepsW3c registers a global filter via enable(). Matrix: REQ-MM-008/009 -> green. REQ-MM-008 also has an E2E cross-ref (MmDistributedFieldE2E#twoHopSameTestId) that lands in Task 9; the matrix has no partial-state notation for a single row, so the row is flipped to green now with this note that the E2E half is still pending. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ
Review findings from Task 6: - Add HeaderStyleTest#noHeadersWhenNoTestActive, a @ParameterizedTest over @EnumSource(HeaderStyle.class) asserting neither the baggage header nor the test.id field header is sent for any style when no test id is active. Uses junit-jupiter-params, already pulled in transitively by the junit-jupiter test aggregate dependency. - Bump the stale coverage rollup line in the requirements doc from 7/15 (47%) to 10/15 (67%) to reflect REQ-MM-001..010 all green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ
e2e-mm-boot3/ is a standalone JDK-17 Gradle build (own settings.gradle.kts, NOT included in the root multi-module build) wiring the already-committed Boot 3 spike app (app/src/main, Maven pom kept for reference) as its main sourceSet, plus MmBoot3BootE2E#bootsWithDefaultIncludes (REQ-MM-011): boots the app as a child process with -javaagent and no agent options (includes=* default), awaits the Tomcat port from the log, asserts GET /sync == 200 and no IllegalAccessError, tears the child down in finally (destroyForcibly + waitFor) on every exit path. App tweaks needed by Task 8/9: AsyncWorker.work is now straight-line (no if/else) for deterministic per-line coverage assertions, and SpikeController's /call-downstream target is configurable via downstream.base-url (default: self) for Task 9's 2-hop E2E. Discovery-method verification surfaced a SECOND boot-crash class beyond Task 4's jdk.proxy* fix: plain Boot 3 component-scan annotation reflection generates jdk.internal.reflect.GeneratedConstructorAccessorN classes via a fresh per-generation DelegatingClassLoader (not the platform loader, so the existing bootstrap/platform skip misses them); instrumenting them crashed boot with NoClassDefFoundError, 3/3 reproducible, confirmed absent without the agent. Extended ProbeInstrumentation's unconditional exclusion to jdk.internal.reflect. (same failure family, same fix pattern as jdk.proxy*) with two new ProxyExclusionTest cases. Ablation-verified: reverting either exclusion alone reproduces the corresponding red; both together are required for REQ-MM-011 to be green. REQ-MM-011: 🔴 planned -> 🟢 green. Coverage 10/15 -> 11/15 (73%). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ
Adds MmTracerPathE2E to e2e-mm-boot3, turning the 2026-08-03 spike's S2/S3 findings into a regression test: a fixed b3 header's traceId becomes the trace-keyed store's file name and carries SyncWorker coverage, and every executed line of AsyncWorker#work (constructor excluded — it runs outside any trace's activation window) attributes to the same request's traceId store after a graceful SIGTERM flush. Adds org.jacoco.core as a test dependency to parse the resulting .exec files. REQ-MM-012: 🔴 -> 🟡 (both non-CI acceptance criteria green; the dedicated JDK 17 CI job criterion is deferred to Task 10). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ
Adds MmDistributedFieldE2E (REQ-MM-013, REQ-MM-008 E2E cross-reference): two child JVMs of the Boot 3 + micrometer-tracing-bridge-brave spike app (A calls B via /call-downstream -> /sink), harness drives A with PjacocoRestAssured.baggageFilter(HeaderStyle.FIELD), and A's own tracer propagates B3 + the test.id field to B. Asserts both execs collect under the same key, that A-only (DownstreamWorker) and B-only (SpikeController#sink, method-level within the shared class) coverage both survive a vanilla JaCoCo union merge (not last-writer-wins), and that the FIELD header itself reaches B on the wire (via the app's existing /sink header dump). finally-block teardown destroys both child JVMs and asserts zero remain alive. Deviates from the task brief's literal explicit /test/start?testId=D1 procedure: on this SUT both instances always have an active Brave tracer, so per ServletAdvice's documented priority (design spec sec 4.6) the baggage/ field-header fallback is unreachable and the coverage key is always the propagated traceId, not the harness testId - confirmed by reproducing the brief's literal procedure (classCount:0, all probes dropped as "unregistered testId=<traceId>") before switching both instances to traceKeyAutoCreate=true (MmTracerPathE2E's already-proven mechanism). Requirements spec updated with a backprop note under REQ-MM-013 documenting this; REQ-MM-013 flipped green, coverage rollup 12/16 -> 13/16 (81%). Also adds the sibling testkit-core/testkit-restassured jar dependency block to e2e-mm-boot3/build.gradle.kts (version-free glob + explicit rest-assured dependency for testkit-restassured's compileOnly requirement). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ
…-013
Review found the merge acceptance criterion ("SUT-A에만 있는 클래스와
SUT-B에만 있는 클래스가 모두 병합 결과에 존재") was silently satisfied at
method granularity for B's side (SpikeController#sink vs #callDownstream,
within the shared class) rather than a wholly separate class, with no
matching backprop note - unlike the testId->traceId deviation, which was
documented. The spike app genuinely has no class dedicated to /sink (it's
an inline method on SpikeController), so this is a SUT structural
constraint, not a test shortcut. Adds a second backprop note under
REQ-MM-013 stating this explicitly and why it's accepted (preserves the
"no last-writer-wins merge" substance of the criterion). Docs-only; no
code or tests changed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ
- Add HotPathGuardTest: SHA-256 hash guard for CoverageBridge.recordCoverage method body (REQ-MM-015b) - Extend AgentOptionsParseWarningsTest#knownKeysSetIsFrozenForThisCycle: options set frozen (REQ-MM-015a) - Wrap recordCoverage method body in HOT-PATH-BEGIN/END markers (comments only, no bytecode impact) - Update requirements matrix: REQ-MM-015 → 🟢 green (14/16 87.5%) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ
- Fix Finding 1: Extend knownKeysSetIsFrozenForThisCycle to use reflection and assert KNOWN_KEYS cardinality = 23 (previous version only checked warning behavior, missing actual set-size guard against new key additions). Reflection-read private static KNOWN_KEYS field, pin size to 23, retain behavioral assertions (all known keys parse, any new key warns). - Fix Finding 2: Mark REQ-MM-015 as 🟡 partial (not 🟢 green) — criteria (a)(b) locally verified; criterion (c) jdk8-compat CI can only be confirmed in PR CI. Update coverage: 14/16 green → 13/16 green (81.25%). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ
Adds a README ko/en section documenting the Micrometer/Brave B3 header conventions (3-header fallback priority, HeaderStyle usage, async attribution precondition), drafts v2.1.0 release notes covering the two behavior changes (fallback-key auto-create exclusion, unconditional proxy/reflection-accessor exclusion), and bumps the version 2.0.0 -> 2.1.0 across the source of truth (build.gradle.kts), maven-plugin/pom.xml, samples, and README coordinate examples. REQ-MM-014 -> green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ
…main) The Gradle Plugin Portal approval already landed on main (PR #37); this branch forked before that merge, so both READMEs still carried the "Portal approval pending" workaround block, including a local-install mvn install:install-file example pinned to the now-stale 2.0.0. Mirror main's fix (swap the pending-approval block for a one-line "published" note) and bump the still-valid GitHub-Release local-install example to 2.1.0 rather than deleting it, since that fallback path (for users without Central/ Portal access) remains generally useful and is unrelated to Portal status. Also swept both files for other "Portal pending" remnants the earlier pass missed (a stale "public release not done yet" note near the Maven example in both files, and a redundant/false "not on Central/Portal yet" aside in the English in-process section) and corrected or removed them so neither README claims the Portal is still pending. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ
…-log cleanup, exclusion enum) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ
…ADME cleanup; keep 2.1.0 coordinates) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ
…reen by PR #38 CI Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pjacoco v2.1.0: Micrometer Tracing / Brave B3 환경 지원 (REQ-MM-001..016, 13-task plan 완료).
Agent (inbound)
test.id,baggage-test.id(W3Cbaggage와 3-way 우선순위: W3C > field > legacy; 유효 값 기준, malformed/empty는 다음 소스로 폴백)fallbackActivations= W3C/field/legacy 카운터 파티션 불변식 + summary 노출traceKeyAutoCreate대상 제외 (행위 변화 — 릴리스 노트 초안 참고)jdk.internal.reflect.리플렉션 접근자 무조건 계측 제외 (REQ-MM-010/016, ablation 검증)Testkit
HeaderStyleAPI (W3C/FIELD/LEGACY):enable()/baggageFilter()오버로드 — no-arg는 기존 W3C 동작 그대로 (소스·바이너리 호환)E2E / CI
e2e-mm-boot3/(JDK 17, micrometer-tracing-bridge-brave): 부팅, 트레이서 경로(sync+async), 2-hop 분산 수집(FIELD 와이어 전파 실단언) — 4 테스트mm-e2e(JDK 17 전용)recordCoverage핫패스 소스 해시 고정Docs
docs/superpowers/plans/2026-08-03-v210-release-notes-draft.mdVerification
🤖 Generated with Claude Code
https://claude.ai/code/session_01FavJfhdcNAHZKWvmSBVLYZ