Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 29, 2025

This PR provides a comprehensive migration plan for incrementally moving all code from cmd/query/app into cmd/jaeger/internal/extension/jaegerquery, establishing jaegerquery as the primary home for query service functionality in Jaeger v2.

Background

The query service configuration was split between cmd/jaeger/internal/extension/jaegerquery and cmd/query/app/flags.go. Initial attempts to consolidate the configuration revealed deeper architectural issues requiring a complete migration strategy.

What This PR Contains

Migration Plan Document

Created MIGRATION_PLAN.md in the jaegerquery extension directory outlining:

  • 8 Incremental Phases to migrate ~6,400 lines of code
  • Proposed Directory Structure for the target state
  • Testing Strategy for each phase
  • Backward Compatibility Approach using re-export layers
  • Risk Assessment and rollback strategy per phase
  • Timeline Estimates: 17-23 days total

Migration Phases Overview

  1. Configuration Consolidation (~100 lines) - Move QueryOptions, UIConfig types
  2. Querysvc Core Logic (~2,000 lines) - Move query service implementation
  3. API v3 Implementation (~500 lines) - Move gRPC + HTTP gateway
  4. UI Assets Handling (~800 lines) - Move static UI serving logic
  5. HTTP/gRPC Handlers (~1,500 lines) - Move request handlers
  6. Utilities and Support Code (~600 lines) - Move utilities, qualitymetrics
  7. Server Integration (~800 lines) - Consolidate server initialization
  8. Deprecation and Cleanup - Mark old package as deprecated

Proposed Target Structure

cmd/jaeger/internal/extension/jaegerquery/
├── config.go, factory.go, server.go (existing)
├── handlers/       # HTTP/gRPC request handlers
├── apiv3/          # API v3 implementation  
├── querysvc/       # Core query service logic
├── ui/             # UI asset handling
├── utils/          # Shared utilities
└── qualitymetrics/ # Quality metrics

Key Principles

  • Incremental Migration: Each phase is independent, testable, and can be rolled back
  • Backward Compatible: Re-export layer maintains API compatibility during transition
  • Risk-Managed: Phases ordered from lowest to highest risk
  • Well-Tested: Clear testing strategy for each phase

Current State

All previous code changes have been reverted. The repository is in its original state, ready for the migration to begin phase by phase.

Next Steps

Review the migration plan and approve starting with Phase 1 (Configuration Consolidation), which has the lowest risk and establishes the foundation for subsequent phases.

Original prompt

jaeger query primary home is how in cmd/jaeger/internal/extension/jaegerquery, but its config cmd/jaeger/internal/extension/jaegerquery/config.go is pointing to a legacy location in cmd/query/app/flags.go. I want to consolidate all query config in cmd/jaeger/internal/extension/jaegerquery/config.go, simplify (i.e. fields from QueryOptions should just be added directly to Config struct instead of the current embedding

type Config struct {
        queryapp.QueryOptions `mapstructure:",squash"`

Move cmd/query/app/flags_test.go as well.

Validate that all tests succeed after refactring (make test)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 3 commits December 29, 2025 17:42
- Move QueryOptions and UIConfig fields from cmd/query/app/flags.go into cmd/jaeger/internal/extension/jaegerquery/config.go (flattened, not embedded)
- Add DefaultConfig() function to create default configuration
- Add ToQueryOptions() method to convert Config to legacy QueryOptions for backward compatibility
- Move flags_test.go to config_defaults_test.go with updated test names
- Update factory.go to use new DefaultConfig()
- Update server.go to use ToQueryOptions() for backward compatibility with cmd/query/app
- Update server_test.go to use new Config structure directly

Co-authored-by: yurishkuro <[email protected]>
Copilot AI changed the title [WIP] Consolidate and simplify query config structure Consolidate query config into jaegerquery extension Dec 29, 2025
Copilot AI requested a review from yurishkuro December 29, 2025 17:55
@yurishkuro
Copy link
Member

@copilot > Move cmd/query/app/flags_test.go as well.

I asked to move the types, not copy them. Move will require some refactoring in cmd/query/app/

@codecov
Copy link

codecov bot commented Dec 29, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.36%. Comparing base (9658822) to head (55ee12b).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7800   +/-   ##
=======================================
  Coverage   95.35%   95.36%           
=======================================
  Files         310      310           
  Lines       16075    16101   +26     
=======================================
+ Hits        15329    15355   +26     
  Misses        578      578           
  Partials      168      168           
Flag Coverage Δ
badger_v1 9.03% <ø> (ø)
badger_v2 1.90% <ø> (ø)
cassandra-4.x-v1-manual 13.35% <ø> (ø)
cassandra-4.x-v2-auto 1.89% <ø> (ø)
cassandra-4.x-v2-manual 1.89% <ø> (ø)
cassandra-5.x-v1-manual 13.35% <ø> (ø)
cassandra-5.x-v2-auto 1.89% <ø> (ø)
cassandra-5.x-v2-manual 1.89% <ø> (ø)
clickhouse 1.82% <ø> (ø)
elasticsearch-6.x-v1 17.24% <ø> (ø)
elasticsearch-7.x-v1 17.28% <ø> (ø)
elasticsearch-8.x-v1 17.43% <ø> (ø)
elasticsearch-8.x-v2 1.90% <ø> (ø)
elasticsearch-9.x-v2 1.90% <ø> (ø)
grpc_v1 8.91% <ø> (ø)
grpc_v2 1.90% <ø> (ø)
kafka-3.x-v2 1.90% <ø> (ø)
memory_v2 1.90% <ø> (ø)
opensearch-1.x-v1 17.32% <ø> (ø)
opensearch-2.x-v1 17.32% <ø> (ø)
opensearch-2.x-v2 1.90% <ø> (ø)
opensearch-3.x-v2 1.90% <ø> (ø)
query 1.90% <ø> (ø)
tailsampling-processor 0.55% <ø> (ø)
unittests 93.93% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

Move types from cmd/query/app/flags.go to new cmd/internal/flags/query.go file.
This allows both cmd/query/app and cmd/jaeger/internal/extension/jaegerquery to import the shared types.

Changes:
- Created cmd/internal/flags/query.go with QueryOptions, UIConfig, and DefaultQueryOptions
- Created cmd/internal/flags/query_test.go with test for DefaultQueryOptions
- Updated cmd/query/app/flags.go to re-export types from cmd/internal/flags for backward compatibility
- Updated cmd/jaeger/internal/extension/jaegerquery/config.go to embed flags.QueryOptions instead of duplicating fields
- Deleted cmd/query/app/flags_test.go (moved to cmd/internal/flags/query_test.go)
- Deleted cmd/jaeger/internal/extension/jaegerquery/config_defaults_test.go (test now in cmd/internal/flags)
- Updated cmd/jaeger/internal/extension/jaegerquery/server_test.go to use flags.QueryOptions

Co-authored-by: yurishkuro <[email protected]>
Copy link
Contributor Author

Copilot AI commented Dec 29, 2025

@copilot > Move cmd/query/app/flags_test.go as well.

I asked to move the types, not copy them. Move will require some refactoring in cmd/query/app/

Done. Types moved to cmd/internal/flags/query.go which can be imported by both packages. cmd/query/app/flags.go now re-exports the types for backward compatibility, and cmd/jaeger/internal/extension/jaegerquery/config.go embeds flags.QueryOptions. Test file also moved. (6c8649c)

Copilot AI changed the title Consolidate query config into jaegerquery extension Move query config types to shared location Dec 29, 2025
@yurishkuro
Copy link
Member

@copilot I don't need a new module cmd/internal/flags/query.go, the types should go into cmd/jaeger/internal/extension/jaegerquery/config.go Using type aliases in the original flags.go file is the right choice.

Copy link
Contributor Author

Copilot AI commented Dec 29, 2025

@copilot I don't need a new module cmd/internal/flags/query.go, the types should go into cmd/jaeger/internal/extension/jaegerquery/config.go Using type aliases in the original flags.go file is the rig...

I understand you want types in cmd/jaeger/internal/extension/jaegerquery/config.go with type aliases in cmd/query/app/flags.go. However, this creates two blockers:

  1. Circular dependency: jaegerquery/server.go imports cmd/query/app (uses queryapp.Server, queryapp.NewServer), so if cmd/query/app/flags.go imports jaegerquery, we get: jaegerquery → app → jaegerquery (cycle rejected by Go compiler)

  2. Go internal package rules: Packages under cmd/jaeger/internal/ can only be imported by code under cmd/jaeger/. The cmd/query/ package cannot import from cmd/jaeger/internal/.

Could you clarify which approach you'd prefer:

  • Move jaegerquery out of internal (e.g., cmd/jaeger/extension/jaegerquery)
  • Keep the shared cmd/internal/flags/query.go approach (current working solution)
  • Break the dependency by refactoring how jaegerquery uses cmd/query/app

@yurishkuro
Copy link
Member

@copilot ok revert all code changes and instead come up with a plan / roadmap for moving all code from under cmd/query/app into cmd/jaeger/internal/extension/jaegerquery. Find a way to do this in increments, not a big bang move, and propose a logical directory structure.

Copilot AI and others added 2 commits December 29, 2025 18:57
Created detailed migration plan with 8 incremental phases to move all code from cmd/query/app into cmd/jaeger/internal/extension/jaegerquery.

Key aspects:
- Incremental migration strategy (not big bang)
- Proposed logical directory structure
- Phase-by-phase breakdown with risks and benefits
- Backward compatibility strategy using re-exports
- Testing strategy per phase
- Timeline estimates (17-23 days)
- Success criteria and rollback strategy

Each phase is independent and can be executed, tested, and rolled back separately.

Co-authored-by: yurishkuro <[email protected]>
Copy link
Contributor Author

Copilot AI commented Dec 29, 2025

@copilot ok revert all code changes and instead come up with a plan / roadmap for moving all code from under cmd/query/app into cmd/jaeger/internal/extension/jaegerquery. Find a way to do this in incr...

Done. All code changes reverted and created comprehensive migration plan in MIGRATION_PLAN.md (dbfc09c).

The plan outlines 8 incremental phases to migrate ~6400 lines from cmd/query/app to cmd/jaeger/internal/extension/jaegerquery:

  1. Configuration Consolidation (~100 lines)
  2. Querysvc Core Logic (~2000 lines)
  3. API v3 Implementation (~500 lines)
  4. UI Assets Handling (~800 lines)
  5. HTTP/gRPC Handlers (~1500 lines)
  6. Utilities (~600 lines)
  7. Server Integration (~800 lines)
  8. Deprecation/Cleanup

Each phase is independent with its own testing strategy and rollback plan. Backward compatibility maintained via re-export layer. Estimated timeline: 17-23 days.

Copilot AI changed the title Move query config types to shared location Migration plan: Move cmd/query/app code into jaegerquery extension Dec 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants