-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[BUG] NextLinkOperationImplementation uses AOT incompatible BinaryData APIs #44127
Comments
Hello, that test app isn't connected to any CI. I added that to manually verify we were AOT compliant for the NET8 release. @m-redding added a CI pipeline in Dec #40629 and it's on my backlog to onboard to this. This was a lower priority because development has slowed on the Exporter. I hadn't considered that new changes in Azure.Core could break us. |
Yes! Anything changing in your dependencies might break/affect your code. A lower level API could decide to do something incompatible and add a So it would be good to get CI checks to ensure libraries that should be AOT compatible, stay AOT compatible. |
@live1206: This is related to the LRO rehydration changes. Please take a look at ways we can make this AOT compatible, given that we can't know what libraries are using them. //cc: @ArthurMa1978, @m-nash |
I'm having some issues onboarding to the AOT CI. I'll need to follow up with @m-redding when she's available. Some other questions/concerns came up while onboarding.
|
Sorry for the delay - was OOF. The AOT CI was passing for this particular PR, I will follow up on why it didn't catch this regression. I'm assuming it's because our pipelines are still running .NET 7 which is a known limitation of the CI. |
After further investigation it turns out this was not a regression in Azure.Core. These warnings had already been baselined before I updated the .NET 8 pipeline. This is the previous version of the expected warnings from December: azure-sdk-for-net/sdk/core/Azure.Core/tests/compatibility/ExpectedAotWarnings.txt Lines 19 to 22 in c47ab4d
The changes in #42686 didn't introduce these. I'll work on getting them fixed regardless, and the pipelines are now updated to .NET 8 so they are more accurate, but I don't entirely understand how/why these warnings are impacting monitor exporter. |
Ok I figured it out. These warnings are coming directly from System.Memory.Data because in Azure.Core, we have a reference on 1.0.2. If I update the reference to 8.0.0 then the warnings are resolved. The downside is that we cannot upgrade to 8.0.0 because then we would have to upgrade STJ and a few other packages to 8 as well, which would impact our compatibility with Functions and/or Powershell. At this point I think the best next steps would be to include a direct reference on System.Memory.Data 8.0.0 in monitor exporter |
Thanks for digging in, @m-redding, and for figuring out the best path forward! |
@m-redding Since we have further fix to resolve this issue, will you take over this issue instead? |
@eerhardt Azure.Core made an update to their library.
Edit |
Library name and version
Azure.Monitor.OpenTelemetry.Exporter 1.3.0-beta.2
Describe the bug
NextLinkOperationImplementation is causing AOT warnings in the following code:
azure-sdk-for-net/sdk/core/Azure.Core/src/Shared/NextLinkOperationImplementation.cs
Line 225 in 34daa9b
azure-sdk-for-net/sdk/core/Azure.Core/src/Shared/NextLinkOperationImplementation.cs
Line 101 in 34daa9b
NOTE: if you use
1.3.0-beta.1
you don't get any warnings. This is new in 1.3.0-beta.2.I believe the warnings were introduced in #42686.
@m-redding @TimothyMothra - do you know why these warnings aren't being caught by https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.AotCompatibilityTestApp/test-aot-compat.ps1?
Expected behavior
The above app shouldn't produce any AOT warnings. See also #35572 which fixed the warnings initially.
Actual behavior
It produces the following warnings:
Reproduction Steps
dotnet publish
the following projectEnvironment
No response
The text was updated successfully, but these errors were encountered: