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.

Sign up for GitHub

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

Jump to bottom

.Net: Add Amazon's AWS Bedrock Connector #7627

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open

.Net: Add Amazon's AWS Bedrock Connector #7627

wants to merge 23 commits into from
+8,011 −7

Conversation

RogerBarreto
Copy link
Member

@RogerBarreto RogerBarreto commented Aug 3, 2024 •
edited
Loading

Add Amazon's AWS Bedrock Connector

Adding support for Amazon AWS Connector to the Semantic Kernel.

@markwallace-microsoft markwallace-microsoft added .NET Issue or Pull requests regarding .NET code documentation labels Aug 3, 2024
RogerBarreto and others added 2 commits August 13, 2024 03:31
@RogerBarreto
Merge branch 'main' into feature-connectors-amazon
246b0a3
@charlizeyeh @charyeh
@RogerBarret0 @RogerBarreto
.Net: Add Bedrock Connector (#7587)
95b82dd 
### Motivation and Context

<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->
There has been a strong demand from developers in the community to be
able to utilize Bedrock with the Semantic Kernel. With the wide variety
of foundation models offered including specialized models tailored for
various domains and use cases, Bedrock's AI capabilities can now be
leveraged within the Semantic Kernel's unified ecosystem with this
connector.

### Description

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->

This pull request implements a connector for Amazon Bedrock, allowing
developers to leverage Bedrock's foundation models within the Semantic
Kernel framework. Key features of this implementation include:
- Core Services: Implementation of `IChatCompletionService` and
`ITextGenerationService` from the Semantic Kernel.
- Authentication and Security: Secure client authentication and API
communication with the Amazon Bedrock Runtime.
- Modular Design: A directory organization and namespace structure
similar to other connectors in the Semantic Kernel, including folders
for core, services, models, and extensions.
- Testing: Inclusion of unit tests and integration tests for available
Bedrock models to demonstrate successful communication with Bedrock API,
request/response mapping, and output generation.

The implementation leverages the following Amazon Bedrock Runtime
actions:
- `InvokeModelAsync` and `InvokeModelWithResponseStreamAsync` for
`GetTextContentsAsync` and `GetStreamingTextContentsAsync`,
respectively.
- `ConverseAsync` and `ConverseStreamAsync` for
`GetChatMessageContentsAsync` and
`GetStreamingChatMessageContentsAsync`, respectively.

The following base models from various providers are currently connected
and available for use under Bedrock:
- AI21 Labs Jamba
- AI21 Labs Jurassic-2 series
- Amazon Titan
- Anthropic Claude
- Cohere Command
- Meta Llama 3
- Mistral AI Mistral

The kernel builder extension methods support two authentication
scenarios:
- User-provided `IAmazonBedrockRuntime` object: Developers can provide a
pre-configured `IAmazonBedrockRuntime` object, allowing for customized
configurations such as specifying custom credentials, endpoints, or
other settings.
- Automatic `IAmazonBedrockRuntime` creation: If developers do not
provide an `IAmazonBedrockRuntime` object, an instance is automatically
created and registered in the dependency injection (DI) container using
the `AWSSDK.Extensions.NETCore.Setup` package. In this case, the
`IAmazonBedrockRuntime` object will be configured with default settings,
such as credentials obtained from environment variables, AWS credentials
files, or instance profile credentials.

Resolve #3094
Resolve #4335
- #3094
- #4335

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [ ] The code builds clean without any errors or warnings
- [ ] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [ ] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄

---------

Co-authored-by: Charlize Yeh <[email protected]>
Co-authored-by: Roger Barreto <[email protected]>
Co-authored-by: Roger Barreto <[email protected]>
@markwallace-microsoft markwallace-microsoft added the kernel Issues or pull requests impacting the core kernel label Aug 14, 2024
@RogerBarreto RogerBarreto marked this pull request as ready for review October 3, 2024 12:16
@RogerBarreto RogerBarreto requested a review from a team as a code owner October 3, 2024 12:16
@RogerBarreto RogerBarreto changed the title .Net: [Draft] main -> feature-connectors-amazon .Net: Add Amazon's AWS Bedrock Connector Oct 3, 2024
SergeyMenshykh
SergeyMenshykh reviewed Oct 7, 2024
View reviewed changes
.../src/Connectors/Connectors.Amazon.UnitTests/Extensions/BedrockKernelBuilderExtensionTests.cs Outdated Show resolved Hide resolved
dotnet/src/Connectors/Connectors.Amazon/Bedrock/AI21JurassicPenalties.cs Outdated Show resolved Hide resolved
dotnet/src/Connectors/Connectors.Amazon/Bedrock/CohereCommandRTools.cs Outdated Show resolved Hide resolved
dotnet/src/Connectors/Connectors.Amazon/Bedrock/Core/BedrockClientIOService.cs
/// <summary>
/// Utilities to get the model IO service and model provider. Used by Bedrock service clients.
/// </summary>
internal sealed class BedrockClientIOService
Copy link
Member

Choose a reason for hiding this comment

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

This class looks like a factory that could have CreateTextIOService and CreateChatIOService methods.

dotnet/src/Connectors/Connectors.Amazon/Bedrock/Core/BedrockModelUtilities.cs Outdated Show resolved Hide resolved
dotnet/src/Connectors/Connectors.Amazon/Bedrock/Core/BedrockModelUtilities.cs Show resolved Hide resolved
dotnet/src/Connectors/Connectors.Amazon/Bedrock/Core/Clients/BedrockTextGenerationClient.cs Outdated Show resolved Hide resolved
dotnet/src/Connectors/Connectors.Amazon/Bedrock/Core/IBedrockTextGenerationIOService.cs
/// <param name="prompt">The input prompt for text generation.</param>
/// <param name="executionSettings">Optional prompt execution settings.</param>
/// <returns>The invoke request body per model requirements for the InvokeAsync Bedrock runtime call.</returns>
internal object GetInvokeModelRequestBody(string modelId, string prompt, PromptExecutionSettings? executionSettings = null);
Copy link
Member

Choose a reason for hiding this comment

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

Is object the right type to represent the request body?

dmytrostruk
dmytrostruk reviewed Oct 15, 2024
View reviewed changes
dotnet/SK-dotnet.sln
Comment on lines +817 to +818
{E059E9B0-1302-474D-B1B5-10A6E0F1A769}.Publish|Any CPU.ActiveCfg = Debug|Any CPU
{E059E9B0-1302-474D-B1B5-10A6E0F1A769}.Publish|Any CPU.Build.0 = Debug|Any CPU
Copy link
Member

Choose a reason for hiding this comment

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

This configuration should be Publish to avoid issues during the release process of this connector.

Suggested change
{E059E9B0-1302-474D-B1B5-10A6E0F1A769}.Publish|Any CPU.ActiveCfg = Debug|Any CPU
{E059E9B0-1302-474D-B1B5-10A6E0F1A769}.Publish|Any CPU.Build.0 = Debug|Any CPU
{E059E9B0-1302-474D-B1B5-10A6E0F1A769}.Publish|Any CPU.ActiveCfg = Publish|Any CPU
{E059E9B0-1302-474D-B1B5-10A6E0F1A769}.Publish|Any CPU.Build.0 = Publish|Any CPU

dotnet/samples/Demos/AmazonBedrockModels/README.md Outdated Show resolved Hide resolved
...et/src/Connectors/Connectors.Amazon/Bedrock/Extensions/BedrockServiceCollectionExtensions.cs Outdated Show resolved Hide resolved
.../src/Connectors/Connectors.Amazon.UnitTests/Extensions/BedrockKernelBuilderExtensionTests.cs Outdated Show resolved Hide resolved
.../src/Connectors/Connectors.Amazon.UnitTests/Extensions/BedrockKernelBuilderExtensionTests.cs Outdated Show resolved Hide resolved
dotnet/src/Connectors/Connectors.Amazon/Bedrock/Services/BedrockChatCompletionService.cs Outdated Show resolved Hide resolved
dotnet/src/Connectors/Connectors.Amazon/Bedrock/Settings/AmazonClaudeExecutionSettings.cs Outdated Show resolved Hide resolved
dotnet/src/Connectors/Connectors.Amazon/Connectors.Amazon.csproj
<!-- THIS PROPERTY GROUP MUST COME FIRST -->
<AssemblyName>Microsoft.SemanticKernel.Connectors.Amazon</AssemblyName>
<RootNamespace>$(AssemblyName)</RootNamespace>
<TargetFramework>net8.0</TargetFramework>
Copy link
Member

Choose a reason for hiding this comment

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

Are we going to support netstandard2.0?

dotnet/src/Connectors/Connectors.Amazon/Connectors.Amazon.csproj
<PackageReference Include="AWSSDK.Core"/>
</ItemGroup>
<ItemGroup>
<PackageVersion Include="AWSSDK.Core" Version="3.7.400"/>
Copy link
Member

Choose a reason for hiding this comment

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

Package version should be specified centrally in Directory.Packages.props file.

dotnet/src/IntegrationTests/Connectors/Amazon/Bedrock/BedrockChatCompletionTests.cs
Comment on lines +12 to +30
[Theory]
[InlineData("ai21.jamba-instruct-v1:0")]
[InlineData("amazon.titan-text-premier-v1:0")]
[InlineData("amazon.titan-text-lite-v1")]
[InlineData("amazon.titan-text-express-v1")]
[InlineData("anthropic.claude-v2")]
[InlineData("anthropic.claude-v2:1")]
[InlineData("anthropic.claude-instant-v1")]
[InlineData("anthropic.claude-3-sonnet-20240229-v1:0")]
[InlineData("anthropic.claude-3-haiku-20240307-v1:0")]
[InlineData("cohere.command-r-v1:0")]
[InlineData("cohere.command-r-plus-v1:0")]
[InlineData("meta.llama3-70b-instruct-v1:0")]
[InlineData("meta.llama3-8b-instruct-v1:0")]
[InlineData("mistral.mistral-7b-instruct-v0:2")]
[InlineData("mistral.mistral-large-2402-v1:0")]
[InlineData("mistral.mistral-small-2402-v1:0")]
[InlineData("mistral.mixtral-8x7b-instruct-v0:1")]
public async Task ChatCompletionReturnsValidResponseAsync(string modelId)
Copy link
Member

Choose a reason for hiding this comment

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

A couple of points here:

  1. The authentication should be configured to run these tests in merge pipeline.
  2. Running the test for each model during merge pipeline every time a new PR is merged looks scary, I think there will be a lot of transient errors. I also think there could be an issue with rate limiting.

I would probably disable these tests by default and run them manually when needed. Later we can setup a job to run them periodically and independently from PR merge pipeline.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmytrostruk dmytrostruk dmytrostruk left review comments

@SergeyMenshykh SergeyMenshykh SergeyMenshykh left review comments

At least 2 approving reviews are required to merge this pull request.

Assignees

@RogerBarreto RogerBarreto

Labels
documentation kernel Issues or pull requests impacting the core kernel .NET Issue or Pull requests regarding .NET code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

.Net: Add connector for AWS Bedrock
5 participants
@RogerBarreto @dmytrostruk @SergeyMenshykh @markwallace-microsoft @charlizeyeh