Skip to content

Refactor ServerComponents into separate classes for OTEL instrumentations#67

Merged
satyakigh merged 1 commit intomainfrom
telemetry-impl
Oct 13, 2025
Merged

Refactor ServerComponents into separate classes for OTEL instrumentations#67
satyakigh merged 1 commit intomainfrom
telemetry-impl

Conversation

@satyakigh
Copy link
Collaborator

@satyakigh satyakigh commented Oct 12, 2025

High-Level Summary of PR #67

This PR refactors the monolithic ServerComponents class into a layered architecture with separate component classes to enable proper initialization order for the Telemetry SDK before other OpenTelemetry instrumentations load.

The Problem

OpenTelemetry (OTEL) instrumentations must be initialized before any application code loads. This is critical because:

  1. Auto-instrumentation requires early registration - OTEL patches modules (HTTP clients, databases, etc.) at import time
  2. Circular dependencies block initialization - If telemetry code imports application components that import telemetry, the instrumentation never gets a chance to initialize first
  3. Import order matters - Once a module is imported, it's too late to instrument it

The Root Cause

The old ServerComponents class created a circular dependency problem:

TelemetryService → ServerComponents → (all components) → LoggerFactory → TelemetryService

This meant:
• Telemetry couldn't start first because it needed ServerComponents
• ServerComponents imported everything, triggering all module loads
• By the time telemetry initialized, instrumentation targets were already loaded

Key Architectural Changes:

  1. Component Separation (4 new classes)
    LspComponents - LSP protocol-level components (diagnostics, workspace, documents, communication, handlers)
    CfnInfraCore - Core infrastructure with no external dependencies (settings, document management, syntax trees, telemetry)
    CfnExternal - AWS services and external APIs (CloudFormation, CCAPI, schemas, linting)
    CfnLspProviders - Business logic and LSP feature providers (completion, hover, code actions, etc.)

  2. Interface Refactoring
    • Renamed Configurable → SettingsConfigurable for clarity
    • Moved Closeable and Configurable interfaces to separate utility files (src/utils/Closeable.ts, src/utils/Configurable.ts)
    • Added Configurables interface for components that contain multiple configurable sub-components
    • Made Subscription generic: Subscription

  3. Dependency Injection Improvements
    • Components now explicitly declare dependencies through constructor parameters
    • Removed static factory methods (.create()) in favor of direct instantiation
    • Better separation of concerns with clear dependency flow: LSP → Core → External → Providers

  4. Telemetry Initialization
    • LoggerFactory and TelemetryService converted to proper singletons with lazy initialization
    • Simplified ClientMessage (removed settings configuration, now just a thin wrapper)
    • Enables telemetry SDK to start before other OTEL instrumentations

  5. Widespread Updates (88 modified files)
    • All components updated to use SettingsConfigurable instead of Configurable
    • Removed unnecessary logger/clientMessage dependencies from many classes
    • Updated test mocks to match new architecture
    • Cleaned up imports and removed circular dependencies

Impact:

Net deletion: -1221 lines removed, +929 added = 292 lines of code eliminated
• Better testability through explicit dependencies
• Clearer component lifecycle management
• Proper telemetry initialization order for OpenTelemetry instrumentation

… can startup before other OTEL instrumentations are loaded
@satyakigh satyakigh changed the title Refactor ServerComponents into separate classes to that Telemetry SDK… Refactor ServerComponents into separate classes for OTEL instrumentations Oct 12, 2025
@satyakigh satyakigh requested review from Zee2413 and kddejong October 12, 2025 21:50
@satyakigh satyakigh marked this pull request as ready for review October 12, 2025 22:24
@satyakigh satyakigh requested a review from a team as a code owner October 12, 2025 22:24
Copy link
Contributor

@Zee2413 Zee2413 left a comment

Choose a reason for hiding this comment

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

Approving large change. Comments should be addressed as necessary in quick followup.

@satyakigh satyakigh merged commit aaf66c1 into main Oct 13, 2025
8 checks passed
@satyakigh satyakigh deleted the telemetry-impl branch October 13, 2025 16:30
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.

3 participants