Merge remote-tracking branch 'origin/main' into feature/lambda-aspnet…

…core-autoinstrumentation
newrelic · Aug 6, 2024 · 2b0633c · 2b0633c
2 parents bc58ac7 + b18158f
commit 2b0633c
Show file tree

Hide file tree

Showing 35 changed files with 164 additions and 173 deletions.
diff --git a/.github/workflows/all_solutions.yml b/.github/workflows/all_solutions.yml
@@ -559,8 +559,8 @@ jobs:
           fi
 
           cd ${{ github.workspace }}/build/Linux
-          docker-compose build build_rpm
-          docker-compose run -e AGENT_VERSION=$agentVersion -e GPG_KEYS=/keys/gpg.tar.bz2 build_rpm
+          docker compose build build_rpm
+          docker compose run -e AGENT_VERSION=$agentVersion -e GPG_KEYS=/keys/gpg.tar.bz2 build_rpm
         shell: bash
 
       - name: Archive RPM Package Artifacts
@@ -612,8 +612,8 @@ jobs:
           fi
 
           cd ${{ github.workspace }}/build/Linux
-          docker-compose build build_deb
-          docker-compose run -e AGENT_VERSION=$agentVersion build_deb
+          docker compose build build_deb
+          docker compose run -e AGENT_VERSION=$agentVersion build_deb
         shell: bash
 
       - name: Archive Debian Package Artifacts

diff --git a/.github/workflows/build_profiler.yml b/.github/workflows/build_profiler.yml
@@ -139,6 +139,7 @@ jobs:
 
     env:
       profiler_path: ${{ github.workspace }}/src/Agent/NewRelic/Profiler
+      CORECLR_NEWRELIC_HOME: ${{ github.workspace }}/src/Agent/NewRelic/newrelichome_x64_coreclr_linux # not used but required by Profiler/docker-compose.yml
 
     steps:
       # intentionally disabled for this job, when enabled it causes a failure in the Build Linux Profiler step
@@ -164,8 +165,8 @@ jobs:
       - name: Build Linux Profiler
         run: |
           cd ${{ env.profiler_path }}
-          docker-compose build build
-          docker-compose run build
+          docker compose build build
+          docker compose run build
         shell: bash
 
       - name: Move Profiler to staging folder

diff --git a/.github/workflows/deploy_agent.yml b/.github/workflows/deploy_agent.yml
@@ -422,12 +422,12 @@ jobs:
           cd ${{ github.workspace }}/deploy/linux/
           find . -name "*.bash" |xargs chmod a+x
           find . -type f |xargs dos2unix
-          docker-compose build
+          docker compose build
           if [ "${{ inputs.deploy }}" == "true" ] ; then
-            docker-compose run deploy_packages
+            docker compose run deploy_packages
           else
             echo "Input:deploy was not true.  The following deploy command was not run:"
-            echo "docker-compose run deploy_packages"
+            echo "docker compose run deploy_packages"
           fi
         shell: bash
 

diff --git a/build/Linux/linux_packaging.md b/build/Linux/linux_packaging.md
@@ -4,15 +4,15 @@ The Linux CoreCLR agent can be packaged into .rpm (for Red Hat/Centos/Oracle/SUS
 
 1. From Visual Studio, build the FullAgent solution, `Release` or `Debug` depending on your needs.
 2. In Powershell, from `build/Linux`:
-      1. `docker-compose build`
-      2. `docker-compose run build_rpm`
-      3. `docker-compose run build_deb`
+      1. `docker compose build`
+      2. `docker compose run build_rpm`
+      3. `docker compose run build_deb`
 
 Optional: sign the .rpm
 
-1. `docker-compose run -e GPG_KEYS=/keys/gpg.tar.bz2 build_rpm`
+1. `docker compose run -e GPG_KEYS=/keys/gpg.tar.bz2 build_rpm`
 
 You can do ad hoc testing inside containers using the `test_debian` and/or `test_centos` services and `bash`.
 
-     docker-compose run test_debian bash
-     docker-compose run test_centos bash
+     docker compose run test_debian bash
+     docker compose run test_centos bash
diff --git a/build/build.ps1 b/build/build.ps1
@@ -79,16 +79,16 @@ Write-Host "===================================="
 Write-Host "Executing Linux builds in Docker for Agent Version: $agentVersion"
 Push-Location "$rootDirectory\build\Linux"
 # Build the docker images
-docker-compose build
+docker compose build
 # Build the Debian package
-docker-compose run -e AGENT_VERSION=$agentVersion build_deb
+docker compose run -e AGENT_VERSION=$agentVersion build_deb
 # Build the RPM package, signing it if a key was supplied
 if (!($gpgKeyPath -eq "")) {
     New-Item keys -Type Directory -Force
     Copy-Item $gpgKeyPath .\keys\gpg.tar.bz2 -Force
-    docker-compose run -e AGENT_VERSION=$agentVersion -e GPG_KEYS=/keys/gpg.tar.bz2 build_rpm
+    docker compose run -e AGENT_VERSION=$agentVersion -e GPG_KEYS=/keys/gpg.tar.bz2 build_rpm
 } else {
-    docker-compose run -e AGENT_VERSION=$agentVersion build_rpm
+    docker compose run -e AGENT_VERSION=$agentVersion build_rpm
 }
 Pop-Location
 Write-Host "===================================="

diff --git a/deploy/linux/README.md b/deploy/linux/README.md
@@ -32,11 +32,11 @@ To deploy the .rpm and .deb packages for a particular release version (e.g. 10.0
 
 5. Build the Docker container:
 
-        docker-compose build
+        docker compose build
 
 6. Run the deploy (or rollback) process:
 
-        docker-compose run deploy_packages
+        docker compose run deploy_packages
 
 Note that the scripts in ./deploy_scripts came from the PHP agent team and have a lot of logic in them to support their particular build/test/release processes, 
 not all of which we are using.  However, since we are sharing the same public package sources with the PHP agent, anything this script does needs to be cautious 

diff --git a/release-please/.release-please-manifest.json b/release-please/.release-please-manifest.json
@@ -1,3 +1,3 @@
 {
-  ".": "10.27.0"
+  ".": "10.28.0"
 }
diff --git a/src/Agent/CHANGELOG.md b/src/Agent/CHANGELOG.md
@@ -4,6 +4,21 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [10.28.0](https://github.com/newrelic/newrelic-dotnet-agent/compare/v10.27.0...v10.28.0) (2024-08-05)
+
+
+### New features
+
+* Improve serverless mode detection ([#2661](https://github.com/newrelic/newrelic-dotnet-agent/issues/2661)) ([5f5dda8](https://github.com/newrelic/newrelic-dotnet-agent/commit/5f5dda860a78152574f71f4f1095248707e8c7e3))
+* Set application name via command line with the .msi installer ([#2648](https://github.com/newrelic/newrelic-dotnet-agent/issues/2648)) ([369dcba](https://github.com/newrelic/newrelic-dotnet-agent/commit/369dcbab4f3fa59354f683bae16b711f45be2387))
+
+
+### Fixes
+
+* Better Lambda web request input parameter validation. ([#2653](https://github.com/newrelic/newrelic-dotnet-agent/issues/2653)) ([810d4af](https://github.com/newrelic/newrelic-dotnet-agent/commit/810d4aff20457200b4166daa9744cefe8dfc699b)), closes [#2652](https://github.com/newrelic/newrelic-dotnet-agent/issues/2652)
+* Revert recent Profiler warning fixes to address reported instability ([#2663](https://github.com/newrelic/newrelic-dotnet-agent/issues/2663)) ([b3c9cd1](https://github.com/newrelic/newrelic-dotnet-agent/commit/b3c9cd10c47dbe5c4654a1dcb1f90c3adeabe90f))
+* SQS instrumentation could cause InvalidOperationException ([#2645](https://github.com/newrelic/newrelic-dotnet-agent/issues/2645)) ([#2646](https://github.com/newrelic/newrelic-dotnet-agent/issues/2646)) ([40b6ad5](https://github.com/newrelic/newrelic-dotnet-agent/commit/40b6ad5b899942eff9241da362a653a010cf2e7f))
+
 ## [10.27.0](https://github.com/newrelic/newrelic-dotnet-agent/compare/v10.26.0...v10.27.0) (2024-07-15)
 
 

diff --git a/src/Agent/NewRelic/Home/Home.csproj b/src/Agent/NewRelic/Home/Home.csproj
@@ -13,7 +13,7 @@
   </Target>
 
   <ItemGroup>
-    <PackageReference Include="NewRelic.Agent.Internal.Profiler" Version="10.25.1.10"/>
+    <PackageReference Include="NewRelic.Agent.Internal.Profiler" Version="10.27.0.20"/>
   </ItemGroup>
 
 </Project>
diff --git a/src/Agent/NewRelic/Profiler/Common/CorStandIn.h b/src/Agent/NewRelic/Profiler/Common/CorStandIn.h
@@ -3,8 +3,6 @@
 * SPDX-License-Identifier: Apache-2.0
 */
 #pragma warning(push)
-// Since this isn't our code, we don't want to mess with it. These warnings can be safely ignored.
-#pragma warning(disable: 4458) // Scope hides class member with same name.
-#pragma warning(disable: 26495) // Uninitialized member variable, even if it's always set before use.
+#pragma warning(disable : 4458)
 #include <corhlpr.h>
 #pragma warning(pop)
diff --git a/src/Agent/NewRelic/Profiler/Configuration/Configuration.h b/src/Agent/NewRelic/Profiler/Configuration/Configuration.h
@@ -47,24 +47,24 @@ namespace NewRelic { namespace Profiler { namespace Configuration {
             , _ignoreList(new IgnoreInstrumentationList())
         {
             try {
-                auto globalNewRelicConfigurationDocument = std::make_shared<rapidxml::xml_document<xchar_t>>();
-                globalNewRelicConfigurationDocument->parse<rapidxml::parse_trim_whitespace | rapidxml::parse_normalize_whitespace>(const_cast<xchar_t*>(globalNewRelicConfiguration.c_str()));
+                rapidxml::xml_document<xchar_t> globalNewRelicConfigurationDocument;
+                globalNewRelicConfigurationDocument.parse<rapidxml::parse_trim_whitespace | rapidxml::parse_normalize_whitespace>(const_cast<xchar_t*>(globalNewRelicConfiguration.c_str()));
 
-                auto globalNewRelicConfigurationNode = GetConfigurationNode(globalNewRelicConfigurationDocument);
+                auto globalNewRelicConfigurationNode  = GetConfigurationNode(globalNewRelicConfigurationDocument);
                 if (globalNewRelicConfigurationNode == nullptr) 
                 {
                     LogError(L"Unable to locate configuration node in the global newrelic.config file.");
                     throw ConfigurationException();
                 }
 
                 auto appliedNewRelicConfigurationNode = globalNewRelicConfigurationNode;
-                auto localNewRelicConfigurationDocument = std::make_shared<rapidxml::xml_document<xchar_t>>();
 
                 if (localNewRelicConfiguration.second)
                 {
                     try
                     {
-                        localNewRelicConfigurationDocument->parse<rapidxml::parse_trim_whitespace | rapidxml::parse_normalize_whitespace>(const_cast<xchar_t*>(localNewRelicConfiguration.first.c_str()));
+                        rapidxml::xml_document<xchar_t> localNewRelicConfigurationDocument;
+                        localNewRelicConfigurationDocument.parse<rapidxml::parse_trim_whitespace | rapidxml::parse_normalize_whitespace>(const_cast<xchar_t*>(localNewRelicConfiguration.first.c_str()));
 
                         auto localNewRelicConfigurationNode = GetConfigurationNode(localNewRelicConfigurationDocument);
                         if (localNewRelicConfigurationNode == nullptr)
@@ -92,7 +92,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration {
                 SetLogLevel(appliedNewRelicConfigurationNode);
                 SetInstrumentationData(appliedNewRelicConfigurationNode);
                 SetApplicationPools(appliedNewRelicConfigurationNode);
-                
+
             } catch (const rapidxml::parse_error& exception) {
                 // We log two separate error messages here because sometimes the logging macros hang when
                 // logging the "where" contents
@@ -196,15 +196,15 @@ namespace NewRelic { namespace Profiler { namespace Configuration {
             return _logLevel;
         }
 
-        bool GetConsoleLogging() const
+        bool GetConsoleLogging()
         {
             return _consoleLogging;
         }
-        bool GetLoggingEnabled() const
+        bool GetLoggingEnabled()
         {
             return _loggingEnabled;
         }
-        IgnoreInstrumentationListPtr GetIgnoreInstrumentationList() const
+        IgnoreInstrumentationListPtr GetIgnoreInstrumentationList()
         {
             return _ignoreList;
         }
@@ -224,9 +224,9 @@ namespace NewRelic { namespace Profiler { namespace Configuration {
         std::shared_ptr<NewRelic::Profiler::Logger::IFileDestinationSystemCalls> _systemCalls;
         IgnoreInstrumentationListPtr _ignoreList;
 
-        rapidxml::xml_node<xchar_t>* GetConfigurationNode(const std::shared_ptr<rapidxml::xml_document<xchar_t>> document)
+        rapidxml::xml_node<xchar_t>* GetConfigurationNode(const rapidxml::xml_document<xchar_t>& document)
         {
-            auto configurationNode = document->first_node(_X("configuration"), 0, false);
+            auto configurationNode = document.first_node(_X("configuration"), 0, false);
             if (configurationNode == nullptr) {
                 return nullptr;
             }
@@ -294,7 +294,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration {
             _logLevel = TryParseLogLevel(level);
         }
 
-        Logger::Level TryParseLogLevel(const xstring_t& logText) const
+        Logger::Level TryParseLogLevel(const xstring_t& logText)
         {
             if (Strings::AreEqualCaseInsensitive(logText, _X("off"))) {
                 return Logger::Level::LEVEL_ERROR;
@@ -423,8 +423,8 @@ namespace NewRelic { namespace Profiler { namespace Configuration {
             if (applicationConfiguration.empty())
                 return;
 
-            auto document = std::make_shared<rapidxml::xml_document<xchar_t>>();
-            document->parse<rapidxml::parse_trim_whitespace | rapidxml::parse_normalize_whitespace>(const_cast<xchar_t*>(applicationConfiguration.c_str()));
+            rapidxml::xml_document<xchar_t> document;
+            document.parse<rapidxml::parse_trim_whitespace | rapidxml::parse_normalize_whitespace>(const_cast<xchar_t*>(applicationConfiguration.c_str()));
             auto configurationNode = GetConfigurationNode(document);
 
             auto appSettingsNode = configurationNode->first_node(_X("appSettings"), 0, false);
@@ -468,7 +468,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration {
         static bool IsProcessInProcessList(const ProcessesPtr& processes, const xstring_t& processName)
         {
             // check the processes loaded from configuration
-            for (auto& validProcessName : *processes) {
+            for (auto validProcessName : *processes) {
                 if (Strings::EndsWith(processName, validProcessName)) {
                     return true;
                 }
@@ -498,7 +498,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration {
             return isIis;
         }
 
-        bool ShouldInstrumentApplicationPool(const xstring_t& appPoolId) const
+        bool ShouldInstrumentApplicationPool(const xstring_t& appPoolId)
         {
             if (ApplicationPoolIsOnBlackList(appPoolId, _applicationPoolsBlackList)) {
                 LogInfo(_X("This application pool (") + appPoolId + _X(") is explicitly configured to NOT be instrumented."));

diff --git a/src/Agent/NewRelic/Profiler/Configuration/IgnoreInstrumentation.h b/src/Agent/NewRelic/Profiler/Configuration/IgnoreInstrumentation.h
@@ -50,7 +50,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration
             }
 
         private:
-            bool Matches(xstring_t assembly, xstring_t className) const
+            bool Matches(xstring_t assembly, xstring_t className)
             {
                 if (!Strings::AreEqualCaseInsensitive(AssemblyName, assembly))
                 {

diff --git a/src/Agent/NewRelic/Profiler/Configuration/InstrumentationConfiguration.h b/src/Agent/NewRelic/Profiler/Configuration/InstrumentationConfiguration.h
@@ -33,7 +33,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration
             , _foundServerlessInstrumentationPoint(false)
         {
             // pull instrumentation points from every xml string
-            for (auto& instrumentationXml : *instrumentationXmls)
+            for (auto instrumentationXml : *instrumentationXmls)
             {
                 try
                 {
@@ -64,13 +64,13 @@ namespace NewRelic { namespace Profiler { namespace Configuration
             , _systemCalls(nullptr)
             , _foundServerlessInstrumentationPoint(false)
         {
-            for (auto& instrumentationPoint : *instrumentationPoints)
+            for (auto instrumentationPoint : *instrumentationPoints)
             {
                 AddInstrumentationPointToCollectionsIfNotIgnored(instrumentationPoint);
             }
         }
 
-        uint16_t GetInvalidFileCount() const
+        uint16_t GetInvalidFileCount()
         {
             return _invalidFileCount;
         }
@@ -100,7 +100,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration
             // We may have multiple matching instrumentation points that target different assembly versions. See if we can find one that meets
             // the version requirements
             AssemblyVersion foundVersion(function->GetAssemblyProps());
-            for (auto& instPoint : instPoints)
+            for (auto instPoint : instPoints)
             {
                 if ((instPoint->MinVersion != nullptr) && (foundVersion < *instPoint->MinVersion))
                 {
@@ -126,30 +126,36 @@ namespace NewRelic { namespace Profiler { namespace Configuration
                 return;
             }
 
-            auto lambdaInstPoint = _systemCalls->TryGetEnvironmentVariable(_X("_HANDLER"));
+            // give precedence to the NEW_RELIC_LAMBDA_HANDLER environment variable
+            auto lambdaInstPoint = _systemCalls->TryGetEnvironmentVariable(_X("NEW_RELIC_LAMBDA_HANDLER"));
             if (lambdaInstPoint != nullptr)
             {
-                AddInstrumentationPointToCollectionFromEnvironment(*lambdaInstPoint);
-                _foundServerlessInstrumentationPoint = true;
-                return;
+                LogDebug("Found NEW_RELIC_LAMBDA_HANDLER environment variable: ", *lambdaInstPoint);
+                if (TryAddInstrumentationPointToCollectionFromEnvironment(*lambdaInstPoint))
+                {
+                    _foundServerlessInstrumentationPoint = true;
+                    return;
+                }
             }
 
-            lambdaInstPoint = _systemCalls->TryGetEnvironmentVariable(_X("NEW_RELIC_LAMBDA_HANDLER"));
+            lambdaInstPoint = _systemCalls->TryGetEnvironmentVariable(_X("_HANDLER"));
             if (lambdaInstPoint != nullptr)
             {
-                AddInstrumentationPointToCollectionFromEnvironment(*lambdaInstPoint);
-                _foundServerlessInstrumentationPoint = true;
+                LogDebug("Found _HANDLER environment variable: ", *lambdaInstPoint);
+                if (TryAddInstrumentationPointToCollectionFromEnvironment(*lambdaInstPoint))
+                    _foundServerlessInstrumentationPoint = true;
             }
         }
 
-        void AddInstrumentationPointToCollectionFromEnvironment(xstring_t text)
+        bool TryAddInstrumentationPointToCollectionFromEnvironment(xstring_t text)
         {
             auto segments = Strings::Split(text, _X("::"));
             if (segments.size() != 3)
             {
                 LogWarn(text, L" is not a valid method descriptor. It must be in the format 'assembly::class::method'");
-                return;
+                return false;
             }
+
             LogInfo(L"Serverless mode detected. Assembly: ", segments[0], L" Class: ", segments[1], L" Method: ", segments[2]);
 
             InstrumentationPointPtr instrumentationPoint(new InstrumentationPoint());
@@ -167,6 +173,8 @@ namespace NewRelic { namespace Profiler { namespace Configuration
 
             (*_instrumentationPointsMap)[instrumentationPoint->GetMatchKey()].insert(instrumentationPoint);
             _instrumentationPointsSet->insert(instrumentationPoint);
+
+            return true;
         }
 
     private:
@@ -228,9 +236,9 @@ namespace NewRelic { namespace Profiler { namespace Configuration
 
         void GetInstrumentationPoints(xstring_t instrumentationXml)
         {
-            auto document = std::make_shared<rapidxml::xml_document<xchar_t>>();
-            document->parse<rapidxml::parse_trim_whitespace | rapidxml::parse_normalize_whitespace>(const_cast<xchar_t*>(instrumentationXml.c_str()));
-            auto extensionNode = document->first_node(_X("extension"), 0, false);
+            rapidxml::xml_document<xchar_t> document;
+            document.parse<rapidxml::parse_trim_whitespace | rapidxml::parse_normalize_whitespace>(const_cast<xchar_t*>(instrumentationXml.c_str()));
+            auto extensionNode = document.first_node(_X("extension"), 0, false);
             if (extensionNode == nullptr)
             {
                 LogWarn(L"extension node not found in instrumentation file. Please validate your instrumentation files against extensions/extension.xsd or contact New Relic support.");
@@ -397,7 +405,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration
             // if the ClassName includes multiple classes, we have to split this into multiple instrumentation points
             auto instrumentationPoints = SplitInstrumentationPointsOnClassNames(instrumentationPoint);
 
-            for (auto& iPoint : instrumentationPoints) {
+            for (auto iPoint : instrumentationPoints) {
 
                 // finally add the new instrumentation point(s) to our set of instrumentation points
                 // Note that there may be "duplicated" instrumentation points that target different assembly versions