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

[AzureMonitorExporter] onboard to aot ci #44128

Merged
merged 12 commits into from
May 31, 2024
Merged

Conversation

TimothyMothra
Copy link
Contributor

@TimothyMothra TimothyMothra commented May 17, 2024

This PR is to onboard to the AOT CI. https://github.com/Azure/azure-sdk-for-net/blob/main/doc/dev/AotRegressionChecks.md

A regression in AOT compatibility was reported in #44127

  • 1.3.0-beta.1 supports AOT
  • 1.3.0-beta.2 does not

The root cause was a change in the Azure.Core dependency.
To help catch regressions sooner, we need to onboard our Exporter to the AOT CI introduced in #40629

@github-actions github-actions bot added the Monitor Monitor, Monitor Ingestion, Monitor Query label May 17, 2024
sdk/monitor/ci.yml Outdated Show resolved Hide resolved
@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

@TimothyMothra
Copy link
Contributor Author

TimothyMothra commented May 31, 2024

Good news, Build is showing 0 warnings!
image

Bad news, MacOS builds are failing

##[error]/Users/runner/hostedtoolcache/dotnet/sdk/8.0.100/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.FrameworkReferenceResolution.targets(90,5): Error NETSDK1204: Ahead-of-time compilation is not supported on the current platform 'osx-x64'.

@m-redding your keyvault pr is not having this problem (#44301). Can you help me with this?

@m-redding
Copy link
Member

It's coming from this I believe - https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.AotCompatibilityTestApp/Azure.Monitor.OpenTelemetry.Exporter.AotCompatibilityTestApp.csproj#L4 I'm not exactly sure how to fix it, I tried to when I was testing the pipeline in https://github.com/Azure/azure-sdk-for-net/pull/44302/files but I couldn't get the targets right

@TimothyMothra
Copy link
Contributor Author

It's coming from this I believe - https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.AotCompatibilityTestApp/Azure.Monitor.OpenTelemetry.Exporter.AotCompatibilityTestApp.csproj#L4 I'm not exactly sure how to fix it, I tried to when I was testing the pipeline in https://github.com/Azure/azure-sdk-for-net/pull/44302/files but I couldn't get the targets right

Oh I missed that it was my AOT project. Weird that it only fails on this PR. Regardless, I think I can fix this. Thank you!

@TimothyMothra
Copy link
Contributor Author

@m-redding thanks for all your help! Can you please review and sign off on these changes?

Copy link
Member

@m-redding m-redding left a comment

Choose a reason for hiding this comment

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

This all looks good! So glad to see the warnings are resolved. And thanks for fixing the md

@TimothyMothra TimothyMothra enabled auto-merge (squash) May 31, 2024 21:53
@TimothyMothra TimothyMothra merged commit 0aaf0db into main May 31, 2024
17 checks passed
@TimothyMothra TimothyMothra deleted the tilee/202405_onboard_aot_ci branch May 31, 2024 23:03
benbp pushed a commit that referenced this pull request Jun 5, 2024
* onboard to aot ci

* testing txt file to fix build

* testing different param name

* update warnings file

* update doc

* Update sdk/monitor/ci.yml

Co-authored-by: Madalyn Redding <66138537+m-redding@users.noreply.github.com>

* testing fix for MacOS

* remove file

* updating compatibility app

---------

Co-authored-by: Madalyn Redding <66138537+m-redding@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Monitor Monitor, Monitor Ingestion, Monitor Query
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants