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

Added telemetry to ImageResizer Shell Extension code (dev/imageResizer) #1272

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 35 additions & 12 deletions src/modules/imageresizer/dll/ContextMenuHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "HDropIterator.h"
#include "Settings.h"
#include "common/icon_helpers.h"
#include "trace.h"

extern HINSTANCE g_hInst_imageResizer;

Expand Down Expand Up @@ -177,7 +178,8 @@ HRESULT CContextMenuHandler::GetCommandString(UINT_PTR idCmd, UINT uType, _In_ U
HRESULT CContextMenuHandler::InvokeCommand(_In_ CMINVOKECOMMANDINFO* pici)
{
BOOL fUnicode = FALSE;

Trace::Invoked();
HRESULT hr = E_FAIL;
if (pici->cbSize == sizeof(CMINVOKECOMMANDINFOEX) && pici->fMask & CMIC_MASK_UNICODE)
{
fUnicode = TRUE;
Expand All @@ -190,15 +192,15 @@ HRESULT CContextMenuHandler::InvokeCommand(_In_ CMINVOKECOMMANDINFO* pici)
{
if (wcscmp(((CMINVOKECOMMANDINFOEX*)pici)->lpVerbW, RESIZE_PICTURES_VERBW) == 0)
{
return ResizePictures(pici, nullptr);
hr = ResizePictures(pici, nullptr);
}
}
else if (LOWORD(pici->lpVerb) == ID_RESIZE_PICTURES)
{
return ResizePictures(pici, nullptr);
hr = ResizePictures(pici, nullptr);
}

return E_FAIL;
Trace::InvokedRet(hr);
return hr;
}

// This function is used for both MSI and MSIX. If pici is null and psiItemArray is not null then this is called by Invoke(MSIX). If pici is not null and psiItemArray is null then this is called by InvokeCommand(MSI).
Expand All @@ -215,8 +217,17 @@ HRESULT CContextMenuHandler::ResizePictures(CMINVOKECOMMANDINFO* pici, IShellIte
sa.nLength = sizeof(SECURITY_ATTRIBUTES);
sa.lpSecurityDescriptor = NULL;
sa.bInheritHandle = TRUE;
CreatePipe(&hReadPipe, &hWritePipe, &sa, 0);
SetHandleInformation(hWritePipe, HANDLE_FLAG_INHERIT, 0);
HRESULT hr = E_FAIL;
if (!CreatePipe(&hReadPipe, &hWritePipe, &sa, 0))
{
Trace::InvokedRet(hr);
return hr;
}
if (!SetHandleInformation(hWritePipe, HANDLE_FLAG_INHERIT, 0))
{
Trace::InvokedRet(hr);
return hr;
}
CAtlFile writePipe(hWritePipe);

CString commandLine;
Expand Down Expand Up @@ -264,8 +275,16 @@ HRESULT CContextMenuHandler::ResizePictures(CMINVOKECOMMANDINFO* pici, IShellIte
&startupInfo,
&processInformation);
delete[] lpszCommandLine;
CloseHandle(processInformation.hProcess);
CloseHandle(processInformation.hThread);
if (!CloseHandle(processInformation.hProcess))
{
Trace::InvokedRet(hr);
return hr;
}
if (!CloseHandle(processInformation.hThread))
{
Trace::InvokedRet(hr);
return hr;
}

// psiItemArray is NULL if called from InvokeCommand. This part is used for the MSI installer. It is not NULL if it is called from Invoke (MSIX).
if (!psiItemArray)
Expand Down Expand Up @@ -302,8 +321,9 @@ HRESULT CContextMenuHandler::ResizePictures(CMINVOKECOMMANDINFO* pici, IShellIte
}

writePipe.Close();

return S_OK;
hr = S_OK;
Trace::InvokedRet(hr);
return hr;
}

HRESULT __stdcall CContextMenuHandler::GetTitle(IShellItemArray* /*psiItemArray*/, LPWSTR* ppszName)
Expand Down Expand Up @@ -380,5 +400,8 @@ HRESULT __stdcall CContextMenuHandler::EnumSubCommands(IEnumExplorerCommand** pp
// psiItemArray contains the list of files that have been selected when the context menu entry is invoked
HRESULT __stdcall CContextMenuHandler::Invoke(IShellItemArray* psiItemArray, IBindCtx* /*pbc*/)
{
return ResizePictures(nullptr, psiItemArray);
Trace::Invoked();
HRESULT hr = ResizePictures(nullptr, psiItemArray);
Trace::InvokedRet(hr);
return hr;
}
2 changes: 2 additions & 0 deletions src/modules/imageresizer/dll/ImageResizerExt.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@
<PrecompiledHeader Condition="'$(Configuration)|$(Platform)'=='Release|Win32'">Create</PrecompiledHeader>
<PrecompiledHeader Condition="'$(Configuration)|$(Platform)'=='Release|x64'">Create</PrecompiledHeader>
</ClCompile>
<ClCompile Include="trace.cpp" />
</ItemGroup>
<ItemGroup>
<ClInclude Include="ContextMenuHandler.h" />
Expand All @@ -283,6 +284,7 @@
<ClInclude Include="ImageResizerExt_i.h" />
<ClInclude Include="stdafx.h" />
<ClInclude Include="targetver.h" />
<ClInclude Include="trace.h" />
</ItemGroup>
<ItemGroup>
<ResourceCompile Include="ImageResizerExt.rc" />
Expand Down
6 changes: 6 additions & 0 deletions src/modules/imageresizer/dll/ImageResizerExt.vcxproj.filters
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@
<ClCompile Include="Settings.cpp">
<Filter>Source Files</Filter>
</ClCompile>
<ClCompile Include="trace.cpp">
<Filter>Source Files</Filter>
</ClCompile>
<ClInclude Include="trace.h">
<Filter>Header Files</Filter>
</ClInclude>
</ItemGroup>
<ItemGroup>
<ClInclude Include="stdafx.h">
Expand Down
37 changes: 10 additions & 27 deletions src/modules/imageresizer/dll/dllmain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <interface/powertoy_module_interface.h>
#include <common/settings_objects.h>
#include "Settings.h"
#include "trace.h"
#include <common/common.h>

CImageResizerExtModule _AtlModule;
Expand All @@ -16,6 +17,11 @@ extern "C" BOOL WINAPI DllMain(HINSTANCE hInstance, DWORD dwReason, LPVOID lpRes
{
case DLL_PROCESS_ATTACH:
g_hInst_imageResizer = hInstance;
Trace::RegisterProvider();
break;
case DLL_PROCESS_DETACH:
Trace::UnregisterProvider();
break;
}
return _AtlModule.DllMain(dwReason, lpReserved);
}
Expand Down Expand Up @@ -83,34 +89,22 @@ class ImageResizerModule : public PowertoyModuleIface
virtual void call_custom_action(const wchar_t* action) override {}

// Called by the runner to pass the updated settings values as a serialized JSON.
virtual void set_config(const wchar_t* config) override
{
try
{
// Parse the input JSON string.
PowerToysSettings::PowerToyValues values = PowerToysSettings::PowerToyValues::from_json_string(config);
// If you don't need to do any custom processing of the settings, proceed
// to persists the values calling:
values.save_to_settings_file();
}
catch (std::exception ex)
{
// Improper JSON.
}
}
virtual void set_config(const wchar_t* config) override {}

// Enable the powertoy
virtual void enable()
{
m_enabled = true;
CSettings::SetEnabled(m_enabled);
Trace::EnableImageResizer(m_enabled);
}

// Disable the powertoy
virtual void disable()
{
m_enabled = false;
CSettings::SetEnabled(m_enabled);
Trace::EnableImageResizer(m_enabled);
}

// Returns if the powertoys is enabled
Expand All @@ -130,18 +124,7 @@ class ImageResizerModule : public PowertoyModuleIface
};

// Load the settings file.
void ImageResizerModule::init_settings()
{
try
{
// Load and parse the settings file for this PowerToy.
PowerToysSettings::PowerToyValues settings = PowerToysSettings::PowerToyValues::load_from_settings_file(get_name());
}
catch (std::exception ex)
{
// Error while loading from the settings file. Let default values stay as they are.
}
}
void ImageResizerModule::init_settings() {}

extern "C" __declspec(dllexport) PowertoyModuleIface* __cdecl powertoy_create()
{
Expand Down
1 change: 1 addition & 0 deletions src/modules/imageresizer/dll/stdafx.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,4 @@
#include <windows.h>

#include <ShlObj.h>
#include <ProjectTelemetry.h>
49 changes: 49 additions & 0 deletions src/modules/imageresizer/dll/trace.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
#include "stdafx.h"
#include "trace.h"

arjunbalgovind marked this conversation as resolved.
Show resolved Hide resolved
TRACELOGGING_DEFINE_PROVIDER(
g_hProvider,
"Microsoft.PowerToys",
// {38e8889b-9731-53f5-e901-e8a7c1753074}
(0x38e8889b, 0x9731, 0x53f5, 0xe9, 0x01, 0xe8, 0xa7, 0xc1, 0x75, 0x30, 0x74),
TraceLoggingOptionProjectTelemetry());

void Trace::RegisterProvider() noexcept
{
TraceLoggingRegister(g_hProvider);
}

void Trace::UnregisterProvider() noexcept
{
TraceLoggingUnregister(g_hProvider);
}

void Trace::EnableImageResizer(_In_ bool enabled) noexcept
{
TraceLoggingWrite(
g_hProvider,
"ImageResizer_EnableImageResizer",
ProjectTelemetryPrivacyDataTag(ProjectTelemetryTag_ProductAndServicePerformance),
TraceLoggingKeyword(PROJECT_KEYWORD_MEASURE),
TraceLoggingBoolean(enabled, "Enabled"));
}


void Trace::Invoked() noexcept
{
TraceLoggingWrite(
g_hProvider,
"ImageResizer_Invoked",
ProjectTelemetryPrivacyDataTag(ProjectTelemetryTag_ProductAndServicePerformance),
TraceLoggingKeyword(PROJECT_KEYWORD_MEASURE));
}

void Trace::InvokedRet(_In_ HRESULT hr) noexcept
{
TraceLoggingWrite(
g_hProvider,
"ImageResizer_InvokedRet",
ProjectTelemetryPrivacyDataTag(ProjectTelemetryTag_ProductAndServicePerformance),
TraceLoggingHResult(hr),
TraceLoggingKeyword(PROJECT_KEYWORD_MEASURE));
}
11 changes: 11 additions & 0 deletions src/modules/imageresizer/dll/trace.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#pragma once

class Trace
{
public:
static void RegisterProvider() noexcept;
static void UnregisterProvider() noexcept;
static void EnableImageResizer(_In_ bool enabled) noexcept;
static void Invoked() noexcept;
static void InvokedRet(_In_ HRESULT hr) noexcept;
};