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.

Sign up for GitHub

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

Jump to bottom

[release/7.0] Fix DiagnosticSource to work with NativeAOT (#76109) #78372

Closed
wants to merge 1 commit into from
Closed

[release/7.0] Fix DiagnosticSource to work with NativeAOT (#76109) #78372

wants to merge 1 commit into from

Conversation

MichalStrehovsky
Copy link
Member 

git cherry-pick 70b33f8f69128eeb1463691e24646987b54a0e3d

@eerhardt @MichalStrehovsky
Fix DiagnosticSource to work with NativeAOT (dotnet#76109)
ea96c9a 
* Fix DiagnosticSource to work with NativeAOT

There were 2 problems:

1. The use of MakeGenericType doesn't work when a property is a ValueType.
An app will crash when a listener is enabled and DiagnosticSourceEventSource tries
writing values.
2. The properties on KeyValuePair were not being preserved correctly, so the Arguments
of the DiagnosticSourceEventSource methods were not being serialized correctly.

Add test (and infrastructure) to ensure DiagnosticSource works in a NativeAOT app

Fix dotnet#75945

* Enable new NativeAotTests in CI

- Only run them in Release configuration
- Suppress IL2026 warning

* Don't run NativeAot published app tests on OSX since it isn't supported

Set EventSourceSupport only on the projects that need it.
@dotnet-issue-labeler
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost
Copy link

ghost commented Nov 15, 2022

Tagging subscribers to this area: @tarekgh, @tommcdon, @pjanotti
See info in area-owners.md if you want to be subscribed.

Issue Details 
git cherry-pick 70b33f8f69128eeb1463691e24646987b54a0e3d
Author: MichalStrehovsky
Assignees: MichalStrehovsky
Labels:

area-System.Diagnostics.Tracing, new-api-needs-documentation
Milestone: -

@MichalStrehovsky MichalStrehovsky mentioned this pull request Nov 15, 2022
Closed
@tommcdon
Copy link
Member

@noahfalk

@carlossanlop
Copy link
Member

@MichalStrehovsky please fill out the template and request approval from @jeffschwMSFT. Once approved, please send an email to Tactics requesting approval and add the servicing-consider label. The branch is closed for changes for the December release, so this will have to go into the January release.

@MichalStrehovsky
Copy link
Member Author

Going to use the other: PR #76109 (comment)

@MichalStrehovsky MichalStrehovsky deleted the backport76109 branch November 18, 2022 01:29
@ghost ghost locked as resolved and limited conversation to collaborators Dec 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@MichalStrehovsky MichalStrehovsky

Labels
area-System.Diagnostics.Tracing new-api-needs-documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@MichalStrehovsky @tommcdon @carlossanlop @eerhardt