FO 3.1.14 - Bug Fix (Plugin loader)

microsoft · Jun 18, 2021 · 0d110e4 · 0d110e4
2 parents a027835 + 68a0657
commit 0d110e4
Show file tree

Hide file tree

Showing 11 changed files with 96 additions and 56 deletions.
diff --git a/Build-SFPkgs.ps1 b/Build-SFPkgs.ps1
@@ -23,11 +23,11 @@ function Build-SFPkg {
 try {
     Push-Location $scriptPath
 
-    Build-SFPkg "Microsoft.ServiceFabricApps.FabricObserver.Linux.SelfContained.3.1.13" "$scriptPath\bin\release\FabricObserver\linux-x64\self-contained\FabricObserverType"
-    Build-SFPkg "Microsoft.ServiceFabricApps.FabricObserver.Linux.FrameworkDependent.3.1.13" "$scriptPath\bin\release\FabricObserver\linux-x64\framework-dependent\FabricObserverType"
+    Build-SFPkg "Microsoft.ServiceFabricApps.FabricObserver.Linux.SelfContained.3.1.14" "$scriptPath\bin\release\FabricObserver\linux-x64\self-contained\FabricObserverType"
+    Build-SFPkg "Microsoft.ServiceFabricApps.FabricObserver.Linux.FrameworkDependent.3.1.14" "$scriptPath\bin\release\FabricObserver\linux-x64\framework-dependent\FabricObserverType"
 
-    Build-SFPkg "Microsoft.ServiceFabricApps.FabricObserver.Windows.SelfContained.3.1.13" "$scriptPath\bin\release\FabricObserver\win-x64\self-contained\FabricObserverType"
-    Build-SFPkg "Microsoft.ServiceFabricApps.FabricObserver.Windows.FrameworkDependent.3.1.13" "$scriptPath\bin\release\FabricObserver\win-x64\framework-dependent\FabricObserverType"
+    Build-SFPkg "Microsoft.ServiceFabricApps.FabricObserver.Windows.SelfContained.3.1.14" "$scriptPath\bin\release\FabricObserver\win-x64\self-contained\FabricObserverType"
+    Build-SFPkg "Microsoft.ServiceFabricApps.FabricObserver.Windows.FrameworkDependent.3.1.14" "$scriptPath\bin\release\FabricObserver\win-x64\framework-dependent\FabricObserverType"
 }
 finally {
     Pop-Location

diff --git a/Documentation/Plugins.md b/Documentation/Plugins.md
@@ -37,6 +37,26 @@ FrameworkDependent = Requires that .NET Core 3.1 is already installed on target
 
 SelfContained = Includes all the binaries necessary for running .NET Core 3.1 applications on target machine. ***This is what you will want to use for your Azure deployments.***
 
+**Plugins and Dependencies** 
+
+- You MUST place all plugin dependency libraries in the same folder as your plugin dll.
+- Plugins (and their dependencies) can live in child folders in the Plugins directory, which will keep things cleaner for folks with multiple plugins.
+
+The Plugins folder/file structure MUST be: 
+
+- Config/Data/Plugins/MyPlugin/MyPlugin.dll (required), MyPlugin.pdb (optional), [ALL of MyPlugin.dll's private dependencies] (required) 
+
+OR 
+
+- Config/Data/Plugins/MyPlugin.dll (required), MyPlugin.pdb(optional), [ALL of MyPlugin.dll's private dependencies] (required).  
+
+A private plugin dependency is any file (typically a dll) that you reference in your plugin project that is not already referenced by FabricObserver. 
+
+So, things like Nuget packages or Project References or COM References that are only used by your plugin. It is important to stress that if a dependency dll has dependencies, then you MUST also place those in the plugin's directory.
+When you build your plugin project you will see the dependencies (if any) in the bin folder. Make sure ALL of them are placed into FO's Plugins folder or your plugin will not work. 
+
+**Build and Publish**  
+
 - Write your observer plugin!
 
 - Build your observer project, drop the output dll into the Data/Plugins folder in FabricObserver/PackageRoot.

diff --git a/Documentation/Using.md b/Documentation/Using.md
@@ -539,7 +539,7 @@ $appParams = @{ "FabricSystemObserverEnabled" = "true"; "FabricSystemObserverMem
 Then execute the application upgrade with
 
 ```Powershell
-Start-ServiceFabricApplicationUpgrade -ApplicationName fabric:/FabricObserver -ApplicationTypeVersion 3.1.13 -ApplicationParameter $appParams -Monitored -FailureAction rollback
+Start-ServiceFabricApplicationUpgrade -ApplicationName fabric:/FabricObserver -ApplicationTypeVersion 3.1.14 -ApplicationParameter $appParams -Monitored -FailureAction rollback
 ```  
 
 Note: On *Linux*, this will restart FO processes (one at a time, UD Walk with safety checks) due to the way Linux Capabilites work. In a nutshell, for any kind of application upgrade, we have to re-run the FO setup script to get the Capabilities in place. For Windows, FO processes will NOT be restarted.
diff --git a/FabricObserver.nuspec.template b/FabricObserver.nuspec.template
@@ -2,12 +2,17 @@
 <package xmlns="http://schemas.microsoft.com/packaging/2013/05/nuspec.xsd">
   <metadata minClientVersion="3.3.0">
     <id>%PACKAGE_ID%</id>
-    <version>3.1.13</version>
+    <version>3.1.14</version>
     <releaseNotes>
-      - New EventSource impl to better support ETW event listening impls (important for plugins that need to capture FO ETW event data). 
-      - ServiceEventSource class modified with new functions, moved to Extensibility library. 
-      - Changes to Logger (removed EtwLogger EventSource) and TelemetryData classes to support new ServiceEventSource impl. 
-      - NetworkUsage class code change: replaced try-finally explicit disposal pattern with using blocks. 
+    - Bug fix in FO plugin loader: native dependencies will not crash loader (and thus FO).
+    - For Plugin authors: You MUST place all plugin dependency libraries in the same folder as your plugin dll.
+    - Plugins (and their dependencies) can now live in child folders in the Plugins directory, which will keep things cleaner for folks with multiple plugins.
+    The Plugins folder/file structure MUST be: 
+    Config/Data/Plugins/MyPlugin/MyPlugin.dll (required), MyPlugin.pdb (optional), [ALL of MyPlugin.dll's private dependencies] (required) 
+    OR Config/Data/Plugins/MyPlugin.dll (required), MyPlugin.pdb(optional), [ALL of MyPlugin.dll's private dependencies] (required).  
+    A private plugin dependency is any package that you reference in your plugin project that is not already referenced by FabricObserver. 
+    So, things like Nuget packages or Project References or COM References that are only used by your plugin. It is important to stress that if a dependency dll has dependencies, 
+    then you MUST also place those in the plugin's directory.
     </releaseNotes>
     <authors>Microsoft</authors>
     <license type="expression">MIT</license>

diff --git a/FabricObserver/FabricObserver.cs b/FabricObserver/FabricObserver.cs
@@ -82,44 +82,59 @@ private void LoadObserversFromPlugins(IServiceCollection services)
                 return;
             }
 
-            string[] pluginDlls = Directory.GetFiles(pluginsDir, "*.dll", SearchOption.TopDirectoryOnly);
+            string[] pluginDlls = Directory.GetFiles(pluginsDir, "*.dll", SearchOption.AllDirectories);
 
             if (pluginDlls.Length == 0)
             {
                 return;
             }
 
             var pluginLoaders = new List<PluginLoader>(pluginDlls.Length);
-
             Type[] sharedTypes = { typeof(FabricObserverStartupAttribute), typeof(IFabricObserverStartup), typeof(IServiceCollection) };
 
-            foreach (string pluginDll in pluginDlls)
+            foreach (string dll in pluginDlls)
             {
-                PluginLoader loader = PluginLoader.CreateFromAssemblyFile(pluginDll, sharedTypes);
+                // This does not create an Assembly. It creates a PluginLoader instance for each dll in the Plugins folder.
+                // TODO: Figure out how to only load the plugin dll in an efficient way. For now, this is fine. This is not resource intensive.
+                PluginLoader loader = PluginLoader.CreateFromAssemblyFile(dll, sharedTypes);
                 pluginLoaders.Add(loader);
             }
 
             foreach (PluginLoader pluginLoader in pluginLoaders)
             {
-                Assembly pluginAssembly = pluginLoader.LoadDefaultAssembly();
-
-                // Note: This is a micro-optimization (including the for below it). It could just as well be left as IEnumerable and then foreach'd over.
-                // It is unlikely that there will be 1000s of plugins...
-                FabricObserverStartupAttribute[] startupAttributes = pluginAssembly.GetCustomAttributes<FabricObserverStartupAttribute>().ToArray();
+                Assembly pluginAssembly;
 
-                for (int i = 0; i < startupAttributes.Length; ++i)
+                try
                 {
-                    object startupObject = Activator.CreateInstance(startupAttributes[i].StartupType);
+                    // If your plugin has native library dependencies (that's fine), then we will land in the catch (BadImageFormatException).
+                    // This is by design. The Managed FO plugin assembly will successfully load, of course.
+                    pluginAssembly = pluginLoader.LoadDefaultAssembly();
 
-                    if (startupObject is IFabricObserverStartup fabricObserverStartup)
-                    {
-                        fabricObserverStartup.ConfigureServices(services, fabricClient, Context);
-                    }
-                    else
+                    FabricObserverStartupAttribute[] startupAttributes = pluginAssembly.GetCustomAttributes<FabricObserverStartupAttribute>().ToArray();
+
+                    for (int i = 0; i < startupAttributes.Length; ++i)
                     {
-                        throw new InvalidOperationException($"{startupAttributes[i].StartupType.FullName} must implement IFabricObserverStartup.");
+                        object startupObject = Activator.CreateInstance(startupAttributes[i].StartupType);
+
+                        if (startupObject is IFabricObserverStartup fabricObserverStartup)
+                        {
+                            fabricObserverStartup.ConfigureServices(services, fabricClient, Context);
+                        }
+                        else
+                        {
+                            // This will bring down FO, which it should: This means your plugin is not supported. Fix your bug.
+                            throw new InvalidOperationException($"{startupAttributes[i].StartupType.FullName} must implement IFabricObserverStartup.");
+                        }
                     }
                 }
+                catch (BadImageFormatException)
+                {
+                    continue;
+                }
+                finally
+                {
+                    pluginLoader?.Dispose();
+                }
             }
         }
     }

diff --git a/FabricObserver/PackageRoot/Data/Plugins/Readme.txt b/FabricObserver/PackageRoot/Data/Plugins/Readme.txt
@@ -29,9 +29,9 @@ so you just program as you did before and everything will work as it used to.
 
 Install .Net Core 3.1.
 
-Grab the latest FO nupkg from Releases section that suits your target OS (Linux or Windows).
-In general, you will want the SelfContained package, not the FrameworkDependent (for example, Azure OS images do not ship with .NET Core 3.1 aboard,
-so you need self-contained build which has all the binaries needed to run a .NET Core 3.1 app.)
+Grab the latest FabricObserver nupkg from https://www.nuget.org/profiles/ServiceFabricApps that suits your target OS (Linux or Windows).
+In general, you will want the SelfContained package, not FrameworkDependent (for example, Azure OS images do not ship with .NET Core 3.1 aboard,
+so you need SelfContained build which has all the binaries needed to run a .NET Core 3.1 app.)
 
 If you want to build your own nupkgs from FO source, then:
 
@@ -43,7 +43,9 @@ Navigate to top level directory (where the SLN lives, for example), then:
 	Target OS - Framework-dependent  = .NET Core 3.1 is already installed on target server
 	Target OS - Self-contained = includes all the files necessary for running .NET Core 3.1 applications
 4. Write an observer plugin!
-5. Build your observer project, drop the output dll into the Data/Plugins folder in FabricObserver/PackageRoot.
+5. Build your observer project, drop the output dll and *ALL* of its dependencies, both managed and native (this is *very* important), into the Config/Data/Plugins folder in FabricObserver/PackageRoot. 
+   You can place your plugin dll and all of its dependencies in its own (*same*) folder under the Plugins directory (useful if you have multiple plugins). 
+   Again, ALL plugin dll dependencies need to live in the *same* folder as the plugin dll. :)
 6. Add a new config section for your observer in FabricObserver/PackageRoot/Config/Settings.xml (see example at bottom of that file)
    Update ApplicationManifest.xml with Parameters if you want to support Application Parameter Updates for your plugin.
    (Look at both FabricObserver/PackageRoot/Config/Settings.xml and FabricObserverApp/ApplicationPackageRoot/ApplicationManifest.xml for several examples of how to do this.)

diff --git a/FabricObserver/PackageRoot/ServiceManifest._linux.xml b/FabricObserver/PackageRoot/ServiceManifest._linux.xml
@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="utf-8"?>
 <ServiceManifest Name="FabricObserverPkg"
-                 Version="3.1.13"
+                 Version="3.1.14"
                  xmlns="http://schemas.microsoft.com/2011/01/fabric"
                  xmlns:xsd="http://www.w3.org/2001/XMLSchema"
                  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
@@ -11,7 +11,7 @@
   </ServiceTypes>
 
   <!-- Code package is your service executable. -->
-  <CodePackage Name="Code" Version="3.1.13">
+  <CodePackage Name="Code" Version="3.1.14">
     <SetupEntryPoint>
       <ExeHost>
         <Program>setcaps.sh</Program>
@@ -27,11 +27,10 @@
 
   <!-- Config package is the contents of the Config directory under PackageRoot that contains an 
        independently-updateable and versioned set of custom configuration settings for your service. -->
-  <ConfigPackage Name="Config" Version="3.1.13" />
+  <ConfigPackage Name="Config" Version="3.1.14" />
 
   <!-- Data package is the contents of the Data directory under PackageRoot that contains an 
        independently-updateable and versioned custom data for your service.
        Observer plugin dlls must live in this folder, in a child folder named Plugins. -->
-  <DataPackage Name="Data" Version="3.1.13" />
-
+  <DataPackage Name="Data" Version="3.1.14" />
 </ServiceManifest>
diff --git a/FabricObserver/PackageRoot/ServiceManifest.xml b/FabricObserver/PackageRoot/ServiceManifest.xml
@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="utf-8"?>
 <ServiceManifest Name="FabricObserverPkg"
-                 Version="3.1.13"
+                 Version="3.1.14"
                  xmlns="http://schemas.microsoft.com/2011/01/fabric"
                  xmlns:xsd="http://www.w3.org/2001/XMLSchema"
                  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
@@ -11,7 +11,7 @@
   </ServiceTypes>
 
   <!-- Code package is your service executable. -->
-  <CodePackage Name="Code" Version="3.1.13">
+  <CodePackage Name="Code" Version="3.1.14">
     <EntryPoint>
       <ExeHost>
         <Program>FabricObserver</Program>
@@ -21,11 +21,10 @@
 
   <!-- Config package is the contents of the Config directory under PackageRoot that contains an 
        independently-updateable and versioned set of custom configuration settings for your service. -->
-  <ConfigPackage Name="Config" Version="3.1.13" />
+  <ConfigPackage Name="Config" Version="3.1.14" />
 
   <!-- Data package is the contents of the Data directory under PackageRoot that contains an 
        independently-updateable and versioned custom data for your service.
        Observer plugin dlls must live in this folder, in a child folder named Plugins. -->
-  <DataPackage Name="Data" Version="3.1.13" />
-
+  <DataPackage Name="Data" Version="3.1.14" />
 </ServiceManifest>
diff --git a/FabricObserverApp/ApplicationPackageRoot/ApplicationManifest.xml b/FabricObserverApp/ApplicationPackageRoot/ApplicationManifest.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="utf-8"?>
-<ApplicationManifest xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" ApplicationTypeName="FabricObserverType" ApplicationTypeVersion="3.1.13" xmlns="http://schemas.microsoft.com/2011/01/fabric">
+<ApplicationManifest xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" ApplicationTypeName="FabricObserverType" ApplicationTypeVersion="3.1.14" xmlns="http://schemas.microsoft.com/2011/01/fabric">
   <!-- Note: Add parameters for your Observer plugins here (e.g., [ObserverName]Enabled, etc.) so you can deploy
              simple configuration updates for them. By design, any observer you make using our 
              extensibility model will employ the same basic config parameters as all observers do.
@@ -18,7 +18,7 @@
     <Parameter Name="OSObserverEnabled" DefaultValue="true" />
     <Parameter Name="SFConfigurationObserverEnabled" DefaultValue="false" />
     <!--Parameter Name="SampleNewObserverEnabled" DefaultValue="true" />-->
-
+    
     <!-- Telemetry - AppInsights or LogAnalytics -->
     <Parameter Name="AppObserverEnableTelemetry" DefaultValue="false" />
     <Parameter Name="CertificateObserverEnableTelemetry" DefaultValue="false" />
@@ -29,7 +29,7 @@
     <Parameter Name="OSObserverEnableTelemetry" DefaultValue="false" />
     <Parameter Name="SFConfigurationObserverEnableTelemetry" DefaultValue="false" />
     <!--Parameter Name="SampleNewObserverEnableTelemetry" DefaultValue="false" /> -->
-
+    
     <!-- ETW - Custom EventSource Tracing -->
     <Parameter Name="AppObserverEnableEtw" DefaultValue="false" />
     <Parameter Name="CertificateObserverEnableEtw" DefaultValue="false" />
@@ -40,12 +40,12 @@
     <Parameter Name="OSObserverEnableEtw" DefaultValue="false" />
     <Parameter Name="SFConfigurationObserverEnableEtw" DefaultValue="false" />
     <!--Parameter Name="SampleNewObserverEnableEtw" DefaultValue="true" /> -->
-
+    
     <!-- CSV Logging -->
     <Parameter Name="AppObserverEnableCSVDataLogging" DefaultValue="false" />
     <Parameter Name="FabricSystemObserverEnableCSVDataLogging" DefaultValue="false" />
     <Parameter Name="NodeObserverEnableCSVDataLogging" DefaultValue="false" />
-
+    
     <!-- Verbose Logging -->
     <Parameter Name="AppObserverEnableVerboseLogging" DefaultValue="false" />
     <Parameter Name="CertificateObserverEnableVerboseLogging" DefaultValue="false" />
@@ -56,13 +56,13 @@
     <Parameter Name="OSObserverEnableVerboseLogging" DefaultValue="false" />
     <Parameter Name="SFConfigurationObserverEnableVerboseLogging" DefaultValue="false" />
     <!--Parameter Name="SampleNewObserverEnableVerboseLogging" DefaultValue="false" /> -->
-
+    
     <!-- Monitor Durations (TimeSpan format, e.g., 00:00:00) -->
     <Parameter Name="AppObserverMonitorDuration" DefaultValue="00:00:01" />
     <Parameter Name="DiskObserverMonitorDuration" DefaultValue="00:00:01" />
     <Parameter Name="FabricSystemObserverMonitorDuration" DefaultValue="00:00:01" />
     <Parameter Name="NodeObserverMonitorDuration" DefaultValue="00:00:02" />
-
+    
     <!-- Run Intervals (TimeSpan format, e.g., 0.00:00:00) -->
     <Parameter Name="AppObserverRunInterval" DefaultValue="" />
     <Parameter Name="CertificateObserverRunInterval" DefaultValue="1.00:00:00" />
@@ -73,7 +73,7 @@
     <Parameter Name="OSObserverRunInterval" DefaultValue="01:00:00" />
     <Parameter Name="SFConfigurationObserverRunInterval" DefaultValue="" />
     <!--Parameter Name="SampleNewObserverRunInterval" DefaultValue="" /> -->
-
+    
     <!-- AppObserver -->
     <Parameter Name="AppObserverClusterOperationTimeoutSeconds" DefaultValue="120" />
     <Parameter Name="AppObserverUseCircularBuffer" DefaultValue="false" />
@@ -145,7 +145,7 @@
        should match the Name and Version attributes of the ServiceManifest element defined in the 
        ServiceManifest.xml file. -->
   <ServiceManifestImport>
-    <ServiceManifestRef ServiceManifestName="FabricObserverPkg" ServiceManifestVersion="3.1.13" />
+    <ServiceManifestRef ServiceManifestName="FabricObserverPkg" ServiceManifestVersion="3.1.14" />
     <ConfigOverrides>
       <ConfigOverride Name="Config">
         <Settings>