-
Notifications
You must be signed in to change notification settings - Fork 25
feat: add WIL-based TraceLogging ETW telemetry (experimental) #493
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
Draft
RamonArjona4
wants to merge
6
commits into
main
Choose a base branch
from
user/ramonarjona4/tracelogging
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
17d211a
feat: add WIL-based TraceLogging ETW telemetry (experimental)
RamonArjona4 434d196
Potential fix for pull request finding
RamonArjona4 dc032a4
Fix potential zip injection vulnerability
RamonArjona4 382b399
Address potential PII leak
RamonArjona4 31fb819
Fix documentation inaccurracy
RamonArjona4 2e88af1
Fix potential WIL version drift
RamonArjona4 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -83,6 +83,59 @@ jobs: | |
| condition: and(eq('${{ item.os }}','linux'), eq('${{ item.arch }}','arm64')) | ||
|
|
||
| - ${{ if eq(parameters.isOfficialBuild, true) }}: | ||
|
|
||
| # --- Telemetry GUID substitution (official builds only, mirrors WinAppSDK) --- | ||
| # For internal builds, the real Microsoft telemetry group GUID is | ||
| # compiled in by overwriting WIL's public no-op traceloggingconfig.h | ||
| # with MicrosoftTelemetry.h from the private Microsoft.Telemetry.Inbox.Native | ||
| # NuGet package. Community/OSS builds skip these steps — build.rs | ||
| # compiles against the public stub and events fire as plain ETW. | ||
|
|
||
| # 1. Authenticate to the private NuGet feed hosting the telemetry package. | ||
| - task: NuGetAuthenticate@1 | ||
| displayName: 'NuGet authenticate for telemetry config' | ||
| inputs: | ||
| nuGetServiceConnections: 'TelemetryInternal' | ||
| condition: and(succeeded(), eq('${{ item.os }}', 'windows')) | ||
|
|
||
| # 2. Restore Microsoft.Telemetry.Inbox.Native into build/telemetry/packages/. | ||
| - task: NuGetCommand@2 | ||
| displayName: 'Restore Microsoft.Telemetry.Inbox.Native' | ||
| inputs: | ||
| command: 'custom' | ||
| arguments: >- | ||
| restore $(Build.SourcesDirectory)/build/telemetry/packages.config | ||
| -ConfigFile $(Build.SourcesDirectory)/build/telemetry/nuget.config | ||
| -PackagesDirectory $(Build.SourcesDirectory)/build/telemetry/packages | ||
| condition: and(succeeded(), eq('${{ item.os }}', 'windows')) | ||
|
|
||
| # 3. Find MicrosoftTelemetry.h and set the env var so build.rs picks it up. | ||
| - powershell: | | ||
| $srcPath = Get-ChildItem -Path '$(Build.SourcesDirectory)/build/telemetry/packages' ` | ||
| -File 'MicrosoftTelemetry.h' -Recurse -ErrorAction SilentlyContinue | ||
| if ($srcPath) { | ||
| Write-Host "Found telemetry config override: $($srcPath.FullName)" | ||
| Write-Host "##vso[task.setvariable variable=MXC_TELEMETRY_CONFIG_OVERRIDE]$($srcPath.FullName)" | ||
| } else { | ||
| Write-Host "MicrosoftTelemetry.h not found under $(Build.SourcesDirectory)/build/telemetry/packages/" | ||
| Get-ChildItem -Path '$(Build.SourcesDirectory)/build/telemetry/packages' -Recurse -ErrorAction SilentlyContinue | ||
| } | ||
| displayName: 'Set MXC_TELEMETRY_CONFIG_OVERRIDE' | ||
| condition: and(succeeded(), eq('${{ item.os }}', 'windows')) | ||
|
|
||
| # --- SDK license override (mirrors telemetry GUID substitution) --- | ||
| # For internal npm publishes, the public MIT-only LICENSE.md is replaced | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: "internal npm publishes" what do you mean by this? We only publish to the public npm repository. See my comment above about chatting about what you want to see happen in the pipelines/release package. It's probably better to handle this part in a follow up PR. |
||
| # with a private EULA containing a Section 2 DATA clause (telemetry | ||
| # disclosure, GDPR). The private EULA is sourced from an internal | ||
| # artifact store and injected via the MXC_LICENSE_OVERRIDE env var. | ||
| # Community/OSS builds skip this step — the SDK ships MIT-only. | ||
|
|
||
| # 4. Apply private EULA if MXC_LICENSE_OVERRIDE is set. | ||
| - powershell: | | ||
| & '$(Build.SourcesDirectory)/scripts/apply-license-override.ps1' | ||
| displayName: 'Apply SDK license override' | ||
| condition: and(succeeded(), ne(variables['MXC_LICENSE_OVERRIDE'], '')) | ||
|
|
||
| - template: Rust.Build.Steps.Official.yml@self | ||
| parameters: | ||
| targetTriple: $(triplet) | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| <?xml version="1.0" encoding="utf-8"?> | ||
| <!-- Scoped NuGet config for restoring the private telemetry package. | ||
| The <clear/> ensures no unintended sources (e.g. nuget.org) are used. | ||
| NuGetAuthenticate@1 injects the feed URL and credentials from the | ||
| TelemetryInternal service connection at build time. --> | ||
| <configuration> | ||
| <packageSources> | ||
| <clear /> | ||
| </packageSources> | ||
| </configuration> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| <?xml version="1.0" encoding="utf-8"?> | ||
| <!-- Telemetry configuration package for internal Microsoft builds. | ||
| Microsoft.Telemetry.Inbox.Native provides MicrosoftTelemetry.h which | ||
| redefines TraceLoggingOptionMicrosoftTelemetry() with the real Microsoft | ||
| telemetry group GUID. This is the same package and version used by | ||
| WinAppSDK (see dev/WindowsAppRuntime_Insights/packages.config in the | ||
| WindowsAppSDK repo). | ||
|
|
||
| The package itself is hosted on a private NuGet feed — see the | ||
| NuGetAuthenticate + NuGetCommand steps in Rust.Build.Job.yml. | ||
| Community/OSS builds do not need this package; the public WIL NuGet | ||
| already ships a no-op stub that compiles cleanly. --> | ||
| <packages> | ||
| <package id="Microsoft.Telemetry.Inbox.Native" | ||
| version="10.0.19041.1-191206-1406.vb-release.x86fre" | ||
| targetFramework="native" /> | ||
| </packages> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
issue: so this pipeline is run in the Dart ADO project https://microsoft.visualstudio.com/Dart/_build?definitionId=190018. In there I don't see a service connection for
TelemetryInternalso I don't think this would work. That said, we should talk about what you want to do in the pipeline offline as there might be a simpler way to do this.FYI we have our own nuget feed in there as well https://microsoft.visualstudio.com/Dart/_artifacts/feed/Mxc-Azure-Feed where we can add internal packages which can be retrieved during build time.