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 #108156

Merged
merged 18 commits into from
Sep 30, 2024

Conversation

lambdageek
Copy link
Member

@lambdageek lambdageek commented Sep 23, 2024

Break up the monolithic cdacreader assembly into four 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.DataContractReader: 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<TContract> interface - this is what the target's ContractRegistry uses to instantiate specific versions of contracts.

Goals:

  1. Make it possible to mock a Target and its ContractRegistry 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 ContractRegistry implementation
  3. Make it possible to consume just the Target and Contracts without the unmanaged entrypoints or the legacy interfaces

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
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Sep 23, 2024
@lambdageek lambdageek removed the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Sep 23, 2024
Copy link
Contributor

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

@lambdageek
Copy link
Member Author

I believe I've addressed the outstanding feedback (except the record struct change which I'd like to do in a follow-up). Could I get one more look

@lambdageek lambdageek merged commit a7e5426 into dotnet:main Sep 30, 2024
148 of 150 checks passed
sirntar pushed a commit to sirntar/runtime that referenced this pull request Oct 3, 2024
Break up the monolithic cdacreader assembly into four 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.DataContractReader`: 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<TContract>` interface - this is what the target's ContractRegistry uses to instantiate specific versions of contracts.

Goals:

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


Changes:
* [cdac] break up cdacreader into 4 separate assemblies
* rename the contract factories using libraries naming convention
* removed unused usings
* document all abstract Target members
* rename Target -> ContractDescriptorTarget
* Add ReadTargetPointerFromSpan to abstract Target
   Allows the TypeNameBuidler and SigFormat to depend on the abstract target
* change SOSDacImpl to depend on the abstract Target
* fixup filenames and namespaces
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants