Don't send AppLifecycle telemetry unless SEND_TELEMETRY is defined #10
Conversation
This file contains 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
danbelcher-MSFT
approved these changes
Feb 4, 2019
|
You said we do this for the main TraceLogger. Does that have its own #define its checking? Should both classes be tied to the same identifier? |
The main tracelogger uses the SEND_TELEMETRY flag too: https://github.com/Microsoft/calculator/blob/ac63c1e1c2c53f702eb94e62dbe3ea22ca775d85/src/CalcViewModel/Common/TraceLogger.cpp#L83-L96 I don't think there's any reason to have separate flags for the two loggers. I don't anticipate any cases where we'd want to turn on only one. |
|
Perfect; I wanted them on the same flag. Ok, no problems. Check it in! |
HowardWolosky
added
the
codebase quality
tian-lt
added a commit
that referenced
this pull request
May 25, 2021
* change CalcViewModel into a WindowsRuntimeComponent project (#5) * change CalcViewModel into a WindowsRuntimeComponent project * remove the old UI codebase (#6) * initially migrated C# codebase by tian (#7) * initial migrated C# codebase by tian * format the codebase * resolve comments * undo: modifications on UI test project * Remove the blocks that have more than 1 empty line. * Register DP using keyword 'nameof' * C# Migration: Initially migrated C# codebase by Han (#8) * C# Migration: Initially migrated C# codebase by Han * Resolved comments and misssing asset * Added three files to Calculator project * Added TODO comment and updated Object * NavCategory: temporary resolution of the hang issue (#9) * Updated CalcViewModel and missing files (#10) * Updated CalcViewModel and WinMeta * Added Calculator.rc * Resolved comment for InitializeLocalizationSettings * add: views/unitconverter.xaml (#11) * add: views/unitconverter.xaml * format the code * remove the extra empty line * add an empty line * check null before invoking event handlers (#12) * fix problems of the migration of OBSERVABLE_PROPERTY_RW (#13) * fixes crash in MathRichEditBox.ctor() (#14) * fixes crash in MathRichEditBox.ctor() * typo * Update azure-pipelines.ci.yaml for Azure Pipelines * Added a link copy of CalcViewModel to temporarily pass Unit Tests (#16) * Updated CalcViewModelCopyForUT configuration (#17) * changes output path of the UI project to align with other projects (#15) * fixes EETypeLoadException issue: export class DelegateCommand (#18) * fixes EETypeLoadException issue: export class DelegateCommand * weak-reference in C++/CX * WeakRef in C# codebase * UTF-8-BOM * spaces in macro * resolve some comments from the offline review * format * rename file * fixes the memory list issue (#20) * fixes a wrongly migrated property * UTF-8-BOM * fixes up the crash of type casting (#21) * Update localized strings 2021-01-04 (#1458) (#23) (cherry picked from commit cdcb956) Co-authored-by: Matt Cooley <[email protected]> * Fixup tests (#1429) (#24) - Removed unneeded "ToString" calls - Fixed typos - Renamed "fEButtonState" to "FEButtonState" (cherry picked from commit 66ad328) Co-authored-by: N <[email protected]> * Update graph internal engine verseion (#1466) (#25) (cherry picked from commit 0048dcb) Co-authored-by: Quentin Al-Timimi <[email protected]> * Turn off DFS file shares in internal build system (#1470) (#26) (cherry picked from commit 885fa23) Co-authored-by: Matt Cooley <[email protected]> * Improve clarity of math expressions in history for Standard Calculator (feature #138) (#1453) (#27) * Implemented feature & added unit tests * Fixed more unit/ui tests * Refactored tests * Update HistoryTests.cpp * Update HistoryTests.cpp * Update HistoryTests.cpp * Update HistoryTests.cpp * Update HistoryTests.cpp * Update HistoryTests.cpp * Update HistoryTests.cpp * Update HistoryTests.cpp (cherry picked from commit 565e3e2) Co-authored-by: Wei (Waley) Zhang <[email protected]> * Adds unit-test cases for NarratorAnnouncement after fixing issue #1386 (#1469) (#28) * fix bug: No confirmation is announced by the narrator after activating 'Remove equation' button #1386 * Unit Test: Add NarratorAnnouncementUnitTests Co-authored-by: tain <[email protected]> (cherry picked from commit 9d8e2ad) Co-authored-by: Tian L <[email protected]> * Move localization pipeline sync schedule to the YAML file (#1478) (#30) (cherry picked from commit 007eccd) Co-authored-by: Matt Cooley <[email protected]> * remove the strong reference carried from delegate (#32) * Remove the finalizer of ControlSizeTrigger (#31) * Normalize the namespace of CalcViewModel (#33) * ViewMode: arrange namespaces * UI build pass * run release * UT build pass * pass build * resolve comment: make the diff results cleaner * resolve comment: make the diff results cleaner (2) * resolve comment: make the diff results cleaner (3) * resolve comment: move impl into a namespace * update: spaces * update: CalculatorButtonUser.h * UTF-8 to UTF-8-BOM * remove ViewState.h/.cpp from CalcViewModel path * revert changes for NavCategory.cpp * remove extra space * remove UCM * remove BOM * Fixed a graphing calculator "permissions" bug caused by PR #1426 (#1471) (#34) - The PR #1426 can cause a crash when no users are returned via `User::FindAllAsync(UserType::LocalUser)` when subsequently trying to access the first user. The existing code also does not guarantee that the returned user is the currently active user. - This fix retrieves the user that opened the app and passes this user into a function to check if this user has the proper permissions to access the graphing mode. This makes sense since the active user is indistinguishable (at least from the app's perspective) to the user who opened the app. This user's permissions are then propagated downwards to properly set up the navigation menu of the app. - Implementation detail worth pointing out: `s_categoryManifest` is what is used to populate the navigation menu of the app, but this variable is static by design, so a separate function was written to override the appropriate `isEnabled` value in `s_categoryManifest`. This function is called by `onLaunched`. - Manual testing Co-authored-by: Wei (Waley) Zhang <[email protected]> * fixes up a bug (#35) * fix csproj (#37) Co-authored-by: hanzhang54 <[email protected]> Co-authored-by: Matt Cooley <[email protected]> Co-authored-by: N <[email protected]> Co-authored-by: Quentin Al-Timimi <[email protected]> Co-authored-by: Wei (Waley) Zhang <[email protected]> Co-authored-by: Tian L <[email protected]>
tian-lt
added a commit
that referenced
this pull request
Jul 15, 2021
* Hello C# - Going to an official feature branch (#1544) * change CalcViewModel into a WindowsRuntimeComponent project (#5) * change CalcViewModel into a WindowsRuntimeComponent project * remove the old UI codebase (#6) * initially migrated C# codebase by tian (#7) * initial migrated C# codebase by tian * format the codebase * resolve comments * undo: modifications on UI test project * Remove the blocks that have more than 1 empty line. * Register DP using keyword 'nameof' * C# Migration: Initially migrated C# codebase by Han (#8) * C# Migration: Initially migrated C# codebase by Han * Resolved comments and misssing asset * Added three files to Calculator project * Added TODO comment and updated Object * NavCategory: temporary resolution of the hang issue (#9) * Updated CalcViewModel and missing files (#10) * Updated CalcViewModel and WinMeta * Added Calculator.rc * Resolved comment for InitializeLocalizationSettings * add: views/unitconverter.xaml (#11) * add: views/unitconverter.xaml * format the code * remove the extra empty line * add an empty line * check null before invoking event handlers (#12) * fix problems of the migration of OBSERVABLE_PROPERTY_RW (#13) * fixes crash in MathRichEditBox.ctor() (#14) * fixes crash in MathRichEditBox.ctor() * typo * Update azure-pipelines.ci.yaml for Azure Pipelines * Added a link copy of CalcViewModel to temporarily pass Unit Tests (#16) * Updated CalcViewModelCopyForUT configuration (#17) * changes output path of the UI project to align with other projects (#15) * fixes EETypeLoadException issue: export class DelegateCommand (#18) * fixes EETypeLoadException issue: export class DelegateCommand * weak-reference in C++/CX * WeakRef in C# codebase * UTF-8-BOM * spaces in macro * resolve some comments from the offline review * format * rename file * fixes the memory list issue (#20) * fixes a wrongly migrated property * UTF-8-BOM * fixes up the crash of type casting (#21) * Update localized strings 2021-01-04 (#1458) (#23) (cherry picked from commit cdcb956) Co-authored-by: Matt Cooley <[email protected]> * Fixup tests (#1429) (#24) - Removed unneeded "ToString" calls - Fixed typos - Renamed "fEButtonState" to "FEButtonState" (cherry picked from commit 66ad328) Co-authored-by: N <[email protected]> * Update graph internal engine verseion (#1466) (#25) (cherry picked from commit 0048dcb) Co-authored-by: Quentin Al-Timimi <[email protected]> * Turn off DFS file shares in internal build system (#1470) (#26) (cherry picked from commit 885fa23) Co-authored-by: Matt Cooley <[email protected]> * Improve clarity of math expressions in history for Standard Calculator (feature #138) (#1453) (#27) * Implemented feature & added unit tests * Fixed more unit/ui tests * Refactored tests * Update HistoryTests.cpp * Update HistoryTests.cpp * Update HistoryTests.cpp * Update HistoryTests.cpp * Update HistoryTests.cpp * Update HistoryTests.cpp * Update HistoryTests.cpp * Update HistoryTests.cpp (cherry picked from commit 565e3e2) Co-authored-by: Wei (Waley) Zhang <[email protected]> * Adds unit-test cases for NarratorAnnouncement after fixing issue #1386 (#1469) (#28) * fix bug: No confirmation is announced by the narrator after activating 'Remove equation' button #1386 * Unit Test: Add NarratorAnnouncementUnitTests Co-authored-by: tain <[email protected]> (cherry picked from commit 9d8e2ad) Co-authored-by: Tian L <[email protected]> * Move localization pipeline sync schedule to the YAML file (#1478) (#30) (cherry picked from commit 007eccd) Co-authored-by: Matt Cooley <[email protected]> * remove the strong reference carried from delegate (#32) * Remove the finalizer of ControlSizeTrigger (#31) * Normalize the namespace of CalcViewModel (#33) * ViewMode: arrange namespaces * UI build pass * run release * UT build pass * pass build * resolve comment: make the diff results cleaner * resolve comment: make the diff results cleaner (2) * resolve comment: make the diff results cleaner (3) * resolve comment: move impl into a namespace * update: spaces * update: CalculatorButtonUser.h * UTF-8 to UTF-8-BOM * remove ViewState.h/.cpp from CalcViewModel path * revert changes for NavCategory.cpp * remove extra space * remove UCM * remove BOM * Fixed a graphing calculator "permissions" bug caused by PR #1426 (#1471) (#34) - The PR #1426 can cause a crash when no users are returned via `User::FindAllAsync(UserType::LocalUser)` when subsequently trying to access the first user. The existing code also does not guarantee that the returned user is the currently active user. - This fix retrieves the user that opened the app and passes this user into a function to check if this user has the proper permissions to access the graphing mode. This makes sense since the active user is indistinguishable (at least from the app's perspective) to the user who opened the app. This user's permissions are then propagated downwards to properly set up the navigation menu of the app. - Implementation detail worth pointing out: `s_categoryManifest` is what is used to populate the navigation menu of the app, but this variable is static by design, so a separate function was written to override the appropriate `isEnabled` value in `s_categoryManifest`. This function is called by `onLaunched`. - Manual testing Co-authored-by: Wei (Waley) Zhang <[email protected]> * fixes up a bug (#35) * fix csproj (#37) Co-authored-by: hanzhang54 <[email protected]> Co-authored-by: Matt Cooley <[email protected]> Co-authored-by: N <[email protected]> Co-authored-by: Quentin Al-Timimi <[email protected]> Co-authored-by: Wei (Waley) Zhang <[email protected]> Co-authored-by: Tian L <[email protected]> * **BYPASS_SECRET_SCANNING** (#1546) * Fixes a bug about the UI of expression tokens (#1547) * fix * [FeatureBranch] Fixes x86/ARM/ARM64 builds for the CI-Pipeline (#1550) * **BYPASS_SECRET_SCANNING** * fix * fixes x86/ARM/ARM64 builds for CI-Pipeline * Add headers missing for compilation with GCC (#1468) (#1551) Things that required such update included: * `wstringstream` * `setprecision` * `SCODE_CODE`, `E_BOUNDS` * Various SAL macros Co-authored-by: Michał Janiszewski <[email protected]> * Update nuget.config file (#1486) (#1552) Co-authored-by: Matt Cooley <[email protected]> * Fixes up some simple miscellaneous TODO items (#1556) * #DEBUG is a known C# preprocessor directive * So far, we haven't observed the problem described in the comment from C# async * fixes misc TODO items * resolve some warnings (#1564) * Add internal CI pipeline (#1553) (#1565) * Add CI-internal pipeline * No ARM64, to match release Co-authored-by: Matt Cooley <[email protected]> * Temporarily disable PGO NuGet package (#1510) (#1566) Co-authored-by: Matt Cooley <[email protected]> * [C# Calc]Removes WinMeta.cs (#1567) * remove WinMeta.cs * undo a trivial change * UTF-8 BOM * [C# Calc] Reverts some changes for Currency constants (#1570) * Update2108release - experimental (#1572) * adjusts Calculator.csproj (#1571) * fixes BinSkim problems (#1573) * fixes an issue around line style (#1575) * fixes the missed NULLs (#1576) (#1578) * Fix the Missing Part in Unit Converter Constructor (#1579) * fixes: calculator doesn't remember its previous mode (#1580) * Fixes: GraphingNumber doesn't work correctly (#1585) * fixes: GraphingNumber doesn't work correctly * Avoid crashing * fixes binding (#1586) * resolve TODO items (#1589) * Improving keyboard support in VariableValueSlider (#1559) (#1595) Co-authored-by: Dave Grochocki <[email protected]> * [C# Calc] Fixes: Keep the value away from getting rounded in Graphing Mode (#1596) * keep the value away from getting rounded * set the display precision to 6 to align with C++ impl * fixes the button-light-up time (#1597) * fixes up merging flaws * Update2108release * fixes (#1599) * keep master for ci pipeline * remove the Resources filter from CalcViewModel project * removes `that` since `this` can be captured automatically * AppxBundlePlatforms * StampAssemblyInfo * removes PreferredToolArchitecture * Change the arg AppVersion into Version * Change the arg AppVersion into Version * from Calculator.rc to AssemblyInfo.cs * Adds assembly-info Co-authored-by: hanzhang54 <[email protected]> Co-authored-by: Matt Cooley <[email protected]> Co-authored-by: N <[email protected]> Co-authored-by: Quentin Al-Timimi <[email protected]> Co-authored-by: Wei (Waley) Zhang <[email protected]> Co-authored-by: Tian L <[email protected]> Co-authored-by: Michał Janiszewski <[email protected]> Co-authored-by: Dave Grochocki <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The app currently logs diagnostic events to ETW in various places. If we add certain flags to those events, the data is eligible to be sent as telemetry. (Whether it actually gets sent is controlled outside of Calculator--for example, there are lots of checks at the system level to ensure privacy settings are honored.)
Currently we set the "eligible for telemetry" flags only in official builds, so no telemetry gets uploaded while the app is in development. We already have this set up for the main TraceLogging provider, but we also need to do this for the AppLifecycle performance logging.