Customizable analysis #164

bhufmann · 2024-10-01T14:08:13Z

Update (Oct 28, 0204):
All the PRs below are merged.

This PR combines different commits to allow for customization of analysis using a json definition. These customable analyses are exposed through data providers used in conjunction with the Trace Compass trace server.

Note that some of the commits are also available as separate pull requests.

List of commits/PRs:

Signed-off-by: Bernd Hufmann [email protected]

PatrickTasse · 2024-10-03T16:09:17Z

...tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/config/TmfJsonConfiguration.java

+           name = name == null ? defaultValues.getName() : name;
+           String description = config.getDescription();
+           description = description == null ? defaultValues.getDescription() : description;
+           return builder.setId(config.getId())


It seems a bit odd that we create a TmfConfiguration with id, name, description that can come from the json string and then the TmfConfiguration also contains the json string, so these values are duplicated inside the TmfConfiguration object.

I agree with this statement for name and description. The ID is generated to remove burden of end-user to create a unique ID. We use this class to serialize a TmfConfiguration that has generated ID. When deserializing the configuration, we read the ID back, no need for regeneration.

abhinava-ericsson · 2024-10-04T22:08:25Z

...compass.tmf.core/src/org/eclipse/tracecompass/tmf/core/dataprovider/DataProviderManager.java

+     * @return list of the available providers for this trace / experiment
+     * @since 9.5
+     */
+    public synchronized @NonNull List<IDataProviderDescriptor> getAvailableProviders(ITmfTrace trace, String configId, String... ids) {


For supporting per base DataProvider conifgurations We would need a getAvailableProviders(ITmfTrace trace, String... ids) to return all DataProviders which contain the (factoryId == baseDataProvider ID).
That, or DataProvider Descriptor object would need to be a tree of descriptor strings.

That could be useful. It would return all derived data providers for all configurations for the base data provider.

bhufmann · 2024-10-08T13:39:05Z

...se.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/config/ITmfConfiguration.java

+     *
+     * @since 9.5
+     */
+    default String getJsonParameters() {


This will not be needed because only the map will be supported:
See eclipse-tracecompass-incubator/org.eclipse.tracecompass.incubator#88 (comment)

@abhinava-ericsson @PatrickTasse

bhufmann · 2024-10-08T13:39:22Z

...cecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/config/ITmfConfigurationSource.java

+     *             If the creation of the configuration fails
+     * @since 9.5
+     */
+    default ITmfConfiguration create(String parameters) throws TmfConfigurationException {


This will not be needed because only the map will be supported:
See eclipse-tracecompass-incubator/org.eclipse.tracecompass.incubator#88 (comment)

@abhinava-ericsson @PatrickTasse

bhufmann · 2024-10-08T13:39:58Z

...cecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/config/ITmfConfigurationSource.java

+            throw new TmfConfigurationException("Can't convert json string to Map to update configuration", e); //$NON-NLS-1$
+        }
+    }
+


This will not be needed because only the map will be supported:
See eclipse-tracecompass-incubator/org.eclipse.tracecompass.incubator#88 (comment)

@abhinava-ericsson @PatrickTasse

bhufmann · 2024-10-08T13:40:58Z

...acecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/config/ITmfDataProviderSource.java

+     *             if an error occurs
+     */
+//    List<IDataProviderDescriptor> createDataProviderDescriptors(String typeId, ITmfTrace trace, Map<String, Object> parameters) throws TmfConfigurationException;
+    List<IDataProviderDescriptor> createDataProviderDescriptors(String typeId, ITmfTrace trace, String jsonParameters) throws TmfConfigurationException;


The API should use Map<String, Object> as discussed here: eclipse-tracecompass-incubator/org.eclipse.tracecompass.incubator#88 (comment)

@abhinava-ericsson @PatrickTasse

bhufmann · 2024-10-08T13:41:40Z

...acecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/config/ITmfDataProviderSource.java

+     *             if an error occurs
+     */
+    void removeDataProviderDescriptor(ITmfTrace trace, IDataProviderDescriptor descriptor) throws TmfConfigurationException;
+}


