Issue #170 Enhance ConnectorStore loading system #176

NassimBtk · 2024-05-17T08:04:12Z

Overview

Each connector object within the ConnectorStore defines a Map<String, EmbeddedFile> for storing associated external files (e.g., awk scripts).
The EmbeddedFile content is stored as a byte array, so that we can regenerate the original file if needed. When executing the awk compute, we convert the embedded file content (awk script) to text using UTF-8.
We keep the current behavior of the Connector Store reader, so that users retain full control over the community connectors in production.
Various enhancement and code factorization.

* Internalized external files. * Updated Windows and OsCommand extensions in order to correctly fetch embedded files.

* Updated http extension to manage header and body using embedded files.

* Formatted code. * Updated unit tests in the agent to use real extensions.

* Updated extension unit tests.

fearture/issue-170-enhance-metricshub-connectorstore-loading-system

* Fixed JUnit tests on oscommand extension

* Fixed formatting and removed useless imports

* Decreased coverage on metricshub-agent due to code removal

fearture/issue-170-enhance-metricshub-connectorstore-loading-system

* Added unit tests for EmbeddedFile methods. * Update README.md * Enhanced log message in EmbeddedFileHelper

* Managed connector store extension providers

MedMaalej · 2024-05-17T08:27:53Z

metricshub-oscommand-extension/pom.xml

@@ -150,7 +150,7 @@
 										<limit>
 											<counter>COMPLEXITY</counter>
 											<value>COVEREDRATIO</value>
-											<minimum>0.51</minimum>
+											<minimum>0.50</minimum>


Why we decreased the test coverage percentage here ?

Even with new tests the coverage decreased, I will look into it.

MedMaalej · 2024-05-17T08:29:06Z

...hub-engine/src/main/java/org/sentrysoftware/metricshub/engine/common/helpers/FileHelper.java

+	 */
+	public static String getExtension(String filename) {
+		// Find the last index of '.' in the filename
+		int lastIndex = filename.lastIndexOf('.');


could be final

MedMaalej · 2024-05-17T08:29:40Z

...hub-engine/src/main/java/org/sentrysoftware/metricshub/engine/common/helpers/FileHelper.java

+	 */
+	public static String getBaseName(String filename) {
+		// Find the last index of '.' in the filename
+		int lastIndex = filename.lastIndexOf('.');


could be final

bertysentry

This looks beautiful! Well done! I only have a question about the id of embedded files, and whether we can ditch it. Thank you! 😊

bertysentry · 2024-05-17T10:52:56Z

...ngine/src/main/java/org/sentrysoftware/metricshub/engine/strategy/utils/OsCommandHelper.java

 		try {
-			return File.createTempFile("SEN_Embedded_", extension);
+			return File.createTempFile("SEN_Embedded_" + baseName, extension);


we don't need SEN_Embedded_

Yes, maybe we need metricshub_embedded_ as prefix.
The reason we have this prefix is to easily identify the files in case of an issue. The user or support can then efficiently clean everything in the user's directory, and possibly even on a production machine where files could fill up the filesystem, which could become critical.

bertysentry · 2024-05-17T10:55:11Z

.../test/java/org/sentrysoftware/metricshub/engine/connector/model/common/EmbeddedFileTest.java

+	void testDescription() {
+		assertEquals(
+			"EmbeddedFile 1: script.bat",
+			EmbeddedFile.builder().content("value".getBytes()).filename("script.bat").id(1).build().description()


Is the id really required? Wouldn't the name be sufficient? (I'm asking because I'm may be misunderstanding something)

The name alone is insufficient since different files with the same name can exist in various subdirectories and be referenced in the connector. Therefore, a unique ID is necessary. Additionally, the global index of the embedded files should be represented using integer values instead of strings.

bertysentry · 2024-05-17T10:57:50Z

.../test/java/org/sentrysoftware/metricshub/engine/connector/model/common/EmbeddedFileTest.java

+	void testGetContentAsString() {
+		final EmbeddedFile embeddedFile = EmbeddedFile
+			.builder()
+			.content("value".getBytes())


You will need to specify the Charset in getBytes() to make sure the tests run properly without relying with the JRE and system configuration.

bertysentry · 2024-05-17T11:02:39Z

...st/java/org/sentrysoftware/metricshub/engine/connector/parser/EmbeddedFilesResolverTest.java

-		assertEquals(uriStrExpected, result.toString());
+		final String actualAwkScriptNormalized = result.getContentAsString().replaceAll("\r\n", "").replaceAll("\n", "");
+		final String expected =
+			"""


bertysentry · 2024-05-17T11:07:53Z

.../src/main/java/org/sentrysoftware/metricshub/engine/connector/model/common/EmbeddedFile.java

 	 */
-	private String reference;
+	private Integer id;


What's the requirement for this id? Can we avoid it?

This is the unique id of the embedded file, this will handle the case where different embedded files have the same name, and enhance slightly the memory of the global embedded file Map<Integer, EmbeddedFile>.

This is generated by the connector parser 😉.

* Increased coverage on OsCommand and Http extension * Fixed Timeout deserialization on the configuration objects.

* Cleaned-up Constants.java in the engine. * Replaced temp file prefix `SEN_Embedded_` with `metricshub_embedded_`.

* Added note to `README.md`.

NassimBtk added 11 commits May 13, 2024 19:55

Issue #170: Enhance MetricsHub ConnectorStore Loading System

72fdb9b

* Internalized external files. * Updated Windows and OsCommand extensions in order to correctly fetch embedded files.

Issue #170: Enhance MetricsHub ConnectorStore Loading System

00c46dd

* Updated http extension to manage header and body using embedded files.

Issue #170: Enhance MetricsHub ConnectorStore Loading System

158ab07

* Formatted code. * Updated unit tests in the agent to use real extensions.

Issue #170: Enhance MetricsHub ConnectorStore Loading System

47cdf04

* Updated extension unit tests.

Merge remote-tracking branch 'origin/feature/EPIC-extension-system' into

faed190

fearture/issue-170-enhance-metricshub-connectorstore-loading-system

Issue #170: Enhance MetricsHub ConnectorStore Loading System

e7b1d62

* Fixed JUnit tests on oscommand extension

Issue #170: Enhance MetricsHub ConnectorStore Loading System

7dfd508

* Fixed formatting and removed useless imports

Issue #170: Enhance MetricsHub ConnectorStore Loading System

56976b2

* Decreased coverage on metricshub-agent due to code removal

Merge remote-tracking branch 'origin/feature/EPIC-extension-system' into

57cfc59

fearture/issue-170-enhance-metricshub-connectorstore-loading-system

Issue #170: Enhance MetricsHub ConnectorStore Loading System

f98a26d

* Added unit tests for EmbeddedFile methods. * Update README.md * Enhanced log message in EmbeddedFileHelper

Issue #170: Enhance MetricsHub ConnectorStore Loading System

f09fa06

* Managed connector store extension providers

NassimBtk requested review from MedMaalej, bertysentry, alexdgilbert, TNohaic, CherfaElyes and SafaeAJ May 17, 2024 08:04

MedMaalej reviewed May 17, 2024

View reviewed changes

bertysentry reviewed May 17, 2024

View reviewed changes

Issue #170: Enhance MetricsHub ConnectorStore Loading System

f50f139

* Increased coverage on OsCommand and Http extension * Fixed Timeout deserialization on the configuration objects.

NassimBtk changed the title ~~Issue #170 enhance ConnectorStore loading system~~ Issue #170 Enhance ConnectorStore loading system May 17, 2024

Issue #170: Enhance MetricsHub ConnectorStore Loading System

9089695

* Cleaned-up Constants.java in the engine. * Replaced temp file prefix `SEN_Embedded_` with `metricshub_embedded_`.

NassimBtk requested review from MedMaalej and bertysentry May 17, 2024 14:35

Issue #170: Enhance MetricsHub ConnectorStore Loading System

ed3ec80

* Added note to `README.md`.

CherfaElyes approved these changes May 17, 2024

View reviewed changes

NassimBtk merged commit c9678c3 into feature/EPIC-extension-system May 17, 2024

NassimBtk deleted the fearture/issue-170-enhance-metricshub-connectorstore-loading-system branch May 17, 2024 15:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue #170 Enhance ConnectorStore loading system #176

Issue #170 Enhance ConnectorStore loading system #176

NassimBtk commented May 17, 2024 •

edited

MedMaalej May 17, 2024

NassimBtk May 17, 2024

MedMaalej May 17, 2024

MedMaalej May 17, 2024

bertysentry left a comment

bertysentry May 17, 2024

NassimBtk May 17, 2024 •

edited

bertysentry May 17, 2024

NassimBtk May 17, 2024

bertysentry May 17, 2024

bertysentry May 17, 2024

bertysentry May 17, 2024

NassimBtk May 17, 2024 •

edited

NassimBtk May 17, 2024

Issue #170 Enhance ConnectorStore loading system #176

Issue #170 Enhance ConnectorStore loading system #176

Conversation

NassimBtk commented May 17, 2024 • edited

Overview

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

bertysentry left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NassimBtk May 17, 2024 • edited

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

NassimBtk May 17, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NassimBtk commented May 17, 2024 •

edited

NassimBtk May 17, 2024 •

edited

NassimBtk May 17, 2024 •

edited