Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cdac] break up cdacreader into 4 separate assemblies #107709

Draft
wants to merge 88 commits into
base: main
Choose a base branch
from

Conversation

lambdageek
Copy link
Member

@lambdageek lambdageek commented Sep 11, 2024

Break up the monolithic cdacreader assembly into three parts:

  1. Microsoft.Diagnostics.DataContractReader.Abstractions just the API surface for contract implementations and clients note everything is internal for now (with IVT for the other assemblies) - we're not committing to a public API surface yet
  2. Microsoft.Diagnostics.DataContractReader.Contracts: the concrete implementations of the contracts and data
  3. Microsoft.Diagnostics.DataCotnractReader: a concrete Target that ties everything together
  4. cdacreader just the unmanaged entrypoints and the legacy DAC API surface SOSDacImpl

To untangle things I had to add a new IContractFactory<TProduct> interface - this is what the target's Registry uses to instantiate specific versions of contracts.

Goals:

  1. Make it possible to mock a ITarget and its IRegistry so that concrete contracts can be tested in isolation for example by making dummy dependent contracts that return canned answers.
  2. Eventually make it possible to inject additional contract implementations into a Registry implementations
  3. Make it possible to consume just the Target and Contracts without the unmanaged entrypoints or the legacy interfaces

Incidentally depends on #106413, although things could probably be teased apart if we want to take this PR first

@lambdageek lambdageek added area-Diagnostics-coreclr NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Sep 11, 2024
Copy link
Contributor

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

@lambdageek lambdageek changed the title [cdac] break up cdacreader into 3 separate assemblies [cdac] break up cdacreader into 4 separate assemblies Sep 12, 2024
make Registry hold a collection of contract factories, optionally with a configure callback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Diagnostics-coreclr NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant