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

MddBootstrap Telemetry & TraceLogging #2321

Merged
merged 7 commits into from
Apr 27, 2022

Conversation

sachintaMSFT
Copy link
Contributor

@sachintaMSFT sachintaMSFT commented Mar 25, 2022

Telemetry and TraceLogging for MddBootstrap

@sachintaMSFT
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@riverar
Copy link
Contributor

riverar commented Mar 25, 2022

Is the intention here to start adding telemetry to the bootstrap code/library? Will my apps start sending telemetry on initialize?

@sachintaMSFT
Copy link
Contributor Author

Contributor

Yes. Add telemetry and also logging. Yes and only when client machine chooses privacy settings to Enhanced/Full telemetry. It's same as any WindowsAppRuntime component's telemetry.

Copy link
Member

@DrusTheAxe DrusTheAxe left a comment

Choose a reason for hiding this comment

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

I think I'm missing something. This looks like telemetry support but I don't see where it's actively invoked. Or not these aren't WIL acctivities or remotely like them, it's WIL's fallback provider hook handling everything flowing through that?

dev/WindowsAppRuntime_BootstrapDLL/tracelogging.cpp Outdated Show resolved Hide resolved
dev/WindowsAppRuntime_BootstrapDLL/tracelogging.cpp Outdated Show resolved Hide resolved
dev/WindowsAppRuntime_BootstrapDLL/tracelogging.cpp Outdated Show resolved Hide resolved
dev/WindowsAppRuntime_BootstrapDLL/tracelogging.cpp Outdated Show resolved Hide resolved
dev/WindowsAppRuntime_BootstrapDLL/tracelogging.cpp Outdated Show resolved Hide resolved
dev/WindowsAppRuntime_BootstrapDLL/tracelogging.h Outdated Show resolved Hide resolved
dev/WindowsAppRuntime_BootstrapDLL/tracelogging.h Outdated Show resolved Hide resolved
dev/WindowsAppRuntime_BootstrapDLL/tracelogging.h Outdated Show resolved Hide resolved
dev/WindowsAppRuntime_BootstrapDLL/tracelogging.h Outdated Show resolved Hide resolved
dev/WindowsAppRuntime_BootstrapDLL/tracelogging.h Outdated Show resolved Hide resolved
@sachintaMSFT
Copy link
Contributor Author

I think I'm missing something. This looks like telemetry support but I don't see where it's actively invoked. Or not these aren't WIL acctivities or remotely like them, it's WIL's fallback provider hook handling everything flowing through that?

Yes, the PR is half complete. Something weird happened when I pushed the PR (I changed the branch name before pushing and not sure if that had any weird impact to the push) and only half changes went in. Getting all up now.

@sachintaMSFT sachintaMSFT changed the title BootStrapTelemetry MddBootstrap Telemetry & TraceLogging Mar 28, 2022
@sachintaMSFT
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Pull request contains merge conflicts.

@sachintaMSFT
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

dev/WindowsAppRuntime_BootstrapDLL/MddBootstrapActivity.h Outdated Show resolved Hide resolved
dev/WindowsAppRuntime_BootstrapDLL/MddBootstrapActivity.h Outdated Show resolved Hide resolved
dev/WindowsAppRuntime_BootstrapDLL/MddBootstrapActivity.h Outdated Show resolved Hide resolved
dev/WindowsAppRuntime_BootstrapDLL/MddBootstrapActivity.h Outdated Show resolved Hide resolved
dev/WindowsAppRuntime_BootstrapDLL/MddBootstrapActivity.h Outdated Show resolved Hide resolved
dev/WindowsAppRuntime_BootstrapDLL/tracelogging.h Outdated Show resolved Hide resolved
dev/WindowsAppRuntime_BootstrapDLL/tracelogging.h Outdated Show resolved Hide resolved
dev/WindowsAppRuntime_BootstrapDLL/tracelogging.h Outdated Show resolved Hide resolved
dev/WindowsAppRuntime_BootstrapDLL/tracelogging.h Outdated Show resolved Hide resolved
dev/WindowsAppRuntime_BootstrapDLL/tracelogging.h Outdated Show resolved Hide resolved
@sachintaMSFT sachintaMSFT force-pushed the user/sachinta/BootStrap_Telemetry branch from dcc2f3a to 81f8804 Compare April 15, 2022 03:07
@sachintaMSFT
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Pull request contains merge conflicts.

dev/WindowsAppRuntime_BootstrapDLL/MddBootstrapActivity.h Outdated Show resolved Hide resolved
dev/WindowsAppRuntime_BootstrapDLL/MddBootstrapActivity.h Outdated Show resolved Hide resolved
dev/WindowsAppRuntime_BootstrapDLL/MddBootstrap.cpp Outdated Show resolved Hide resolved
WindowsAppRuntimeBootstrap_TraceLogger::Initialize m_bootstrapInitializeActivity;
std::atomic<uint32_t> g_initializationCount;
wil::unique_cotaskmem_string g_initializationPackageFullName;
Copy link
Member

Choose a reason for hiding this comment

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

Do we ever need to know the value is null (vs empty)? At a glance it seems not so this could be std::wstring.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you had it as wil::unique_cotaskmem_string (which I maintianed) because IDynamicDependencyLifetimeManager's GetPackageFullName accepts LPWSTR and std::wstring return LPCWSTR. Do you have suggestion on elegant way to handle this ?

Copy link
Member

Choose a reason for hiding this comment

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

Ach. OK, so mote it be.

Nothing to see here. Move along. Nothing to see ;-)

dev/WindowsAppRuntime_BootstrapDLL/MddBootstrapActivity.h Outdated Show resolved Hide resolved
@sachintaMSFT sachintaMSFT force-pushed the user/sachinta/BootStrap_Telemetry branch 6 times, most recently from f8bfa45 to 33438fb Compare April 15, 2022 09:58
@sachintaMSFT
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@sachintaMSFT sachintaMSFT force-pushed the user/sachinta/BootStrap_Telemetry branch from 33438fb to c1cf06b Compare April 18, 2022 13:38
@sachintaMSFT sachintaMSFT force-pushed the user/sachinta/BootStrap_Telemetry branch from c1cf06b to 1b59ed6 Compare April 18, 2022 14:56
@sachintaMSFT
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@sachintaMSFT
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@sachintaMSFT sachintaMSFT merged commit 87298dd into main Apr 27, 2022
@sachintaMSFT sachintaMSFT deleted the user/sachinta/BootStrap_Telemetry branch April 27, 2022 17:12
return m_initializationCount;
}

const WindowsAppRuntime::MddBootstrap::Activity::IntegrityFlags& WindowsAppRuntime::MddBootstrap::Activity::Context::GetIntegrityFlags(HANDLE token)
Copy link
Member

Choose a reason for hiding this comment

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

You are returning a local variable by reference, which is undefined behavior. This has also triggered a build warning in the pipelines. Please fix.

kythant added a commit that referenced this pull request May 11, 2022
* Rename CsWinRT Dependency Variable for Windows SDK

* update Version.Details.xml

* Under a .NET SDK version check, bring back our (broken) default file inclusion solution for RESW and image files (applies to non-single project MSIX apps only). (#2424)

* Under a .NET SDK version check, continue to use our (broken) default file inclusion solution for RESW and image files (applies to non-single project MSIX apps only).

* Fixes: use the right version number and use the right property for the version number! Tested.

* add end dev library adoption section, link to community call (#2428)

* Replace WAR_ prefix with WINDOWSAPPRUNTIME_ prefix (#2430)

Co-authored-by: Santosh Chintalapati <sachinta@ntdev.microsoft.com>

* Fix IsSelfContained() to lookup framework packagefamilyname at runtime (#2411)

* Added Microsoft::WindowsAppRuntime::VersionInfo::RuntimeInformation as a (partial) runtime-equivalent WindowsAppSDK-VersionInfo.h to workaround the chicken-egg problem where we need to know the versioninfo now but it's nto defined until higher in the build tree (the Aggregator pipeline at the top)

* Replaced WindowsAppRuntime_SelfContainedTestInitialize with WindowsAppRuntime_VersionInfo_TestInitialize as we moved the functionality down a level

* Removed dead code

* Update docs for IsSelfContained

* Update test to match SelfContained/VersionInfo::TestInitialize change

* Add missing #include

* Fix compiler warnings (#2420)

* Update WinRT contracts per specs\WinRT\WinRTAPIContracts.md (some contract names didn't match or were absent)

* Fixed warnings

* Use correct type to avoid warnings on some platform (#2436)

Co-authored-by: Eric Langlois <erlangl@microsoft.com>

* Trim Resource Id prefix from WINDOWSAPPRUNTIME_ to MSIX_ (#2442)

Co-authored-by: Santosh Chintalapati <sachinta@ntdev.microsoft.com>

* MddBootstrap Telemetry & TraceLogging (#2321)

* MddBootStrap API telemetry

* Use ThreadLoggingCallback instead of Process wide callback for wil result logging callback

* Add safegaurds to call initialize and shutdown activity's StopWithResult only when the activity is running.

Co-authored-by: Santosh Chintalapati <sachinta@ntdev.microsoft.com>

* Update WindowsAppSDK-RunHelixTests-Job.yml (#2455)

Appended windows.11.amd64.client.open.reunion to helixTargetQueuesOpen and Windows.11.Amd64.client.Reunion to helixTargetQueuesClosed.

* UndockedRegFreeWinRT auto-initialization (#2452)

* Remove dead code

* Add URWF auto-initializer

* New URFW files

* Connect the dots for URFW auto-initialization support

* Added missing file. Fixed compiler warnings

* Simplified the C# auto-initializer. Added a comment explaining why it works the way it does

* Updated C# auto-initializers to shortcircuit any work if loaded for reflecction (vs execution)

* Added missing using

* Fix returning a reference to a local variable in GetActivityFlags(); returns a const enum so no reason it can't be by-value and the obvious correctness reason it can't be by-ref

* Added build macro WindowsAppSDKCleanIntermediateFiles=<boolean> to scrub intermediate outputs as we build. Defaults to 'true' for build pipeline else undefined and thus not acted on (e.g. dev inner-loop)

* Fix prototype for GetIntegrityFlags

* Move WindowsAppSDKCleanIntermediateFiles to be a parameter defined by the pipeline, for better inner-loop (for those who want to use the option)

* Deploy Main and Singleton in BreakAway process when Deployment API is called from MediumILHigherIL  Non-Microsoft publisher package (#2454)

Create DeploymentAgent.exe and run it in a breakaway process from Deployment API when the caller is packaged process and has MediumIL/HigherIL integrity level.

Co-authored-by: Santosh Chintalapati <sachinta@ntdev.microsoft.com>

* Log Installer events to disk (#2460)

* Log Installer events to disk (Application.evtx or Eventvwr.exe -> Windows Logs -> Application) in "WindowsAppRuntime Installer" source.

Co-authored-by: Santosh Chintalapati <sachinta@ntdev.microsoft.com>

* Change Singleton package Name to Microsoft.WinAppRuntime.Singleton[-shorttag]

* Yet Moar CorpII

* Replace WARmain with WARsingleton to point to singleton package

* Update Deployment ACIDs in Framework package manifest (#2472)

Update Deployment ACIDs in Framework package manifest

Co-authored-by: Santosh Chintalapati <sachinta@ntdev.microsoft.com>

* Fix UndockedRegFreeWinRT auto-initializer issues (#2476)

* Fix bad filename

* No precompiledheaders when compiling the auto-initializer

* Copy DeploymentAgent executable to TestFwkPkg (#2467)

* Copy DeploymentAgent executable to TestFwkPkg

* Make DeploymentAgent a dependency to build Microsoft.Windows.Framework

Co-authored-by: Santosh Chintalapati <sachinta@ntdev.microsoft.com>

* Add PushNotificationsLongRunningTask.ProxyStub pdb to symbols dir (#2361)

Co-authored-by: Eric Langlois <erlangl@microsoft.com>

* Some refinements on the UndockedRegFreeWinRT self-contained and auto-initialization support (#2493)

Co-authored-by: Joshua Larkin <70237359+j0shuams@users.noreply.github.com>
Co-authored-by: Rohan Palaniappan <35987549+rohanp-msft@users.noreply.github.com>
Co-authored-by: Scott Jones <sjones@microsoft.com>
Co-authored-by: sachintaMSFT <80828309+sachintaMSFT@users.noreply.github.com>
Co-authored-by: Santosh Chintalapati <sachinta@ntdev.microsoft.com>
Co-authored-by: Howard Kapustein <howardk@microsoft.com>
Co-authored-by: eric langlois <email@ericlanglois.com>
Co-authored-by: Eric Langlois <erlangl@microsoft.com>
Co-authored-by: alexlamtest <68841560+alexlamtest@users.noreply.github.com>
Co-authored-by: Paul Purifoy <33183370+pmpurifoy@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants