System.Net.ClientModel and Azure.Core client building blocks

[annelo-msft]

This issue tracks updates to Azure.Core and System.Net.ClientModel to move us toward a path to generate Azure SDK and unbranded clients in a consistent fashion.

System.Net.ClientModel

• Finalize Message abstraction - pending feedback from @KrzysztofCwalina
  • Request
    • Create instance of HttpRequestMessage inside HttpPipelineRequest in transport.CreateMessage and optimize header use
    • Follow-ups in comments on this PR System library response abstraction #39293
  • Response
• Finalize Policy abstraction
  • Where does the different logic live in the pipeline?
    • Response buffering
    • Network timeouts
    • Setting IsError and other meta-data on response?
  • How does this work when transports are entirely replaceable?
    • What happens if a custom transport doesn't, e.g. set IsError on response?
  • Should Policy be a class or an interface?
    • Do we want to be able to create general adapters for any public policy in the System dll, or does any policy we would make public have enough purpose-specific APIs that such a general adapter would be unhelpful?
  • Can PipelineEnumerator be an IAsyncEnumerator? Why or why not?
    • What platform was IAsyncEnumerator introduced?
    • What would that look like? Are the APIs helpful to customers?
  • Should PipelineEnumerator be a class or an interface?
• Finalize Options abstraction and extensibility points
  • Do we need to do this?
  • Do we need to do this?
  • Follow-ups in comments on this PR Move options off Pipeline #39192
  • Separate out types responsible for error classification from retry classification
  • Work through pipeline construction/pipeline invocation issues and extensibility story around each from POV of different customer needs and "separate use from construction"
• Finalize Result abstraction
  • Validate values passed to Result.FromValue are not null. This preserves correctness of semantics in NullableResult<T> vs. Result<T>
• Pull in retry functionality
• Pull in model content read/write APIs, teaming with @m-nash
• Finalize KeyCredential as part of larger TypeSpec auth types story, teaming with @schaabs and @christothes
• Reconcile our TelemetrySource and TelemetrySpan APIs with BCL System.Diagnostics APIs
• Remove dependency on Azure.Core shared source types (shown in .csproj file)
• Add tests to validate all functionality.
  • Per Revert "Adjust test projects" #39342 (comment), make sure nullability is enabled in test projects.
  • We may need to migrate parts of TestFramework to add tests - give this consideration about how we can do this.
• Remove azureicon.png coming in from https://github.com/Azure/azure-sdk-for-net/blob/main/eng/Directory.Build.Common.props#L219

Remove Shared Source

• Argument.cs
• JsonElementExtensions.cs
• RawRequestUriBuilder.cs
• RequestUriBuilderExtensions.cs
• TypeFormatters.cs
• Utf8JsonRequestContent.cs
• DiagnosticScope/DiagnosticScopeFactory -> we're providing public equivalents of these in TelemetryScope and TelemetrySpan classes, but System.ServiceModel.Rest will not support the netcoreapp2.1 TFM. DiagnosticScope files will need to pull functionality from public types but add netcoreapp2.1 support if we want to keep that in Azure.Core.

Rework System.*.Internal Types

• ClientUtilities
• IUtf8JsonWriteable -> this will be replaced by the model content read/write interfaces
• KeyCredentialPolicy -> this will move to the Core.Pipeline namespace but we'd like to have the generator inline the auth header addition in the default case rather than requiring a policy
• Optional: OptionalProperty, OptionalList, OptionalDictionary -> it might make sense to rework these as part of the model serialization implementation story and/or with holistic thinking about serialization for JSON Merge Patch work
• PipelineProtocolExtensions
• ModelSerializationExtensions
• RequestUri -> we will remove this and will need to inline any generation of Uris in the clients.
• Utf8JsonRequestBody -> we should make this internal and provide a method on one of the basic pipeline types to create it. This will require updates to the generator.

Update Generator

• Inline content read/write calls by removing shared source types:
  • Use BCL UriBuilder instead of RequestUriBuilder as shown in this example
  • Remove Utf8JsonRequestContent in favor of a pattern such as this one
  • Remove ModelSerizaliationExtensions
  • Remove PipelineProtocolExtensions
• Change request header assignment as shown in this example
• Inline auth header addition to request in the general case; don't add auth policy to pipeline by default
• Move FromCancellationToken method into a public reusable API for converting from CancellationToken to RequestOptions

Validations

• Ensure we haven't regressed key performance benchmarks - teaming with @mikeharder
• Ensure no breaking changes to GA service client libraries - teaming with @weshaggard
• Validate that it is possible for the generator to have a single code path for Azure SDK and Unbranded libraries that differs only where it needs to take advantage of "Azure specific features", i.e. things that are add-ons in Azure.Core (like we discussed)

Library Targets

• We ran into issues with the netcoreapp2.1 target with ClientDiagnostics and nuget package downgrades. One solution to this is to remove netcoreapp2.1 target from Azure.Core and @KrzysztofCwalina has said he is ok with removing it if it is only a perf degradation, i.e. it will not result in any breaks in functionality. This would be a good outcome for Azure.Core, but we need to validate that removing this target will not break any functionality.

[annelo-msft]

All items here are represented by other GH issues.