Skip to content

trace2: improve lifecycle and instrumentation - #2408

Open
mjcheetham wants to merge 18 commits into
git-ecosystem:vnextfrom
mjcheetham:trace2-v3
Open

trace2: improve lifecycle and instrumentation#2408
mjcheetham wants to merge 18 commits into
git-ecosystem:vnextfrom
mjcheetham:trace2-v3

Conversation

@mjcheetham

Copy link
Copy Markdown
Contributor

GCM already emits Trace2 events, but the implementation is owned by CommandContext even though Trace2 represents a single process-wide event stream. This prevents tracing from starting before context construction and requires the tracer to be passed through services that cannot meaningfully replace it.

The existing implementation also attributes asynchronous work to physical managed threads, reports some errors through tracing-specific exception types, and leaves major command, UI, and authentication boundaries uninstrumented.

This PR:

  • reorganises and simplifies the Trace2 message and writer code;
  • initialises one process-wide Trace2 session at program entry;
  • removes tracer dependencies from contexts, services, and OAuth clients;
  • centralises exception reporting at the application boundary;
  • gives child-process events stable correlation IDs and accurate timing;
  • tracks logical execution contexts across asynchronous continuations;
  • adds command-name, scalar-data, and structured-data events;
  • instruments command dispatch, Git configuration, credential storage, UI startup, and authentication flows.

Trace2 continues to honour the existing Git environment variables and configuration settings, and supports event, normal, and performance output formats. The added instrumentation records control-flow and configuration choices without recording credentials.

The series is organised as follows:

  • commits 1-2 reorganise and refactor the tracing implementation;
  • commits 3-8 migrate Trace2 to process-wide ownership;
  • commits 9-14 improve event lifecycle, context, and event types;
  • commits 15-18 add focused instrumentation to application workflows.

To keep the cross-cutting ownership migration reviewable, commits 3-7 are intentionally not independently buildable. Commit 8 completes the migration and restores a buildable tree. The remaining commits build normally.

@mjcheetham
mjcheetham requested a review from a team as a code owner August 13, 2026 12:33
@mjcheetham
mjcheetham requested a balanced review from Copilot August 13, 2026 12:33

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Moves Trace2 to a process-wide lifecycle and adds instrumentation across command, authentication, configuration, storage, process, and UI workflows.

Changes:

  • Reworks Trace2 messages, writers, process correlation, and logical contexts.
  • Removes injected Trace2 dependencies and centralizes exception reporting.
  • Adds command, data, UI, authentication, and credential-store instrumentation.

Reviewed changes

Copilot reviewed 97 out of 99 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
src/TestInfrastructure/Objects/TestSettings.cs Removes injected Trace2 settings.
src/TestInfrastructure/Objects/TestCommandContext.cs Removes context-owned Trace2.
src/TestInfrastructure/Objects/NullTrace.cs Removes obsolete Trace2 stub.
src/TestInfrastructure/GitTestUtilities.cs Updates child-process API usage.
src/Microsoft.AzureRepos/AzureReposHostProvider.cs Removes tracing-specific exceptions.
src/Microsoft.AzureRepos/AzureDevOpsRestApi.cs Uses standard exceptions.
src/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs Updates exception assertions.
src/Microsoft.AzureRepos.Tests/AzureDevOpsApiTests.cs Updates exception assertions.
src/GitLab/UI/Commands/CredentialsCommand.cs Uses standard cancellation exception.
src/GitLab/GitLabOAuth2Client.cs Removes Trace2 dependency.
src/GitLab/GitLabHostProvider.cs Uses standard exceptions.
src/GitLab/GitLabAuthentication.cs Removes injected tracing and specialized exceptions.
src/GitLab.Tests/GitLabAuthenticationTests.cs Updates exception assertions.
src/GitHub/UI/Commands/TwoFactorCommand.cs Uses standard cancellation exception.
src/GitHub/UI/Commands/DeviceCommand.cs Uses standard cancellation exception.
src/GitHub/UI/Commands/CredentialsCommand.cs Uses standard cancellation exception.
src/GitHub/GitHubOAuth2Client.cs Removes Trace2 dependency.
src/GitHub/GitHubHostProvider.cs Uses process-wide Trace2.
src/GitHub/GitHubAuthentication.cs Adds authentication instrumentation.
src/GitHub.Tests/GitHubHostProviderTests.cs Updates exception assertions.
src/GitHub.Tests/GitHubAuthenticationTests.cs Updates exception assertions.
src/git-credential-manager/Program.cs Owns Trace2 process lifecycle.
src/Core/WslUtils.cs Updates WSL child-process creation.
src/Core/UI/HelperApplication.cs Reports exceptions through Trace2.
src/Core/UI/Commands/OAuthCommand.cs Uses standard cancellation exception.
src/Core/UI/Commands/DeviceCodeCommand.cs Uses standard cancellation exception.
src/Core/UI/Commands/DefaultAccountCommand.cs Uses standard cancellation exception.
src/Core/UI/Commands/CredentialsCommand.cs Uses standard cancellation exception.
src/Core/UI/AvaloniaUi.cs Instruments UI initialization and windows.
src/Core/Tty/AnsiConsoleFactory.cs Instruments console creation.
src/Core/Tracing/TraceUtils.cs Centralizes source formatting.
src/Core/Tracing/Trace2Writer.cs Makes Trace2 writers abstract.
src/Core/Tracing/Trace2TextWriter.cs Adds text-writer output.
src/Core/Tracing/Trace2ProcessClass.cs Defines child-process classifications.
src/Core/Tracing/Trace2PipeWriter.cs Refactors named-pipe output.
src/Core/Tracing/Trace2Message.cs Defines Trace2 event messages and formats.
src/Core/Tracing/Trace2FormatTarget.cs Defines output formats.
src/Core/Tracing/Trace2FileWriter.cs Simplifies file output.
src/Core/Tracing/Trace2.cs Implements process-wide Trace2.
src/Core/Tracing/Trace.cs Relocates standard tracing.
src/Core/Trace2StreamWriter.cs Removes obsolete stream writer.
src/Core/Trace2Message.cs Removes previous message implementation.
src/Core/Trace2Exception.cs Removes tracing-specific exceptions.
src/Core/Trace2.cs Removes context-owned Trace2.
src/Core/Settings.cs Removes Trace2 settings ownership.
src/Core/ProcessManager.cs Moves process classification to creation.
src/Core/PlatformUtils.cs Removes Trace2 parameters.
src/Core/Interop/Windows/WindowsProcessManager.cs Updates Windows process creation.
src/Core/Interop/Windows/Native/Win32Error.cs Removes tracing-specific interop errors.
src/Core/HttpClientFactory.cs Uses global Trace2 reporting.
src/Core/HostProviderRegistry.cs Uses global Trace2 reporting.
src/Core/Gpg.cs Updates process tracing and exceptions.
src/Core/GitConfiguration.cs Instruments configuration operations.
src/Core/Git.cs Updates Git process tracing.
src/Core/GenericHostProvider.cs Removes Trace2 injection.
src/Core/Diagnostics/GitDiagnostic.cs Updates child-process API usage.
src/Core/Diagnostics/EnvironmentDiagnostic.cs Removes Trace2 parameter.
src/Core/CredentialStore.cs Instruments credential-store operations.
src/Core/Constants.cs Removes Trace2 user-agent dependency.
src/Core/ConsoleService.cs Lazily creates consoles.
src/Core/Commands/StoreCommand.cs Instruments provider storage.
src/Core/Commands/GitCommandBase.cs Instruments command parsing and dispatch.
src/Core/Commands/GetCommand.cs Instruments provider retrieval.
src/Core/Commands/EraseCommand.cs Instruments provider erasure.
src/Core/Commands/DiagnoseCommand.cs Instruments diagnostics.
src/Core/Commands/ConfigurationCommands.cs Instruments configuration commands.
src/Core/Commands/CapabilityCommand.cs Instruments capability command.
src/Core/CommandContext.cs Removes context-owned Trace2.
src/Core/ChildProcess.cs Adds stable Trace2 child IDs and timing.
src/Core/Authentication/OAuthAuthentication.cs Uses standard exceptions.
src/Core/Authentication/OAuth/OAuth2Client.cs Adds OAuth instrumentation.
src/Core/Authentication/Entra/EntraAuthentication.PublicClient.cs Uses standard exceptions.
src/Core/Authentication/Entra/EntraAuthentication.Caching.cs Uses global Trace2 reporting.
src/Core/Authentication/BasicAuthentication.cs Uses standard exceptions.
src/Core/Authentication/AuthenticationBase.cs Removes injected Trace2 usage.
src/Core/Application.cs Instruments command setup and errors.
src/Core.Tests/WslUtilsTests.cs Updates WSL process tests.
src/Core.Tests/Trace2Tests.cs Adds SID and depth tests.
src/Core.Tests/Trace2MessageTests.cs Tests new Trace2 event formats.
src/Core.Tests/TestProcessManager.cs Updates process manager test double.
src/Core.Tests/ProcessManagerTests.cs Removes obsolete SID tests.
src/Core.Tests/HttpClientFactoryTests.cs Updates factory construction.
src/Core.Tests/HostProviderRegistryTests.cs Updates exception assertions.
src/Core.Tests/GitTests.cs Updates Git construction.
src/Core.Tests/GitConfigurationTests.cs Updates Git construction.
src/Core.Tests/ConsoleServiceTests.cs Tests lazy console factories.
src/Core.Tests/Authentication/OAuth2ClientTests.cs Removes Trace2 test dependency.
src/Core.Tests/Authentication/Entra/EntraAuthenticationTests.cs Updates exception assertions.
src/Core.Tests/Authentication/BasicAuthenticationTests.cs Updates exception assertions.
src/Atlassian.Bitbucket/UI/Commands/CredentialsCommand.cs Uses standard cancellation exception.
src/Atlassian.Bitbucket/OAuth2ClientRegistry.cs Removes Trace2 client injection.
src/Atlassian.Bitbucket/DataCenter/BitbucketOAuth2Client.cs Removes Trace2 dependency.
src/Atlassian.Bitbucket/Cloud/BitbucketOAuth2Client.cs Removes Trace2 dependency.
src/Atlassian.Bitbucket/BitbucketOAuth2Client.cs Updates base OAuth construction.
src/Atlassian.Bitbucket/BitbucketHostProvider.cs Uses global Trace2 reporting.
src/Atlassian.Bitbucket/BitbucketAuthentication.cs Adds authentication instrumentation.
src/Atlassian.Bitbucket.Tests/DataCenter/BitbucketOAuth2ClientTest.cs Updates client construction.
src/Atlassian.Bitbucket.Tests/Cloud/BitbucketOAuth2ClientTest.cs Updates client construction and assertions.
docs/development.md Documents added Trace2 events.
Suppressed comments (1)

src/Core.Tests/Trace2Tests.cs:90

  • This catch converts every assertion failure into a passing test and fails to restore the process environment on success. Change it to finally so cleanup is unconditional without suppressing failures.
        catch
        {
            // Restore original environment variable for this process
            Environment.SetEnvironmentVariable(Trace2.SidEnvar, originalSid);
        }

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/Core/Commands/StoreCommand.cs Outdated
Comment thread src/Core/Commands/EraseCommand.cs Outdated
Comment thread src/Core/Interop/Windows/WindowsProcessManager.cs Outdated
Comment thread src/Core/UI/AvaloniaUi.cs Outdated
Comment thread src/Core.Tests/Trace2Tests.cs Outdated
Comment thread docs/development.md Outdated
Comment thread src/Core/Commands/ConfigurationCommands.cs Outdated
Gather tracing implementation files under a dedicated directory so the
subsequent behavioral changes are easier to review. Rename writer source
files at the same time and align the public region-scope API with its
lifetime by naming the operation StartRegion.

Folding that terminology into the source reorganization gives every later
change the final scope vocabulary instead of preserving a transitional API
name that would need a standalone cleanup.

Assisted-by: GPT-5.6 Sol
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Consolidate event serialization and normal/performance formatting so each
message declares only its event-specific data. Extract the shared format and
process classifications, and align writer implementations with their source
names to make later lifecycle changes smaller and clearer.

Assisted-by: GPT-5.6 Sol
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
TRACE2 represents one process-wide event stream, so an injected
instance obscures its lifetime and makes region state appear isolatable
when it is not. Make event emission and writer state static first so
the API shape can be reviewed apart from startup ownership and the
downstream constructor migration.

Assisted-by: GPT-5.6 Sol
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Tracing begins before CommandContext exists and survives until
dispatcher shutdown, so injected services create a circular dependency
for process identity and settings. Let Trace2 establish its SID, depth,
targets, and initial events at entry, then close writers after dispatch
ends.

Assisted-by: GPT-5.6 Sol
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
CommandContext should expose replaceable execution services, not own a
process-wide event stream. Removing TRACE2 from that surface makes the
remaining injected dependencies explicit and avoids preserving a fake
test tracer for state that can no longer vary by context.

Assisted-by: GPT-5.6 Sol
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Core infrastructure cannot meaningfully substitute a process-wide tracer,
yet carrying it through process, Git, HTTP, and platform constructors
forces every helper to mirror global lifetime. Remove that dependency
from the service layer and emit its existing events through the static
API, while leaving leaf exception conversion for the final pass.

Assisted-by: GPT-5.6 Sol
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
OAuth clients only forward the tracer so every provider can emit into
the same process stream. That parameter creates a constructor cascade
without representing per-client state. Remove it at the OAuth boundary
so provider wiring follows the real ownership model, while reserving
leaf exception changes for the final cleanup.

Assisted-by: GPT-5.6 Sol
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
After constructor ownership is gone, leaf exception wrappers and command
call sites still refer to a tracer that cannot vary by caller. Route
those final authentication, provider, storage, UI, and Win32 events
through the process API so the migration becomes buildable again
without introducing an adapter that would immediately disappear.

Assisted-by: GPT-5.6 Sol
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Exception subclasses that emit while being constructed can report failures
more than once and make ordinary error handling depend on tracing. Emit once at
the application boundary, preserve the richer Git and interop formats there,
and return call sites to standard exception types.

Assisted-by: GPT-5.6 Sol
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Child start and exit events need a stable correlation identity and timing
that survives fast process termination. Give each child wrapper its own Trace2
identifier, classify it when it is created, and report the operating system
exit timestamp from the process notification.

Assisted-by: GPT-5.6 Sol
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Physical managed threads do not describe asynchronous work once execution
crosses awaits or dispatcher boundaries. Flow an explicit Trace2 context with
the execution context so thread identities, nested regions, and relative timing
remain coherent, while allowing callers to restore the main context when
needed.

Assisted-by: GPT-5.6 Sol
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Repeated fully qualified caller-info attribute names obscure the
Trace2 API signatures. Import the namespace to keep those signatures
readable without changing behavior.

Assisted-by: GPT-5.6 Sol
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
A process start records argv but does not identify which command parser
branch ultimately won. Emit the canonical command name after parsing and carry
an inherited hierarchy through child processes so nested helper activity can
be related to its originating command.

Assisted-by: GPT-5.6 Sol
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Regions describe elapsed work but cannot explain the values that shaped
that work. Add scalar and structured data events whose thread, nesting, and
relative timing come from the active logical context, allowing later
instrumentation to attach useful measurements without inventing new regions.

Assisted-by: GPT-5.6 Sol
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Top-level traces currently stop before command parsing and provider
selection, leaving the most important dispatch decisions invisible. Add focused
regions and metadata around setup, input parsing, provider resolution, and each
command entry point so command execution can be followed end to end.

Assisted-by: GPT-5.6 Sol
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Command dispatch alone cannot explain time spent discovering Git, reading
configuration, or opening the selected credential store. Trace those service
boundaries and record the configuration scope and backing-store choices that
drive their behavior.

Assisted-by: GPT-5.6 Sol
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Eager console creation and dispatcher hand-offs hide UI startup cost and
can initialize terminal state before it is needed. Create consoles lazily and
trace ANSI, Avalonia, rendering, and window-display boundaries so interactive
latency is attributed to the correct lifecycle stage.

Assisted-by: GPT-5.6 Sol
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Authentication spans browser, device-code, refresh, account selection,
and provider-specific decisions that are otherwise indistinguishable in a
trace. Add regions and mode data at those boundaries so delays and user-flow
choices can be diagnosed without recording credentials.

Assisted-by: GPT-5.6 Sol
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
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