From 3ed1ec36d29328553c1a4131a6bb23c3d025a276 Mon Sep 17 00:00:00 2001 From: Helen Zhang Date: Wed, 13 Sep 2023 13:42:23 -0700 Subject: [PATCH 1/7] Add template plugin manifest --- .../PluginTemplate/pluginManifest.json | 19 ++++++++++ .../PluginTool-CLI.md | 36 ++++++++++++++++++- .../Manifest/PluginManifestSchema.json | 6 ++-- 3 files changed, 57 insertions(+), 4 deletions(-) create mode 100644 devel-template/templates/PluginTemplate/pluginManifest.json diff --git a/devel-template/templates/PluginTemplate/pluginManifest.json b/devel-template/templates/PluginTemplate/pluginManifest.json new file mode 100644 index 00000000..c7aa0fe4 --- /dev/null +++ b/devel-template/templates/PluginTemplate/pluginManifest.json @@ -0,0 +1,19 @@ +{ + "$schema": "https://raw.githubusercontent.com/microsoft/microsoft-performance-toolkit-sdk/main/src/PluginsSystem/Tools/Microsoft.Performance.Toolkit.Plugins.Cli/Manifest/PluginManifestSchema.json", + "identity": { + "id": "__PLUGIN_ID__", + "version": "__PLUGIN_VERSION__" + }, + "displayName": "__PLUGIN_DISPLAY_NAME__", + "description": "__PLUGIN_DESCRIPTION__", + "owners": [ + { + "name": "__PLUGIN_OWNER_NAME__", + "address": "__PLUGIN_OWNER_ADDRESS__", + "emailAddresses": [ "__PLUGIN_OWNER_EMAIL__A__", "__PLUGIN_OWNER_EMAIL__B__" ], + "phoneNumbers": [ "__PLUGIN_OWNER_PHONE__A__", "__PLUGIN_OWNER_PHONE__B__" ] + } + ], + "projectUrl": "__PLUGIN_PROJECT_URL__", + "manifestVersion": 1.0 +} diff --git a/documentation/Plugins-System-Preview-Only/PluginTool-CLI.md b/documentation/Plugins-System-Preview-Only/PluginTool-CLI.md index 6e914561..5964654e 100644 --- a/documentation/Plugins-System-Preview-Only/PluginTool-CLI.md +++ b/documentation/Plugins-System-Preview-Only/PluginTool-CLI.md @@ -25,4 +25,38 @@ The `plugintool` cli can be used to generate plugin metadata files (`metadata.js - `-m|--manifest ` Specifies the path to the manifest file. If not provided, the tool will look for a `pluginManifest.json` file in the source directory. - `-w|--overwrite` - Specifies whether to overwrite existing metadata files if they exist. It's only valid if the `-o|--output` option is specified. \ No newline at end of file + Specifies whether to overwrite existing metadata files if they exist. It's only valid if the `-o|--output` option is specified. + +### Manifest File +The manifest file is an editable JSON file that allows users to specify the metadata for a plugin. To generate metadata files or package a plugin, a manifest file must be provided. The tool will look for a `pluginManifest.json` file in the source directory by default. The manifest file can also be specified using the `-m|--manifest` option. A template of the manifest file can be found [here](https://raw.githubusercontent.com/microsoft/microsoft-performance-toolkit-sdk/main/devel-template/templates/PluginTemplate/pluginManifest.json). + +The manifest file contains the following properties: + +#### `identity` + +`id` - An identifier for the plugin. Plugin IDs must be unique within the context of a plugin feed. It's recommended to use dots to create a namespace-like structure that reflects the organization and projects the plugin belongs to. e.g. `MyCompany.MyPlugin` or `MyCompany.MyProject.MyPlugin`. + +`version` - The version of the plugin in the format of sematic versioning. e.g. `1.0.0` or `1.0.0-beta.1`. + +#### `displayName` +A human-readable name for the plugin. e.g. `My Plugin`. + +#### `description` +A human-readable description for the plugin. e.g. `This is a plugin for my application.` + +#### `owners` +A list of owners for the plugin. + +`name` - The name of the owner. e.g. `John Doe`. + +`address` - The address of the owner. e.g. `My Company, 123 Main St, New York, NY 10001`. + +`emailAddresses` - A list of email addresses for the owner. e.g. `[ "myemail1@hotmail.com", "myemail2@outlook.com" ]` + +`phoneNumbers` - A list of phone numbers for the owner. e.g. `[ "123-456-7890", "098-765-4321" ]` + +#### `projectUrl` +The URL of the project repository. e.g. `https://github.com/mycompany/myplugin`. + +#### `manifestVersion` +The version of the manifest file defined in the schema this manifest file conforms to. e.g. `1.0`. diff --git a/src/PluginsSystem/Tools/Microsoft.Performance.Toolkit.Plugins.Cli/Manifest/PluginManifestSchema.json b/src/PluginsSystem/Tools/Microsoft.Performance.Toolkit.Plugins.Cli/Manifest/PluginManifestSchema.json index f5c312e6..b1b148a4 100644 --- a/src/PluginsSystem/Tools/Microsoft.Performance.Toolkit.Plugins.Cli/Manifest/PluginManifestSchema.json +++ b/src/PluginsSystem/Tools/Microsoft.Performance.Toolkit.Plugins.Cli/Manifest/PluginManifestSchema.json @@ -1,6 +1,6 @@ { "$schema": "http://json-schema.org/draft-07/schema", - "title": "JSON Schema a plugin manifest", + "title": "Plugin Manifest Schema", "type": "object", "required": [ "identity", @@ -24,8 +24,8 @@ }, "version": { "type": "string", - "pattern": "^(\\d+\\.\\d+\\.\\d+)(?:-([0-9A-Za-z-]+(?:\\.[0-9A-Za-z-]+)*))?(?:\\+([0-9A-Za-z-]+(?:\\.[0-9A-Za-z-]+)*))?$", - "description": "The version number of the plugin." + "pattern": "^(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)(?:-((?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\\.(?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\\+([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?$", + "description": "Semantic version of the plugin." } } }, From db5e04546d0c26788047fae87d2a750bff3c3220 Mon Sep 17 00:00:00 2001 From: Jayson Maxson Date: Tue, 26 Sep 2023 14:40:46 -0700 Subject: [PATCH 2/7] Prevent double loading and add logging fixes. --- .../Plugins/PluginsLoader.cs | 10 ++-- .../TableExtensionReferenceTests.cs | 2 + .../ReflectionProcessingSourceCatalog.cs | 16 +++++- .../CompositeDataCookerReference.cs | 11 ++-- .../DataCookers/SourceDataCookerReference.cs | 8 +-- .../DataExtensions/DataExtensionFactory.cs | 57 +++++++++++++++++-- .../DataExtensions/DataExtensionReflector.cs | 23 +++++--- .../DataExtensions/IDataExtensionFactory.cs | 41 ++++++++++++- .../Tables/TableExtensionReference.cs | 8 ++- .../SourceDataCookerReferenceFactory.cs | 33 ++++++++++- .../PluginSet.cs | 2 +- 11 files changed, 179 insertions(+), 32 deletions(-) diff --git a/src/Microsoft.Performance.SDK.Runtime.NetCoreApp/Plugins/PluginsLoader.cs b/src/Microsoft.Performance.SDK.Runtime.NetCoreApp/Plugins/PluginsLoader.cs index 315472d8..58bcb639 100644 --- a/src/Microsoft.Performance.SDK.Runtime.NetCoreApp/Plugins/PluginsLoader.cs +++ b/src/Microsoft.Performance.SDK.Runtime.NetCoreApp/Plugins/PluginsLoader.cs @@ -109,7 +109,7 @@ public PluginsLoader( // The constructor for this object hooks up the repository to the extension provider // (the ExtensionDiscovery in this case). We do not need to hold onto this object // explicitly, only call its constructor. - new DataExtensionReflector(this.extensionDiscovery, extensionRepository); + new DataExtensionReflector(this.extensionDiscovery, extensionRepository, logger); this.isDisposed = false; this.logger = logger; @@ -254,7 +254,7 @@ public bool TryLoadPlugins(IEnumerable directories, out IDictionary(); if (!directories.Any()) @@ -280,21 +280,21 @@ public bool TryLoadPlugins(IEnumerable directories, out IDictionary /// Tries to create an instance of based on the /// . @@ -71,7 +72,7 @@ internal static bool TryCreateReference( { return TryCreateReference( candidateType, - Runtime.Logger.Create(), + Microsoft.Performance.SDK.Runtime.Logger.Null, out reference); } @@ -105,8 +106,8 @@ internal static bool TryCreateReference( ILogger logger, out ICompositeDataCookerReference reference) { - Guard.NotNull(candidateType, nameof(candidateType)); - Guard.NotNull(logger, nameof(logger)); + Debug.Assert(candidateType != null, $"{nameof(candidateType)} cannot be null."); + Debug.Assert(logger != null, $"{nameof(logger)} cannot be null."); reference = null; diff --git a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataCookers/SourceDataCookerReference.cs b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataCookers/SourceDataCookerReference.cs index 382cc5d4..6b52a4dc 100644 --- a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataCookers/SourceDataCookerReference.cs +++ b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataCookers/SourceDataCookerReference.cs @@ -10,7 +10,6 @@ using Microsoft.Performance.SDK.Extensibility.DataCooking; using Microsoft.Performance.SDK.Extensibility.DataCooking.SourceDataCooking; using Microsoft.Performance.SDK.Processing; -using Microsoft.Performance.SDK.Runtime.Extensibility.DataExtensions.DataProcessors; using Microsoft.Performance.SDK.Runtime.Extensibility.DataExtensions.Dependency; namespace Microsoft.Performance.SDK.Runtime.Extensibility.DataExtensions.DataCookers @@ -141,7 +140,7 @@ internal static bool TryCreateReference( { return TryCreateReference( candidateType, - Runtime.Logger.Create(), + Runtime.Logger.Null, out reference); } @@ -176,7 +175,8 @@ internal static bool TryCreateReference( ILogger logger, out ISourceDataCookerReference reference) { - Guard.NotNull(candidateType, nameof(candidateType)); + Debug.Assert(candidateType != null, $"{nameof(candidateType)} cannot be null."); + Debug.Assert(logger != null, $"{nameof(logger)} cannot be null."); reference = null; @@ -202,7 +202,7 @@ internal static bool TryCreateReference( // There must be an empty, public constructor if (!candidateType.TryGetEmptyPublicConstructor(out var constructor)) { - Console.Error.WriteLine( + logger.Error( $"Warning: type {candidateType} seems to be a data cooker, but is missing an empty public " + "constructor."); return false; diff --git a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionFactory.cs b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionFactory.cs index 4580d9a9..4ae0de30 100644 --- a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionFactory.cs +++ b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionFactory.cs @@ -7,7 +7,6 @@ using Microsoft.Performance.SDK.Extensibility.SourceParsing; using Microsoft.Performance.SDK.Processing; using Microsoft.Performance.SDK.Runtime.Extensibility.DataExtensions.DataCookers; -using Microsoft.Performance.SDK.Runtime.Extensibility.DataExtensions.DataProcessors; using Microsoft.Performance.SDK.Runtime.Extensibility.DataExtensions.Repository; using Microsoft.Performance.SDK.Runtime.Extensibility.DataExtensions.Tables; @@ -19,13 +18,13 @@ namespace Microsoft.Performance.SDK.Runtime.Extensibility.DataExtensions public class DataExtensionFactory : IDataExtensionFactory { - private static readonly DataExtensionRepository SingletonRepository + private static readonly DataExtensionRepository SingletonRepository = new DataExtensionRepository(); /// /// Gets a singleton instance of this class. /// - public IDataExtensionRepositoryBuilder SingletonDataExtensionRepository + public IDataExtensionRepositoryBuilder SingletonDataExtensionRepository => DataExtensionFactory.SingletonRepository; /// @@ -43,10 +42,34 @@ public IDataExtensionRepositoryBuilder SingletonDataExtensionRepository public bool TryCreateSourceDataCookerReference( Type candidateType, out ISourceDataCookerReference reference) + { + return TryCreateSourceDataCookerReference(candidateType, Logger.Null, out reference); + } + + /// + /// Generate a source data cooker reference from a given type. + /// + /// + /// Data extension type. + /// + /// + /// Data extension reference. + /// + /// + /// Logs messages during reference creation. + /// + /// + /// true if succeeded, false otherwise. + /// + public bool TryCreateSourceDataCookerReference( + Type candidateType, + ILogger logger, + out ISourceDataCookerReference reference) { Guard.NotNull(candidateType, nameof(candidateType)); + Guard.NotNull(logger, nameof(logger)); - return SourceDataCookerReference.TryCreateReference(candidateType, out reference); + return SourceDataCookerReference.TryCreateReference(candidateType, logger, out reference); } /// @@ -108,10 +131,34 @@ public bool TryCreateCompositeDataCookerReference( public bool TryCreateTableReference( Type candidateType, out ITableExtensionReference reference) + { + return TryCreateTableReference(candidateType, Logger.Null, out reference); + } + + /// + /// Generate a table reference from a given type. + /// + /// + /// Data extension type. + /// + /// + /// Logs messages during reference creation. + /// + /// + /// Data extension reference. + /// + /// + /// true if succeeded, false otherwise. + /// + public bool TryCreateTableReference( + Type candidateType, + ILogger logger, + out ITableExtensionReference reference) { Guard.NotNull(candidateType, nameof(candidateType)); + Guard.NotNull(logger, nameof(logger)); - return TableExtensionReference.TryCreateReference(candidateType, out reference); + return TableExtensionReference.TryCreateReference(candidateType, logger, out reference); } /// diff --git a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionReflector.cs b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionReflector.cs index a6a2f660..da28bc7e 100644 --- a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionReflector.cs +++ b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionReflector.cs @@ -1,13 +1,13 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System; +using System.Diagnostics; +using Microsoft.Performance.SDK.Processing; using Microsoft.Performance.SDK.Runtime.Discovery; using Microsoft.Performance.SDK.Runtime.Extensibility.DataExtensions.DataCookers; -using Microsoft.Performance.SDK.Runtime.Extensibility.DataExtensions.DataProcessors; using Microsoft.Performance.SDK.Runtime.Extensibility.DataExtensions.Repository; using Microsoft.Performance.SDK.Runtime.Extensibility.DataExtensions.Tables; -using System; -using System.Diagnostics; namespace Microsoft.Performance.SDK.Runtime.Extensibility.DataExtensions { @@ -18,6 +18,7 @@ public class DataExtensionReflector : IExtensionTypeObserver { private readonly IDataExtensionRepositoryBuilder repoBuilder; + private readonly ILogger logger; /// /// Initializes a new instance of the DataExtensionReflector class. @@ -28,11 +29,19 @@ public class DataExtensionReflector /// /// Provides an output to store discovered extensions. /// + /// + /// Logs messages during reference creation. + /// public DataExtensionReflector( IExtensionTypeProvider extensionProvider, - IDataExtensionRepositoryBuilder repoBuilder) + IDataExtensionRepositoryBuilder repoBuilder, + ILogger logger) { + Guard.NotNull(logger, nameof(logger)); + this.repoBuilder = repoBuilder; + this.logger = logger; + extensionProvider.RegisterTypeConsumer(this); } @@ -40,21 +49,21 @@ public DataExtensionReflector( public void ProcessType(Type type, string sourceName) { // Find source data cookers - if (SourceDataCookerReference.TryCreateReference(type, out var sourceDataCookerReference)) + if (SourceDataCookerReference.TryCreateReference(type, this.logger, out var sourceDataCookerReference)) { Debug.Assert(sourceDataCookerReference != null); this.repoBuilder.AddSourceDataCookerReference(sourceDataCookerReference); } // Find composite data cookers - if (CompositeDataCookerReference.TryCreateReference(type, out var compositeDataCookerReference)) + if (CompositeDataCookerReference.TryCreateReference(type, this.logger, out var compositeDataCookerReference)) { Debug.Assert(compositeDataCookerReference != null); this.repoBuilder.AddCompositeDataCookerReference(compositeDataCookerReference); } // Find tables - if (TableExtensionReference.TryCreateReference(type, out var dataExtensionReference)) + if (TableExtensionReference.TryCreateReference(type, this.logger, out var dataExtensionReference)) { Debug.Assert(dataExtensionReference != null); this.repoBuilder.AddTableExtensionReference(dataExtensionReference); diff --git a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/IDataExtensionFactory.cs b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/IDataExtensionFactory.cs index 2ce67faf..adcd8c32 100644 --- a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/IDataExtensionFactory.cs +++ b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/IDataExtensionFactory.cs @@ -7,7 +7,6 @@ using Microsoft.Performance.SDK.Extensibility.SourceParsing; using Microsoft.Performance.SDK.Processing; using Microsoft.Performance.SDK.Runtime.Extensibility.DataExtensions.DataCookers; -using Microsoft.Performance.SDK.Runtime.Extensibility.DataExtensions.DataProcessors; using Microsoft.Performance.SDK.Runtime.Extensibility.DataExtensions.Repository; using Microsoft.Performance.SDK.Runtime.Extensibility.DataExtensions.Tables; @@ -39,6 +38,26 @@ bool TryCreateSourceDataCookerReference( Type candidateType, out ISourceDataCookerReference reference); + /// + /// Generate a source data cooker reference from a given type. + /// + /// + /// data extension type. + /// + /// + /// Logs messages during reference creation. + /// + /// + /// data extension reference. + /// + /// + /// true if succeeded. + /// + bool TryCreateSourceDataCookerReference( + Type candidateType, + ILogger logger, + out ISourceDataCookerReference reference); + /// /// Generate a composite data cooker reference from a given type. /// @@ -88,6 +107,26 @@ bool TryCreateTableReference( Type candidateType, out ITableExtensionReference reference); + /// + /// Generate a table reference from a given type. + /// + /// + /// data extension type. + /// + /// + /// data extension reference. + /// + /// + /// Logs messages during reference creation. + /// + /// + /// true if succeeded. + /// + bool TryCreateTableReference( + Type candidateType, + ILogger logger, + out ITableExtensionReference reference); + /// /// Creates a data extension repository. /// diff --git a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/Tables/TableExtensionReference.cs b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/Tables/TableExtensionReference.cs index 07370c36..c44bb94c 100644 --- a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/Tables/TableExtensionReference.cs +++ b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/Tables/TableExtensionReference.cs @@ -134,6 +134,9 @@ public Func IsDataAvailableFunc /// /// Candidate for the /// + /// + /// Logs messages during reference creation. + /// /// /// Out /// @@ -146,15 +149,18 @@ public Func IsDataAvailableFunc /// internal static bool TryCreateReference( Type candidateType, + ILogger logger, out ITableExtensionReference reference) { - Guard.NotNull(candidateType, nameof(candidateType)); + Debug.Assert(candidateType != null, $"{nameof(candidateType)} cannot be null."); + Debug.Assert(logger != null, $"{nameof(logger)} cannot be null."); reference = null; if (TableDescriptorFactory.TryCreate( candidateType, tableConfigSerializer, + logger, out var tableDescriptor, out var tableBuildAction, out var tableIsDataAvailableFunc)) diff --git a/src/Microsoft.Performance.SDK.Runtime/SourceDataCookerReferenceFactory.cs b/src/Microsoft.Performance.SDK.Runtime/SourceDataCookerReferenceFactory.cs index bc64708f..6466fc0b 100644 --- a/src/Microsoft.Performance.SDK.Runtime/SourceDataCookerReferenceFactory.cs +++ b/src/Microsoft.Performance.SDK.Runtime/SourceDataCookerReferenceFactory.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System; +using Microsoft.Performance.SDK.Processing; using Microsoft.Performance.SDK.Runtime.Extensibility.DataExtensions.DataCookers; namespace Microsoft.Performance.SDK.Runtime @@ -31,7 +32,37 @@ public static bool TryCreateReference( Type candidateType, out ISourceDataCookerReference reference) { - return SourceDataCookerReference.TryCreateReference(candidateType, out reference); + return TryCreateReference(candidateType, Logger.Null, out reference); + } + + /// + /// Creates an for a given type. + /// + /// + /// This will not add the generated reference to a cooker repository, and the cooker's data will not be + /// available through the standard cooker framework. + /// + /// + /// Type that represents a source data cooker. + /// + /// + /// Logs messages during reference creation. + /// + /// + /// A reference to a source data cooker for the given type. + /// + /// + /// true if a reference was generated; otherwise false + /// + public static bool TryCreateReference( + Type candidateType, + ILogger logger, + out ISourceDataCookerReference reference) + { + Guard.NotNull(candidateType, nameof(candidateType)); + Guard.NotNull(logger, nameof(logger)); + + return SourceDataCookerReference.TryCreateReference(candidateType, logger, out reference); } } } \ No newline at end of file diff --git a/src/Microsoft.Performance.Toolkit.Engine/PluginSet.cs b/src/Microsoft.Performance.Toolkit.Engine/PluginSet.cs index 51348353..97805198 100644 --- a/src/Microsoft.Performance.Toolkit.Engine/PluginSet.cs +++ b/src/Microsoft.Performance.Toolkit.Engine/PluginSet.cs @@ -355,7 +355,7 @@ public static PluginSet Load( var factory = new DataExtensionFactory(); repo = factory.CreateDataExtensionRepository(); - var reflector = new DataExtensionReflector(assemblyDiscovery, repo); + var reflector = new DataExtensionReflector(assemblyDiscovery, repo, logger ?? Logger.Null); assemblyDiscovery.ProcessAssemblies(extensionDirectoriesFullPaths, out var discoveryError); From 0ed830980ee0261c40e2e25f48934c3645da082b Mon Sep 17 00:00:00 2001 From: Jayson Maxson Date: Tue, 26 Sep 2023 15:00:42 -0700 Subject: [PATCH 3/7] Add IDataExtensionFactory.TryCreateCompositeDataCookerReference method override. --- .../DataExtensions/DataExtensionFactory.cs | 26 ++++++++++++++++++- .../DataExtensions/IDataExtensionFactory.cs | 19 ++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionFactory.cs b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionFactory.cs index 4ae0de30..e201477e 100644 --- a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionFactory.cs +++ b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionFactory.cs @@ -87,10 +87,34 @@ public bool TryCreateSourceDataCookerReference( public bool TryCreateCompositeDataCookerReference( Type candidateType, out ICompositeDataCookerReference reference) + { + return TryCreateCompositeDataCookerReference(candidateType, Logger.Null, out reference); + } + + /// + /// Generate a composite data cooker reference from a given type. + /// + /// + /// Data extension type. + /// + /// + /// Logs messages during reference creation. + /// + /// + /// Data extension reference. + /// + /// + /// true if succeeded, false otherwise. + /// + public bool TryCreateCompositeDataCookerReference( + Type candidateType, + ILogger logger, + out ICompositeDataCookerReference reference) { Guard.NotNull(candidateType, nameof(candidateType)); + Guard.NotNull(logger, nameof(logger)); - return CompositeDataCookerReference.TryCreateReference(candidateType, out reference); + return CompositeDataCookerReference.TryCreateReference(candidateType, logger, out reference); } // TODO: __SDK_DP__ diff --git a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/IDataExtensionFactory.cs b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/IDataExtensionFactory.cs index adcd8c32..3ebf574d 100644 --- a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/IDataExtensionFactory.cs +++ b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/IDataExtensionFactory.cs @@ -58,6 +58,25 @@ bool TryCreateSourceDataCookerReference( ILogger logger, out ISourceDataCookerReference reference); + /// + /// Generate a composite data cooker reference from a given type. + /// + /// + /// data extension type + /// + /// Logs messages during reference creation. + /// + /// + /// Data extension reference. + /// + /// + /// true if succeeded. + /// + bool TryCreateCompositeDataCookerReference( + Type candidateType, + ILogger logger, + out ICompositeDataCookerReference reference); + /// /// Generate a composite data cooker reference from a given type. /// From e67705babaa1bd601749adc0f1c6000c0fe7bf34 Mon Sep 17 00:00:00 2001 From: Jayson Maxson Date: Tue, 26 Sep 2023 15:11:52 -0700 Subject: [PATCH 4/7] Make PluginSet.Load logging more consistent --- src/Microsoft.Performance.Toolkit.Engine/PluginSet.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Microsoft.Performance.Toolkit.Engine/PluginSet.cs b/src/Microsoft.Performance.Toolkit.Engine/PluginSet.cs index 97805198..9a629fd3 100644 --- a/src/Microsoft.Performance.Toolkit.Engine/PluginSet.cs +++ b/src/Microsoft.Performance.Toolkit.Engine/PluginSet.cs @@ -340,9 +340,7 @@ public static PluginSet Load( try { - assemblyLoader ??= logger is null - ? new AssemblyLoader() - : new AssemblyLoader(logger); + assemblyLoader ??= new AssemblyLoader(logger ?? Logger.Null); var validatorFactory = new Func, IPreloadValidator>(_ => new NullValidator()); From 5d8ff9c4bace0f8afb084afc79754f3df89d22d4 Mon Sep 17 00:00:00 2001 From: Jayson Maxson Date: Tue, 26 Sep 2023 15:14:04 -0700 Subject: [PATCH 5/7] PR feedback --- .../SourceDataCookerReferenceFactory.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Performance.SDK.Runtime/SourceDataCookerReferenceFactory.cs b/src/Microsoft.Performance.SDK.Runtime/SourceDataCookerReferenceFactory.cs index 6466fc0b..4211e6ad 100644 --- a/src/Microsoft.Performance.SDK.Runtime/SourceDataCookerReferenceFactory.cs +++ b/src/Microsoft.Performance.SDK.Runtime/SourceDataCookerReferenceFactory.cs @@ -65,4 +65,4 @@ public static bool TryCreateReference( return SourceDataCookerReference.TryCreateReference(candidateType, logger, out reference); } } -} \ No newline at end of file +} From c1fcad2d8913587fe7202b436d79fa6b395eb845 Mon Sep 17 00:00:00 2001 From: Jayson Maxson Date: Tue, 26 Sep 2023 18:27:53 -0700 Subject: [PATCH 6/7] Hybrid solution update. --- .../CompositeDataCookerReference.cs | 2 +- .../DataCookers/SourceDataCookerReference.cs | 2 +- .../DataExtensions/DataExtensionFactory.cs | 12 ++++-- .../DataExtensions/DataExtensionReflector.cs | 5 ++- .../Tables/TableExtensionReference.cs | 37 +++++++++++++++++++ .../SourceDataCookerReferenceFactory.cs | 4 +- .../Engine.cs | 7 +++- 7 files changed, 60 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataCookers/CompositeDataCookerReference.cs b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataCookers/CompositeDataCookerReference.cs index e06f7ffc..52bd68af 100644 --- a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataCookers/CompositeDataCookerReference.cs +++ b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataCookers/CompositeDataCookerReference.cs @@ -72,7 +72,7 @@ internal static bool TryCreateReference( { return TryCreateReference( candidateType, - Microsoft.Performance.SDK.Runtime.Logger.Null, + Runtime.Logger.Create(), out reference); } diff --git a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataCookers/SourceDataCookerReference.cs b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataCookers/SourceDataCookerReference.cs index 6b52a4dc..1038b7eb 100644 --- a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataCookers/SourceDataCookerReference.cs +++ b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataCookers/SourceDataCookerReference.cs @@ -140,7 +140,7 @@ internal static bool TryCreateReference( { return TryCreateReference( candidateType, - Runtime.Logger.Null, + Runtime.Logger.Create(), out reference); } diff --git a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionFactory.cs b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionFactory.cs index e201477e..382f1f2c 100644 --- a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionFactory.cs +++ b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionFactory.cs @@ -43,7 +43,9 @@ public bool TryCreateSourceDataCookerReference( Type candidateType, out ISourceDataCookerReference reference) { - return TryCreateSourceDataCookerReference(candidateType, Logger.Null, out reference); + Guard.NotNull(candidateType, nameof(candidateType)); + + return SourceDataCookerReference.TryCreateReference(candidateType, out reference); } /// @@ -88,7 +90,9 @@ public bool TryCreateCompositeDataCookerReference( Type candidateType, out ICompositeDataCookerReference reference) { - return TryCreateCompositeDataCookerReference(candidateType, Logger.Null, out reference); + Guard.NotNull(candidateType, nameof(candidateType)); + + return CompositeDataCookerReference.TryCreateReference(candidateType, out reference); } /// @@ -156,7 +160,9 @@ public bool TryCreateTableReference( Type candidateType, out ITableExtensionReference reference) { - return TryCreateTableReference(candidateType, Logger.Null, out reference); + Guard.NotNull(candidateType, nameof(candidateType)); + + return TableExtensionReference.TryCreateReference(candidateType, out reference); } /// diff --git a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionReflector.cs b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionReflector.cs index da28bc7e..1d0c3ff7 100644 --- a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionReflector.cs +++ b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionReflector.cs @@ -37,7 +37,10 @@ public DataExtensionReflector( IDataExtensionRepositoryBuilder repoBuilder, ILogger logger) { - Guard.NotNull(logger, nameof(logger)); + if (logger == null) + { + logger = Logger.Create(); + } this.repoBuilder = repoBuilder; this.logger = logger; diff --git a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/Tables/TableExtensionReference.cs b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/Tables/TableExtensionReference.cs index c44bb94c..2a0ca69c 100644 --- a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/Tables/TableExtensionReference.cs +++ b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/Tables/TableExtensionReference.cs @@ -117,6 +117,43 @@ public Func IsDataAvailableFunc } } + /// + /// Tries to create an instance of based on the + /// . + /// + /// A must satisfy the following criteria in order to + /// be eligible as a reference: + /// + /// + /// must expose a valid . + /// See + /// for details on how a table is to expose a descriptor. + /// + /// + /// + /// + /// Candidate for the + /// + /// + /// Out + /// + /// + /// true if the is valid and can create a instance of ; + /// false otherwise. + /// + /// + /// is null. + /// + internal static bool TryCreateReference( + Type candidateType, + out ITableExtensionReference reference) + { + return TryCreateReference( + candidateType, + Logger.Create(), + out reference); + } + /// /// Tries to create an instance of based on the /// . diff --git a/src/Microsoft.Performance.SDK.Runtime/SourceDataCookerReferenceFactory.cs b/src/Microsoft.Performance.SDK.Runtime/SourceDataCookerReferenceFactory.cs index 4211e6ad..e31b8472 100644 --- a/src/Microsoft.Performance.SDK.Runtime/SourceDataCookerReferenceFactory.cs +++ b/src/Microsoft.Performance.SDK.Runtime/SourceDataCookerReferenceFactory.cs @@ -32,7 +32,9 @@ public static bool TryCreateReference( Type candidateType, out ISourceDataCookerReference reference) { - return TryCreateReference(candidateType, Logger.Null, out reference); + Guard.NotNull(candidateType, nameof(candidateType)); + + return SourceDataCookerReference.TryCreateReference(candidateType, out reference); } /// diff --git a/src/Microsoft.Performance.Toolkit.Engine/Engine.cs b/src/Microsoft.Performance.Toolkit.Engine/Engine.cs index 89dfe78c..845de335 100644 --- a/src/Microsoft.Performance.Toolkit.Engine/Engine.cs +++ b/src/Microsoft.Performance.Toolkit.Engine/Engine.cs @@ -750,9 +750,12 @@ private static Engine CreateCore(EngineCreateInfo createInfo, DataSourceSet inte { Debug.Assert(createInfo != null); - var loggerFactory = createInfo.LoggerFactory ?? Logger.Create; + if (createInfo.LoggerFactory != null) + { + Logger.Factory = createInfo.LoggerFactory; + } - var logger = loggerFactory(typeof(Engine)); + var logger = Logger.Create(typeof(Engine)); Engine instance = null; try From 818933dc90db17ed25da58586b43b4c99f5c2e3e Mon Sep 17 00:00:00 2001 From: Jayson Maxson Date: Tue, 26 Sep 2023 18:52:29 -0700 Subject: [PATCH 7/7] Pushes logger creation down when not specified, so that the logger type is more often applicable. --- .../DataCookers/CompositeDataCookerReference.cs | 11 ++++++----- .../DataCookers/SourceDataCookerReference.cs | 11 ++++++----- .../DataExtensions/DataExtensionFactory.cs | 15 +++------------ .../DataExtensions/DataExtensionReflector.cs | 5 ----- .../Tables/TableExtensionReference.cs | 11 ++++++----- .../SourceDataCookerReferenceFactory.cs | 5 +---- .../PluginSet.cs | 6 ++++-- 7 files changed, 26 insertions(+), 38 deletions(-) diff --git a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataCookers/CompositeDataCookerReference.cs b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataCookers/CompositeDataCookerReference.cs index 52bd68af..88dcc0d0 100644 --- a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataCookers/CompositeDataCookerReference.cs +++ b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataCookers/CompositeDataCookerReference.cs @@ -70,10 +70,7 @@ internal static bool TryCreateReference( Type candidateType, out ICompositeDataCookerReference reference) { - return TryCreateReference( - candidateType, - Runtime.Logger.Create(), - out reference); + return TryCreateReference(candidateType, null, out reference); } /// @@ -107,10 +104,14 @@ internal static bool TryCreateReference( out ICompositeDataCookerReference reference) { Debug.Assert(candidateType != null, $"{nameof(candidateType)} cannot be null."); - Debug.Assert(logger != null, $"{nameof(logger)} cannot be null."); reference = null; + if (logger == null) + { + logger = Runtime.Logger.Create(); + } + if (candidateType.IsInstantiatableOfType(typeof(ICompositeDataCookerDescriptor))) { // There must be an empty, public constructor diff --git a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataCookers/SourceDataCookerReference.cs b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataCookers/SourceDataCookerReference.cs index 1038b7eb..2b7baf94 100644 --- a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataCookers/SourceDataCookerReference.cs +++ b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataCookers/SourceDataCookerReference.cs @@ -138,10 +138,7 @@ internal static bool TryCreateReference( Type candidateType, out ISourceDataCookerReference reference) { - return TryCreateReference( - candidateType, - Runtime.Logger.Create(), - out reference); + return TryCreateReference(candidateType, null, out reference); } /// @@ -176,10 +173,14 @@ internal static bool TryCreateReference( out ISourceDataCookerReference reference) { Debug.Assert(candidateType != null, $"{nameof(candidateType)} cannot be null."); - Debug.Assert(logger != null, $"{nameof(logger)} cannot be null."); reference = null; + if (logger == null) + { + logger = Runtime.Logger.Create(); + } + // perform this basic check first, as it's cheaper than a more specific test below if (!candidateType.Implements(typeof(IDataCooker))) { diff --git a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionFactory.cs b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionFactory.cs index 382f1f2c..3c79c222 100644 --- a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionFactory.cs +++ b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionFactory.cs @@ -43,9 +43,7 @@ public bool TryCreateSourceDataCookerReference( Type candidateType, out ISourceDataCookerReference reference) { - Guard.NotNull(candidateType, nameof(candidateType)); - - return SourceDataCookerReference.TryCreateReference(candidateType, out reference); + return TryCreateSourceDataCookerReference(candidateType, null, out reference); } /// @@ -69,7 +67,6 @@ public bool TryCreateSourceDataCookerReference( out ISourceDataCookerReference reference) { Guard.NotNull(candidateType, nameof(candidateType)); - Guard.NotNull(logger, nameof(logger)); return SourceDataCookerReference.TryCreateReference(candidateType, logger, out reference); } @@ -90,9 +87,7 @@ public bool TryCreateCompositeDataCookerReference( Type candidateType, out ICompositeDataCookerReference reference) { - Guard.NotNull(candidateType, nameof(candidateType)); - - return CompositeDataCookerReference.TryCreateReference(candidateType, out reference); + return TryCreateCompositeDataCookerReference(candidateType, null, out reference); } /// @@ -116,7 +111,6 @@ public bool TryCreateCompositeDataCookerReference( out ICompositeDataCookerReference reference) { Guard.NotNull(candidateType, nameof(candidateType)); - Guard.NotNull(logger, nameof(logger)); return CompositeDataCookerReference.TryCreateReference(candidateType, logger, out reference); } @@ -160,9 +154,7 @@ public bool TryCreateTableReference( Type candidateType, out ITableExtensionReference reference) { - Guard.NotNull(candidateType, nameof(candidateType)); - - return TableExtensionReference.TryCreateReference(candidateType, out reference); + return TryCreateTableReference(candidateType, null, out reference); } /// @@ -186,7 +178,6 @@ public bool TryCreateTableReference( out ITableExtensionReference reference) { Guard.NotNull(candidateType, nameof(candidateType)); - Guard.NotNull(logger, nameof(logger)); return TableExtensionReference.TryCreateReference(candidateType, logger, out reference); } diff --git a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionReflector.cs b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionReflector.cs index 1d0c3ff7..7e0d3154 100644 --- a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionReflector.cs +++ b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/DataExtensionReflector.cs @@ -37,11 +37,6 @@ public DataExtensionReflector( IDataExtensionRepositoryBuilder repoBuilder, ILogger logger) { - if (logger == null) - { - logger = Logger.Create(); - } - this.repoBuilder = repoBuilder; this.logger = logger; diff --git a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/Tables/TableExtensionReference.cs b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/Tables/TableExtensionReference.cs index 2a0ca69c..d9357ee2 100644 --- a/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/Tables/TableExtensionReference.cs +++ b/src/Microsoft.Performance.SDK.Runtime/Extensibility/DataExtensions/Tables/TableExtensionReference.cs @@ -148,10 +148,7 @@ internal static bool TryCreateReference( Type candidateType, out ITableExtensionReference reference) { - return TryCreateReference( - candidateType, - Logger.Create(), - out reference); + return TryCreateReference(candidateType, null, out reference); } /// @@ -190,10 +187,14 @@ internal static bool TryCreateReference( out ITableExtensionReference reference) { Debug.Assert(candidateType != null, $"{nameof(candidateType)} cannot be null."); - Debug.Assert(logger != null, $"{nameof(logger)} cannot be null."); reference = null; + if (logger == null) + { + logger = Logger.Create(); + } + if (TableDescriptorFactory.TryCreate( candidateType, tableConfigSerializer, diff --git a/src/Microsoft.Performance.SDK.Runtime/SourceDataCookerReferenceFactory.cs b/src/Microsoft.Performance.SDK.Runtime/SourceDataCookerReferenceFactory.cs index e31b8472..3543098a 100644 --- a/src/Microsoft.Performance.SDK.Runtime/SourceDataCookerReferenceFactory.cs +++ b/src/Microsoft.Performance.SDK.Runtime/SourceDataCookerReferenceFactory.cs @@ -32,9 +32,7 @@ public static bool TryCreateReference( Type candidateType, out ISourceDataCookerReference reference) { - Guard.NotNull(candidateType, nameof(candidateType)); - - return SourceDataCookerReference.TryCreateReference(candidateType, out reference); + return TryCreateReference(candidateType, null, out reference); } /// @@ -62,7 +60,6 @@ public static bool TryCreateReference( out ISourceDataCookerReference reference) { Guard.NotNull(candidateType, nameof(candidateType)); - Guard.NotNull(logger, nameof(logger)); return SourceDataCookerReference.TryCreateReference(candidateType, logger, out reference); } diff --git a/src/Microsoft.Performance.Toolkit.Engine/PluginSet.cs b/src/Microsoft.Performance.Toolkit.Engine/PluginSet.cs index 9a629fd3..01548dd3 100644 --- a/src/Microsoft.Performance.Toolkit.Engine/PluginSet.cs +++ b/src/Microsoft.Performance.Toolkit.Engine/PluginSet.cs @@ -340,7 +340,9 @@ public static PluginSet Load( try { - assemblyLoader ??= new AssemblyLoader(logger ?? Logger.Null); + assemblyLoader ??= logger is null + ? new AssemblyLoader() + : new AssemblyLoader(logger); var validatorFactory = new Func, IPreloadValidator>(_ => new NullValidator()); @@ -353,7 +355,7 @@ public static PluginSet Load( var factory = new DataExtensionFactory(); repo = factory.CreateDataExtensionRepository(); - var reflector = new DataExtensionReflector(assemblyDiscovery, repo, logger ?? Logger.Null); + var reflector = new DataExtensionReflector(assemblyDiscovery, repo, logger); assemblyDiscovery.ProcessAssemblies(extensionDirectoriesFullPaths, out var discoveryError);