I think this interface needs also a method to remove the config which will then remove the data providers.

bhufmann · 2024-10-08T13:44:49Z

...tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/config/TmfJsonConfiguration.java

This class might not be needed and json serialization and deserialization to disk can be in the TmfConfiguration class after removing the jsonParameter string support, as discussed here:

eclipse-tracecompass-incubator/org.eclipse.tracecompass.incubator#88 (comment)

@abhinava-ericsson @PatrickTasse

bhufmann · 2024-10-08T13:46:12Z

...tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/config/TmfJsonConfiguration.java

+           name = name == null ? defaultValues.getName() : name;
+           String description = config.getDescription();
+           description = description == null ? defaultValues.getDescription() : description;
+           return builder.setId(config.getId())


I agree with this statement for name and description. The ID is generated to remove burden of end-user to create a unique ID. We use this class to serialize a TmfConfiguration that has generated ID. When deserializing the configuration, we read the ID back, no need for regeneration.

bhufmann · 2024-10-08T13:51:41Z

...ass.tmf.core/src/org/eclipse/tracecompass/tmf/core/dataprovider/IDataProviderDescriptor.java

+     * @since 9.5
+     */
+    default @Nullable String getParentId() {
+        return null;


I opted to only have the parentId in the descriptor to create an hierarchy and not having also a getChildren() method. This is easier to implement at the moment, but we can opt to implement it at a later time if we realize it's to iterating through the list of descriptors to find the parent - children hierarchy.

If we opt to have the getChildren() I suggest to implement later and not in the initial version of the fullstack support of data provider configurations.

FYI @PatrickTasse @abhinava-ericsson

That sounds right. Currently, DataProviderManager maintains a list of all Data Providers, and we don't have nesting of derived data providers. So, base data providers don't really need to be aware of their children.
As you said, after full-stack implementation, if we see any performance or simplicity benefit by adding getChildren(), we can implement it.

bhufmann · 2024-10-08T13:52:28Z

...ass.tmf.core/src/org/eclipse/tracecompass/tmf/core/dataprovider/IDataProviderDescriptor.java

+     * data provider, or null if not applicable
+     * @since 9.5
+     */
+    default @Nullable ITmfConfiguration getCreationConfiguration() {


I don't like the name of the method. Any suggestions for a better name? @PatrickTasse @abhinava-ericsson

Just getConfiguration() is fine till we don't allow modification of an existing configuration. Later, if we do allow that, we can differentiate between getConfiguration() and getInitialConfiguration().

getConfiguration() sounds like there can be only one. We might (I don't have use case yet) some other configuration, that does something else than creating a data provider. That's why I used creationConfiguration, but I don't like it. Maybe for now we call it getConfiguration() as you suggested and that's the configuration that was used for creation. If we in the future support another configuration we can find a name for the new type configuration.

bhufmann · 2024-10-08T14:11:41Z

...org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/trace/ITmfTrace.java

@@ -273,6 +273,37 @@ public interface ITmfTrace extends ITmfEventProvider {
     */
    @NonNull Iterable<@NonNull IAnalysisModule> getAnalysisModules();

+


@PatrickTasse @abhinava-ericsson

This API needs explanation. In Trace Compass analysis modules are managed by the TmfAnalysisFramework through IAnalysisModuleSource extension. The ITmfTrace has a method getAnalysisModules() and that collection has only analysis modules that are configured through the analysis framework when opening a trace TmfTraceOpenSignal() processed which triggers the method executeAnalysis(). This then call TmfAnalysisManager.getAnalysisModules() which will then instantiate the modules and store in the trace object

Many data provider factories often use TmfTraceUtils.getAnalysisModulesOfClass(trace, IFlameChartProvider.class); which call ITmfTrace.getAnalysisModules(). So, if an analysis module is not know by the analysis framework, then analysis module is not instantiated and not return by the trace.

These 2 methods will by-pass the analysis framework and custom analysis modules can be added and removed. This is needed for the custom analysis implementation.

Please note that the analysis framework, reads some global configuration to instantiate so-called IAnalysisModuleHelpers which globally available and assigned to applicable traces. These helpers are used to instantiate the actual analysis module and then configure it with parameters provided by the extension point or xml definition.

One thing to remember, if we would like to use the analysis framework for custom analysis, which is doable and I prototyped, there needs to global storage of configuration files (e.g. json), a manager class that can add and remove such configurations as well as assign it to a trace. However, the design of customizable analysis through data providers (factories) doesn't the provision for having such manager class and the required server endpoint.

bhufmann · 2024-10-08T14:24:45Z

...se.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/analysis/IAnalysisModule.java

+     * @return true if it was successful or false if invalid config (TODO should we through exception?)
+     * @since 9.5
+     */
+    default boolean setConfiguration(ITmfConfiguration configuration) {


This use needed to be able to add the configuration IDataProviderDescriptors for derived data providers. This is not used by the analysis framework, and hence doesn't have equivalent methods in IAnalysisModuleHelper.

@PatrickTasse @abhinava-ericsson

bhufmann · 2024-10-08T14:24:52Z

...se.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/analysis/IAnalysisModule.java

+     * @return true if it was successful or false if invalid config (TODO should we through exception?)
+     * @since 9.5
+     */
+    default @Nullable ITmfConfiguration getConfiguration() {


This use needed to be able to add the configuration IDataProviderDescriptors for derived data providers. This is not used by the analysis framework.

bhufmann · 2024-10-08T14:26:58Z

...se.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/analysis/IAnalysisModule.java

+     * @return the configuration root path
+     * @since 9.5
+     */
+    default @Nullable String getConfigurationRoot() {


This can be removed. It's an artifact from a protoype that uses the anlaysis framework.

bhufmann mentioned this pull request Oct 1, 2024

Custom analysis API and InAndOut Implementation eclipse-tracecompass-incubator/org.eclipse.tracecompass.incubator#88

Merged

bhufmann requested a review from PatrickTasse October 1, 2024 14:08

PatrickTasse reviewed Oct 3, 2024

View reviewed changes

abhinava-ericsson reviewed Oct 4, 2024

View reviewed changes

bhufmann commented Oct 8, 2024

View reviewed changes

bhufmann mentioned this pull request Oct 8, 2024

tmf: Add API to add and remove analysis modules to/from ITmfTrace #161

Merged

bhufmann commented Oct 8, 2024

View reviewed changes

bhufmann force-pushed the json-schema-json-if-factory-fixes branch from d013347 to 140ffae Compare October 28, 2024 12:37

bhufmann closed this Oct 28, 2024

bhufmann force-pushed the json-schema-json-if-factory-fixes branch from 140ffae to 751b02c Compare October 28, 2024 17:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Customizable analysis #164

Customizable analysis #164

bhufmann commented Oct 1, 2024 •

edited

Loading

PatrickTasse Oct 3, 2024

bhufmann Oct 8, 2024

abhinava-ericsson Oct 4, 2024

bhufmann Oct 7, 2024 •

edited

Loading

bhufmann Oct 8, 2024

bhufmann Oct 8, 2024

bhufmann Oct 8, 2024

bhufmann Oct 8, 2024

bhufmann Oct 8, 2024

bhufmann Oct 8, 2024

bhufmann Oct 8, 2024

bhufmann Oct 8, 2024

abhinava-ericsson Oct 10, 2024

bhufmann Oct 8, 2024

abhinava-ericsson Oct 10, 2024

bhufmann Oct 10, 2024

bhufmann Oct 8, 2024 •

edited

Loading

bhufmann Oct 8, 2024

bhufmann Oct 8, 2024

bhufmann Oct 8, 2024

		@@ -273,6 +273,37 @@ public interface ITmfTrace extends ITmfEventProvider {
		*/
		@NonNull Iterable<@NonNull IAnalysisModule> getAnalysisModules();

Customizable analysis #164

Customizable analysis #164

Conversation

bhufmann commented Oct 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bhufmann Oct 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bhufmann Oct 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bhufmann commented Oct 1, 2024 •

edited

Loading

bhufmann Oct 7, 2024 •

edited

Loading

bhufmann Oct 8, 2024 •

edited

Loading