Skip to content

Deduplicate command tracing provider lifecycle wiring#7840

Merged
lpcox merged 3 commits into
mainfrom
copilot/duplicate-code-tracing-provider-lifecycle
Jun 20, 2026
Merged

Deduplicate command tracing provider lifecycle wiring#7840
lpcox merged 3 commits into
mainfrom
copilot/duplicate-code-tracing-provider-lifecycle

Conversation

Copilot AI commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

run() and runProxy() had copy-pasted tracing lifecycle code (provider init + deferred shutdown + warning callbacks), which created avoidable drift risk. This PR centralizes that lifecycle wiring in one helper while preserving each command’s existing tracing-config semantics.

  • Tracing lifecycle extraction

    • Added setupCommandTracing(...) in internal/cmd/tracing.go to own provider initialization and deferred shutdown wiring.
    • Added shared warning logger helper logTracingWarnf(...) to remove repeated inline warning lambdas.
  • Command integration

    • internal/cmd/root.go now uses setupCommandTracing(...) with:
      • init warnings via logger.StartupWarn
      • shutdown warnings via logTracingWarnf
    • internal/cmd/proxy.go now uses setupCommandTracing(...) with logTracingWarnf for both init/shutdown warnings.
    • Proxy enablement logging remains keyed to explicit proxy tracing config (tracingCfg != nil), matching prior behavior.
  • Focused helper coverage

    • Extended internal/cmd/tracing_helpers_test.go with TestSetupCommandTracing to assert the helper returns a provider + cleanup function and that noop cleanup is safe.
provider, cleanup := setupCommandTracing(
    ctx,
    tracingCfg,
    "Failed to initialize tracing provider: %v",
    logger.StartupWarn,
    logTracingWarnf,
)
defer cleanup()

GitHub Advanced Security started work on behalf of lpcox June 20, 2026 16:36 View session
GitHub Advanced Security finished work on behalf of lpcox June 20, 2026 16:37
GitHub Advanced Security started work on behalf of lpcox June 20, 2026 16:43 View session
GitHub Advanced Security finished work on behalf of lpcox June 20, 2026 16:44
Copilot AI changed the title [WIP] Refactor duplicate tracing provider lifecycle in cmd commands Deduplicate command tracing provider lifecycle wiring Jun 20, 2026
Copilot finished work on behalf of lpcox June 20, 2026 16:46
Copilot AI requested a review from lpcox June 20, 2026 16:46
GitHub Advanced Security started work on behalf of lpcox June 20, 2026 16:47 View session
@lpcox lpcox marked this pull request as ready for review June 20, 2026 16:48
Copilot AI review requested due to automatic review settings June 20, 2026 16:48
GitHub Advanced Security finished work on behalf of lpcox June 20, 2026 16:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 reduces drift risk in the CLI by centralizing OpenTelemetry tracing provider lifecycle wiring (init + deferred shutdown + warning callbacks) into a shared helper in internal/cmd/tracing.go, while preserving the existing tracing enablement semantics for run() and runProxy().

Changes:

  • Added setupCommandTracing(...) to initialize the tracing provider and return a cleanup function that performs shutdown with timeout.
  • Added logTracingWarnf(...) to remove repeated inline log.Printf("Warning: ...") lambdas.
  • Updated root and proxy commands to use the shared helper, and extended unit tests to cover the new helper behavior.
Show a summary per file
File Description
internal/cmd/tracing.go Adds shared helper(s) for tracing provider init + shutdown lifecycle and shared warning logging.
internal/cmd/tracing_helpers_test.go Adds TestSetupCommandTracing to validate helper returns a provider + safe cleanup.
internal/cmd/root.go Replaces duplicated tracing lifecycle code with setupCommandTracing(...) while keeping startup warning semantics via logger.StartupWarn.
internal/cmd/proxy.go Uses the shared helper for tracing lifecycle while keeping proxy enablement logging tied to explicit proxy tracing config.

Copilot's findings

Tip

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

  • Files reviewed: 4/4 changed files
  • Comments generated: 0

@lpcox lpcox merged commit 7f5a68b into main Jun 20, 2026
40 checks passed
@lpcox lpcox deleted the copilot/duplicate-code-tracing-provider-lifecycle branch June 20, 2026 16:59
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