Giter Club home page Giter Club logo

Comments (11)

jsquire avatar jsquire commented on September 23, 2024 2

@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

from azure-sdk-for-net.

m-redding avatar m-redding commented on September 23, 2024 2

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

from azure-sdk-for-net.

eerhardt avatar eerhardt commented on September 23, 2024 1

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 [RequiresUnreferencedCode] attribute to a method you call, and now you have warnings.

So it would be good to get CI checks to ensure libraries that should be AOT compatible, stay AOT compatible.

from azure-sdk-for-net.

m-redding avatar m-redding commented on September 23, 2024 1

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.
And @TimothyMothra I'll respond to your email today about onboarding exporter (although like you mentioned, this wouldn't have prevented this).

from azure-sdk-for-net.

m-redding avatar m-redding commented on September 23, 2024 1

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:

ILC : Trim analysis warning IL2026: System\.BinaryData\.BinaryData\(Object,JsonSerializerOptions,Type\): Using member 'System\.Text\.Json\.JsonSerializer\.SerializeToUtf8Bytes\(Object,Type,JsonSerializerOptions\)' which has 'RequiresUnreferencedCodeAttribute'
ILC : AOT analysis warning IL3050: System\.BinaryData\.BinaryData\(Object,JsonSerializerOptions,Type\): Using member 'System\.Text\.Json\.JsonSerializer\.SerializeToUtf8Bytes\(Object,Type,JsonSerializerOptions\)' which has 'RequiresDynamicCodeAttribute'
ILC : Trim analysis warning IL2026: System\.BinaryData\.ToObjectFromJson<T>\(JsonSerializerOptions\): Using member 'System\.Text\.Json\.JsonSerializer\.Deserialize\(ReadOnlySpan`1<Byte>,Type,JsonSerializerOptions\)' which has 'RequiresUnreferencedCodeAttribute'
ILC : AOT analysis warning IL3050: System\.BinaryData\.ToObjectFromJson<T>\(JsonSerializerOptions\): Using member 'System\.Text\.Json\.JsonSerializer\.Deserialize\(ReadOnlySpan`1<Byte>,Type,JsonSerializerOptions\)' which has 'RequiresDynamicCodeAttribute'

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.

from azure-sdk-for-net.

m-redding avatar m-redding commented on September 23, 2024 1

@live1206 this can be closed since there is no further action for Core. We already have a tracking issue for upgrading System.Memory.Data in #41941

from azure-sdk-for-net.

TimothyMothra avatar TimothyMothra commented on September 23, 2024

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?

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.

from azure-sdk-for-net.

TimothyMothra avatar TimothyMothra commented on September 23, 2024

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.

  • Onboarding our Exporter to the AOT CI will only run in the monitor subdirectory. This would catch regressions in my library before releasing, but this wouldn't have caught changes as they occur in the Azure.Core library.
  • Azure.Core is already onboarded to the AOT CI (link) and it didn't catch this regression.

from azure-sdk-for-net.

jsquire avatar jsquire commented on September 23, 2024

Thanks for digging in, @m-redding, and for figuring out the best path forward!

from azure-sdk-for-net.

live1206 avatar live1206 commented on September 23, 2024

@m-redding Since we have further fix to resolve this issue, will you take over this issue instead?

from azure-sdk-for-net.

TimothyMothra avatar TimothyMothra commented on September 23, 2024

@eerhardt Azure.Core made an update to their library.
Can you retest and confirm if this resolved the issue?

Azure.Core's nightly build is available here: https://dev.azure.com/azure-sdk/public/_artifacts/feed/azure-sdk-for-net/NuGet/Azure.Core/versions/1.40.0-alpha.20240603.1


Edit
Spoke to Eric offline. there isn't a new nightly build of the Exporter.
I tested locally using the reproduction steps he originally shared and everything looks good.

from azure-sdk-for-net.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.