-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add mobile telemetry hooks for Build
target
#50561
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
base: main
Are you sure you want to change the base?
Conversation
This PR is targeting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds mobile telemetry hooks to collect build-time information for mobile platforms (Android, iOS, macOS Catalyst, and tvOS). The telemetry captures key properties like target platform, runtime identifier, and publishing configuration settings when the Build target is executed.
- Creates a new mobile telemetry target file that collects platform-specific properties
- Conditionally imports mobile telemetry only for mobile platforms in the build pipeline
- Registers the new "MobileProperties" telemetry event in the MSBuild logger
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
Microsoft.NET.Sdk.BeforeCommon.targets | Conditionally imports mobile telemetry targets for mobile platforms |
Microsoft.NET.MobileTelemetry.targets | New file defining build telemetry collection for mobile properties |
MSBuildLogger.cs | Adds MobileProperties event name to the telemetry logger |
Condition="'$(TargetPlatformIdentifier)' == 'android' or | ||
'$(TargetPlatformIdentifier)' == 'ios' or | ||
'$(TargetPlatformIdentifier)' == 'maccatalyst' or | ||
'$(TargetPlatformIdentifier)' == 'tvos'" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The condition uses string equality comparisons with 'or' operators spanning multiple lines. Consider using an ItemGroup with an 'in' operator for better readability and maintainability: Condition="$(TargetPlatformIdentifier) != '' and '$(TargetPlatformIdentifier)' in 'android;ios;maccatalyst;tvos'"
Condition="'$(TargetPlatformIdentifier)' == 'android' or | |
'$(TargetPlatformIdentifier)' == 'ios' or | |
'$(TargetPlatformIdentifier)' == 'maccatalyst' or | |
'$(TargetPlatformIdentifier)' == 'tvos'" /> | |
Condition="'$(TargetPlatformIdentifier)' != '' and '$(TargetPlatformIdentifier)' in 'android;ios;maccatalyst;tvos'" /> |
Copilot uses AI. Check for mistakes.
<ItemGroup> | ||
<MobileTelemetry Remove="@(MobileTelemetry)" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The ItemGroup removes all MobileTelemetry items and then recreates them. This pattern could be simplified by using a condition on the target or by ensuring the ItemGroup is only populated once, which would be more efficient and clearer in intent.
<ItemGroup> | |
<MobileTelemetry Remove="@(MobileTelemetry)" /> |
Copilot uses AI. Check for mistakes.
Triage/proposal:
|
Description
Adding simple mobile telemetry triggered by
Build
target forandroid
,ios
,maccatalyst
, andtvos
. Collecting:TargetPlatformIdentifier
,RuntimeIdentifier
,UseMonoRuntime
,PublishAot
,PublishReadyToRun
,PublishReadyToRunComposite
properties.Sample Output Telemetry
Testing
Tested locally by installing
android
workload to.dotnet/dotnet
and running the following tests:Publish:
Build:
Another testing was done by locally modified version of SDK, where I modified the .target files and verified that the mobile telemetry targets get executed for
dotnet new android
app.Open Questions
.dotnet/dotnet
)Build
target. How could we distinguish in the telemetry if the invocation came fromdotnet build
ordotnet publish
?I noticed that without
"--property:TargetPlatformIdentifier=android"
, theTargetPlatformIdentifier
inBeforeCommon.targets
was left empty and the telemetry wasn't included. My concerns is ifTargetPlatformIdentifier
is populated soon enough so that the check inBeforeCommon.targets
succeeds when building for mobile targets.