From 470ae38086e01dac556c19beefb769738b317808 Mon Sep 17 00:00:00 2001
From: Derek Brown <derek.brown@datadoghq.com>
Date: Thu, 24 Mar 2022 14:26:19 -0700
Subject: [PATCH 1/4] fix npm upgrade bug

---
 omnibus/resources/agent/msi/source.wxs.erb    |  7 ++++-
 .../notes/fixnpmupgrade-ef6b42d87c980541.yaml | 11 +++++++
 .../install-help/cal/customactiondata.cpp     | 29 ++++++++-----------
 3 files changed, 29 insertions(+), 18 deletions(-)
 create mode 100644 releasenotes/notes/fixnpmupgrade-ef6b42d87c980541.yaml

diff --git a/omnibus/resources/agent/msi/source.wxs.erb b/omnibus/resources/agent/msi/source.wxs.erb
index c993503902f79..865366946f3de 100644
--- a/omnibus/resources/agent/msi/source.wxs.erb
+++ b/omnibus/resources/agent/msi/source.wxs.erb
@@ -79,6 +79,7 @@
     <Property Id="APIKEY" Hidden="yes" />
     <Property Id="SITE" Value="datadoghq.com" />
     <Property Id="FinalizeInstall" Hidden="yes" />
+    <Property Id="NPMFEATURESTATE" />
      <PropertyRef Id="WIX_ACCOUNT_ADMINISTRATORS" />
      <PropertyRef Id="WIX_ACCOUNT_LOCALSYSTEM" />
      <PropertyRef Id="WIX_ACCOUNT_USERS" />
@@ -101,7 +102,9 @@
     <SetProperty Id="PROJECTLOCATIONONCMDLINESLASH" Value="[PROJECTLOCATION]\" Before="AppSearch">PROJECTLOCATION </SetProperty>
     <SetProperty Id="CONFDIRONCOMMANDLINE" Value="[APPLICATIONDATADIRECTORY]" Before="AppSearch">APPLICATIONDATADIRECTORY </SetProperty>
     <SetProperty Id="CONFDIRONCOMMANDLINESLASH" Value="[APPLICATIONDATADIRECTORY]\" Before="AppSearch">APPLICATIONDATADIRECTORY </SetProperty>
-
+    
+    <SetProperty Action="SetNPMOff" Id="NPMFEATURESTATE" Before="SetFinalizeProperty" Value="off" Sequence="execute"><![CDATA[not (&NPM = 3)]]></SetProperty>
+    <SetProperty Action="SetNPMOn"  Id="NPMFEATURESTATE" Before="SetFinalizeProperty" Value="on"  Sequence="execute"><![CDATA[(&NPM = 3)]]></SetProperty>
     <!-- This condition allows the install if
         1) the user didn't supply the binary directory on the command line
         2) this is NOT an upgrade
@@ -651,6 +654,7 @@
       This does not affect the final artefact, as it seems to affects only the base string, not the expanded one.
       At runtime this limitation doesn\'t seem to apply.
     -->
+    
     <CustomAction Id='SetFinalizeProperty' Return='check' Property='FinalizeInstall'
         Value='
             UILevel=[UILevel]
@@ -659,6 +663,7 @@
             DDAGENTUSER_NAME=[DDAGENTUSER_NAME]
             DDAGENTUSER_PASSWORD=[DDAGENTUSER_PASSWORD]
             SYSPROBE_PRESENT=[SYSPROBE_PRESENT]
+            NPMFEATURE=[NPMFEATURESTATE]
             ADDLOCAL=[ADDLOCAL]
             APIKEY=[APIKEY]
             TAGS=[TAGS]
diff --git a/releasenotes/notes/fixnpmupgrade-ef6b42d87c980541.yaml b/releasenotes/notes/fixnpmupgrade-ef6b42d87c980541.yaml
new file mode 100644
index 0000000000000..2db628f56e63b
--- /dev/null
+++ b/releasenotes/notes/fixnpmupgrade-ef6b42d87c980541.yaml
@@ -0,0 +1,11 @@
+# Each section from every release note are combined when the
+# CHANGELOG.rst is rendered. So the text needs to be worded so that
+# it does not depend on any information only available in another
+# section. This may mean repeating some details, but each section
+# must be readable independently of the other.
+#
+# Each section note must be formatted as reStructuredText.
+---
+fixes:
+  - |
+    For Windows, fixes problem in upgrade wherein NPM driver is not automatically started by system probe.
diff --git a/tools/windows/install-help/cal/customactiondata.cpp b/tools/windows/install-help/cal/customactiondata.cpp
index 04376e1d6b204..c5734f04af186 100644
--- a/tools/windows/install-help/cal/customactiondata.cpp
+++ b/tools/windows/install-help/cal/customactiondata.cpp
@@ -145,6 +145,7 @@ bool CustomActionData::parseSysprobeData()
     std::wstring sysprobePresent;
     std::wstring addlocal;
     std::wstring npm;
+    std::wstring npmFeature;
     this->_doInstallSysprobe = false;
     this->_ddnpmPresent = false;
     if (!this->value(L"SYSPROBE_PRESENT", sysprobePresent))
@@ -172,24 +173,18 @@ bool CustomActionData::parseSysprobeData()
         this->_ddnpmPresent = true;
     }
 
-    // now check to see if we're installing the driver
-    if (!this->value(L"ADDLOCAL", addlocal))
-    {
-        // should never happen.  But if the addlocalkey isn't there,
-        // don't bother trying
-        WcaLog(LOGMSG_STANDARD, "ADDLOCAL not present");
-
-        return true;
-    }
-    WcaLog(LOGMSG_STANDARD, "ADDLOCAL is (%S)", addlocal.c_str());
-    if (_wcsicmp(addlocal.c_str(), L"ALL") == 0)
+    
+    if(this->value(L"NPMFEATURE", npmFeature))
     {
-        // installing all components, do it
-        this->_ddnpmPresent = true;
-        WcaLog(LOGMSG_STANDARD, "ADDLOCAL is ALL");
-    } else if (addlocal.find(L"NPM") != std::wstring::npos) {
-        WcaLog(LOGMSG_STANDARD, "ADDLOCAL contains NPM %S", addlocal.c_str());
-        this->_ddnpmPresent = true;
+        // this property is set to "on" or "off" depending on the desired installed state
+        // of the NPM feature.
+        WcaLog(LOGMSG_STANDARD, "NPMFEATURE key is present and (%S)", npmFeature.c_str());
+        if (_wcsicmp(npmFeature.c_str(), L"on") == 0)
+        {
+            this->_ddnpmPresent = true;
+        }
+    } else {
+        WcaLog(LOGMSG_STANDARD, "NPMFEATURE not present");
     }
 
     return true;

From 55dff990c056ecc316512a0474afedc4498b53c8 Mon Sep 17 00:00:00 2001
From: Derek Brown <derek.brown@datadoghq.com>
Date: Thu, 24 Mar 2022 16:12:47 -0700
Subject: [PATCH 2/4] add helpful comment

---
 omnibus/resources/agent/msi/source.wxs.erb | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/omnibus/resources/agent/msi/source.wxs.erb b/omnibus/resources/agent/msi/source.wxs.erb
index 865366946f3de..961ce7d6bd895 100644
--- a/omnibus/resources/agent/msi/source.wxs.erb
+++ b/omnibus/resources/agent/msi/source.wxs.erb
@@ -103,6 +103,19 @@
     <SetProperty Id="CONFDIRONCOMMANDLINE" Value="[APPLICATIONDATADIRECTORY]" Before="AppSearch">APPLICATIONDATADIRECTORY </SetProperty>
     <SetProperty Id="CONFDIRONCOMMANDLINESLASH" Value="[APPLICATIONDATADIRECTORY]\" Before="AppSearch">APPLICATIONDATADIRECTORY </SetProperty>
     
+    <!--
+      The following sets a property (to be used by the custom action).
+      the "&" indicates the action state of a feature (so &NPM refers to the NPM _feature_ not the NPM property that might be set on the command line)
+      "3" refers to the action state "on the local computer" (which is the only one we support besides "not present", which happens to be "2")
+
+      So, the NPMFEATURESTATE property will be set to "off" if the NPM feature is no installed on the local computer
+      and will be set to "on" if it is used on the local computer.  
+
+      This flag will be used in the custom action to decide what service dependencies need to be set
+
+      For more information on this syntax see
+      https://www.firegiant.com/wix/tutorial/com-expression-syntax-miscellanea/expression-syntax/
+      -->
     <SetProperty Action="SetNPMOff" Id="NPMFEATURESTATE" Before="SetFinalizeProperty" Value="off" Sequence="execute"><![CDATA[not (&NPM = 3)]]></SetProperty>
     <SetProperty Action="SetNPMOn"  Id="NPMFEATURESTATE" Before="SetFinalizeProperty" Value="on"  Sequence="execute"><![CDATA[(&NPM = 3)]]></SetProperty>
     <!-- This condition allows the install if

From b9f8732129fe7c64497f8d790120eb1a62b6abbf Mon Sep 17 00:00:00 2001
From: Derek Brown <derek.brown@datadoghq.com>
Date: Wed, 30 Mar 2022 08:22:37 -0700
Subject: [PATCH 3/4] Update
 tools/windows/install-help/cal/customactiondata.cpp

Co-authored-by: Julien Lebot <julien.lebot@datadoghq.com>
---
 tools/windows/install-help/cal/customactiondata.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/windows/install-help/cal/customactiondata.cpp b/tools/windows/install-help/cal/customactiondata.cpp
index c5734f04af186..b0096fb2e5fe8 100644
--- a/tools/windows/install-help/cal/customactiondata.cpp
+++ b/tools/windows/install-help/cal/customactiondata.cpp
@@ -174,7 +174,7 @@ bool CustomActionData::parseSysprobeData()
     }
 
     
-    if(this->value(L"NPMFEATURE", npmFeature))
+    if (this->value(L"NPMFEATURE", npmFeature))
     {
         // this property is set to "on" or "off" depending on the desired installed state
         // of the NPM feature.

From 64d2b1ef8a9380e3424db26b039011dbc509ee62 Mon Sep 17 00:00:00 2001
From: Derek Brown <derek.brown@datadoghq.com>
Date: Wed, 30 Mar 2022 08:22:45 -0700
Subject: [PATCH 4/4] Update
 tools/windows/install-help/cal/customactiondata.cpp

Co-authored-by: Julien Lebot <julien.lebot@datadoghq.com>
---
 tools/windows/install-help/cal/customactiondata.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/windows/install-help/cal/customactiondata.cpp b/tools/windows/install-help/cal/customactiondata.cpp
index b0096fb2e5fe8..d2ee3fa55f5be 100644
--- a/tools/windows/install-help/cal/customactiondata.cpp
+++ b/tools/windows/install-help/cal/customactiondata.cpp
@@ -183,7 +183,9 @@ bool CustomActionData::parseSysprobeData()
         {
             this->_ddnpmPresent = true;
         }
-    } else {
+    }
+    else
+    {
         WcaLog(LOGMSG_STANDARD, "NPMFEATURE not present");
     }