Skip to content

docs(adr): add state management implementation plan (0008)#3704

Merged
yurishkuro merged 6 commits intojaegertracing:mainfrom
Parship12:rewrite-ADR-004
Apr 9, 2026
Merged

docs(adr): add state management implementation plan (0008)#3704
yurishkuro merged 6 commits intojaegertracing:mainfrom
Parship12:rewrite-ADR-004

Conversation

@Parship12
Copy link
Copy Markdown
Member

@Parship12 Parship12 commented Apr 4, 2026

Add ADR 0008 as the implementation / phased migration companion to ADR 0004

Signed-off-by: Parship Chowdhury <[email protected]>
Signed-off-by: Parship Chowdhury <[email protected]>
Copilot AI review requested due to automatic review settings April 4, 2026 04:49
@Parship12 Parship12 requested a review from a team as a code owner April 4, 2026 04:49
@Parship12 Parship12 changed the title Rewrite ADR 004 Rewrite/Update ADR 004 Apr 4, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR is a comprehensive rewrite of ADR 0004 (State Management Strategy for Jaeger UI), transforming it from an extensive 1425-line evaluation document into a concise, action-oriented 220-line decision and implementation roadmap.

Changes:

  • Status update: Changed from "Proposed" to "In progress" and dated 2026-04-04, reflecting active implementation work
  • Document restructuring: Simplified the evaluation while preserving key decision rationale, removing detailed deep-dives (which are preserved in git history per line 56)
  • Clear decision statement: Upfront commitment to Zustand + TanStack Query with explicit rejection table for Context and Redux-only approaches
  • Phased migration plan: Organized into 4 phases (Foundations, UI State, Server State, Cleanup) with 16+ specific sub-tasks, tracking progress with checkmarks
  • Enhanced cross-references: Stronger links to ADR 0005 and ADR 0002, acknowledging the interconnected nature of architecture decisions
  • README.md update: Updated the ADR summary line to reflect the new structure and decision clarity

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
docs/adr/README.md Updated ADR-0004 description to emphasize decision clarity and phased migration plan aligned with ADR 0005
docs/adr/0004-state-management-strategy.md Complete strategic rewrite from evaluation-heavy to decision-and-action focused, with detailed phased migration roadmap


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.21%. Comparing base (01c7e88) to head (59ee5b8).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3704      +/-   ##
==========================================
+ Coverage   89.19%   89.21%   +0.01%     
==========================================
  Files         330      330              
  Lines        9894     9900       +6     
  Branches     2567     2570       +3     
==========================================
+ Hits         8825     8832       +7     
+ Misses        927      926       -1     
  Partials      142      142              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@yurishkuro
Copy link
Copy Markdown
Member

the research part was valuable, you now just provide decisions / claims without backing. I suggest writing a new ADR that focuses on implementation plan and leaving this one alone as the motivation / reference.

@github-actions github-actions bot added the waiting-for-author PR is waiting for author to respond to maintainer's comments label Apr 4, 2026
Signed-off-by: Parship Chowdhury <[email protected]>
@github-actions github-actions bot removed the waiting-for-author PR is waiting for author to respond to maintainer's comments label Apr 4, 2026
@Parship12 Parship12 changed the title Rewrite/Update ADR 004 docs(adr): add state management implementation plan (0008) Apr 4, 2026
@Parship12
Copy link
Copy Markdown
Member Author

@yurishkuro ptal

Copy link
Copy Markdown
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I look at ADR-4 it has research / comparison / decision, and it has a high level migration strategy. But it does not have the final design (similar to ADR-5 with the current design).

Perhaps your ADR-8 should focus on being the representation of the current state which keeps morphing into future / desired state so that eventually it could serve as a claer architectural document of "this is how we do things". Meanwhile updates regadring migration steps should go back to ADR-4, since it already has that but the steps could be expanded with more details that you tried adding here.

@Parship12
Copy link
Copy Markdown
Member Author

Perhaps your ADR-8 should focus on being the representation of the current state which keeps morphing into future / desired state so that eventually it could serve as a claer architectural document of "this is how we do things". Meanwhile updates regadring migration steps should go back to ADR-4, since it already has that but the steps could be expanded with more details that you tried adding here.

Ok, I will update it.

Signed-off-by: Parship Chowdhury <[email protected]>
Copilot AI review requested due to automatic review settings April 7, 2026 18:29
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: Parship Chowdhury <[email protected]>
Signed-off-by: Parship Chowdhury <[email protected]>
Copilot AI review requested due to automatic review settings April 7, 2026 18:52
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


---

## TL;DR
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love the table! There's just one thing that gives me pause - why have a split brain situation between Zustand state and URL state? Is this a transient migration artifact or long term state? Why not consolidate everything in the URL state, which makes shared URLs much more descriptive / usable?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's imagine 2 types of things:

  1. which span is my mouse hovering over right now? as we move our mouse across the trace timeline, this changes...say 60 times per second. If we try to save that in the URL, Will not the URL would be changing 60 times per second? browsers would crash.

  2. Which of the 10,000 spans did I manually expand? A span id looks like: a3f2b1c4d5e6f7a8. If we expand 200 spans, we have to stuff 200 of those into the URL. The URL would become thousands of characters long.

@yurishkuro yurishkuro added the changelog:skip Trivial change that does not require an entry in CHANGELOG label Apr 9, 2026
@yurishkuro yurishkuro merged commit f9226f8 into jaegertracing:main Apr 9, 2026
17 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog:skip Trivial change that does not require an entry in CHANGELOG

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants