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

Extend AoT support #1541

Merged
merged 12 commits into from
Jan 23, 2024
Merged

Conversation

martincostello
Copy link
Contributor

@martincostello martincostello commented Jan 20, 2024

Changes

  • Make the following assemblies AoT compatible:
    • OpenTelemetry.Extensions.Enrichment
    • OpenTelemetry.ResourceDetectors.AWS
    • OpenTelemetry.Sampler.AWS
  • Test that the following assemblies are AoT compatible;
    • OpenTelemetry.Extensions
    • OpenTelemetry.Extensions.AWS
    • OpenTelemetry.ResourceDetectors.Container
  • Appropriate CHANGELOG.md updated for non-trivial changes

Make `OpenTelemetry.ResourceDetectors.AWS` and `OpenTelemetry.Sampler.AWS` AoT compatible.
Test that `OpenTelemetry.ResourceDetectors.Container` is AoT compatible.
Add annotations to make OpenTelemetry.Extensions.Enrichment AoT compatible.
Copy link

codecov bot commented Jan 20, 2024

Codecov Report

Attention: 56 lines in your changes are missing coverage. Please review.

Comparison is base (71655ce) 73.91% compared to head (5390e3c) 66.63%.
Report is 121 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1541      +/-   ##
==========================================
- Coverage   73.91%   66.63%   -7.29%     
==========================================
  Files         267      249      -18     
  Lines        9615     9105     -510     
==========================================
- Hits         7107     6067    -1040     
- Misses       2508     3038     +530     
Flag Coverage Δ
unittests-Exporter.Geneva 44.47% <44.08%> (?)
unittests-Exporter.OneCollector 89.46% <100.00%> (?)
unittests-Extensions 83.33% <100.00%> (?)
unittests-Instrumentation.AspNet 75.96% <ø> (?)
unittests-Instrumentation.EventCounters 75.92% <ø> (?)
unittests-Instrumentation.Owin 85.71% <ø> (?)
unittests-Instrumentation.Process 100.00% <ø> (?)
unittests-Instrumentation.Runtime 100.00% <ø> (?)
unittests-Instrumentation.StackExchangeRedis 68.02% <ø> (?)
unittests-Instrumentation.Wcf 48.91% <ø> (?)
unittests-PersistentStorage 58.47% <ø> (?)
unittests-ResourceDetectors.Azure 81.48% <ø> (?)
unittests-ResourceDetectors.Host 62.50% <ø> (?)
unittests-ResourceDetectors.Process 100.00% <ø> (?)
unittests-ResourceDetectors.ProcessRuntime 93.33% <ø> (?)
unittests-Solution 79.61% <88.57%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
....Exporter.Geneva/GenevaExporterHelperExtensions.cs 95.45% <ø> (+27.27%) ⬆️
...Telemetry.Exporter.Geneva/GenevaExporterOptions.cs 90.00% <ø> (ø)
...OpenTelemetry.Exporter.Geneva/GenevaLogExporter.cs 57.50% <ø> (-20.00%) ⬇️
...lemetry.Exporter.Geneva/GenevaLoggingExtensions.cs 85.71% <ø> (ø)
...enTelemetry.Exporter.Geneva/GenevaTraceExporter.cs 52.50% <ø> (-22.50%) ⬇️
...xporter.Geneva/Internal/ConnectionStringBuilder.cs 91.66% <100.00%> (-1.28%) ⬇️
...ry.Exporter.Geneva/Internal/ExporterEventSource.cs 14.28% <ø> (+9.52%) ⬆️
...eneva/Internal/ReentrantActivityExportProcessor.cs 0.00% <ø> (-100.00%) ⬇️
...porter.Geneva/Internal/ReentrantExportProcessor.cs 0.00% <ø> (-100.00%) ⬇️
...ry.Exporter.Geneva/Internal/TableNameSerializer.cs 98.93% <ø> (ø)
... and 90 more

... and 180 files with indirect coverage changes

Update CHANGELOGS.
Leftovers from local refactoring before a68bb05.
Fix IDE0005 warning.
Fix IDE0005 warnings.
- Add new filters for AWS and resource detector projects.
- Add new filter for projects validated by AoT that don't otherwise fall under an existing category.
Add OpenTelemetry.Extensions to the AoT test app.
Add OpenTelemetry.Extensions.AWS to the AoT test app.
@martincostello martincostello marked this pull request as ready for review January 20, 2024 16:59
@martincostello martincostello requested a review from a team January 20, 2024 16:59
@Kielek
Copy link
Contributor

Kielek commented Jan 22, 2024

@martincostello, can we make changes for each package separately? It will be much easier to review.

It will be great to both enable AoT and switcc to the new build pipeline.
You can find example here for a brand new project, but changes for the exisiting one should be similar #1507

@martincostello
Copy link
Contributor Author

Can do, though I didn't think this was a particularly big/contentious PR so that's why I didn't do them separately (like I did for #1544).

I'd prefer to get #1540 get merged before I do that as otherwise I have to do branch gymnastics locally to validate the changes I'm making.

Only mention the relevant change in each file.
@cijothomas cijothomas merged commit 397e9f8 into open-telemetry:main Jan 23, 2024
111 checks passed
@martincostello martincostello deleted the extend-aot-support branch January 23, 2024 00:10
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 this pull request may close these issues.

7 participants