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

Smeantic-Kernal NuGets fail in Function Apps #1793

Closed
wrharper-AASP opened this issue Jun 30, 2023 · 5 comments · Fixed by #1799
Closed

Smeantic-Kernal NuGets fail in Function Apps #1793

wrharper-AASP opened this issue Jun 30, 2023 · 5 comments · Fixed by #1799

Comments

@wrharper-AASP
Copy link

Describe the bug
If you use Azure Function Apps with version 15.230531.5-preview it will work.
Every version past that gives the error in the screenshot. 0.15.320609.2-preview and onward.

To Reproduce
Just make an Azure Function App in Visual Studio 2022 as a httptrigger and import these files. After you run it, you will see the error.

Expected behavior
Needs to be able to load latest versions.

Screenshots
image

Desktop (please complete the following information):
In descriptions already, on windows 11

@wrharper-AASP
Copy link
Author

I believe this is the problem in the patch notes:
Upgrade System.Text.Json and Microsoft.Extensions.Logging versions to 7.0 by @stephentoub in #1297

@wrharper-AASP
Copy link
Author

I reverted changes (manually created a build) the best I could from: bc7627a until this is resolved.

@ericstj
Copy link
Member

ericstj commented Jun 30, 2023

cc @fabiocav
AzureFunctions has a managed host and loads functions into the same process -- as a result the host will pin a number of the assemblies it uses since it needs to share types from those assemblies with functions and agree on the type definitions. As a result - those functions cannot upgrade the assemblies pinned by the host. It looks like Azure functions doesn't do anything to notify the function project when it's in a state where it's referenced versions of these that cannot be loaded by the host.

Here's a repro project for the broken in-process azure function:
FunctionApp1.zip

Azure functions did create a solution for this problem, and that's the "Isolated" function model. When you create your function, make sure to select "Isolated" as shown below.
image

Here's a repro project for the working isolated process azure function:
FunctionApp2.zip

Here's more detail on the different types of Azure functions: https://learn.microsoft.com/en-us/azure/azure-functions/dotnet-isolated-process-guide

@fabiocav it would be nice if Azure Functions SDK would notify users when they were in this state - either with a better runtime exception - or even better - a build time error letting them know the application was referencing an assembly version that wouldn't be loadable in the shared host.

@wrharper-AASP
Copy link
Author

I guess the only choice is to wait for in-process to get fixed. Isolated won't work for what I am doing.

@wrharper-AASP
Copy link
Author

the custom build I have created is working. so I will just use it until in-process is fixed.

github-merge-queue bot pushed a commit that referenced this issue Jul 4, 2023
This pains me, as it's a technical step backwards. But there are two
problems today:
1) The .NET support policy today unfortunately applies not only to the
.NET SDK and runtime but also to nuget packages released in the same
wave, which means that if someone is relying on eg System.Text.Json and
is running on .NET 6, having SK depend on System.Text.Json with a
minimum version of 7.0 forces the consumer to also use the 7.0 version
of System.Text.Json from the nuget, which then means that one assembly
falls under the STS support policy instead of LTS support policy, which
means that one assembly will have support end for it a few months
earlier than it otherwise would. I've raised this issue for further
evaluation on the .NET side of things, but in the meantime, this lowers
the version number to remove the perceived problem and possible barrier
to adoption.
2) Azure Functions has two deployment models: in-process and isolated.
Ideally functions use isolated, which gives them the freedom to
reference whatever they need. The in-process model is exactly what it
sounds like: the function runs in the same process as the host, and the
host pins several dependencies at 6.0 versions. That means that if a
function references a 7.0 version, it'll fail to load in the in-process
model. While we'd like for folks to be using the isolated model, we
can't force it, and we don't want a need for the in-process model to
block SK usage.

This commit downgrades back to the 6.0 versions, at least where
possible. Some of the connectors reference libraries (e.g. NRedisStack,
pgvector, etc.) that themselves have a 7.0 dependency.

Closes #1793

---------

Co-authored-by: Shawn Callegari <36091529+shawncal@users.noreply.github.com>
shawncal added a commit to shawncal/semantic-kernel that referenced this issue Jul 6, 2023
This pains me, as it's a technical step backwards. But there are two
problems today:
1) The .NET support policy today unfortunately applies not only to the
.NET SDK and runtime but also to nuget packages released in the same
wave, which means that if someone is relying on eg System.Text.Json and
is running on .NET 6, having SK depend on System.Text.Json with a
minimum version of 7.0 forces the consumer to also use the 7.0 version
of System.Text.Json from the nuget, which then means that one assembly
falls under the STS support policy instead of LTS support policy, which
means that one assembly will have support end for it a few months
earlier than it otherwise would. I've raised this issue for further
evaluation on the .NET side of things, but in the meantime, this lowers
the version number to remove the perceived problem and possible barrier
to adoption.
2) Azure Functions has two deployment models: in-process and isolated.
Ideally functions use isolated, which gives them the freedom to
reference whatever they need. The in-process model is exactly what it
sounds like: the function runs in the same process as the host, and the
host pins several dependencies at 6.0 versions. That means that if a
function references a 7.0 version, it'll fail to load in the in-process
model. While we'd like for folks to be using the isolated model, we
can't force it, and we don't want a need for the in-process model to
block SK usage.

This commit downgrades back to the 6.0 versions, at least where
possible. Some of the connectors reference libraries (e.g. NRedisStack,
pgvector, etc.) that themselves have a 7.0 dependency.

Closes microsoft#1793

---------

Co-authored-by: Shawn Callegari <36091529+shawncal@users.noreply.github.com>
Woland2k referenced this issue Aug 30, 2023
### 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.
-->

This PR contains changes to improve observability for Semantic Kernel
users, including:
- Logging: using already existing `ILogger` interface with some
improvements described below.
- Metering: implemented using `Meter` and `MeterListener` classes from
`System.Diagnostics.Metrics` namespace.
- Tracing: implemented using `Activity`, `ActivitySource` and
`ActivityListener` classes from `System.Diagnostics` namespace.

Added telemetry uses native .NET methods, which means that it's not
dependent on specific telemetry tool.
Current PR contains console application using Application Insights as
example to show telemetry capabilities in Kernel.

Changes include instrumentation for `SequentialPlanner` as a starting
point and later there will be new PRs for other types of planners and
more improvements for kernel telemetry.

Also, there are updates for `LogLevel` handling across codebase to align
with described log level purpose:

https://learn.microsoft.com/en-us/dotnet/api/microsoft.extensions.logging.loglevel?view=dotnet-plat-ext-7.0

Particularly, there are changes for `LogLevel.Trace` to be used for
sensitive data and most detailed messages, which should not be enabled
in prodution environments.

### Description
<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->
Changes:
- Marked `.WithLogger` method in KernelBuilder as obsolete and added new
method `.WithLogging`.
- Added new methods `.WithMetering` and `.WithTracing` in KernelBuilder.
- Improved `LogLevel` handling across codebase to cover cases of logging
sensitive data.
- Implemented `InstrumentedSequentialPlanner` for instrumentation using
decorator pattern.
- Extended `InvokeAsync` method in `Plan` for instrumentation.

### Contribution Checklist
<!-- Before submitting this PR, please make sure: -->
- [x] The code builds clean without any errors or warnings
- [x] The PR follows SK Contribution Guidelines
(https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
- [x] The code follows the .NET coding conventions
(https://learn.microsoft.com/dotnet/csharp/fundamentals/coding-style/coding-conventions)
verified with `dotnet format`
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄

---------

Co-authored-by: Shawn Callegari <36091529+shawncal@users.noreply.github.com>
johnoliver pushed a commit to johnoliver/semantic-kernel that referenced this issue Jun 5, 2024
This pains me, as it's a technical step backwards. But there are two
problems today:
1) The .NET support policy today unfortunately applies not only to the
.NET SDK and runtime but also to nuget packages released in the same
wave, which means that if someone is relying on eg System.Text.Json and
is running on .NET 6, having SK depend on System.Text.Json with a
minimum version of 7.0 forces the consumer to also use the 7.0 version
of System.Text.Json from the nuget, which then means that one assembly
falls under the STS support policy instead of LTS support policy, which
means that one assembly will have support end for it a few months
earlier than it otherwise would. I've raised this issue for further
evaluation on the .NET side of things, but in the meantime, this lowers
the version number to remove the perceived problem and possible barrier
to adoption.
2) Azure Functions has two deployment models: in-process and isolated.
Ideally functions use isolated, which gives them the freedom to
reference whatever they need. The in-process model is exactly what it
sounds like: the function runs in the same process as the host, and the
host pins several dependencies at 6.0 versions. That means that if a
function references a 7.0 version, it'll fail to load in the in-process
model. While we'd like for folks to be using the isolated model, we
can't force it, and we don't want a need for the in-process model to
block SK usage.

This commit downgrades back to the 6.0 versions, at least where
possible. Some of the connectors reference libraries (e.g. NRedisStack,
pgvector, etc.) that themselves have a 7.0 dependency.

Closes microsoft#1793

---------

Co-authored-by: Shawn Callegari <36091529+shawncal@users.noreply.github.com>
johnoliver pushed a commit to johnoliver/semantic-kernel that referenced this issue Jun 5, 2024
This pains me, as it's a technical step backwards. But there are two
problems today:
1) The .NET support policy today unfortunately applies not only to the
.NET SDK and runtime but also to nuget packages released in the same
wave, which means that if someone is relying on eg System.Text.Json and
is running on .NET 6, having SK depend on System.Text.Json with a
minimum version of 7.0 forces the consumer to also use the 7.0 version
of System.Text.Json from the nuget, which then means that one assembly
falls under the STS support policy instead of LTS support policy, which
means that one assembly will have support end for it a few months
earlier than it otherwise would. I've raised this issue for further
evaluation on the .NET side of things, but in the meantime, this lowers
the version number to remove the perceived problem and possible barrier
to adoption.
2) Azure Functions has two deployment models: in-process and isolated.
Ideally functions use isolated, which gives them the freedom to
reference whatever they need. The in-process model is exactly what it
sounds like: the function runs in the same process as the host, and the
host pins several dependencies at 6.0 versions. That means that if a
function references a 7.0 version, it'll fail to load in the in-process
model. While we'd like for folks to be using the isolated model, we
can't force it, and we don't want a need for the in-process model to
block SK usage.

This commit downgrades back to the 6.0 versions, at least where
possible. Some of the connectors reference libraries (e.g. NRedisStack,
pgvector, etc.) that themselves have a 7.0 dependency.

Closes microsoft#1793

---------

Co-authored-by: Shawn Callegari <36091529+shawncal@users.noreply.github.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 a pull request may close this issue.

2 participants