From 6532522c0e9d755bc96386ba3fa946f73aa8e5a9 Mon Sep 17 00:00:00 2001 From: EddeCCC Date: Thu, 1 Feb 2024 11:03:56 +0100 Subject: [PATCH 01/17] remove extended logging --- .../src/components/views/status/StatusTable.js | 15 +++------------ .../ocelot/agentstatus/AgentStatusManager.java | 1 - .../ocelot/rest/agent/AgentController.java | 1 - .../src/main/resources/logback.xml | 4 ---- 4 files changed, 3 insertions(+), 18 deletions(-) diff --git a/components/inspectit-ocelot-configurationserver-ui/src/components/views/status/StatusTable.js b/components/inspectit-ocelot-configurationserver-ui/src/components/views/status/StatusTable.js index b7ad9f1a32..6be7443a78 100644 --- a/components/inspectit-ocelot-configurationserver-ui/src/components/views/status/StatusTable.js +++ b/components/inspectit-ocelot-configurationserver-ui/src/components/views/status/StatusTable.js @@ -293,19 +293,10 @@ class StatusTable extends React.Component { agentHealthTemplate = (rowData) => { const { onShowHealthStateDialog } = this.props; - const { onShowDownloadDialog } = this.props; - const { metaInformation } = rowData; + const { healthState, metaInformation } = rowData; + const { health } = healthState; const { agentId } = metaInformation; - // TODO Remove console.error() after agent health issues are solved - let health; - let healthState; - try { - healthState = rowData['healthState']; - health = healthState['health']; - } catch (error) { - console.error(`Could not read agent health from ${agentId}`, error); - console.error(rowData); - } + const { onShowDownloadDialog } = this.props; let { agentCommandsEnabled, supportArchiveAvailable } = this.resolveServiceAvailability(metaInformation); diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentstatus/AgentStatusManager.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentstatus/AgentStatusManager.java index 00b2c26ec6..ac9c5e585d 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentstatus/AgentStatusManager.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentstatus/AgentStatusManager.java @@ -97,7 +97,6 @@ public void notifyAgentConfigurationFetched(Map agentAttributes, logHealthIfChanged(statusKey, agentHealthState); } - log.debug("Storing agent status of {}: ({})", statusKey, agentStatus); attributesToAgentStatusCache.put(statusKey, agentStatus); } diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/agent/AgentController.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/agent/AgentController.java index cc37ea00a5..29d8fd7fcf 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/agent/AgentController.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/agent/AgentController.java @@ -62,7 +62,6 @@ public void e(Exception e) { @GetMapping(value = {"agent/configuration", "agent/configuration/"}, produces = "application/x-yaml") public ResponseEntity fetchConfiguration(@Parameter(description = "The agent attributes used to select the correct mapping") @RequestParam Map attributes, @RequestHeader Map headers) { log.debug("Fetching the agent configuration for agent ({})", attributes.toString()); - log.debug("Receiving agent headers ({})", headers.toString()); AgentConfiguration configuration = configManager.getConfiguration(attributes); statusManager.notifyAgentConfigurationFetched(attributes, headers, configuration); if (configuration == null) { diff --git a/components/inspectit-ocelot-configurationserver/src/main/resources/logback.xml b/components/inspectit-ocelot-configurationserver/src/main/resources/logback.xml index 3d3d591731..1e6cdfb1c3 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/resources/logback.xml +++ b/components/inspectit-ocelot-configurationserver/src/main/resources/logback.xml @@ -54,10 +54,6 @@ - - - - From a7a5f7ae25b7572abadde5a4cf9f6db28a18c211 Mon Sep 17 00:00:00 2001 From: EddeCCC Date: Thu, 1 Feb 2024 13:42:29 +0100 Subject: [PATCH 02/17] refactor AgentConfigurationReloadTask --- .../AgentConfiguration.java | 11 +- .../AgentConfigurationReloadTask.java | 110 ++++++++++++------ .../AgentConfigurationReloadTaskTest.java | 47 ++++---- 3 files changed, 104 insertions(+), 64 deletions(-) diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java index 2fa970a553..b75b54a033 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java @@ -1,6 +1,8 @@ package rocks.inspectit.ocelot.agentconfiguration; import lombok.Builder; +import lombok.Data; +import lombok.Setter; import lombok.Value; import org.springframework.util.DigestUtils; import rocks.inspectit.ocelot.mappings.model.AgentMapping; @@ -14,16 +16,17 @@ * An {@link AgentMapping} which has its configuration loaded in-memory. * In addition, a cryptographic hash is computed to detect changes of configurations. */ -@Value +@Data public class AgentConfiguration { /** * The agent mapping for which this instance represents the loaded configuration. */ - private AgentMapping mapping; + private final AgentMapping mapping; /** * The set of defined documentable objects in this configuration for each file.
+ * The map might be initialized after constructing the AgentConfiguration.
* - Key: the file path
* - Value: the set of objects, like actions, scopes, rules & metrics */ @@ -32,12 +35,12 @@ public class AgentConfiguration { /** * The merged YAML configuration for the given mapping. */ - private String configYaml; + private final String configYaml; /** * Cryptographic hash for {@link #configYaml}. */ - private String hash; + private final String hash; @Builder private AgentConfiguration(AgentMapping mapping, Map> docsObjectsByFile, String configYaml) { diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java index 20dd499dfe..693bf6cd4c 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java @@ -64,53 +64,55 @@ public void run() { List newConfigurations = new ArrayList<>(); for (AgentMapping mapping : mappingsToLoad) { try { - AgentConfiguration agentConfiguration = createAgentConfiguration(mapping); + String configYaml = loadConfigForMapping(mapping); if (isCanceled()) { log.debug("Configuration reloading canceled"); return; } + AgentConfiguration agentConfiguration = AgentConfiguration.builder() + .mapping(mapping) + .configYaml(configYaml) + .docsObjectsByFile(new HashMap<>()) // leave docsObjectsByFile empty for now + .build(); newConfigurations.add(agentConfiguration); } catch (Exception e) { - log.error("Could not load agent configuration for agent mapping '{}'.", mapping.name(), e); + log.error("Could not load agent mapping '{}'.", mapping.name(), e); } } + onTaskSuccess(newConfigurations); + log.info("Finishing configuration reloading..."); + + // add docsObjects afterward to provide agent configurations earlier + addDocsObjectsTask(newConfigurations); } - /** - * Creates the configuration for one agent with the provided agent mapping - * @param mapping the mapping to load - * - * @return Configuration for the agent mapping - */ - @VisibleForTesting - AgentConfiguration createAgentConfiguration(AgentMapping mapping) { - AbstractFileAccessor fileAccessor = getFileAccessorForMapping(mapping); + private void addDocsObjectsTask(List newConfigurations) { + log.info("Starting configuration reloading with documented objects..."); + for (AgentConfiguration configuration : newConfigurations) { + AgentMapping mapping = configuration.getMapping(); + try { + AbstractFileAccessor fileAccessor = getFileAccessorForMapping(mapping); - LinkedHashSet allYamlFiles = new LinkedHashSet<>(); - for (String path : mapping.sources()) { - if (isCanceled()) return null; - allYamlFiles.addAll(getAllYamlFiles(fileAccessor, path)); - } + LinkedHashSet allYamlFiles = loadAllYamlFilesForMapping(fileAccessor, mapping); - Object yamlResult = null; - Map> docsObjectsByFile = new HashMap<>(); + Map> docsObjectsByFile = new HashMap<>(); + for (String path : allYamlFiles) { + if (isCanceled()) return; + String src = fileAccessor.readConfigurationFile(path).orElse(""); - for (String path : allYamlFiles) { - if (isCanceled()) return null; - String src = fileAccessor.readConfigurationFile(path).orElse(""); + Set loadedObjects = loadDocsObjects(src, path); + docsObjectsByFile.put(path, loadedObjects); + } - Set loadedObjects = loadDocsObjects(src, path); - docsObjectsByFile.put(path, loadedObjects); - yamlResult = loadAndMergeYaml(yamlResult, src, path); + configuration.setDocsObjectsByFile(docsObjectsByFile); + } catch (Exception e) { + log.error("Could not load documented objects for agent mapping '{}'.", mapping.name(), e); + } } - String configYaml = yamlResult == null ? "" : new Yaml().dump(yamlResult); - return AgentConfiguration.builder() - .mapping(mapping) - .docsObjectsByFile(docsObjectsByFile) - .configYaml(configYaml) - .build(); + onTaskSuccess(newConfigurations); + log.info("Finishing configuration reloading with documented objects..."); } /** @@ -131,17 +133,59 @@ private Set loadDocsObjects(String src, String filePath) { return objects; } + /** + * Loads the given mapping as yaml string. + * + * @param mapping the mapping to load + * + * @return the merged yaml for the given mapping or an empty string if the mapping does not contain any existing files + * If this task has been canceled, null is returned. + */ + @VisibleForTesting + String loadConfigForMapping(AgentMapping mapping) { + AbstractFileAccessor fileAccessor = getFileAccessorForMapping(mapping); + + LinkedHashSet allYamlFiles = loadAllYamlFilesForMapping(fileAccessor, mapping); + + Object result = null; + for (String path : allYamlFiles) { + if (isCanceled()) { + return null; + } + result = loadAndMergeYaml(fileAccessor, result, path); + } + return result == null ? "" : new Yaml().dump(result); + } + + /** + * + * @param fileAccessor + * @param mapping + * @return + */ + private LinkedHashSet loadAllYamlFilesForMapping(AbstractFileAccessor fileAccessor, AgentMapping mapping) { + LinkedHashSet allYamlFiles = new LinkedHashSet<>(); + for (String path : mapping.sources()) { + if (isCanceled()) { + return null; + } + List yamlFiles = getAllYamlFiles(fileAccessor, path); + allYamlFiles.addAll(yamlFiles); + } + return allYamlFiles; + } + /** * Loads a yaml file as a Map/List structure and merges it with an existing map/list structure * * @param toMerge the existing structure of nested maps / lists with which the loaded yaml will be merged. - * @param src the yaml string - * @param path the path of the yaml file to load + * @param path the path of the yaml file to load * * @return the merged structure */ - private Object loadAndMergeYaml(Object toMerge, String src, String path) { + private Object loadAndMergeYaml(AbstractFileAccessor fileAccessor, Object toMerge, String path) { Yaml yaml = new Yaml(); + String src = fileAccessor.readConfigurationFile(path).orElse(""); try { Map loadedYaml = yaml.load(src); if (toMerge == null) { diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java index 615a3d3155..55ed8e2a0b 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java @@ -227,10 +227,9 @@ public void loadYaml() { .source("/test") .sourceBranch(WORKSPACE) .build(); - AgentConfiguration config = reloadTask.createAgentConfiguration(mapping); - String yaml = config.getConfigYaml(); + String string = reloadTask.loadConfigForMapping(mapping); - assertThat(yaml).isEqualTo("{key: value}\n"); + assertThat(string).isEqualTo("{key: value}\n"); } @Test @@ -248,7 +247,7 @@ public void yamlWithTab() { .sourceBranch(WORKSPACE) .build(); - assertThatExceptionOfType(AgentConfigurationReloadTask.InvalidConfigurationFileException.class).isThrownBy(() -> reloadTask.createAgentConfiguration(mapping)) + assertThatExceptionOfType(AgentConfigurationReloadTask.InvalidConfigurationFileException.class).isThrownBy(() -> reloadTask.loadConfigForMapping(mapping)) .withMessage("The configuration file '/test.yml' is invalid and cannot be parsed."); } } @@ -257,49 +256,46 @@ public void yamlWithTab() { class LoadConfigForMapping { @Test - void noSourcesSpecified() { - AgentConfiguration config = reloadTask.createAgentConfiguration(AgentMapping.builder().build()); - String yaml = config.getConfigYaml(); + void noSourcesSpecified() throws IOException { + String result = reloadTask.loadConfigForMapping(AgentMapping.builder().build()); - assertThat(yaml).isEmpty(); + assertThat(result).isEmpty(); } @Test - void liveBranchSpecified() { + void liveBranchSpecified() throws IOException { AgentMapping mapping = AgentMapping.builder().source("a.yml").sourceBranch(Branch.LIVE).build(); doReturn(true).when(liveAccessor).configurationFileExists("a.yml"); doReturn(false).when(liveAccessor).configurationFileIsDirectory("a.yml"); doReturn(Optional.of("key: value")).when(liveAccessor).readConfigurationFile("a.yml"); - AgentConfiguration config = reloadTask.createAgentConfiguration(mapping); - String yaml = config.getConfigYaml(); + String result = reloadTask.loadConfigForMapping(mapping); - assertThat(yaml).isEqualToIgnoringWhitespace("{key: value}"); + assertThat(result).isEqualToIgnoringWhitespace("{key: value}"); } @Test - void nonExistingSourcesSpecified() { + void nonExistingSourcesSpecified() throws IOException { doReturn(false).when(workspaceAccessor).configurationFileExists("a.yml"); doReturn(false).when(workspaceAccessor).configurationFileExists("some/folder"); - AgentConfiguration config = reloadTask.createAgentConfiguration(AgentMapping.builder() + String result = reloadTask.loadConfigForMapping(AgentMapping.builder() .source("a.yml") .source("/some/folder") .sourceBranch(WORKSPACE) .build()); - String yaml = config.getConfigYaml(); - assertThat(yaml).isEmpty(); + assertThat(result).isEmpty(); } @Test - void nonYamlIgnored() { + void nonYamlIgnored() throws IOException { doReturn(true).when(workspaceAccessor).configurationFileExists(any()); doReturn(false).when(workspaceAccessor).configurationFileIsDirectory(any()); doReturn(Optional.of("")).when(workspaceAccessor).readConfigurationFile(any()); - AgentConfiguration config = reloadTask.createAgentConfiguration(AgentMapping.builder() + String result = reloadTask.loadConfigForMapping(AgentMapping.builder() .source("a.yml") .source("b.YmL") .source("c.yaml") @@ -307,9 +303,7 @@ void nonYamlIgnored() { .sourceBranch(WORKSPACE) .build()); - String yaml = config.getConfigYaml(); - - assertThat(yaml).isEmpty(); + assertThat(result).isEmpty(); verify(workspaceAccessor).readConfigurationFile("a.yml"); verify(workspaceAccessor).readConfigurationFile("b.YmL"); verify(workspaceAccessor).readConfigurationFile("c.yaml"); @@ -318,12 +312,12 @@ void nonYamlIgnored() { } @Test - void leadingSlashesInSourcesRemoved() { + void leadingSlashesInSourcesRemoved() throws IOException { doReturn(false).when(workspaceAccessor).configurationFileExists("a.yml"); lenient().doThrow(new RuntimeException()).when(workspaceAccessor).configurationFileExists(startsWith("/")); - reloadTask.createAgentConfiguration(AgentMapping.builder() + reloadTask.loadConfigForMapping(AgentMapping.builder() .source("/a.yml") .sourceBranch(WORKSPACE) .build()); @@ -332,7 +326,7 @@ void leadingSlashesInSourcesRemoved() { } @Test - void priorityRespected() { + void priorityRespected() throws IOException { when(workspaceAccessor.configurationFileExists(any())).thenReturn(true); @@ -354,14 +348,13 @@ void priorityRespected() { doReturn(Optional.of("{ val1: b, val2: b, val3: b}")).when(workspaceAccessor) .readConfigurationFile("folder/b.yml"); - AgentConfiguration config = reloadTask.createAgentConfiguration(AgentMapping.builder() + String result = reloadTask.loadConfigForMapping(AgentMapping.builder() .source("/z.yml") .source("/folder") .sourceBranch(WORKSPACE) .build()); - String yaml = config.getConfigYaml(); - assertThat(yaml).isEqualTo("{val1: z, val2: a, val3: b}\n"); + assertThat(result).isEqualTo("{val1: z, val2: a, val3: b}\n"); verify(workspaceAccessor, never()).readConfigurationFile("folder/somethingelse"); } } From 8654bcbdbd7c5fc2d9920af39ecf41f4d7ea6501 Mon Sep 17 00:00:00 2001 From: EddeCCC Date: Thu, 1 Feb 2024 14:02:17 +0100 Subject: [PATCH 03/17] add docsObjectsTask test --- .../AgentConfigurationManager.java | 5 +- .../AgentConfigurationReloadTask.java | 8 ++- .../agentconfiguration/DocsObjectsLoader.java | 6 +- .../ConfigurationController.java | 5 +- .../AgentConfigurationReloadTaskTest.java | 72 ++++++++++++++----- .../DocsObjectsLoaderTest.java | 10 +-- 6 files changed, 79 insertions(+), 27 deletions(-) diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationManager.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationManager.java index 9561fac1aa..11851093d9 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationManager.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationManager.java @@ -49,6 +49,9 @@ public class AgentConfigurationManager { @Autowired private FileManager fileManager; + @Autowired + private DocsObjectsLoader docsObjectsLoader; + /** * List of current AgentConfigurations for retrieval of AgentConfiguration corresponding to AgentMapping in * {@link AgentConfigurationManager#getConfigurationForMapping(AgentMapping)}. @@ -78,7 +81,7 @@ private synchronized void reloadConfigurationAsync() { if (reloadTask != null) { reloadTask.cancel(); } - reloadTask = new AgentConfigurationReloadTask(mappingsSerializer, fileManager, this::replaceConfigurations); + reloadTask = new AgentConfigurationReloadTask(mappingsSerializer, fileManager, docsObjectsLoader, this::replaceConfigurations); executorService.submit(reloadTask); } diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java index 693bf6cd4c..caf2ac0338 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java @@ -32,6 +32,8 @@ class AgentConfigurationReloadTask extends CancellableTask> onLoadCallback) { + public AgentConfigurationReloadTask(AgentMappingSerializer mappingsSerializer, FileManager fileManager, + DocsObjectsLoader docsObjectsLoader, Consumer> onLoadCallback) { super(onLoadCallback); this.mappingsSerializer = mappingsSerializer; this.fileManager = fileManager; + this.docsObjectsLoader = docsObjectsLoader; } /** @@ -126,7 +130,7 @@ private void addDocsObjectsTask(List newConfigurations) { private Set loadDocsObjects(String src, String filePath) { Set objects = Collections.emptySet(); try { - objects = DocsObjectsLoader.loadObjects(src); + objects = docsObjectsLoader.loadObjects(src); } catch (Exception e) { log.warn("Could not parse configuration: {}", filePath, e); } diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoader.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoader.java index db06d1d703..f63c7de539 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoader.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoader.java @@ -2,6 +2,7 @@ import inspectit.ocelot.configdocsgenerator.parsing.ConfigParser; import lombok.extern.slf4j.Slf4j; +import org.springframework.stereotype.Component; import org.yaml.snakeyaml.Yaml; import rocks.inspectit.ocelot.config.model.InspectitConfig; import rocks.inspectit.ocelot.config.model.instrumentation.InstrumentationSettings; @@ -13,6 +14,7 @@ * Helper class to load documentable objects from a yaml string */ @Slf4j +@Component public class DocsObjectsLoader { /** @@ -25,7 +27,7 @@ public class DocsObjectsLoader { * @param src the source yaml * @return a set of defined objects in this yaml */ - public static Set loadObjects(String src) throws IOException { + public Set loadObjects(String src) throws IOException { Yaml yaml = new Yaml(); Object rawYaml = yaml.load(src); Set objects = new HashSet<>(); @@ -52,7 +54,7 @@ public static Set loadObjects(String src) throws IOException { * @param defaultYamls A map of the default file paths and their yaml content * @return A set of defined objects for each file */ - public static Map> loadDefaultDocsObjectsByFile(Map defaultYamls) { + public Map> loadDefaultDocsObjectsByFile(Map defaultYamls) { Map> defaultDocsObjectsByFile = new HashMap<>(); for(Map.Entry entry : defaultYamls.entrySet()) { String path = OCELOT_DEFAULT_CONFIG_PREFIX + entry.getKey(); diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationController.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationController.java index 9d6483b631..969d3297a2 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationController.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationController.java @@ -51,6 +51,9 @@ public class ConfigurationController extends AbstractBaseController { @Autowired private DefaultConfigController defaultConfigController; + @Autowired + private DocsObjectsLoader docsObjectsLoader; + // Not final to make mocking in test possible private ConfigDocsGenerator configDocsGenerator = new ConfigDocsGenerator(); @@ -125,7 +128,7 @@ public ResponseEntity getConfigDocumentation( combined = ObjectStructureMerger.merge(combined, loadedYaml); } configYaml = yaml.dump(combined); - Map> defaultObjectsByFile = DocsObjectsLoader.loadDefaultDocsObjectsByFile(defaultYamls); + Map> defaultObjectsByFile = docsObjectsLoader.loadDefaultDocsObjectsByFile(defaultYamls); docsObjectsByFile.putAll(defaultObjectsByFile); } diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java index 55ed8e2a0b..50b8efc45b 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java @@ -16,10 +16,7 @@ import rocks.inspectit.ocelot.mappings.model.AgentMapping; import java.io.IOException; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; -import java.util.Optional; +import java.util.*; import java.util.function.Consumer; import java.util.stream.Stream; @@ -41,6 +38,9 @@ public class AgentConfigurationReloadTaskTest { @Mock FileManager fileManager; + @Mock + DocsObjectsLoader docsObjectsLoader; + @Mock RevisionAccess liveAccessor; @@ -48,7 +48,7 @@ public class AgentConfigurationReloadTaskTest { RevisionAccess workspaceAccessor; @BeforeEach - public void beforeEach() { + void beforeEach() { lenient().when(fileManager.getWorkspaceRevision()).thenReturn(workspaceAccessor); lenient().when(fileManager.getLiveRevision()).thenReturn(liveAccessor); lenient().when(serializer.getRevisionAccess()).thenReturn(workspaceAccessor); @@ -58,7 +58,7 @@ public void beforeEach() { class Run { @Test - public void loadWithException() { + void loadWithException() { FileInfo fileInfo = mock(FileInfo.class); when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of("/test.yml"), Stream.of("/test.yml")); when(workspaceAccessor.agentMappingsExist()).thenReturn(true); @@ -83,7 +83,7 @@ public void loadWithException() { MutableObject> configurations = new MutableObject<>(); Consumer> consumer = configurations::setValue; - AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, consumer); + AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, docsObjectsLoader, consumer); task.run(); @@ -95,7 +95,7 @@ public void loadWithException() { } @Test - public void loadWithExceptionOnlyString() { + void loadWithExceptionOnlyString() { FileInfo fileInfo = mock(FileInfo.class); when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of("/test.yml"), Stream.of("/test.yml")); when(workspaceAccessor.agentMappingsExist()).thenReturn(true); @@ -120,7 +120,7 @@ public void loadWithExceptionOnlyString() { MutableObject> configurations = new MutableObject<>(); Consumer> consumer = configurations::setValue; - AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, consumer); + AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, docsObjectsLoader, consumer); task.run(); @@ -132,7 +132,7 @@ public void loadWithExceptionOnlyString() { } @Test - public void loadWithExceptionOnlyList() { + void loadWithExceptionOnlyList() { FileInfo fileInfo = mock(FileInfo.class); when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of("/test.yml"), Stream.of("/test.yml")); when(workspaceAccessor.agentMappingsExist()).thenReturn(true); @@ -157,7 +157,7 @@ public void loadWithExceptionOnlyList() { MutableObject> configurations = new MutableObject<>(); Consumer> consumer = configurations::setValue; - AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, consumer); + AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, docsObjectsLoader, consumer); task.run(); @@ -169,7 +169,7 @@ public void loadWithExceptionOnlyList() { } @Test - public void loadMappingFromWorkspace() { + void loadMappingFromWorkspace() { when(workspaceAccessor.agentMappingsExist()).thenReturn(true); AgentMapping mapping = AgentMapping.builder() @@ -181,7 +181,7 @@ public void loadMappingFromWorkspace() { MutableObject> configurations = new MutableObject<>(); Consumer> consumer = configurations::setValue; - AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, consumer); + AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, docsObjectsLoader, consumer); task.run(); verify(serializer, times(1)).readAgentMappings(workspaceAccessor); @@ -189,7 +189,7 @@ public void loadMappingFromWorkspace() { } @Test - public void loadMappingFromLive() { + void loadMappingFromLive() { lenient().when(serializer.getRevisionAccess()).thenReturn(liveAccessor); when(liveAccessor.agentMappingsExist()).thenReturn(true); @@ -202,7 +202,7 @@ public void loadMappingFromLive() { MutableObject> configurations = new MutableObject<>(); Consumer> consumer = configurations::setValue; - AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, consumer); + AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, docsObjectsLoader, consumer); task.run(); verify(serializer, times(0)).readAgentMappings(workspaceAccessor); @@ -210,11 +210,49 @@ public void loadMappingFromLive() { } } + @Nested + class AddDocsObjectsTask { + + @Test + void verifyDocsObjectsHaveBeenAdded() throws IOException { + FileInfo fileInfo = mock(FileInfo.class); + String fileName = "/test.yml"; + when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of(fileName), Stream.of(fileName)); + when(workspaceAccessor.agentMappingsExist()).thenReturn(true); + when(workspaceAccessor.configurationFileExists(anyString())).thenReturn(true); + when(workspaceAccessor.configurationFileIsDirectory(anyString())).thenReturn(true); + when(workspaceAccessor.listConfigurationFiles(anyString())).thenReturn(Collections.singletonList(fileInfo)); + + Set docsObjects = Set.of("s_myScope", "r_myRule", "a_myAction", "myMetric"); + when(docsObjectsLoader.loadObjects(any())).thenReturn(docsObjects); + Map> docsObjectsByFile = new HashMap<>(); + docsObjectsByFile.put(fileName, docsObjects); + + AgentMapping mapping = AgentMapping.builder() + .name("test") + .source("/test") + .sourceBranch(WORKSPACE) + .build(); + doReturn(Collections.singletonList(mapping)).when(serializer).readAgentMappings(any()); + MutableObject> configurations = new MutableObject<>(); + Consumer> consumer = configurations::setValue; + + AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, docsObjectsLoader, consumer); + task.run(); + + List configurationList = configurations.getValue(); + assertThat(configurationList).hasSize(1); + assertThat(configurationList).element(0) + .extracting(AgentConfiguration::getDocsObjectsByFile) + .isEqualTo(docsObjectsByFile); + } + } + @Nested class LoadAndMergeYaml { @Test - public void loadYaml() { + void loadYaml() { FileInfo fileInfo = mock(FileInfo.class); when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of("/test.yml")); when(workspaceAccessor.configurationFileExists("test")).thenReturn(true); @@ -233,7 +271,7 @@ public void loadYaml() { } @Test - public void yamlWithTab() { + void yamlWithTab() { FileInfo fileInfo = mock(FileInfo.class); when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of("/test.yml")); when(workspaceAccessor.configurationFileExists("test")).thenReturn(true); diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoaderTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoaderTest.java index e2796da909..739e975b09 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoaderTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoaderTest.java @@ -13,6 +13,8 @@ public class DocsObjectsLoaderTest { + private final DocsObjectsLoader docsObjectsLoader = new DocsObjectsLoader(); + private final String srcYaml = """ inspectit: instrumentation: @@ -34,21 +36,21 @@ public class DocsObjectsLoaderTest { @Test void verifyLoadObjectsSuccessful() throws IOException { - Set objects = DocsObjectsLoader.loadObjects(srcYaml); + Set objects = docsObjectsLoader.loadObjects(srcYaml); assertTrue(objects.contains("s_jdbc_statement_execute")); } @Test void verifyLoadObjectsEmpty() throws IOException { - Set objects = DocsObjectsLoader.loadObjects(srcYamlWithoutDocsObjects); + Set objects = docsObjectsLoader.loadObjects(srcYamlWithoutDocsObjects); assertTrue(objects.isEmpty()); } @Test void verifyLoadThrowsException() { - assertThrows(NoSuchElementException.class, () -> DocsObjectsLoader.loadObjects("invalid-config")); + assertThrows(NoSuchElementException.class, () -> docsObjectsLoader.loadObjects("invalid-config")); } @Test @@ -58,7 +60,7 @@ void verifyLoadDefaultDocsObjectsByFile() { Map configs = new HashMap<>(); configs.put(file, srcYaml); - Map> docsObjectsByFile = DocsObjectsLoader.loadDefaultDocsObjectsByFile(configs); + Map> docsObjectsByFile = docsObjectsLoader.loadDefaultDocsObjectsByFile(configs); assertTrue(docsObjectsByFile.containsKey(fileWithPrefix)); assertTrue(docsObjectsByFile.containsValue(Collections.singleton("s_jdbc_statement_execute"))); } From 35e2ea854f825d0f4866e44d153115d8e4d66464 Mon Sep 17 00:00:00 2001 From: EddeCCC Date: Thu, 1 Feb 2024 15:56:22 +0100 Subject: [PATCH 04/17] optimize task and fix test --- .../AgentConfigurationManager.java | 2 +- .../AgentConfigurationReloadTask.java | 39 +++++++++++++------ .../ocelot/utils/CancellableTask.java | 15 ++++++- .../ConfigurationControllerTest.java | 4 ++ 4 files changed, 45 insertions(+), 15 deletions(-) diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationManager.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationManager.java index 11851093d9..9872779ac7 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationManager.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationManager.java @@ -123,7 +123,7 @@ private synchronized void replaceConfigurations(List newConf attributesToConfigurationCache = CacheBuilder.newBuilder() .maximumSize(config.getMaxAgents()) .expireAfterAccess(config.getAgentEvictionDelay().toMillis(), TimeUnit.MILLISECONDS) - .build(new CacheLoader, AgentConfiguration>() { + .build(new CacheLoader<>() { @Override public AgentConfiguration load(Map agentAttributes) { return newConfigurations.stream() diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java index caf2ac0338..289a8bb963 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java @@ -88,17 +88,22 @@ public void run() { log.info("Finishing configuration reloading..."); // add docsObjects afterward to provide agent configurations earlier - addDocsObjectsTask(newConfigurations); + runDocsObjectsTask(newConfigurations); } - private void addDocsObjectsTask(List newConfigurations) { + /** + * After the loading of the agent configurations, these should be appended by their specific docsObjects. + * + * @param newConfigurations the list of agent configurations, which were loaded + */ + private void runDocsObjectsTask(List newConfigurations) { log.info("Starting configuration reloading with documented objects..."); for (AgentConfiguration configuration : newConfigurations) { AgentMapping mapping = configuration.getMapping(); try { AbstractFileAccessor fileAccessor = getFileAccessorForMapping(mapping); - LinkedHashSet allYamlFiles = loadAllYamlFilesForMapping(fileAccessor, mapping); + LinkedHashSet allYamlFiles = getAllYamlFilesForMapping(fileAccessor, mapping); Map> docsObjectsByFile = new HashMap<>(); for (String path : allYamlFiles) { @@ -115,12 +120,13 @@ private void addDocsObjectsTask(List newConfigurations) { } } - onTaskSuccess(newConfigurations); + String taskName = getClass().getSimpleName() + ".DocsObjectsTask"; + onTaskSuccess(newConfigurations, taskName); log.info("Finishing configuration reloading with documented objects..."); } /** - * Loads all documentable objects of the yaml source string + * Loads all documentable objects of the yaml source string. * * @param src the yaml string * @param filePath the path to the yaml file @@ -149,7 +155,7 @@ private Set loadDocsObjects(String src, String filePath) { String loadConfigForMapping(AgentMapping mapping) { AbstractFileAccessor fileAccessor = getFileAccessorForMapping(mapping); - LinkedHashSet allYamlFiles = loadAllYamlFilesForMapping(fileAccessor, mapping); + LinkedHashSet allYamlFiles = getAllYamlFilesForMapping(fileAccessor, mapping); Object result = null; for (String path : allYamlFiles) { @@ -162,12 +168,15 @@ String loadConfigForMapping(AgentMapping mapping) { } /** + * Returns the set of yaml files, which is defined in the agent mapping sources. + * + * @param fileAccessor the accessor to use for reading the file + * @param mapping the mapping, which contains a list of source file paths * - * @param fileAccessor - * @param mapping - * @return + * @return the set of yaml file paths for the provided mapping + * If this task has been canceled, null is returned. */ - private LinkedHashSet loadAllYamlFilesForMapping(AbstractFileAccessor fileAccessor, AgentMapping mapping) { + private LinkedHashSet getAllYamlFilesForMapping(AbstractFileAccessor fileAccessor, AgentMapping mapping) { LinkedHashSet allYamlFiles = new LinkedHashSet<>(); for (String path : mapping.sources()) { if (isCanceled()) { @@ -180,7 +189,7 @@ private LinkedHashSet loadAllYamlFilesForMapping(AbstractFileAccessor fi } /** - * Loads a yaml file as a Map/List structure and merges it with an existing map/list structure + * Loads a yaml file as a Map/List structure and merges it with an existing map/list structure. * * @param toMerge the existing structure of nested maps / lists with which the loaded yaml will be merged. * @param path the path of the yaml file to load @@ -202,11 +211,17 @@ private Object loadAndMergeYaml(AbstractFileAccessor fileAccessor, Object toMerg } } + /** + * Returns the file accessor with regard to the source branch of the agent mapping. + * + * @param mapping the agent mapping with source branch + * + * @return the file accessor for the mapping source branch + */ private AbstractFileAccessor getFileAccessorForMapping(AgentMapping mapping) { return switch (mapping.sourceBranch()) { case LIVE -> fileManager.getLiveRevision(); case WORKSPACE -> fileManager.getWorkspaceRevision(); - default -> throw new UnsupportedOperationException("Unhandled branch: " + mapping.sourceBranch()); }; } diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/utils/CancellableTask.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/utils/CancellableTask.java index 13adcd27e7..366c020d2d 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/utils/CancellableTask.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/utils/CancellableTask.java @@ -43,13 +43,24 @@ public final boolean isCanceled() { * @param result the result of the task, which will be provided to the configured {@link #onLoadCallback}. */ protected final void onTaskSuccess(R result) { + onTaskSuccess(result, getClass().getSimpleName()); + } + + /** + * Should be invoked by the {@link #run()} method when this task has finished. + * This guarantees that {@link #onLoadCallback} is only invoked if the task has not been canceled. + * + * @param result the result of the task, which will be provided to the configured {@link #onLoadCallback}. + * @param taskName the custom name of the task + */ + protected final void onTaskSuccess(R result, String taskName) { synchronized (this) { if (cancelFlag) { - log.debug("{} canceled", getClass().getSimpleName()); + log.debug("{} canceled", taskName); return; } onLoadCallback.accept(result); - log.info("{} successfully finished", getClass().getSimpleName()); + log.info("{} successfully finished", taskName); } } } diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java index 556d71e01f..0ea3d66349 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java @@ -18,6 +18,7 @@ import org.yaml.snakeyaml.Yaml; import rocks.inspectit.ocelot.agentconfiguration.AgentConfiguration; import rocks.inspectit.ocelot.agentconfiguration.AgentConfigurationManager; +import rocks.inspectit.ocelot.agentconfiguration.DocsObjectsLoader; import rocks.inspectit.ocelot.agentconfiguration.ObjectStructureMerger; import rocks.inspectit.ocelot.file.FileManager; import rocks.inspectit.ocelot.mappings.AgentMappingManager; @@ -53,6 +54,9 @@ public class ConfigurationControllerTest { @Mock private DefaultConfigController defaultConfigController; + @Mock + private DocsObjectsLoader docsObjectsLoader; + @Mock private Yaml yaml; From 6349c49e46eec76664fd847e15472a07b816f242 Mon Sep 17 00:00:00 2001 From: EddeCCC Date: Thu, 1 Feb 2024 16:04:59 +0100 Subject: [PATCH 05/17] optimize cached agent mappings --- .../inspectit/ocelot/mappings/AgentMappingSerializer.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/mappings/AgentMappingSerializer.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/mappings/AgentMappingSerializer.java index 19832c415f..e9c0120455 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/mappings/AgentMappingSerializer.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/mappings/AgentMappingSerializer.java @@ -87,8 +87,8 @@ public void postConstruct() throws IOException { * @return List of current {@link AgentMapping}s representing the content of the given file */ public List readCachedAgentMappings(){ - if(currentMappings != null) return currentMappings; - else return readAgentMappings(fileManager.getWorkspaceRevision()); + if(currentMappings == null) currentMappings = readAgentMappings(fileManager.getWorkspaceRevision()); + return currentMappings; } /** From f5ff8632d5a973e1f0e50f271a561bd045685c23 Mon Sep 17 00:00:00 2001 From: EddeCCC Date: Thu, 1 Feb 2024 16:51:00 +0100 Subject: [PATCH 06/17] remove redundant logs --- .../agentconfiguration/AgentConfigurationReloadTask.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java index 289a8bb963..34e221c429 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java @@ -85,8 +85,6 @@ public void run() { } onTaskSuccess(newConfigurations); - log.info("Finishing configuration reloading..."); - // add docsObjects afterward to provide agent configurations earlier runDocsObjectsTask(newConfigurations); } @@ -122,7 +120,6 @@ private void runDocsObjectsTask(List newConfigurations) { String taskName = getClass().getSimpleName() + ".DocsObjectsTask"; onTaskSuccess(newConfigurations, taskName); - log.info("Finishing configuration reloading with documented objects..."); } /** From 8f65a7e48516b1da67691596f6e6a1e70c3b4e9d Mon Sep 17 00:00:00 2001 From: EddeCCC Date: Wed, 7 Feb 2024 09:57:18 +0100 Subject: [PATCH 07/17] update naming convention --- .../docs/instrumentation/instrumentation.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/inspectit-ocelot-documentation/docs/instrumentation/instrumentation.md b/inspectit-ocelot-documentation/docs/instrumentation/instrumentation.md index 049cb1af0a..3a2316338a 100644 --- a/inspectit-ocelot-documentation/docs/instrumentation/instrumentation.md +++ b/inspectit-ocelot-documentation/docs/instrumentation/instrumentation.md @@ -43,8 +43,11 @@ Therefore, to increase readability of your configuration files the following nam * Action names always start with `a_`, e.g. `a_my_action`. * Rule names always start with `r_`, e.g. `r_my_rule`. * Context variables names should start with `d_`, e.g. `d_transaction_name`. -* Fields which are defined by the user should always be put in single quotations marks, e.g. `input: 'my_input'`. This rule also applies to keys which - can be entirely defined by the user, for example when defining the name of a custom action or attribute names. +* Fields which are defined by the user should always be put in single quotations marks, e.g. `input: 'my_input'`. +* This rule also applies to keys which can be entirely defined by the user, for example when defining the name of a + custom action or attribute names. +* Field names should be written in **snake case**, e.g. `s_my_scope`. **Do not use** dots instead of underscores, since + this will create a _nested structure_ in yaml and might lead to configuration errors. This naming convention is used both in this documentation and the default configuration provided. From 1fa0c60fde5ffc5d1e890cc06345abbf00b1fc99 Mon Sep 17 00:00:00 2001 From: EddeCCC Date: Wed, 7 Feb 2024 09:57:34 +0100 Subject: [PATCH 08/17] add requested changes --- .../AgentConfiguration.java | 184 +++++++++++++- .../AgentConfigurationManager.java | 12 +- .../AgentConfigurationReloadTask.java | 192 +-------------- .../agentconfiguration/DocsObjectsLoader.java | 5 +- .../ObjectStructureMerger.java | 34 +++ .../mappings/AgentMappingSerializer.java | 2 +- .../ConfigurationController.java | 5 +- .../ocelot/utils/CancellableTask.java | 15 +- .../AgentConfigurationReloadTaskTest.java | 195 ++++----------- .../AgentConfigurationTest.java | 225 ++++++++++++++++++ .../DocsObjectsLoaderTest.java | 10 +- .../agentstatus/AgentStatusManagerTest.java | 11 +- .../rest/agent/AgentControllerTest.java | 4 +- .../ocelot/rest/agent/AgentServiceTest.java | 7 +- .../ConfigurationControllerTest.java | 28 +-- 15 files changed, 506 insertions(+), 423 deletions(-) create mode 100644 components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java index b75b54a033..4b1eb3458f 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java @@ -1,28 +1,43 @@ package rocks.inspectit.ocelot.agentconfiguration; -import lombok.Builder; -import lombok.Data; -import lombok.Setter; +import com.google.common.annotations.VisibleForTesting; import lombok.Value; +import lombok.extern.slf4j.Slf4j; import org.springframework.util.DigestUtils; +import org.yaml.snakeyaml.Yaml; +import rocks.inspectit.ocelot.file.FileInfo; +import rocks.inspectit.ocelot.file.accessor.AbstractFileAccessor; import rocks.inspectit.ocelot.mappings.model.AgentMapping; import java.nio.charset.Charset; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; +import java.util.function.Predicate; +import java.util.function.Supplier; +import java.util.stream.Collectors; /** * An {@link AgentMapping} which has its configuration loaded in-memory. * In addition, a cryptographic hash is computed to detect changes of configurations. */ -@Data +@Value +@Slf4j public class AgentConfiguration { + /** + * Used as maker in {@link #attributesToConfigurationCache} to mark attribute-maps for which no mapping matches. + */ + public static final AgentConfiguration NO_MATCHING_MAPPING = createDefaultConfiguration(); + + /** + * Predicate to check if a given file path ends with .yml or .yaml + */ + private static final Predicate HAS_YAML_ENDING = filePath -> filePath.toLowerCase() + .endsWith(".yml") || filePath.toLowerCase().endsWith(".yaml"); + /** * The agent mapping for which this instance represents the loaded configuration. */ - private final AgentMapping mapping; + private AgentMapping mapping; /** * The set of defined documentable objects in this configuration for each file.
@@ -32,21 +47,164 @@ public class AgentConfiguration { */ private Map> docsObjectsByFile; + /** + * The set of suppliers for the defined documentable objects in this configuration for each file + */ + private static final Map>> docsObjectsByFileSuppliers = new HashMap<>(); + /** * The merged YAML configuration for the given mapping. */ - private final String configYaml; + private String configYaml; /** * Cryptographic hash for {@link #configYaml}. */ - private final String hash; + private String hash; - @Builder - private AgentConfiguration(AgentMapping mapping, Map> docsObjectsByFile, String configYaml) { + private AgentConfiguration(AgentMapping mapping, Map> docsObjectsByFile, String configYaml, String hash) { this.mapping = mapping; this.docsObjectsByFile = docsObjectsByFile; this.configYaml = configYaml; - hash = DigestUtils.md5DigestAsHex(configYaml.getBytes(Charset.defaultCharset())); + this.hash = hash; + } + + /** + * Factory method to create AgentConfigurations. + * Also creates a cryptographic hash. + * + * @param mapping The agent mapping for which this instance represents the loaded configuration + * @return Created AgentConfiguration + */ + public static AgentConfiguration create(AgentMapping mapping, AbstractFileAccessor fileAccessor) { + String configYaml = loadConfigForMapping(mapping, fileAccessor); + String hash = DigestUtils.md5DigestAsHex(configYaml.getBytes(Charset.defaultCharset())); + return new AgentConfiguration(mapping, new HashMap<>(), configYaml, hash); + } + + /** + * Factory method to create AgentConfigurations. + * Also creates a cryptographic hash. + * + * @param mapping The agent mapping for which this instance represents the loaded configuration + * @param configYaml The yaml string, which contains the configuration + * @param docsObjectsByFile The set of defined documentable objects in this configuration for each file + * @return Created AgentConfiguration + */ + public static AgentConfiguration create(AgentMapping mapping, Map> docsObjectsByFile, String configYaml) { + String hash = DigestUtils.md5DigestAsHex(configYaml.getBytes(Charset.defaultCharset())); + return new AgentConfiguration(mapping, docsObjectsByFile, configYaml, hash); + } + + /** + * Apply suppliers to get the documentable objects by file and store them + */ + public void supplyDocsObjectsByFile() { + for (Map.Entry>> entry : docsObjectsByFileSuppliers.entrySet()) { + this.docsObjectsByFile.put(entry.getKey(), entry.getValue().get()); + } + } + + /** + * Factory method to create default AgentConfiguration. + * + * @return Created default AgentConfiguration + */ + private static AgentConfiguration createDefaultConfiguration() { + String configYaml = ""; + String hash = DigestUtils.md5DigestAsHex(configYaml.getBytes(Charset.defaultCharset())); + return new AgentConfiguration(null, new HashMap<>(), configYaml, hash); + } + + /** + * Loads the given mapping as yaml string. + * + * @param mapping the mapping to load + * + * @return the merged yaml for the given mapping or an empty string if the mapping does not contain any existing files + * If this task has been canceled, null is returned. + */ + @VisibleForTesting + static String loadConfigForMapping(AgentMapping mapping, AbstractFileAccessor fileAccessor) { + LinkedHashSet allYamlFiles = getAllYamlFilesForMapping(fileAccessor, mapping); + + Object result = null; + for (String path : allYamlFiles) { + String src = fileAccessor.readConfigurationFile(path).orElse(""); + result = ObjectStructureMerger.loadAndMergeYaml(src, result, path); + + Supplier> docsObjectsSupplier = () -> loadDocsObjects(src, path); + docsObjectsByFileSuppliers.put(path, docsObjectsSupplier); + } + return result == null ? "" : new Yaml().dump(result); + } + + /** + * Returns the set of yaml files, which is defined in the agent mapping sources. + * + * @param fileAccessor the accessor to use for reading the file + * @param mapping the mapping, which contains a list of source file paths + * + * @return the set of yaml file paths for the provided mapping + * If this task has been canceled, null is returned. + */ + private static LinkedHashSet getAllYamlFilesForMapping(AbstractFileAccessor fileAccessor, AgentMapping mapping) { + LinkedHashSet allYamlFiles = new LinkedHashSet<>(); + for (String path : mapping.sources()) { + List yamlFiles = getAllYamlFiles(fileAccessor, path); + allYamlFiles.addAll(yamlFiles); + } + return allYamlFiles; + } + + /** + * If the given path is a yaml file, a list containing only it is returned. + * If the path is a directory, the absolute path of all contained yaml files is returned in alphabetical order. + * If it is neither, an empty list is returned. + * + * @param path the path to check for yaml files, can start with a slash which will be ignored + * + * @return a list of absolute paths of contained YAML files + */ + private static List getAllYamlFiles(AbstractFileAccessor fileAccessor, String path) { + String cleanedPath; + if (path.startsWith("/")) { + cleanedPath = path.substring(1); + } else { + cleanedPath = path; + } + + if (fileAccessor.configurationFileExists(cleanedPath)) { + if (fileAccessor.configurationFileIsDirectory(cleanedPath)) { + List fileInfos = fileAccessor.listConfigurationFiles(cleanedPath); + + return fileInfos.stream() + .flatMap(file -> file.getAbsoluteFilePaths(cleanedPath)) + .filter(HAS_YAML_ENDING) + .sorted() + .collect(Collectors.toList()); + } else if (HAS_YAML_ENDING.test(cleanedPath)) { + return Collections.singletonList(cleanedPath); + } + } + return Collections.emptyList(); + } + + /** + * Loads all documentable objects of the yaml source string. + * + * @param src the yaml string + * @param filePath the path to the yaml file + * + * @return the set of documentable objects + */ + private static Set loadDocsObjects(String src, String filePath) { + Set objects = Collections.emptySet(); + try { + objects = DocsObjectsLoader.loadObjects(src); + } catch (Exception e) { + log.warn("Could not parse configuration: {}", filePath, e); + } + return objects; } } diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationManager.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationManager.java index 9872779ac7..b08151223b 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationManager.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationManager.java @@ -24,6 +24,8 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; +import static rocks.inspectit.ocelot.agentconfiguration.AgentConfiguration.NO_MATCHING_MAPPING; + /** * Manager responsible for serving the agent configuration based on the set of {@link AgentMapping}s. */ @@ -31,11 +33,6 @@ @Slf4j public class AgentConfigurationManager { - /** - * Used as maker in {@link #attributesToConfigurationCache} to mark attribute-maps for which no mapping matches. - */ - private static final AgentConfiguration NO_MATCHING_MAPPING = AgentConfiguration.builder().configYaml("").build(); - @Autowired @VisibleForTesting InspectitServerSettings config; @@ -49,9 +46,6 @@ public class AgentConfigurationManager { @Autowired private FileManager fileManager; - @Autowired - private DocsObjectsLoader docsObjectsLoader; - /** * List of current AgentConfigurations for retrieval of AgentConfiguration corresponding to AgentMapping in * {@link AgentConfigurationManager#getConfigurationForMapping(AgentMapping)}. @@ -81,7 +75,7 @@ private synchronized void reloadConfigurationAsync() { if (reloadTask != null) { reloadTask.cancel(); } - reloadTask = new AgentConfigurationReloadTask(mappingsSerializer, fileManager, docsObjectsLoader, this::replaceConfigurations); + reloadTask = new AgentConfigurationReloadTask(mappingsSerializer, fileManager, this::replaceConfigurations); executorService.submit(reloadTask); } diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java index 34e221c429..79210b324f 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java @@ -1,9 +1,6 @@ package rocks.inspectit.ocelot.agentconfiguration; -import com.google.common.annotations.VisibleForTesting; import lombok.extern.slf4j.Slf4j; -import org.yaml.snakeyaml.Yaml; -import rocks.inspectit.ocelot.file.FileInfo; import rocks.inspectit.ocelot.file.FileManager; import rocks.inspectit.ocelot.file.accessor.AbstractFileAccessor; import rocks.inspectit.ocelot.file.accessor.git.RevisionAccess; @@ -12,9 +9,9 @@ import rocks.inspectit.ocelot.utils.CancellableTask; import java.util.*; +import java.util.concurrent.CompletableFuture; import java.util.function.Consumer; -import java.util.function.Predicate; -import java.util.stream.Collectors; +import java.util.function.Supplier; /** * A task for asynchronously loading the configurations based on a given list of mappings. @@ -22,18 +19,10 @@ @Slf4j class AgentConfigurationReloadTask extends CancellableTask> { - /** - * Predicate to check if a given file path ends with .yml or .yaml - */ - private static final Predicate HAS_YAML_ENDING = filePath -> filePath.toLowerCase() - .endsWith(".yml") || filePath.toLowerCase().endsWith(".yaml"); - private FileManager fileManager; private AgentMappingSerializer mappingsSerializer; - private DocsObjectsLoader docsObjectsLoader; - /** * Creates a new reload task, but does NOT start it. * The loading process is done in {@link #run()}. @@ -42,12 +31,10 @@ class AgentConfigurationReloadTask extends CancellableTask> onLoadCallback) { + public AgentConfigurationReloadTask(AgentMappingSerializer mappingsSerializer, FileManager fileManager, Consumer> onLoadCallback) { super(onLoadCallback); this.mappingsSerializer = mappingsSerializer; this.fileManager = fileManager; - this.docsObjectsLoader = docsObjectsLoader; } /** @@ -68,16 +55,12 @@ public void run() { List newConfigurations = new ArrayList<>(); for (AgentMapping mapping : mappingsToLoad) { try { - String configYaml = loadConfigForMapping(mapping); if (isCanceled()) { log.debug("Configuration reloading canceled"); return; } - AgentConfiguration agentConfiguration = AgentConfiguration.builder() - .mapping(mapping) - .configYaml(configYaml) - .docsObjectsByFile(new HashMap<>()) // leave docsObjectsByFile empty for now - .build(); + AbstractFileAccessor fileAccessor = getFileAccessorForMapping(mapping); + AgentConfiguration agentConfiguration = AgentConfiguration.create(mapping, fileAccessor); newConfigurations.add(agentConfiguration); } catch (Exception e) { log.error("Could not load agent mapping '{}'.", mapping.name(), e); @@ -85,127 +68,9 @@ public void run() { } onTaskSuccess(newConfigurations); - // add docsObjects afterward to provide agent configurations earlier - runDocsObjectsTask(newConfigurations); - } - - /** - * After the loading of the agent configurations, these should be appended by their specific docsObjects. - * - * @param newConfigurations the list of agent configurations, which were loaded - */ - private void runDocsObjectsTask(List newConfigurations) { - log.info("Starting configuration reloading with documented objects..."); - for (AgentConfiguration configuration : newConfigurations) { - AgentMapping mapping = configuration.getMapping(); - try { - AbstractFileAccessor fileAccessor = getFileAccessorForMapping(mapping); - - LinkedHashSet allYamlFiles = getAllYamlFilesForMapping(fileAccessor, mapping); - - Map> docsObjectsByFile = new HashMap<>(); - for (String path : allYamlFiles) { - if (isCanceled()) return; - String src = fileAccessor.readConfigurationFile(path).orElse(""); - - Set loadedObjects = loadDocsObjects(src, path); - docsObjectsByFile.put(path, loadedObjects); - } - - configuration.setDocsObjectsByFile(docsObjectsByFile); - } catch (Exception e) { - log.error("Could not load documented objects for agent mapping '{}'.", mapping.name(), e); - } - } - - String taskName = getClass().getSimpleName() + ".DocsObjectsTask"; - onTaskSuccess(newConfigurations, taskName); - } - - /** - * Loads all documentable objects of the yaml source string. - * - * @param src the yaml string - * @param filePath the path to the yaml file - * - * @return the set of documentable objects - */ - private Set loadDocsObjects(String src, String filePath) { - Set objects = Collections.emptySet(); - try { - objects = docsObjectsLoader.loadObjects(src); - } catch (Exception e) { - log.warn("Could not parse configuration: {}", filePath, e); - } - return objects; - } - - /** - * Loads the given mapping as yaml string. - * - * @param mapping the mapping to load - * - * @return the merged yaml for the given mapping or an empty string if the mapping does not contain any existing files - * If this task has been canceled, null is returned. - */ - @VisibleForTesting - String loadConfigForMapping(AgentMapping mapping) { - AbstractFileAccessor fileAccessor = getFileAccessorForMapping(mapping); - - LinkedHashSet allYamlFiles = getAllYamlFilesForMapping(fileAccessor, mapping); - Object result = null; - for (String path : allYamlFiles) { - if (isCanceled()) { - return null; - } - result = loadAndMergeYaml(fileAccessor, result, path); - } - return result == null ? "" : new Yaml().dump(result); - } - - /** - * Returns the set of yaml files, which is defined in the agent mapping sources. - * - * @param fileAccessor the accessor to use for reading the file - * @param mapping the mapping, which contains a list of source file paths - * - * @return the set of yaml file paths for the provided mapping - * If this task has been canceled, null is returned. - */ - private LinkedHashSet getAllYamlFilesForMapping(AbstractFileAccessor fileAccessor, AgentMapping mapping) { - LinkedHashSet allYamlFiles = new LinkedHashSet<>(); - for (String path : mapping.sources()) { - if (isCanceled()) { - return null; - } - List yamlFiles = getAllYamlFiles(fileAccessor, path); - allYamlFiles.addAll(yamlFiles); - } - return allYamlFiles; - } - - /** - * Loads a yaml file as a Map/List structure and merges it with an existing map/list structure. - * - * @param toMerge the existing structure of nested maps / lists with which the loaded yaml will be merged. - * @param path the path of the yaml file to load - * - * @return the merged structure - */ - private Object loadAndMergeYaml(AbstractFileAccessor fileAccessor, Object toMerge, String path) { - Yaml yaml = new Yaml(); - String src = fileAccessor.readConfigurationFile(path).orElse(""); - try { - Map loadedYaml = yaml.load(src); - if (toMerge == null) { - return loadedYaml; - } else { - return ObjectStructureMerger.merge(toMerge, loadedYaml); - } - } catch (Exception e) { - throw new InvalidConfigurationFileException(path, e); - } + newConfigurations.forEach(AgentConfiguration::supplyDocsObjectsByFile); + log.info("Successfully supplied documented Objects for agent configurations"); } /** @@ -221,47 +86,4 @@ private AbstractFileAccessor getFileAccessorForMapping(AgentMapping mapping) { case WORKSPACE -> fileManager.getWorkspaceRevision(); }; } - - /** - * If the given path is a yaml file, a list containing only it is returned. - * If the path is a directory, the absolute path of all contained yaml files is returned in alphabetical order. - * If it is neither, an empty list is returned. - * - * @param path the path to check for yaml files, can start with a slash which will be ignored - * - * @return a list of absolute paths of contained YAML files - */ - private List getAllYamlFiles(AbstractFileAccessor fileAccessor, String path) { - String cleanedPath; - if (path.startsWith("/")) { - cleanedPath = path.substring(1); - } else { - cleanedPath = path; - } - - if (fileAccessor.configurationFileExists(cleanedPath)) { - if (fileAccessor.configurationFileIsDirectory(cleanedPath)) { - List fileInfos = fileAccessor.listConfigurationFiles(cleanedPath); - - return fileInfos.stream() - .flatMap(file -> file.getAbsoluteFilePaths(cleanedPath)) - .filter(HAS_YAML_ENDING) - .sorted() - .collect(Collectors.toList()); - } else if (HAS_YAML_ENDING.test(cleanedPath)) { - return Collections.singletonList(cleanedPath); - } - } - return Collections.emptyList(); - } - - /** - * This exception will be thrown if a configuration file cannot be parsed, e.g. it contains invalid characters. - */ - static class InvalidConfigurationFileException extends RuntimeException { - - public InvalidConfigurationFileException(String path, Exception e) { - super(String.format("The configuration file '%s' is invalid and cannot be parsed.", path), e); - } - } } diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoader.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoader.java index f63c7de539..0c851f1200 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoader.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoader.java @@ -14,7 +14,6 @@ * Helper class to load documentable objects from a yaml string */ @Slf4j -@Component public class DocsObjectsLoader { /** @@ -27,7 +26,7 @@ public class DocsObjectsLoader { * @param src the source yaml * @return a set of defined objects in this yaml */ - public Set loadObjects(String src) throws IOException { + public static Set loadObjects(String src) throws IOException { Yaml yaml = new Yaml(); Object rawYaml = yaml.load(src); Set objects = new HashSet<>(); @@ -54,7 +53,7 @@ public Set loadObjects(String src) throws IOException { * @param defaultYamls A map of the default file paths and their yaml content * @return A set of defined objects for each file */ - public Map> loadDefaultDocsObjectsByFile(Map defaultYamls) { + public static Map> loadDefaultDocsObjectsByFile(Map defaultYamls) { Map> defaultDocsObjectsByFile = new HashMap<>(); for(Map.Entry entry : defaultYamls.entrySet()) { String path = OCELOT_DEFAULT_CONFIG_PREFIX + entry.getKey(); diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/ObjectStructureMerger.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/ObjectStructureMerger.java index 7dc7c49ff2..93b37bfd80 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/ObjectStructureMerger.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/ObjectStructureMerger.java @@ -1,5 +1,7 @@ package rocks.inspectit.ocelot.agentconfiguration; +import org.yaml.snakeyaml.Yaml; +import rocks.inspectit.ocelot.file.accessor.AbstractFileAccessor; import java.util.*; import java.util.stream.Stream; @@ -10,6 +12,29 @@ */ public class ObjectStructureMerger { + /** + * Loads a yaml file as a Map/List structure and merges it with an existing map/list structure. + * + * @param src the source yaml, which should be merged + * @param toMerge the existing structure of nested maps / lists with which the loaded yaml will be merged. + * @param path the path of the yaml file to load + * + * @return the merged structure + */ + public static Object loadAndMergeYaml(String src, Object toMerge, String path) { + Yaml yaml = new Yaml(); + try { + Map loadedYaml = yaml.load(src); + if (toMerge == null) { + return loadedYaml; + } else { + return ObjectStructureMerger.merge(toMerge, loadedYaml); + } + } catch (Exception e) { + throw new InvalidConfigurationFileException(path, e); + } + } + /** * Tries to merge the given two Object structure, giving precedence to the first one. * If both Objects are maps or bot hare lists, they will be deeply merged. @@ -82,4 +107,13 @@ private static List mergeLists(List first, List second) { return result; } + /** + * This exception will be thrown if a configuration file cannot be parsed, e.g. it contains invalid characters. + */ + static class InvalidConfigurationFileException extends RuntimeException { + + public InvalidConfigurationFileException(String path, Exception e) { + super(String.format("The configuration file '%s' is invalid and cannot be parsed.", path), e); + } + } } diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/mappings/AgentMappingSerializer.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/mappings/AgentMappingSerializer.java index e9c0120455..2209605fe5 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/mappings/AgentMappingSerializer.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/mappings/AgentMappingSerializer.java @@ -86,7 +86,7 @@ public void postConstruct() throws IOException { * Read cached agent mappings to avoid long ymlMapper-processing time * @return List of current {@link AgentMapping}s representing the content of the given file */ - public List readCachedAgentMappings(){ + public List readCachedAgentMappings() { if(currentMappings == null) currentMappings = readAgentMappings(fileManager.getWorkspaceRevision()); return currentMappings; } diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationController.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationController.java index 969d3297a2..9d6483b631 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationController.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationController.java @@ -51,9 +51,6 @@ public class ConfigurationController extends AbstractBaseController { @Autowired private DefaultConfigController defaultConfigController; - @Autowired - private DocsObjectsLoader docsObjectsLoader; - // Not final to make mocking in test possible private ConfigDocsGenerator configDocsGenerator = new ConfigDocsGenerator(); @@ -128,7 +125,7 @@ public ResponseEntity getConfigDocumentation( combined = ObjectStructureMerger.merge(combined, loadedYaml); } configYaml = yaml.dump(combined); - Map> defaultObjectsByFile = docsObjectsLoader.loadDefaultDocsObjectsByFile(defaultYamls); + Map> defaultObjectsByFile = DocsObjectsLoader.loadDefaultDocsObjectsByFile(defaultYamls); docsObjectsByFile.putAll(defaultObjectsByFile); } diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/utils/CancellableTask.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/utils/CancellableTask.java index 366c020d2d..13adcd27e7 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/utils/CancellableTask.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/utils/CancellableTask.java @@ -43,24 +43,13 @@ public final boolean isCanceled() { * @param result the result of the task, which will be provided to the configured {@link #onLoadCallback}. */ protected final void onTaskSuccess(R result) { - onTaskSuccess(result, getClass().getSimpleName()); - } - - /** - * Should be invoked by the {@link #run()} method when this task has finished. - * This guarantees that {@link #onLoadCallback} is only invoked if the task has not been canceled. - * - * @param result the result of the task, which will be provided to the configured {@link #onLoadCallback}. - * @param taskName the custom name of the task - */ - protected final void onTaskSuccess(R result, String taskName) { synchronized (this) { if (cancelFlag) { - log.debug("{} canceled", taskName); + log.debug("{} canceled", getClass().getSimpleName()); return; } onLoadCallback.accept(result); - log.info("{} successfully finished", taskName); + log.info("{} successfully finished", getClass().getSimpleName()); } } } diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java index 50b8efc45b..e62ab4ec03 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java @@ -5,23 +5,19 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import rocks.inspectit.ocelot.file.FileInfo; import rocks.inspectit.ocelot.file.FileManager; import rocks.inspectit.ocelot.file.accessor.git.RevisionAccess; -import rocks.inspectit.ocelot.file.versioning.Branch; import rocks.inspectit.ocelot.mappings.AgentMappingSerializer; import rocks.inspectit.ocelot.mappings.model.AgentMapping; -import java.io.IOException; import java.util.*; import java.util.function.Consumer; import java.util.stream.Stream; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; import static rocks.inspectit.ocelot.file.versioning.Branch.WORKSPACE; @@ -29,18 +25,12 @@ @ExtendWith(MockitoExtension.class) public class AgentConfigurationReloadTaskTest { - @InjectMocks - AgentConfigurationReloadTask reloadTask; - @Mock AgentMappingSerializer serializer; @Mock FileManager fileManager; - @Mock - DocsObjectsLoader docsObjectsLoader; - @Mock RevisionAccess liveAccessor; @@ -83,7 +73,7 @@ void loadWithException() { MutableObject> configurations = new MutableObject<>(); Consumer> consumer = configurations::setValue; - AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, docsObjectsLoader, consumer); + AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, consumer); task.run(); @@ -120,7 +110,7 @@ void loadWithExceptionOnlyString() { MutableObject> configurations = new MutableObject<>(); Consumer> consumer = configurations::setValue; - AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, docsObjectsLoader, consumer); + AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, consumer); task.run(); @@ -157,7 +147,7 @@ void loadWithExceptionOnlyList() { MutableObject> configurations = new MutableObject<>(); Consumer> consumer = configurations::setValue; - AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, docsObjectsLoader, consumer); + AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, consumer); task.run(); @@ -181,7 +171,7 @@ void loadMappingFromWorkspace() { MutableObject> configurations = new MutableObject<>(); Consumer> consumer = configurations::setValue; - AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, docsObjectsLoader, consumer); + AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, consumer); task.run(); verify(serializer, times(1)).readAgentMappings(workspaceAccessor); @@ -202,7 +192,7 @@ void loadMappingFromLive() { MutableObject> configurations = new MutableObject<>(); Consumer> consumer = configurations::setValue; - AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, docsObjectsLoader, consumer); + AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, consumer); task.run(); verify(serializer, times(0)).readAgentMappings(workspaceAccessor); @@ -211,10 +201,21 @@ void loadMappingFromLive() { } @Nested - class AddDocsObjectsTask { + class docsObjectsTaskSupplier { + + private final String srcYaml = """ + inspectit: + instrumentation: + scopes: + s_jdbc_statement_execute: + docs: + description: 'Scope for executed JDBC statements.' + methods: + - name: execute + """; @Test - void verifyDocsObjectsHaveBeenAdded() throws IOException { + void verifyDocsObjectsHaveBeenSupplied() { FileInfo fileInfo = mock(FileInfo.class); String fileName = "/test.yml"; when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of(fileName), Stream.of(fileName)); @@ -222,9 +223,9 @@ void verifyDocsObjectsHaveBeenAdded() throws IOException { when(workspaceAccessor.configurationFileExists(anyString())).thenReturn(true); when(workspaceAccessor.configurationFileIsDirectory(anyString())).thenReturn(true); when(workspaceAccessor.listConfigurationFiles(anyString())).thenReturn(Collections.singletonList(fileInfo)); + when(workspaceAccessor.readConfigurationFile(anyString())).thenReturn(Optional.of(srcYaml)); - Set docsObjects = Set.of("s_myScope", "r_myRule", "a_myAction", "myMetric"); - when(docsObjectsLoader.loadObjects(any())).thenReturn(docsObjects); + Set docsObjects = Set.of("s_jdbc_statement_execute"); Map> docsObjectsByFile = new HashMap<>(); docsObjectsByFile.put(fileName, docsObjects); @@ -237,7 +238,7 @@ void verifyDocsObjectsHaveBeenAdded() throws IOException { MutableObject> configurations = new MutableObject<>(); Consumer> consumer = configurations::setValue; - AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, docsObjectsLoader, consumer); + AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, consumer); task.run(); List configurationList = configurations.getValue(); @@ -246,154 +247,38 @@ void verifyDocsObjectsHaveBeenAdded() throws IOException { .extracting(AgentConfiguration::getDocsObjectsByFile) .isEqualTo(docsObjectsByFile); } - } - - @Nested - class LoadAndMergeYaml { @Test - void loadYaml() { + void verifyNoDocsObjectsCouldBeSupplied() { FileInfo fileInfo = mock(FileInfo.class); - when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of("/test.yml")); - when(workspaceAccessor.configurationFileExists("test")).thenReturn(true); - when(workspaceAccessor.configurationFileIsDirectory("test")).thenReturn(true); + String fileName = "/test.yml"; + when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of(fileName), Stream.of(fileName)); + when(workspaceAccessor.agentMappingsExist()).thenReturn(true); + when(workspaceAccessor.configurationFileExists(anyString())).thenReturn(true); + when(workspaceAccessor.configurationFileIsDirectory(anyString())).thenReturn(true); when(workspaceAccessor.listConfigurationFiles(anyString())).thenReturn(Collections.singletonList(fileInfo)); - when(workspaceAccessor.readConfigurationFile("/test.yml")).thenReturn(Optional.of("key: value")); - - AgentMapping mapping = AgentMapping.builder() - .name("test") - .source("/test") - .sourceBranch(WORKSPACE) - .build(); - String string = reloadTask.loadConfigForMapping(mapping); - - assertThat(string).isEqualTo("{key: value}\n"); - } + when(workspaceAccessor.readConfigurationFile(anyString())).thenReturn(Optional.empty()); - @Test - void yamlWithTab() { - FileInfo fileInfo = mock(FileInfo.class); - when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of("/test.yml")); - when(workspaceAccessor.configurationFileExists("test")).thenReturn(true); - when(workspaceAccessor.configurationFileIsDirectory("test")).thenReturn(true); - when(workspaceAccessor.listConfigurationFiles(anyString())).thenReturn(Collections.singletonList(fileInfo)); - when(workspaceAccessor.readConfigurationFile("/test.yml")).thenReturn(Optional.of("key:\tvalue")); + Map> docsObjectsByFile = new HashMap<>(); + docsObjectsByFile.put(fileName, new HashSet<>()); AgentMapping mapping = AgentMapping.builder() .name("test") .source("/test") .sourceBranch(WORKSPACE) .build(); + doReturn(Collections.singletonList(mapping)).when(serializer).readAgentMappings(any()); + MutableObject> configurations = new MutableObject<>(); + Consumer> consumer = configurations::setValue; - assertThatExceptionOfType(AgentConfigurationReloadTask.InvalidConfigurationFileException.class).isThrownBy(() -> reloadTask.loadConfigForMapping(mapping)) - .withMessage("The configuration file '/test.yml' is invalid and cannot be parsed."); - } - } - - @Nested - class LoadConfigForMapping { - - @Test - void noSourcesSpecified() throws IOException { - String result = reloadTask.loadConfigForMapping(AgentMapping.builder().build()); - - assertThat(result).isEmpty(); - } - - @Test - void liveBranchSpecified() throws IOException { - AgentMapping mapping = AgentMapping.builder().source("a.yml").sourceBranch(Branch.LIVE).build(); - - doReturn(true).when(liveAccessor).configurationFileExists("a.yml"); - doReturn(false).when(liveAccessor).configurationFileIsDirectory("a.yml"); - doReturn(Optional.of("key: value")).when(liveAccessor).readConfigurationFile("a.yml"); - - String result = reloadTask.loadConfigForMapping(mapping); - - assertThat(result).isEqualToIgnoringWhitespace("{key: value}"); - } - - @Test - void nonExistingSourcesSpecified() throws IOException { - doReturn(false).when(workspaceAccessor).configurationFileExists("a.yml"); - doReturn(false).when(workspaceAccessor).configurationFileExists("some/folder"); - - String result = reloadTask.loadConfigForMapping(AgentMapping.builder() - .source("a.yml") - .source("/some/folder") - .sourceBranch(WORKSPACE) - .build()); - - assertThat(result).isEmpty(); - } - - @Test - void nonYamlIgnored() throws IOException { - doReturn(true).when(workspaceAccessor).configurationFileExists(any()); - doReturn(false).when(workspaceAccessor).configurationFileIsDirectory(any()); - doReturn(Optional.of("")).when(workspaceAccessor).readConfigurationFile(any()); - - String result = reloadTask.loadConfigForMapping(AgentMapping.builder() - .source("a.yml") - .source("b.YmL") - .source("c.yaml") - .source("d.txt") - .sourceBranch(WORKSPACE) - .build()); - - assertThat(result).isEmpty(); - verify(workspaceAccessor).readConfigurationFile("a.yml"); - verify(workspaceAccessor).readConfigurationFile("b.YmL"); - verify(workspaceAccessor).readConfigurationFile("c.yaml"); - - verify(workspaceAccessor, never()).readConfigurationFile("d.txt"); - } - - @Test - void leadingSlashesInSourcesRemoved() throws IOException { - doReturn(false).when(workspaceAccessor).configurationFileExists("a.yml"); - - lenient().doThrow(new RuntimeException()).when(workspaceAccessor).configurationFileExists(startsWith("/")); - - reloadTask.loadConfigForMapping(AgentMapping.builder() - .source("/a.yml") - .sourceBranch(WORKSPACE) - .build()); - - verify(workspaceAccessor).configurationFileExists(eq("a.yml")); - } - - @Test - void priorityRespected() throws IOException { - - when(workspaceAccessor.configurationFileExists(any())).thenReturn(true); - - doReturn(true).when(workspaceAccessor).configurationFileIsDirectory("folder"); - doReturn(false).when(workspaceAccessor).configurationFileIsDirectory("z.yml"); - - List fileInfos = Arrays.asList(FileInfo.builder() - .type(FileInfo.Type.FILE) - .name("b.yml") - .build(), FileInfo.builder().type(FileInfo.Type.FILE).name("a.yml").build(), FileInfo.builder() - .type(FileInfo.Type.FILE) - .name("somethingelse") - .build()); - - when(workspaceAccessor.listConfigurationFiles("folder")).thenReturn(fileInfos); - - doReturn(Optional.of("{ val1: z}")).when(workspaceAccessor).readConfigurationFile("z.yml"); - doReturn(Optional.of("{ val1: a, val2: a}")).when(workspaceAccessor).readConfigurationFile("folder/a.yml"); - doReturn(Optional.of("{ val1: b, val2: b, val3: b}")).when(workspaceAccessor) - .readConfigurationFile("folder/b.yml"); - - String result = reloadTask.loadConfigForMapping(AgentMapping.builder() - .source("/z.yml") - .source("/folder") - .sourceBranch(WORKSPACE) - .build()); + AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, consumer); + task.run(); - assertThat(result).isEqualTo("{val1: z, val2: a, val3: b}\n"); - verify(workspaceAccessor, never()).readConfigurationFile("folder/somethingelse"); + List configurationList = configurations.getValue(); + assertThat(configurationList).hasSize(1); + assertThat(configurationList).element(0) + .extracting(AgentConfiguration::getDocsObjectsByFile) + .isEqualTo(docsObjectsByFile); } } } diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java new file mode 100644 index 0000000000..975cd9236a --- /dev/null +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java @@ -0,0 +1,225 @@ +package rocks.inspectit.ocelot.agentconfiguration; + +import org.apache.commons.lang3.mutable.MutableObject; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import rocks.inspectit.ocelot.file.FileInfo; +import rocks.inspectit.ocelot.file.accessor.git.RevisionAccess; +import rocks.inspectit.ocelot.file.versioning.Branch; +import rocks.inspectit.ocelot.mappings.model.AgentMapping; + +import java.util.*; +import java.util.function.Consumer; +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.*; +import static rocks.inspectit.ocelot.file.versioning.Branch.WORKSPACE; + +@ExtendWith(MockitoExtension.class) +public class AgentConfigurationTest { + + @Mock + RevisionAccess revisionAccess; + + @Nested + class LoadAndMergeYaml { + + @Test + void loadYaml() { + FileInfo fileInfo = mock(FileInfo.class); + when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of("/test.yml")); + when(revisionAccess.configurationFileExists("test")).thenReturn(true); + when(revisionAccess.configurationFileIsDirectory("test")).thenReturn(true); + when(revisionAccess.listConfigurationFiles(anyString())).thenReturn(Collections.singletonList(fileInfo)); + when(revisionAccess.readConfigurationFile("/test.yml")).thenReturn(Optional.of("key: value")); + + AgentMapping mapping = AgentMapping.builder() + .name("test") + .source("/test") + .sourceBranch(WORKSPACE) + .build(); + String string = AgentConfiguration.loadConfigForMapping(mapping, revisionAccess); + + assertThat(string).isEqualTo("{key: value}\n"); + } + + @Test + void yamlWithTab() { + FileInfo fileInfo = mock(FileInfo.class); + when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of("/test.yml")); + when(revisionAccess.configurationFileExists("test")).thenReturn(true); + when(revisionAccess.configurationFileIsDirectory("test")).thenReturn(true); + when(revisionAccess.listConfigurationFiles(anyString())).thenReturn(Collections.singletonList(fileInfo)); + when(revisionAccess.readConfigurationFile("/test.yml")).thenReturn(Optional.of("key:\tvalue")); + + AgentMapping mapping = AgentMapping.builder() + .name("test") + .source("/test") + .sourceBranch(WORKSPACE) + .build(); + + assertThatExceptionOfType(ObjectStructureMerger.InvalidConfigurationFileException.class).isThrownBy( + () -> AgentConfiguration.loadConfigForMapping(mapping, revisionAccess) + ).withMessage("The configuration file '/test.yml' is invalid and cannot be parsed."); + } + } + + @Nested + class LoadConfigForMapping { + + @Test + void noSourcesSpecified() { + AgentMapping mapping = AgentMapping.builder().build(); + String result = AgentConfiguration.loadConfigForMapping(mapping, revisionAccess); + + assertThat(result).isEmpty(); + } + + @Test + void nonExistingSourcesSpecified() { + doReturn(false).when(revisionAccess).configurationFileExists("a.yml"); + doReturn(false).when(revisionAccess).configurationFileExists("some/folder"); + + AgentMapping mapping = AgentMapping.builder() + .source("a.yml") + .source("/some/folder") + .sourceBranch(WORKSPACE) + .build(); + String result = AgentConfiguration.loadConfigForMapping(mapping, revisionAccess); + + assertThat(result).isEmpty(); + } + + @Test + void nonYamlIgnored() { + doReturn(true).when(revisionAccess).configurationFileExists(any()); + doReturn(false).when(revisionAccess).configurationFileIsDirectory(any()); + doReturn(Optional.of("")).when(revisionAccess).readConfigurationFile(any()); + + AgentMapping mapping = AgentMapping.builder() + .source("a.yml") + .source("b.YmL") + .source("c.yaml") + .source("d.txt") + .sourceBranch(WORKSPACE) + .build(); + String result = AgentConfiguration.loadConfigForMapping(mapping, revisionAccess); + + assertThat(result).isEmpty(); + verify(revisionAccess).readConfigurationFile("a.yml"); + verify(revisionAccess).readConfigurationFile("b.YmL"); + verify(revisionAccess).readConfigurationFile("c.yaml"); + + verify(revisionAccess, never()).readConfigurationFile("d.txt"); + } + + @Test + void leadingSlashesInSourcesRemoved() { + doReturn(false).when(revisionAccess).configurationFileExists("a.yml"); + + lenient().doThrow(new RuntimeException()).when(revisionAccess).configurationFileExists(startsWith("/")); + + AgentMapping mapping = AgentMapping.builder() + .source("/a.yml") + .sourceBranch(WORKSPACE) + .build(); + AgentConfiguration.loadConfigForMapping(mapping, revisionAccess); + + verify(revisionAccess).configurationFileExists(eq("a.yml")); + } + + @Test + void priorityRespected() { + + when(revisionAccess.configurationFileExists(any())).thenReturn(true); + + doReturn(true).when(revisionAccess).configurationFileIsDirectory("folder"); + doReturn(false).when(revisionAccess).configurationFileIsDirectory("z.yml"); + + List fileInfos = Arrays.asList(FileInfo.builder() + .type(FileInfo.Type.FILE) + .name("b.yml") + .build(), FileInfo.builder().type(FileInfo.Type.FILE).name("a.yml").build(), FileInfo.builder() + .type(FileInfo.Type.FILE) + .name("somethingelse") + .build()); + + when(revisionAccess.listConfigurationFiles("folder")).thenReturn(fileInfos); + + doReturn(Optional.of("{ val1: z}")).when(revisionAccess).readConfigurationFile("z.yml"); + doReturn(Optional.of("{ val1: a, val2: a}")).when(revisionAccess).readConfigurationFile("folder/a.yml"); + doReturn(Optional.of("{ val1: b, val2: b, val3: b}")).when(revisionAccess) + .readConfigurationFile("folder/b.yml"); + + AgentMapping mapping = AgentMapping.builder() + .source("/z.yml") + .source("/folder") + .sourceBranch(WORKSPACE) + .build(); + String result = AgentConfiguration.loadConfigForMapping(mapping, revisionAccess); + + assertThat(result).isEqualTo("{val1: z, val2: a, val3: b}\n"); + verify(revisionAccess, never()).readConfigurationFile("folder/somethingelse"); + } + } + + @Nested + class SupplyDocsObjects { + + private final String srcYaml = """ + inspectit: + instrumentation: + scopes: + s_jdbc_statement_execute: + docs: + description: 'Scope for executed JDBC statements.' + methods: + - name: execute + """; + + @Test + void verifySuccessfulSupply() { + FileInfo fileInfo = mock(FileInfo.class); + String fileName = "/test.yml"; + when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of(fileName), Stream.of(fileName)); + when(revisionAccess.configurationFileExists(anyString())).thenReturn(true); + when(revisionAccess.configurationFileIsDirectory(anyString())).thenReturn(true); + when(revisionAccess.listConfigurationFiles(anyString())).thenReturn(Collections.singletonList(fileInfo)); + when(revisionAccess.readConfigurationFile(anyString())).thenReturn(Optional.of(srcYaml)); + + Set docsObjects = Set.of("s_jdbc_statement_execute"); + Map> docsObjectsByFile = new HashMap<>(); + docsObjectsByFile.put(fileName, docsObjects); + + AgentMapping mapping = AgentMapping.builder() + .name("test") + .source("/test") + .sourceBranch(WORKSPACE) + .build(); + + AgentConfiguration config = AgentConfiguration.create(mapping, revisionAccess); + config.supplyDocsObjectsByFile(); + + assertThat(config.getDocsObjectsByFile()).isEqualTo(docsObjectsByFile); + } + + @Test + void verifyNoSupply() { + AgentMapping mapping = AgentMapping.builder() + .name("test") + .source("/test") + .sourceBranch(WORKSPACE) + .build(); + AgentConfiguration config = AgentConfiguration.create(mapping, new HashMap<>(), srcYaml); + config.supplyDocsObjectsByFile(); + + assertThat(config.getDocsObjectsByFile()).isEmpty(); + } + } +} diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoaderTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoaderTest.java index 739e975b09..e2796da909 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoaderTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoaderTest.java @@ -13,8 +13,6 @@ public class DocsObjectsLoaderTest { - private final DocsObjectsLoader docsObjectsLoader = new DocsObjectsLoader(); - private final String srcYaml = """ inspectit: instrumentation: @@ -36,21 +34,21 @@ public class DocsObjectsLoaderTest { @Test void verifyLoadObjectsSuccessful() throws IOException { - Set objects = docsObjectsLoader.loadObjects(srcYaml); + Set objects = DocsObjectsLoader.loadObjects(srcYaml); assertTrue(objects.contains("s_jdbc_statement_execute")); } @Test void verifyLoadObjectsEmpty() throws IOException { - Set objects = docsObjectsLoader.loadObjects(srcYamlWithoutDocsObjects); + Set objects = DocsObjectsLoader.loadObjects(srcYamlWithoutDocsObjects); assertTrue(objects.isEmpty()); } @Test void verifyLoadThrowsException() { - assertThrows(NoSuchElementException.class, () -> docsObjectsLoader.loadObjects("invalid-config")); + assertThrows(NoSuchElementException.class, () -> DocsObjectsLoader.loadObjects("invalid-config")); } @Test @@ -60,7 +58,7 @@ void verifyLoadDefaultDocsObjectsByFile() { Map configs = new HashMap<>(); configs.put(file, srcYaml); - Map> docsObjectsByFile = docsObjectsLoader.loadDefaultDocsObjectsByFile(configs); + Map> docsObjectsByFile = DocsObjectsLoader.loadDefaultDocsObjectsByFile(configs); assertTrue(docsObjectsByFile.containsKey(fileWithPrefix)); assertTrue(docsObjectsByFile.containsValue(Collections.singleton("s_jdbc_statement_execute"))); } diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentstatus/AgentStatusManagerTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentstatus/AgentStatusManagerTest.java index 68e4623736..74b3a52a85 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentstatus/AgentStatusManagerTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentstatus/AgentStatusManagerTest.java @@ -15,6 +15,7 @@ import java.time.Duration; import java.util.Collections; import java.util.Date; +import java.util.HashMap; import java.util.Map; import static org.assertj.core.api.Assertions.assertThat; @@ -43,7 +44,7 @@ class NotifyAgentConfigurationFetched { void testWithAgentIdHeader() { Branch testBranch = Branch.WORKSPACE; AgentMapping agentMapping = AgentMapping.builder().name("test-conf").sourceBranch(testBranch).build(); - AgentConfiguration config = AgentConfiguration.builder().mapping(agentMapping).configYaml("").build(); + AgentConfiguration config = AgentConfiguration.create(agentMapping, new HashMap<>(), ""); Map attributes = ImmutableMap.of("service", "test"); manager.notifyAgentConfigurationFetched(attributes, Collections.singletonMap(HEADER_AGENT_ID, "aid"), config); @@ -75,10 +76,10 @@ void testNoMappingFound() { void testMappingFound() { Branch testBranch = Branch.WORKSPACE; AgentMapping agentMapping = AgentMapping.builder().name("test-conf").sourceBranch(testBranch).build(); - AgentConfiguration conf = AgentConfiguration.builder().mapping(agentMapping).configYaml("").build(); + AgentConfiguration config = AgentConfiguration.create(agentMapping, new HashMap<>(), "");; Map attributes = ImmutableMap.of("service", "test"); - manager.notifyAgentConfigurationFetched(attributes, Collections.emptyMap(), conf); + manager.notifyAgentConfigurationFetched(attributes, Collections.emptyMap(), config); assertThat(manager.getAgentStatuses()).hasSize(1).anySatisfy(status -> { assertThat(status.getAttributes()).isEqualTo(attributes); @@ -92,7 +93,7 @@ void testMappingFound() { void testOverriding() throws Exception { Branch testBranch = Branch.WORKSPACE; AgentMapping agentMapping = AgentMapping.builder().name("test-conf").sourceBranch(testBranch).build(); - AgentConfiguration conf = AgentConfiguration.builder().mapping(agentMapping).configYaml("").build(); + AgentConfiguration config = AgentConfiguration.create(agentMapping, new HashMap<>(), ""); Map attributes = ImmutableMap.of("service", "test"); manager.notifyAgentConfigurationFetched(attributes, Collections.emptyMap(), null); @@ -107,7 +108,7 @@ void testOverriding() throws Exception { Thread.sleep(1); - manager.notifyAgentConfigurationFetched(attributes, Collections.emptyMap(), conf); + manager.notifyAgentConfigurationFetched(attributes, Collections.emptyMap(), config); assertThat(manager.getAgentStatuses()).hasSize(1).anySatisfy(status -> { assertThat(status.getAttributes()).isEqualTo(attributes); diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/agent/AgentControllerTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/agent/AgentControllerTest.java index 509daf04e9..34909a3114 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/agent/AgentControllerTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/agent/AgentControllerTest.java @@ -58,7 +58,7 @@ public void noMappingFound() { @Test public void mappingFound() { - AgentConfiguration config = AgentConfiguration.builder().configYaml("foo : bar").build(); + AgentConfiguration config = AgentConfiguration.create(null, new HashMap<>(), "foo : bar"); doReturn(config).when(configManager).getConfiguration(anyMap()); HashMap attributes = new HashMap<>(); @@ -71,7 +71,7 @@ public void mappingFound() { @Test public void etagPresent() { - AgentConfiguration config = AgentConfiguration.builder().configYaml("foo : bar").build(); + AgentConfiguration config = AgentConfiguration.create(null, new HashMap<>(), "foo : bar"); doReturn(config).when(configManager).getConfiguration(anyMap()); ResponseEntity firstResult = controller.fetchConfiguration(new HashMap<>(), Collections.emptyMap()); diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/agent/AgentServiceTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/agent/AgentServiceTest.java index 45e0580a79..3976a45584 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/agent/AgentServiceTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/agent/AgentServiceTest.java @@ -57,11 +57,10 @@ public class BuildSupportArchive { public void setupTestData(boolean logPreloadingEnabled) { serviceSpy = Mockito.spy(cut); + String configYaml = String.format("inspectit.log-preloading: {enabled: %b}", logPreloadingEnabled); //set config - config = AgentConfiguration.builder() - .configYaml(String.format("inspectit.log-preloading: {enabled: %b}", logPreloadingEnabled)) - .build(); + config = AgentConfiguration.create(null, new HashMap<>(), configYaml); //set attributes attributes = new HashMap() {{ @@ -111,7 +110,7 @@ public void shouldBuildSupportArchive() throws ExecutionException { DeferredResult> actualResult = serviceSpy.buildSupportArchive(attributes, configManager); ResponseEntity unwrappedResult = (ResponseEntity) actualResult.getResult(); - + assertThat(unwrappedResult.getStatusCode()).isEqualTo(HttpStatus.OK); assertThat(unwrappedResult.getBody()).isEqualTo(expectedResult); } diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java index 0ea3d66349..499b17d11f 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java @@ -54,9 +54,6 @@ public class ConfigurationControllerTest { @Mock private DefaultConfigController defaultConfigController; - @Mock - private DocsObjectsLoader docsObjectsLoader; - @Mock private Yaml yaml; @@ -65,10 +62,7 @@ public class FetchConfiguration { @Test public void returningConfiguration() { - AgentConfiguration configuration = AgentConfiguration.builder() - .configYaml("yaml") - .docsObjectsByFile(new HashMap<>()) - .build(); + AgentConfiguration configuration = AgentConfiguration.create(null, new HashMap<>(), "yaml"); when(agentConfigurationManager.getConfiguration(any())).thenReturn(configuration); ResponseEntity output = configurationController.fetchConfiguration(null); @@ -116,10 +110,7 @@ void withDefaultConfig() throws IOException { AgentMapping agentMapping = AgentMapping.builder().build(); final String configYaml = "yaml"; - AgentConfiguration agentConfiguration = AgentConfiguration.builder() - .configYaml(configYaml) - .docsObjectsByFile(docsObjectsByFile) - .build(); + AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, docsObjectsByFile, configYaml); Map defaultYamls = new HashMap<>(); final String defaultYamlContent = "defaultYaml"; @@ -170,10 +161,7 @@ void errorWhenGettingDefaultConfig() throws IOException { AgentMapping agentMapping = AgentMapping.builder().build(); final String configYaml = "yaml"; - AgentConfiguration agentConfiguration = AgentConfiguration.builder() - .configYaml(configYaml) - .docsObjectsByFile(docsObjectsByFile) - .build(); + AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, docsObjectsByFile, configYaml); IOException exception = new IOException(); @@ -206,10 +194,7 @@ void withoutDefaultConfig() throws IOException { AgentMapping agentMapping = AgentMapping.builder().build(); final String configYaml = "yaml"; - AgentConfiguration agentConfiguration = AgentConfiguration.builder() - .configYaml(configYaml) - .docsObjectsByFile(docsObjectsByFile) - .build(); + AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, docsObjectsByFile, configYaml); ConfigDocumentation configDocumentationMock = mock(ConfigDocumentation.class); @@ -258,10 +243,7 @@ void invalidYaml() throws IOException { AgentMapping agentMapping = AgentMapping.builder().build(); final String configYaml = "yaml"; - AgentConfiguration agentConfiguration = AgentConfiguration.builder() - .configYaml(configYaml) - .docsObjectsByFile(docsObjectsByFile) - .build(); + AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, docsObjectsByFile, configYaml); JsonProcessingException exception = mock(JsonProcessingException.class); final String errorMessage = "JsonProcessingException: Yaml could not be processed."; From f7cc582a4ff6713e949e9bc4afea2b51083260bd Mon Sep 17 00:00:00 2001 From: EddeCCC Date: Wed, 7 Feb 2024 13:40:36 +0100 Subject: [PATCH 09/17] refactor supplier --- .../AgentConfiguration.java | 67 +++++++-------- .../AgentConfigurationReloadTask.java | 3 - .../AgentDocumentation.java | 13 +++ .../AgentDocumentationSupplier.java | 30 +++++++ .../agentconfiguration/DocsObjectsLoader.java | 3 +- .../ConfigurationController.java | 2 +- .../AgentConfigurationReloadTaskTest.java | 82 ------------------- .../AgentConfigurationTest.java | 65 +++++---------- .../agentstatus/AgentStatusManagerTest.java | 11 +-- .../rest/agent/AgentControllerTest.java | 5 +- .../ocelot/rest/agent/AgentServiceTest.java | 3 +- .../ConfigurationControllerTest.java | 33 +++++--- 12 files changed, 127 insertions(+), 190 deletions(-) create mode 100644 components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentation.java create mode 100644 components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentationSupplier.java diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java index 4b1eb3458f..aee5115c8f 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java @@ -12,7 +12,6 @@ import java.nio.charset.Charset; import java.util.*; import java.util.function.Predicate; -import java.util.function.Supplier; import java.util.stream.Collectors; /** @@ -26,7 +25,7 @@ public class AgentConfiguration { /** * Used as maker in {@link #attributesToConfigurationCache} to mark attribute-maps for which no mapping matches. */ - public static final AgentConfiguration NO_MATCHING_MAPPING = createDefaultConfiguration(); + public static AgentConfiguration NO_MATCHING_MAPPING = createDefault(); /** * Predicate to check if a given file path ends with .yml or .yaml @@ -40,17 +39,10 @@ public class AgentConfiguration { private AgentMapping mapping; /** - * The set of defined documentable objects in this configuration for each file.
- * The map might be initialized after constructing the AgentConfiguration.
- * - Key: the file path
- * - Value: the set of objects, like actions, scopes, rules & metrics + * The list of suppliers for documentable objects.
+ * There is a setset of defined documentable objects in this configuration for each file. */ - private Map> docsObjectsByFile; - - /** - * The set of suppliers for the defined documentable objects in this configuration for each file - */ - private static final Map>> docsObjectsByFileSuppliers = new HashMap<>(); + private static Set docsSuppliers = new HashSet<>(); /** * The merged YAML configuration for the given mapping. @@ -62,9 +54,15 @@ public class AgentConfiguration { */ private String hash; - private AgentConfiguration(AgentMapping mapping, Map> docsObjectsByFile, String configYaml, String hash) { + private AgentConfiguration(AgentMapping mapping, String configYaml, String hash) { this.mapping = mapping; - this.docsObjectsByFile = docsObjectsByFile; + this.configYaml = configYaml; + this.hash = hash; + } + + private AgentConfiguration(AgentMapping mapping, Set agentDocsSuppliers, String configYaml, String hash) { + this.mapping = mapping; + docsSuppliers = agentDocsSuppliers; this.configYaml = configYaml; this.hash = hash; } @@ -79,7 +77,7 @@ private AgentConfiguration(AgentMapping mapping, Map> docsOb public static AgentConfiguration create(AgentMapping mapping, AbstractFileAccessor fileAccessor) { String configYaml = loadConfigForMapping(mapping, fileAccessor); String hash = DigestUtils.md5DigestAsHex(configYaml.getBytes(Charset.defaultCharset())); - return new AgentConfiguration(mapping, new HashMap<>(), configYaml, hash); + return new AgentConfiguration(mapping, configYaml, hash); } /** @@ -87,22 +85,13 @@ public static AgentConfiguration create(AgentMapping mapping, AbstractFileAccess * Also creates a cryptographic hash. * * @param mapping The agent mapping for which this instance represents the loaded configuration + * @param docsSuppliers The set of * @param configYaml The yaml string, which contains the configuration - * @param docsObjectsByFile The set of defined documentable objects in this configuration for each file * @return Created AgentConfiguration */ - public static AgentConfiguration create(AgentMapping mapping, Map> docsObjectsByFile, String configYaml) { + public static AgentConfiguration create(AgentMapping mapping, Set docsSuppliers, String configYaml) { String hash = DigestUtils.md5DigestAsHex(configYaml.getBytes(Charset.defaultCharset())); - return new AgentConfiguration(mapping, docsObjectsByFile, configYaml, hash); - } - - /** - * Apply suppliers to get the documentable objects by file and store them - */ - public void supplyDocsObjectsByFile() { - for (Map.Entry>> entry : docsObjectsByFileSuppliers.entrySet()) { - this.docsObjectsByFile.put(entry.getKey(), entry.getValue().get()); - } + return new AgentConfiguration(mapping, docsSuppliers, configYaml, hash); } /** @@ -110,10 +99,22 @@ public void supplyDocsObjectsByFile() { * * @return Created default AgentConfiguration */ - private static AgentConfiguration createDefaultConfiguration() { + public static AgentConfiguration createDefault() { String configYaml = ""; String hash = DigestUtils.md5DigestAsHex(configYaml.getBytes(Charset.defaultCharset())); - return new AgentConfiguration(null, new HashMap<>(), configYaml, hash); + return new AgentConfiguration(null, new HashSet<>(), configYaml, hash); + } + + /** + * Convert the set of agent documentation suppliers to a map + * @return the sets of documentable objects for each file + */ + public Map> getDocsObjectsAsMap() { + return docsSuppliers.stream() + .collect(Collectors.toMap( + docsSupplier -> docsSupplier.get().filePath(), + docsSupplier -> docsSupplier.get().objects() + )); } /** @@ -133,8 +134,8 @@ static String loadConfigForMapping(AgentMapping mapping, AbstractFileAccessor fi String src = fileAccessor.readConfigurationFile(path).orElse(""); result = ObjectStructureMerger.loadAndMergeYaml(src, result, path); - Supplier> docsObjectsSupplier = () -> loadDocsObjects(src, path); - docsObjectsByFileSuppliers.put(path, docsObjectsSupplier); + AgentDocumentationSupplier docsSupplier = new AgentDocumentationSupplier(() -> loadDocsObjects(src, path)); + docsSuppliers.add(docsSupplier); } return result == null ? "" : new Yaml().dump(result); } @@ -198,13 +199,13 @@ private static List getAllYamlFiles(AbstractFileAccessor fileAccessor, S * * @return the set of documentable objects */ - private static Set loadDocsObjects(String src, String filePath) { + private static AgentDocumentation loadDocsObjects(String src, String filePath) { Set objects = Collections.emptySet(); try { objects = DocsObjectsLoader.loadObjects(src); } catch (Exception e) { log.warn("Could not parse configuration: {}", filePath, e); } - return objects; + return new AgentDocumentation(filePath, objects); } } diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java index 79210b324f..82a2a75238 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java @@ -68,9 +68,6 @@ public void run() { } onTaskSuccess(newConfigurations); - - newConfigurations.forEach(AgentConfiguration::supplyDocsObjectsByFile); - log.info("Successfully supplied documented Objects for agent configurations"); } /** diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentation.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentation.java new file mode 100644 index 0000000000..25f2237ea1 --- /dev/null +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentation.java @@ -0,0 +1,13 @@ +package rocks.inspectit.ocelot.agentconfiguration; + +import java.util.Set; + +/** + * Model, to store documentable objects of a specific yaml file. + * Documentable objects are actions, scopes, rules & metrics. + * + * @param filePath file, which contains the documentable objects + * @param objects documentable objects of the file + */ +public record AgentDocumentation(String filePath, Set objects) { +} diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentationSupplier.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentationSupplier.java new file mode 100644 index 0000000000..2de261862d --- /dev/null +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentationSupplier.java @@ -0,0 +1,30 @@ +package rocks.inspectit.ocelot.agentconfiguration; + +import java.util.function.Supplier; + +/** + * Supplies the logic to provide an agent documentation. + * The agent documentation will be created lazy. + */ +public class AgentDocumentationSupplier implements Supplier { + + /** + * The supplier containing the logic to provide an agent documentation + */ + private final Supplier supplier; + + /** + * The objects, which should be supplied + */ + private AgentDocumentation documentation; + + public AgentDocumentationSupplier(Supplier supplier) { + this.supplier = supplier; + } + + @Override + public AgentDocumentation get() { + if(documentation == null) documentation = supplier.get(); + return documentation; + } +} diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoader.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoader.java index 0c851f1200..0a14eb261b 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoader.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoader.java @@ -2,7 +2,6 @@ import inspectit.ocelot.configdocsgenerator.parsing.ConfigParser; import lombok.extern.slf4j.Slf4j; -import org.springframework.stereotype.Component; import org.yaml.snakeyaml.Yaml; import rocks.inspectit.ocelot.config.model.InspectitConfig; import rocks.inspectit.ocelot.config.model.instrumentation.InstrumentationSettings; @@ -19,7 +18,7 @@ public class DocsObjectsLoader { /** * Use the same constant as the ui in 'src/data/constants.js' */ - final static String OCELOT_DEFAULT_CONFIG_PREFIX = "/$%$%$%$%Ocelot-default-key/"; + public final static String OCELOT_DEFAULT_CONFIG_PREFIX = "/$%$%$%$%Ocelot-default-key/"; /** * Loads all documentable objects, like actions, scopes, rules & metrics from the provided inspectIT yaml diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationController.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationController.java index 9d6483b631..19ca674927 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationController.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationController.java @@ -110,7 +110,7 @@ public ResponseEntity getConfigDocumentation( AgentConfiguration configuration = configManager.getConfigurationForMapping(agentMapping.get()); String configYaml = configuration.getConfigYaml(); - Map> docsObjectsByFile = configuration.getDocsObjectsByFile(); + Map> docsObjectsByFile = configuration.getDocsObjectsAsMap(); try { if (includeDefault) { diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java index e62ab4ec03..0424d02464 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java @@ -199,86 +199,4 @@ void loadMappingFromLive() { verify(serializer, times(1)).readAgentMappings(liveAccessor); } } - - @Nested - class docsObjectsTaskSupplier { - - private final String srcYaml = """ - inspectit: - instrumentation: - scopes: - s_jdbc_statement_execute: - docs: - description: 'Scope for executed JDBC statements.' - methods: - - name: execute - """; - - @Test - void verifyDocsObjectsHaveBeenSupplied() { - FileInfo fileInfo = mock(FileInfo.class); - String fileName = "/test.yml"; - when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of(fileName), Stream.of(fileName)); - when(workspaceAccessor.agentMappingsExist()).thenReturn(true); - when(workspaceAccessor.configurationFileExists(anyString())).thenReturn(true); - when(workspaceAccessor.configurationFileIsDirectory(anyString())).thenReturn(true); - when(workspaceAccessor.listConfigurationFiles(anyString())).thenReturn(Collections.singletonList(fileInfo)); - when(workspaceAccessor.readConfigurationFile(anyString())).thenReturn(Optional.of(srcYaml)); - - Set docsObjects = Set.of("s_jdbc_statement_execute"); - Map> docsObjectsByFile = new HashMap<>(); - docsObjectsByFile.put(fileName, docsObjects); - - AgentMapping mapping = AgentMapping.builder() - .name("test") - .source("/test") - .sourceBranch(WORKSPACE) - .build(); - doReturn(Collections.singletonList(mapping)).when(serializer).readAgentMappings(any()); - MutableObject> configurations = new MutableObject<>(); - Consumer> consumer = configurations::setValue; - - AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, consumer); - task.run(); - - List configurationList = configurations.getValue(); - assertThat(configurationList).hasSize(1); - assertThat(configurationList).element(0) - .extracting(AgentConfiguration::getDocsObjectsByFile) - .isEqualTo(docsObjectsByFile); - } - - @Test - void verifyNoDocsObjectsCouldBeSupplied() { - FileInfo fileInfo = mock(FileInfo.class); - String fileName = "/test.yml"; - when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of(fileName), Stream.of(fileName)); - when(workspaceAccessor.agentMappingsExist()).thenReturn(true); - when(workspaceAccessor.configurationFileExists(anyString())).thenReturn(true); - when(workspaceAccessor.configurationFileIsDirectory(anyString())).thenReturn(true); - when(workspaceAccessor.listConfigurationFiles(anyString())).thenReturn(Collections.singletonList(fileInfo)); - when(workspaceAccessor.readConfigurationFile(anyString())).thenReturn(Optional.empty()); - - Map> docsObjectsByFile = new HashMap<>(); - docsObjectsByFile.put(fileName, new HashSet<>()); - - AgentMapping mapping = AgentMapping.builder() - .name("test") - .source("/test") - .sourceBranch(WORKSPACE) - .build(); - doReturn(Collections.singletonList(mapping)).when(serializer).readAgentMappings(any()); - MutableObject> configurations = new MutableObject<>(); - Consumer> consumer = configurations::setValue; - - AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, consumer); - task.run(); - - List configurationList = configurations.getValue(); - assertThat(configurationList).hasSize(1); - assertThat(configurationList).element(0) - .extracting(AgentConfiguration::getDocsObjectsByFile) - .isEqualTo(docsObjectsByFile); - } - } } diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java index 975cd9236a..1602e63f36 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java @@ -1,6 +1,6 @@ package rocks.inspectit.ocelot.agentconfiguration; -import org.apache.commons.lang3.mutable.MutableObject; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -8,11 +8,9 @@ import org.mockito.junit.jupiter.MockitoExtension; import rocks.inspectit.ocelot.file.FileInfo; import rocks.inspectit.ocelot.file.accessor.git.RevisionAccess; -import rocks.inspectit.ocelot.file.versioning.Branch; import rocks.inspectit.ocelot.mappings.model.AgentMapping; import java.util.*; -import java.util.function.Consumer; import java.util.stream.Stream; import static org.assertj.core.api.Assertions.*; @@ -136,7 +134,6 @@ void leadingSlashesInSourcesRemoved() { @Test void priorityRespected() { - when(revisionAccess.configurationFileExists(any())).thenReturn(true); doReturn(true).when(revisionAccess).configurationFileIsDirectory("folder"); @@ -170,56 +167,32 @@ void priorityRespected() { } @Nested - class SupplyDocsObjects { - - private final String srcYaml = """ - inspectit: - instrumentation: - scopes: - s_jdbc_statement_execute: - docs: - description: 'Scope for executed JDBC statements.' - methods: - - name: execute - """; + class getDocsObjects { @Test - void verifySuccessfulSupply() { - FileInfo fileInfo = mock(FileInfo.class); - String fileName = "/test.yml"; - when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of(fileName), Stream.of(fileName)); - when(revisionAccess.configurationFileExists(anyString())).thenReturn(true); - when(revisionAccess.configurationFileIsDirectory(anyString())).thenReturn(true); - when(revisionAccess.listConfigurationFiles(anyString())).thenReturn(Collections.singletonList(fileInfo)); - when(revisionAccess.readConfigurationFile(anyString())).thenReturn(Optional.of(srcYaml)); + void verifyEmptyGetDocsObjectsAsMap() { + AgentConfiguration config = AgentConfiguration.NO_MATCHING_MAPPING; + Map> map = config.getDocsObjectsAsMap(); - Set docsObjects = Set.of("s_jdbc_statement_execute"); - Map> docsObjectsByFile = new HashMap<>(); - docsObjectsByFile.put(fileName, docsObjects); + assertThat(map).isEmpty(); + } - AgentMapping mapping = AgentMapping.builder() - .name("test") - .source("/test") - .sourceBranch(WORKSPACE) - .build(); + @Test + void verifyGetDocsObjectsAsMap() { + Map> docsObjectsByFile = new HashMap<>(); + Set docsSuppliers = new HashSet<>(); - AgentConfiguration config = AgentConfiguration.create(mapping, revisionAccess); - config.supplyDocsObjectsByFile(); + String filePath = "test.yml"; + Set objects = Collections.singleton("yaml"); + docsObjectsByFile.put(filePath, objects); - assertThat(config.getDocsObjectsByFile()).isEqualTo(docsObjectsByFile); - } + AgentDocumentationSupplier supplier = new AgentDocumentationSupplier(() -> new AgentDocumentation(filePath, objects)); + docsSuppliers.add(supplier); - @Test - void verifyNoSupply() { - AgentMapping mapping = AgentMapping.builder() - .name("test") - .source("/test") - .sourceBranch(WORKSPACE) - .build(); - AgentConfiguration config = AgentConfiguration.create(mapping, new HashMap<>(), srcYaml); - config.supplyDocsObjectsByFile(); + AgentConfiguration config = AgentConfiguration.create(null, docsSuppliers, ""); + Map> map = config.getDocsObjectsAsMap(); - assertThat(config.getDocsObjectsByFile()).isEmpty(); + assertThat(map).isEqualTo(docsObjectsByFile); } } } diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentstatus/AgentStatusManagerTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentstatus/AgentStatusManagerTest.java index 74b3a52a85..4a08030bcf 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentstatus/AgentStatusManagerTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentstatus/AgentStatusManagerTest.java @@ -13,10 +13,7 @@ import rocks.inspectit.ocelot.mappings.model.AgentMapping; import java.time.Duration; -import java.util.Collections; -import java.util.Date; -import java.util.HashMap; -import java.util.Map; +import java.util.*; import static org.assertj.core.api.Assertions.assertThat; @@ -44,7 +41,7 @@ class NotifyAgentConfigurationFetched { void testWithAgentIdHeader() { Branch testBranch = Branch.WORKSPACE; AgentMapping agentMapping = AgentMapping.builder().name("test-conf").sourceBranch(testBranch).build(); - AgentConfiguration config = AgentConfiguration.create(agentMapping, new HashMap<>(), ""); + AgentConfiguration config = AgentConfiguration.create(agentMapping, new HashSet<>(), ""); Map attributes = ImmutableMap.of("service", "test"); manager.notifyAgentConfigurationFetched(attributes, Collections.singletonMap(HEADER_AGENT_ID, "aid"), config); @@ -76,7 +73,7 @@ void testNoMappingFound() { void testMappingFound() { Branch testBranch = Branch.WORKSPACE; AgentMapping agentMapping = AgentMapping.builder().name("test-conf").sourceBranch(testBranch).build(); - AgentConfiguration config = AgentConfiguration.create(agentMapping, new HashMap<>(), "");; + AgentConfiguration config = AgentConfiguration.create(agentMapping, new HashSet<>(),"");; Map attributes = ImmutableMap.of("service", "test"); manager.notifyAgentConfigurationFetched(attributes, Collections.emptyMap(), config); @@ -93,7 +90,7 @@ void testMappingFound() { void testOverriding() throws Exception { Branch testBranch = Branch.WORKSPACE; AgentMapping agentMapping = AgentMapping.builder().name("test-conf").sourceBranch(testBranch).build(); - AgentConfiguration config = AgentConfiguration.create(agentMapping, new HashMap<>(), ""); + AgentConfiguration config = AgentConfiguration.create(agentMapping, new HashSet<>(),""); Map attributes = ImmutableMap.of("service", "test"); manager.notifyAgentConfigurationFetched(attributes, Collections.emptyMap(), null); diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/agent/AgentControllerTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/agent/AgentControllerTest.java index 34909a3114..d6569eeb52 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/agent/AgentControllerTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/agent/AgentControllerTest.java @@ -19,6 +19,7 @@ import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; @@ -58,7 +59,7 @@ public void noMappingFound() { @Test public void mappingFound() { - AgentConfiguration config = AgentConfiguration.create(null, new HashMap<>(), "foo : bar"); + AgentConfiguration config = AgentConfiguration.create(null, new HashSet<>(), "foo : bar"); doReturn(config).when(configManager).getConfiguration(anyMap()); HashMap attributes = new HashMap<>(); @@ -71,7 +72,7 @@ public void mappingFound() { @Test public void etagPresent() { - AgentConfiguration config = AgentConfiguration.create(null, new HashMap<>(), "foo : bar"); + AgentConfiguration config = AgentConfiguration.create(null, new HashSet<>(), "foo : bar"); doReturn(config).when(configManager).getConfiguration(anyMap()); ResponseEntity firstResult = controller.fetchConfiguration(new HashMap<>(), Collections.emptyMap()); diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/agent/AgentServiceTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/agent/AgentServiceTest.java index 3976a45584..5fe774ed03 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/agent/AgentServiceTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/agent/AgentServiceTest.java @@ -18,6 +18,7 @@ import java.lang.management.ManagementFactory; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; import java.util.concurrent.ExecutionException; @@ -60,7 +61,7 @@ public void setupTestData(boolean logPreloadingEnabled) { String configYaml = String.format("inspectit.log-preloading: {enabled: %b}", logPreloadingEnabled); //set config - config = AgentConfiguration.create(null, new HashMap<>(), configYaml); + config = AgentConfiguration.create(null, new HashSet<>(), configYaml); //set attributes attributes = new HashMap() {{ diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java index 499b17d11f..245c6beee5 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java @@ -16,10 +16,7 @@ import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.yaml.snakeyaml.Yaml; -import rocks.inspectit.ocelot.agentconfiguration.AgentConfiguration; -import rocks.inspectit.ocelot.agentconfiguration.AgentConfigurationManager; -import rocks.inspectit.ocelot.agentconfiguration.DocsObjectsLoader; -import rocks.inspectit.ocelot.agentconfiguration.ObjectStructureMerger; +import rocks.inspectit.ocelot.agentconfiguration.*; import rocks.inspectit.ocelot.file.FileManager; import rocks.inspectit.ocelot.mappings.AgentMappingManager; import rocks.inspectit.ocelot.mappings.model.AgentMapping; @@ -32,6 +29,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.*; +import static rocks.inspectit.ocelot.agentconfiguration.DocsObjectsLoader.OCELOT_DEFAULT_CONFIG_PREFIX; @ExtendWith(MockitoExtension.class) public class ConfigurationControllerTest { @@ -62,7 +60,7 @@ public class FetchConfiguration { @Test public void returningConfiguration() { - AgentConfiguration configuration = AgentConfiguration.create(null, new HashMap<>(), "yaml"); + AgentConfiguration configuration = AgentConfiguration.create(null, new HashSet<>(), "yaml"); when(agentConfigurationManager.getConfiguration(any())).thenReturn(configuration); ResponseEntity output = configurationController.fetchConfiguration(null); @@ -95,12 +93,20 @@ public void initiatesCommit() throws GitAPIException { @Nested public class GetConfigDocumentationTest { - private final static Map> docsObjectsByFile = new HashMap<>(); + private Map> docsObjectsByFile; - @BeforeAll - static void setUp() { + private Set docsSuppliers; + + @BeforeEach + void setUp() { + String filePath = "test.yml"; Set objects = Collections.singleton("yaml"); - docsObjectsByFile.put("test.yml", objects); + docsObjectsByFile = new HashMap<>(); + docsObjectsByFile.put(filePath, objects); + + AgentDocumentationSupplier supplier = new AgentDocumentationSupplier(() -> new AgentDocumentation(filePath, objects)); + docsSuppliers = new HashSet<>(); + docsSuppliers.add(supplier); } @Test @@ -110,11 +116,12 @@ void withDefaultConfig() throws IOException { AgentMapping agentMapping = AgentMapping.builder().build(); final String configYaml = "yaml"; - AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, docsObjectsByFile, configYaml); + AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, docsSuppliers, configYaml); Map defaultYamls = new HashMap<>(); final String defaultYamlContent = "defaultYaml"; defaultYamls.put("firstYaml", defaultYamlContent); + docsObjectsByFile.put(OCELOT_DEFAULT_CONFIG_PREFIX + "firstYaml", Collections.emptySet()); Map configYamlMap = new HashMap<>(); configYamlMap.put("entry", "value"); @@ -161,7 +168,7 @@ void errorWhenGettingDefaultConfig() throws IOException { AgentMapping agentMapping = AgentMapping.builder().build(); final String configYaml = "yaml"; - AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, docsObjectsByFile, configYaml); + AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, docsSuppliers, configYaml); IOException exception = new IOException(); @@ -194,7 +201,7 @@ void withoutDefaultConfig() throws IOException { AgentMapping agentMapping = AgentMapping.builder().build(); final String configYaml = "yaml"; - AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, docsObjectsByFile, configYaml); + AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, docsSuppliers, configYaml); ConfigDocumentation configDocumentationMock = mock(ConfigDocumentation.class); @@ -243,7 +250,7 @@ void invalidYaml() throws IOException { AgentMapping agentMapping = AgentMapping.builder().build(); final String configYaml = "yaml"; - AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, docsObjectsByFile, configYaml); + AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, docsSuppliers, configYaml); JsonProcessingException exception = mock(JsonProcessingException.class); final String errorMessage = "JsonProcessingException: Yaml could not be processed."; From 0225569bf0d2b2277921dae177e89bfd15ef8915 Mon Sep 17 00:00:00 2001 From: EddeCCC Date: Fri, 9 Feb 2024 08:51:50 +0100 Subject: [PATCH 10/17] refactor AgentConfiguration --- .../ConfigDocsGenerator.java | 2 +- .../AgentConfiguration.java | 57 ++++++++----- .../AgentConfigurationReloadTask.java | 4 + .../AgentDocumentation.java | 2 +- .../AgentDocumentationSupplier.java | 30 ------- .../ConfigurationController.java | 2 +- .../AgentConfigurationReloadTaskTest.java | 85 +++++++++++++++++++ .../AgentConfigurationTest.java | 17 ++-- .../ConfigurationControllerTest.java | 16 ++-- 9 files changed, 146 insertions(+), 69 deletions(-) delete mode 100644 components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentationSupplier.java diff --git a/components/inspectit-ocelot-configdocsgenerator/src/main/java/inspectit/ocelot/configdocsgenerator/ConfigDocsGenerator.java b/components/inspectit-ocelot-configdocsgenerator/src/main/java/inspectit/ocelot/configdocsgenerator/ConfigDocsGenerator.java index ae2d8e95f1..44e35bb96d 100644 --- a/components/inspectit-ocelot-configdocsgenerator/src/main/java/inspectit/ocelot/configdocsgenerator/ConfigDocsGenerator.java +++ b/components/inspectit-ocelot-configdocsgenerator/src/main/java/inspectit/ocelot/configdocsgenerator/ConfigDocsGenerator.java @@ -237,7 +237,7 @@ private Set findFiles(String name) { .filter(entry -> entry.getValue().contains(name)) .map(Map.Entry::getKey) .collect(Collectors.toSet()); - if(files.isEmpty()) log.warn("No file found with definition of " + name); + if(files.isEmpty()) log.debug("No file found with definition of " + name); return files; } diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java index aee5115c8f..ba3b890feb 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java @@ -12,6 +12,7 @@ import java.nio.charset.Charset; import java.util.*; import java.util.function.Predicate; +import java.util.function.Supplier; import java.util.stream.Collectors; /** @@ -39,10 +40,15 @@ public class AgentConfiguration { private AgentMapping mapping; /** - * The list of suppliers for documentable objects.
- * There is a setset of defined documentable objects in this configuration for each file. + * The set of suppliers for documentable objects.
*/ - private static Set docsSuppliers = new HashSet<>(); + private static final Set> documentationSuppliers = Collections.synchronizedSet(new HashSet<>()); + + /** + * The set of documentable objects.
+ * There is a set of defined documentable objects in this configuration for each file. + */ + private Set documentations; /** * The merged YAML configuration for the given mapping. @@ -56,13 +62,14 @@ public class AgentConfiguration { private AgentConfiguration(AgentMapping mapping, String configYaml, String hash) { this.mapping = mapping; + this.documentations = new HashSet<>(); this.configYaml = configYaml; this.hash = hash; } - private AgentConfiguration(AgentMapping mapping, Set agentDocsSuppliers, String configYaml, String hash) { + private AgentConfiguration(AgentMapping mapping, Set documentations, String configYaml, String hash) { this.mapping = mapping; - docsSuppliers = agentDocsSuppliers; + this.documentations = documentations; this.configYaml = configYaml; this.hash = hash; } @@ -81,21 +88,21 @@ public static AgentConfiguration create(AgentMapping mapping, AbstractFileAccess } /** - * Factory method to create AgentConfigurations. + * Factory method to create an AgentConfiguration. * Also creates a cryptographic hash. * * @param mapping The agent mapping for which this instance represents the loaded configuration - * @param docsSuppliers The set of + * @param docs The set of agent documentation objects * @param configYaml The yaml string, which contains the configuration * @return Created AgentConfiguration */ - public static AgentConfiguration create(AgentMapping mapping, Set docsSuppliers, String configYaml) { + public static AgentConfiguration create(AgentMapping mapping, Set docs, String configYaml) { String hash = DigestUtils.md5DigestAsHex(configYaml.getBytes(Charset.defaultCharset())); - return new AgentConfiguration(mapping, docsSuppliers, configYaml, hash); + return new AgentConfiguration(mapping, docs, configYaml, hash); } /** - * Factory method to create default AgentConfiguration. + * Factory method to create a default AgentConfiguration. * * @return Created default AgentConfiguration */ @@ -106,14 +113,26 @@ public static AgentConfiguration createDefault() { } /** - * Convert the set of agent documentation suppliers to a map - * @return the sets of documentable objects for each file + * Use the suppliers to create the agent documentations for this configuration. + * The suppliers are cleared after they all have been used. + */ + public synchronized void supplyDocumentations() { + documentationSuppliers.forEach(supplier -> documentations.add(supplier.get())); + documentationSuppliers.clear(); + } + + /** + * Get the current agent documentations as a map.
+ * - Key: the file path
+ * - Value: the set of objects, like actions, scopes, rules & metrics + * + * @return The agent documentations as a map */ - public Map> getDocsObjectsAsMap() { - return docsSuppliers.stream() + public Map> getDocumentationsAsMap() { + return documentations.stream() .collect(Collectors.toMap( - docsSupplier -> docsSupplier.get().filePath(), - docsSupplier -> docsSupplier.get().objects() + AgentDocumentation::filePath, + AgentDocumentation::objects )); } @@ -134,8 +153,8 @@ static String loadConfigForMapping(AgentMapping mapping, AbstractFileAccessor fi String src = fileAccessor.readConfigurationFile(path).orElse(""); result = ObjectStructureMerger.loadAndMergeYaml(src, result, path); - AgentDocumentationSupplier docsSupplier = new AgentDocumentationSupplier(() -> loadDocsObjects(src, path)); - docsSuppliers.add(docsSupplier); + Supplier documentationSupplier = () -> loadDocsObjects(path, src); + documentationSuppliers.add(documentationSupplier); } return result == null ? "" : new Yaml().dump(result); } @@ -199,7 +218,7 @@ private static List getAllYamlFiles(AbstractFileAccessor fileAccessor, S * * @return the set of documentable objects */ - private static AgentDocumentation loadDocsObjects(String src, String filePath) { + private static AgentDocumentation loadDocsObjects(String filePath, String src) { Set objects = Collections.emptySet(); try { objects = DocsObjectsLoader.loadObjects(src); diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java index 82a2a75238..51a30f0cd2 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java @@ -68,6 +68,10 @@ public void run() { } onTaskSuccess(newConfigurations); + + // supply agent documentations after executing the callback to provide the agent configurations faster + newConfigurations.stream().takeWhile(config -> !isCanceled()).forEach(AgentConfiguration::supplyDocumentations); + log.info("Successfully supplied documented objects for agent configurations"); } /** diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentation.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentation.java index 25f2237ea1..49cee90fdb 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentation.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentation.java @@ -4,7 +4,7 @@ /** * Model, to store documentable objects of a specific yaml file. - * Documentable objects are actions, scopes, rules & metrics. + * Documentable objects can be actions, scopes, rules & metrics. * * @param filePath file, which contains the documentable objects * @param objects documentable objects of the file diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentationSupplier.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentationSupplier.java deleted file mode 100644 index 2de261862d..0000000000 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentationSupplier.java +++ /dev/null @@ -1,30 +0,0 @@ -package rocks.inspectit.ocelot.agentconfiguration; - -import java.util.function.Supplier; - -/** - * Supplies the logic to provide an agent documentation. - * The agent documentation will be created lazy. - */ -public class AgentDocumentationSupplier implements Supplier { - - /** - * The supplier containing the logic to provide an agent documentation - */ - private final Supplier supplier; - - /** - * The objects, which should be supplied - */ - private AgentDocumentation documentation; - - public AgentDocumentationSupplier(Supplier supplier) { - this.supplier = supplier; - } - - @Override - public AgentDocumentation get() { - if(documentation == null) documentation = supplier.get(); - return documentation; - } -} diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationController.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationController.java index 19ca674927..c05b2bd950 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationController.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationController.java @@ -110,7 +110,7 @@ public ResponseEntity getConfigDocumentation( AgentConfiguration configuration = configManager.getConfigurationForMapping(agentMapping.get()); String configYaml = configuration.getConfigYaml(); - Map> docsObjectsByFile = configuration.getDocsObjectsAsMap(); + Map> docsObjectsByFile = configuration.getDocumentationsAsMap(); try { if (includeDefault) { diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java index 0424d02464..943119c690 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java @@ -17,6 +17,7 @@ import java.util.function.Consumer; import java.util.stream.Stream; +import static java.util.Collections.emptySet; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; @@ -199,4 +200,88 @@ void loadMappingFromLive() { verify(serializer, times(1)).readAgentMappings(liveAccessor); } } + + @Nested + class supplyAgentDocumentation { + + private static final String srcYaml = """ + inspectit: + instrumentation: + scopes: + s_jdbc_statement_execute: + docs: + description: 'Scope for executed JDBC statements.' + methods: + - name: execute + """; + + private static final String fileName = "/test.yml"; + + @Test + void verifySuccessfulSupply() { + FileInfo fileInfo = mock(FileInfo.class); + when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of(fileName), Stream.of(fileName)); + when(workspaceAccessor.agentMappingsExist()).thenReturn(true); + when(workspaceAccessor.configurationFileExists(anyString())).thenReturn(true); + when(workspaceAccessor.configurationFileIsDirectory(anyString())).thenReturn(true); + when(workspaceAccessor.listConfigurationFiles(anyString())).thenReturn(Collections.singletonList(fileInfo)); + when(workspaceAccessor.readConfigurationFile(anyString())).thenReturn(Optional.of(srcYaml)); + + AgentMapping mapping = AgentMapping.builder() + .name("test") + .source("/test") + .sourceBranch(WORKSPACE) + .build(); + doReturn(Collections.singletonList(mapping)).when(serializer).readAgentMappings(any()); + + MutableObject> configurations = new MutableObject<>(); + Consumer> consumer = configurations::setValue; + + AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, consumer); + AgentDocumentation documentation = new AgentDocumentation(fileName, Set.of("s_jdbc_statement_execute")); + + task.run(); + + List configurationList = configurations.getValue(); + assertThat(configurationList).hasSize(1); + + AgentConfiguration config = configurationList.stream().findFirst().get(); + assertThat(config.getDocumentations()).hasSize(1); + assertThat(config.getDocumentationsAsMap()).hasSize(1); + assertThat(config.getDocumentations()).contains(documentation); + } + + @Test + void verifyEmptySupply() { + FileInfo fileInfo = mock(FileInfo.class); + when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of(fileName), Stream.of(fileName)); + when(workspaceAccessor.agentMappingsExist()).thenReturn(true); + when(workspaceAccessor.configurationFileExists(anyString())).thenReturn(true); + when(workspaceAccessor.configurationFileIsDirectory(anyString())).thenReturn(true); + when(workspaceAccessor.listConfigurationFiles(anyString())).thenReturn(Collections.singletonList(fileInfo)); + when(workspaceAccessor.readConfigurationFile(anyString())).thenReturn(Optional.of("key: valid")); + + AgentMapping mapping = AgentMapping.builder() + .name("test") + .source("/test") + .sourceBranch(WORKSPACE) + .build(); + doReturn(Collections.singletonList(mapping)).when(serializer).readAgentMappings(any()); + + MutableObject> configurations = new MutableObject<>(); + Consumer> consumer = configurations::setValue; + + AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, consumer); + + task.run(); + + List configurationList = configurations.getValue(); + assertThat(configurationList).hasSize(1); + + AgentConfiguration config = configurationList.stream().findFirst().get(); + assertThat(config.getDocumentations()).hasSize(1); + assertThat(config.getDocumentationsAsMap()).hasSize(1); + assertThat(config.getDocumentations()).contains(new AgentDocumentation(fileName, emptySet())); + } + } } diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java index 1602e63f36..f0851b71a3 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java @@ -1,6 +1,5 @@ package rocks.inspectit.ocelot.agentconfiguration; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -167,30 +166,30 @@ void priorityRespected() { } @Nested - class getDocsObjects { + class getDocumentations { @Test void verifyEmptyGetDocsObjectsAsMap() { AgentConfiguration config = AgentConfiguration.NO_MATCHING_MAPPING; - Map> map = config.getDocsObjectsAsMap(); + Map> map = config.getDocumentationsAsMap(); assertThat(map).isEmpty(); } @Test void verifyGetDocsObjectsAsMap() { + String filePath = "test.yml"; Map> docsObjectsByFile = new HashMap<>(); - Set docsSuppliers = new HashSet<>(); + Set documentations = new HashSet<>(); - String filePath = "test.yml"; Set objects = Collections.singleton("yaml"); docsObjectsByFile.put(filePath, objects); - AgentDocumentationSupplier supplier = new AgentDocumentationSupplier(() -> new AgentDocumentation(filePath, objects)); - docsSuppliers.add(supplier); + AgentDocumentation documentation = new AgentDocumentation(filePath, objects); + documentations.add(documentation); - AgentConfiguration config = AgentConfiguration.create(null, docsSuppliers, ""); - Map> map = config.getDocsObjectsAsMap(); + AgentConfiguration config = AgentConfiguration.create(null, documentations, ""); + Map> map = config.getDocumentationsAsMap(); assertThat(map).isEqualTo(docsObjectsByFile); } diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java index 245c6beee5..d07deafeef 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java @@ -95,7 +95,7 @@ public class GetConfigDocumentationTest { private Map> docsObjectsByFile; - private Set docsSuppliers; + private Set documentations; @BeforeEach void setUp() { @@ -104,9 +104,9 @@ void setUp() { docsObjectsByFile = new HashMap<>(); docsObjectsByFile.put(filePath, objects); - AgentDocumentationSupplier supplier = new AgentDocumentationSupplier(() -> new AgentDocumentation(filePath, objects)); - docsSuppliers = new HashSet<>(); - docsSuppliers.add(supplier); + AgentDocumentation documentation = new AgentDocumentation(filePath, objects); + documentations = new HashSet<>(); + documentations.add(documentation); } @Test @@ -116,7 +116,7 @@ void withDefaultConfig() throws IOException { AgentMapping agentMapping = AgentMapping.builder().build(); final String configYaml = "yaml"; - AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, docsSuppliers, configYaml); + AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, documentations, configYaml); Map defaultYamls = new HashMap<>(); final String defaultYamlContent = "defaultYaml"; @@ -168,7 +168,7 @@ void errorWhenGettingDefaultConfig() throws IOException { AgentMapping agentMapping = AgentMapping.builder().build(); final String configYaml = "yaml"; - AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, docsSuppliers, configYaml); + AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, documentations, configYaml); IOException exception = new IOException(); @@ -201,7 +201,7 @@ void withoutDefaultConfig() throws IOException { AgentMapping agentMapping = AgentMapping.builder().build(); final String configYaml = "yaml"; - AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, docsSuppliers, configYaml); + AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, documentations, configYaml); ConfigDocumentation configDocumentationMock = mock(ConfigDocumentation.class); @@ -250,7 +250,7 @@ void invalidYaml() throws IOException { AgentMapping agentMapping = AgentMapping.builder().build(); final String configYaml = "yaml"; - AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, docsSuppliers, configYaml); + AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, documentations, configYaml); JsonProcessingException exception = mock(JsonProcessingException.class); final String errorMessage = "JsonProcessingException: Yaml could not be processed."; From c94c172e5529369a4861e29dc6871993993e49aa Mon Sep 17 00:00:00 2001 From: EddeCCC Date: Fri, 9 Feb 2024 11:01:05 +0100 Subject: [PATCH 11/17] remove static supplier --- .../AgentConfiguration.java | 102 +++++++----------- .../AgentConfigurationReloadTask.java | 4 - .../AgentDocumentationSupplier.java | 24 +++++ .../AgentConfigurationReloadTaskTest.java | 84 --------------- .../AgentConfigurationTest.java | 34 +++--- .../ConfigurationControllerTest.java | 17 +-- 6 files changed, 89 insertions(+), 176 deletions(-) create mode 100644 components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentationSupplier.java diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java index ba3b890feb..29cf222fe3 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java @@ -1,6 +1,5 @@ package rocks.inspectit.ocelot.agentconfiguration; -import com.google.common.annotations.VisibleForTesting; import lombok.Value; import lombok.extern.slf4j.Slf4j; import org.springframework.util.DigestUtils; @@ -12,7 +11,6 @@ import java.nio.charset.Charset; import java.util.*; import java.util.function.Predicate; -import java.util.function.Supplier; import java.util.stream.Collectors; /** @@ -24,7 +22,8 @@ public class AgentConfiguration { /** - * Used as maker in {@link #attributesToConfigurationCache} to mark attribute-maps for which no mapping matches. + * Used as maker in {@link AgentConfigurationManager#attributesToConfigurationCache} to mark attribute-maps + * for which no mapping matches. */ public static AgentConfiguration NO_MATCHING_MAPPING = createDefault(); @@ -40,15 +39,11 @@ public class AgentConfiguration { private AgentMapping mapping; /** - * The set of suppliers for documentable objects.
+ * The set of suppliers for documentable objects. + * There is a set of defined documentable objects in this configuration for each file.
+ * The documentable objects will be loaded lazy. */ - private static final Set> documentationSuppliers = Collections.synchronizedSet(new HashSet<>()); - - /** - * The set of documentable objects.
- * There is a set of defined documentable objects in this configuration for each file. - */ - private Set documentations; + private Set documentationSuppliers; /** * The merged YAML configuration for the given mapping. @@ -60,45 +55,52 @@ public class AgentConfiguration { */ private String hash; - private AgentConfiguration(AgentMapping mapping, String configYaml, String hash) { + private AgentConfiguration(AgentMapping mapping, Set documentationSuppliers, String configYaml, String hash) { this.mapping = mapping; - this.documentations = new HashSet<>(); - this.configYaml = configYaml; - this.hash = hash; - } - - private AgentConfiguration(AgentMapping mapping, Set documentations, String configYaml, String hash) { - this.mapping = mapping; - this.documentations = documentations; + this.documentationSuppliers = documentationSuppliers; this.configYaml = configYaml; this.hash = hash; } /** - * Factory method to create AgentConfigurations. - * Also creates a cryptographic hash. + * Factory method to create AgentConfigurations. Also creates a cryptographic hash. * * @param mapping The agent mapping for which this instance represents the loaded configuration + * @param fileAccessor The accessor to use for reading the files + * * @return Created AgentConfiguration */ public static AgentConfiguration create(AgentMapping mapping, AbstractFileAccessor fileAccessor) { - String configYaml = loadConfigForMapping(mapping, fileAccessor); + LinkedHashSet allYamlFiles = getAllYamlFilesForMapping(fileAccessor, mapping); + + Set documentationSuppliers = new HashSet<>(); + Object yamlResult = null; + + for (String path : allYamlFiles) { + String src = fileAccessor.readConfigurationFile(path).orElse(""); + yamlResult = ObjectStructureMerger.loadAndMergeYaml(src, yamlResult, path); + + AgentDocumentationSupplier supplier = new AgentDocumentationSupplier(() -> loadDocsObjects(path, src)); + documentationSuppliers.add(supplier); + } + + String configYaml = yamlResult == null ? "" : new Yaml().dump(yamlResult); String hash = DigestUtils.md5DigestAsHex(configYaml.getBytes(Charset.defaultCharset())); - return new AgentConfiguration(mapping, configYaml, hash); + + return new AgentConfiguration(mapping, documentationSuppliers, configYaml, hash); } /** - * Factory method to create an AgentConfiguration. - * Also creates a cryptographic hash. + * Factory method to create an AgentConfiguration. Also creates a cryptographic hash. * * @param mapping The agent mapping for which this instance represents the loaded configuration - * @param docs The set of agent documentation objects + * @param documentationSuppliers The set of suppliers for agent documentations * @param configYaml The yaml string, which contains the configuration * @return Created AgentConfiguration */ - public static AgentConfiguration create(AgentMapping mapping, Set docs, String configYaml) { + public static AgentConfiguration create(AgentMapping mapping, Set documentationSuppliers, String configYaml) { String hash = DigestUtils.md5DigestAsHex(configYaml.getBytes(Charset.defaultCharset())); - return new AgentConfiguration(mapping, docs, configYaml, hash); + return new AgentConfiguration(mapping, documentationSuppliers, configYaml, hash); } /** @@ -106,59 +108,27 @@ public static AgentConfiguration create(AgentMapping mapping, Set(), configYaml, hash); } /** - * Use the suppliers to create the agent documentations for this configuration. - * The suppliers are cleared after they all have been used. - */ - public synchronized void supplyDocumentations() { - documentationSuppliers.forEach(supplier -> documentations.add(supplier.get())); - documentationSuppliers.clear(); - } - - /** - * Get the current agent documentations as a map.
+ * Get the current agent documentations as a map. The documentation will be loaded lazy.
* - Key: the file path
* - Value: the set of objects, like actions, scopes, rules & metrics * * @return The agent documentations as a map */ public Map> getDocumentationsAsMap() { - return documentations.stream() + return documentationSuppliers.stream() .collect(Collectors.toMap( - AgentDocumentation::filePath, - AgentDocumentation::objects + supplier -> supplier.get().filePath(), + supplier -> supplier.get().objects() )); } - /** - * Loads the given mapping as yaml string. - * - * @param mapping the mapping to load - * - * @return the merged yaml for the given mapping or an empty string if the mapping does not contain any existing files - * If this task has been canceled, null is returned. - */ - @VisibleForTesting - static String loadConfigForMapping(AgentMapping mapping, AbstractFileAccessor fileAccessor) { - LinkedHashSet allYamlFiles = getAllYamlFilesForMapping(fileAccessor, mapping); - - Object result = null; - for (String path : allYamlFiles) { - String src = fileAccessor.readConfigurationFile(path).orElse(""); - result = ObjectStructureMerger.loadAndMergeYaml(src, result, path); - - Supplier documentationSupplier = () -> loadDocsObjects(path, src); - documentationSuppliers.add(documentationSupplier); - } - return result == null ? "" : new Yaml().dump(result); - } - /** * Returns the set of yaml files, which is defined in the agent mapping sources. * diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java index 51a30f0cd2..82a2a75238 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java @@ -68,10 +68,6 @@ public void run() { } onTaskSuccess(newConfigurations); - - // supply agent documentations after executing the callback to provide the agent configurations faster - newConfigurations.stream().takeWhile(config -> !isCanceled()).forEach(AgentConfiguration::supplyDocumentations); - log.info("Successfully supplied documented objects for agent configurations"); } /** diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentationSupplier.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentationSupplier.java new file mode 100644 index 0000000000..eb572f99cd --- /dev/null +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentationSupplier.java @@ -0,0 +1,24 @@ +package rocks.inspectit.ocelot.agentconfiguration; + +import java.util.function.Supplier; + +/** + * Supplier to load an agent documentation lazy. + * The documentation will be persisted after initial loading. + */ +public class AgentDocumentationSupplier implements Supplier { + + private final Supplier supplier; + + private AgentDocumentation documentation; + + public AgentDocumentationSupplier(Supplier supplier) { + this.supplier = supplier; + } + + @Override + public AgentDocumentation get() { + if(documentation == null) documentation = supplier.get(); + return documentation; + } +} diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java index 943119c690..bdbc12562e 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java @@ -200,88 +200,4 @@ void loadMappingFromLive() { verify(serializer, times(1)).readAgentMappings(liveAccessor); } } - - @Nested - class supplyAgentDocumentation { - - private static final String srcYaml = """ - inspectit: - instrumentation: - scopes: - s_jdbc_statement_execute: - docs: - description: 'Scope for executed JDBC statements.' - methods: - - name: execute - """; - - private static final String fileName = "/test.yml"; - - @Test - void verifySuccessfulSupply() { - FileInfo fileInfo = mock(FileInfo.class); - when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of(fileName), Stream.of(fileName)); - when(workspaceAccessor.agentMappingsExist()).thenReturn(true); - when(workspaceAccessor.configurationFileExists(anyString())).thenReturn(true); - when(workspaceAccessor.configurationFileIsDirectory(anyString())).thenReturn(true); - when(workspaceAccessor.listConfigurationFiles(anyString())).thenReturn(Collections.singletonList(fileInfo)); - when(workspaceAccessor.readConfigurationFile(anyString())).thenReturn(Optional.of(srcYaml)); - - AgentMapping mapping = AgentMapping.builder() - .name("test") - .source("/test") - .sourceBranch(WORKSPACE) - .build(); - doReturn(Collections.singletonList(mapping)).when(serializer).readAgentMappings(any()); - - MutableObject> configurations = new MutableObject<>(); - Consumer> consumer = configurations::setValue; - - AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, consumer); - AgentDocumentation documentation = new AgentDocumentation(fileName, Set.of("s_jdbc_statement_execute")); - - task.run(); - - List configurationList = configurations.getValue(); - assertThat(configurationList).hasSize(1); - - AgentConfiguration config = configurationList.stream().findFirst().get(); - assertThat(config.getDocumentations()).hasSize(1); - assertThat(config.getDocumentationsAsMap()).hasSize(1); - assertThat(config.getDocumentations()).contains(documentation); - } - - @Test - void verifyEmptySupply() { - FileInfo fileInfo = mock(FileInfo.class); - when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of(fileName), Stream.of(fileName)); - when(workspaceAccessor.agentMappingsExist()).thenReturn(true); - when(workspaceAccessor.configurationFileExists(anyString())).thenReturn(true); - when(workspaceAccessor.configurationFileIsDirectory(anyString())).thenReturn(true); - when(workspaceAccessor.listConfigurationFiles(anyString())).thenReturn(Collections.singletonList(fileInfo)); - when(workspaceAccessor.readConfigurationFile(anyString())).thenReturn(Optional.of("key: valid")); - - AgentMapping mapping = AgentMapping.builder() - .name("test") - .source("/test") - .sourceBranch(WORKSPACE) - .build(); - doReturn(Collections.singletonList(mapping)).when(serializer).readAgentMappings(any()); - - MutableObject> configurations = new MutableObject<>(); - Consumer> consumer = configurations::setValue; - - AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, consumer); - - task.run(); - - List configurationList = configurations.getValue(); - assertThat(configurationList).hasSize(1); - - AgentConfiguration config = configurationList.stream().findFirst().get(); - assertThat(config.getDocumentations()).hasSize(1); - assertThat(config.getDocumentationsAsMap()).hasSize(1); - assertThat(config.getDocumentations()).contains(new AgentDocumentation(fileName, emptySet())); - } - } } diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java index f0851b71a3..f252f5eb46 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java @@ -10,6 +10,7 @@ import rocks.inspectit.ocelot.mappings.model.AgentMapping; import java.util.*; +import java.util.function.Supplier; import java.util.stream.Stream; import static org.assertj.core.api.Assertions.*; @@ -41,9 +42,10 @@ void loadYaml() { .source("/test") .sourceBranch(WORKSPACE) .build(); - String string = AgentConfiguration.loadConfigForMapping(mapping, revisionAccess); + AgentConfiguration config = AgentConfiguration.create(mapping, revisionAccess); + String result = config.getConfigYaml(); - assertThat(string).isEqualTo("{key: value}\n"); + assertThat(result).isEqualTo("{key: value}\n"); } @Test @@ -62,7 +64,7 @@ void yamlWithTab() { .build(); assertThatExceptionOfType(ObjectStructureMerger.InvalidConfigurationFileException.class).isThrownBy( - () -> AgentConfiguration.loadConfigForMapping(mapping, revisionAccess) + () -> AgentConfiguration.create(mapping, revisionAccess) ).withMessage("The configuration file '/test.yml' is invalid and cannot be parsed."); } } @@ -73,7 +75,8 @@ class LoadConfigForMapping { @Test void noSourcesSpecified() { AgentMapping mapping = AgentMapping.builder().build(); - String result = AgentConfiguration.loadConfigForMapping(mapping, revisionAccess); + AgentConfiguration config = AgentConfiguration.create(mapping, revisionAccess); + String result = config.getConfigYaml(); assertThat(result).isEmpty(); } @@ -88,7 +91,8 @@ void nonExistingSourcesSpecified() { .source("/some/folder") .sourceBranch(WORKSPACE) .build(); - String result = AgentConfiguration.loadConfigForMapping(mapping, revisionAccess); + AgentConfiguration config = AgentConfiguration.create(mapping, revisionAccess); + String result = config.getConfigYaml(); assertThat(result).isEmpty(); } @@ -106,7 +110,8 @@ void nonYamlIgnored() { .source("d.txt") .sourceBranch(WORKSPACE) .build(); - String result = AgentConfiguration.loadConfigForMapping(mapping, revisionAccess); + AgentConfiguration config = AgentConfiguration.create(mapping, revisionAccess); + String result = config.getConfigYaml(); assertThat(result).isEmpty(); verify(revisionAccess).readConfigurationFile("a.yml"); @@ -126,7 +131,7 @@ void leadingSlashesInSourcesRemoved() { .source("/a.yml") .sourceBranch(WORKSPACE) .build(); - AgentConfiguration.loadConfigForMapping(mapping, revisionAccess); + AgentConfiguration.create(mapping, revisionAccess); verify(revisionAccess).configurationFileExists(eq("a.yml")); } @@ -158,7 +163,8 @@ void priorityRespected() { .source("/folder") .sourceBranch(WORKSPACE) .build(); - String result = AgentConfiguration.loadConfigForMapping(mapping, revisionAccess); + AgentConfiguration config = AgentConfiguration.create(mapping, revisionAccess); + String result = config.getConfigYaml(); assertThat(result).isEqualTo("{val1: z, val2: a, val3: b}\n"); verify(revisionAccess, never()).readConfigurationFile("folder/somethingelse"); @@ -169,7 +175,7 @@ void priorityRespected() { class getDocumentations { @Test - void verifyEmptyGetDocsObjectsAsMap() { + void verifyGetEmptyDocumentationsAsMap() { AgentConfiguration config = AgentConfiguration.NO_MATCHING_MAPPING; Map> map = config.getDocumentationsAsMap(); @@ -177,18 +183,18 @@ void verifyEmptyGetDocsObjectsAsMap() { } @Test - void verifyGetDocsObjectsAsMap() { + void verifyGetDocumentationsAsMap() { String filePath = "test.yml"; Map> docsObjectsByFile = new HashMap<>(); - Set documentations = new HashSet<>(); + Set suppliers = new HashSet<>(); Set objects = Collections.singleton("yaml"); docsObjectsByFile.put(filePath, objects); - AgentDocumentation documentation = new AgentDocumentation(filePath, objects); - documentations.add(documentation); + AgentDocumentationSupplier supplier = new AgentDocumentationSupplier(() -> new AgentDocumentation(filePath, objects)); + suppliers.add(supplier); - AgentConfiguration config = AgentConfiguration.create(null, documentations, ""); + AgentConfiguration config = AgentConfiguration.create(null, suppliers, ""); Map> map = config.getDocumentationsAsMap(); assertThat(map).isEqualTo(docsObjectsByFile); diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java index d07deafeef..aa3716e32f 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java @@ -24,6 +24,7 @@ import java.io.IOException; import java.util.*; +import java.util.function.Supplier; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; @@ -95,7 +96,7 @@ public class GetConfigDocumentationTest { private Map> docsObjectsByFile; - private Set documentations; + private Set suppliers; @BeforeEach void setUp() { @@ -104,9 +105,9 @@ void setUp() { docsObjectsByFile = new HashMap<>(); docsObjectsByFile.put(filePath, objects); - AgentDocumentation documentation = new AgentDocumentation(filePath, objects); - documentations = new HashSet<>(); - documentations.add(documentation); + AgentDocumentationSupplier supplier = new AgentDocumentationSupplier(() -> new AgentDocumentation(filePath, objects)); + suppliers = new HashSet<>(); + suppliers.add(supplier); } @Test @@ -116,7 +117,7 @@ void withDefaultConfig() throws IOException { AgentMapping agentMapping = AgentMapping.builder().build(); final String configYaml = "yaml"; - AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, documentations, configYaml); + AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, suppliers, configYaml); Map defaultYamls = new HashMap<>(); final String defaultYamlContent = "defaultYaml"; @@ -168,7 +169,7 @@ void errorWhenGettingDefaultConfig() throws IOException { AgentMapping agentMapping = AgentMapping.builder().build(); final String configYaml = "yaml"; - AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, documentations, configYaml); + AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, suppliers, configYaml); IOException exception = new IOException(); @@ -201,7 +202,7 @@ void withoutDefaultConfig() throws IOException { AgentMapping agentMapping = AgentMapping.builder().build(); final String configYaml = "yaml"; - AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, documentations, configYaml); + AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, suppliers, configYaml); ConfigDocumentation configDocumentationMock = mock(ConfigDocumentation.class); @@ -250,7 +251,7 @@ void invalidYaml() throws IOException { AgentMapping agentMapping = AgentMapping.builder().build(); final String configYaml = "yaml"; - AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, documentations, configYaml); + AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, suppliers, configYaml); JsonProcessingException exception = mock(JsonProcessingException.class); final String errorMessage = "JsonProcessingException: Yaml could not be processed."; From 30f05d98ca88051b1fc9b6df4a4bd903aecf8f53 Mon Sep 17 00:00:00 2001 From: EddeCCC Date: Fri, 9 Feb 2024 11:09:55 +0100 Subject: [PATCH 12/17] rename method --- .../ocelot/agentconfiguration/AgentConfiguration.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java index 29cf222fe3..eb2fb3a39b 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java @@ -80,7 +80,7 @@ public static AgentConfiguration create(AgentMapping mapping, AbstractFileAccess String src = fileAccessor.readConfigurationFile(path).orElse(""); yamlResult = ObjectStructureMerger.loadAndMergeYaml(src, yamlResult, path); - AgentDocumentationSupplier supplier = new AgentDocumentationSupplier(() -> loadDocsObjects(path, src)); + AgentDocumentationSupplier supplier = new AgentDocumentationSupplier(() -> loadDocumentation(path, src)); documentationSuppliers.add(supplier); } @@ -181,14 +181,14 @@ private static List getAllYamlFiles(AbstractFileAccessor fileAccessor, S } /** - * Loads all documentable objects of the yaml source string. + * Loads all documentable objects of the yaml source string for the provided file. * - * @param src the yaml string * @param filePath the path to the yaml file + * @param src the yaml string * - * @return the set of documentable objects + * @return the set of documentable objects for the provided file */ - private static AgentDocumentation loadDocsObjects(String filePath, String src) { + private static AgentDocumentation loadDocumentation(String filePath, String src) { Set objects = Collections.emptySet(); try { objects = DocsObjectsLoader.loadObjects(src); From 69eb4bc9efb41da195d7d9f3fa314649f378c7ec Mon Sep 17 00:00:00 2001 From: EddeCCC Date: Mon, 19 Feb 2024 11:26:36 +0100 Subject: [PATCH 13/17] move AgentDocumentation to configdocsgenerator --- .../ConfigDocsGenerator.java | 10 ++++---- .../model/AgentDocumentation.java | 24 +++++++++++++++++++ .../ConfigDocsGeneratorTest.java | 6 +++-- .../AgentConfiguration.java | 15 ++++-------- .../AgentDocumentation.java | 13 ---------- .../AgentDocumentationSupplier.java | 2 ++ .../agentconfiguration/DocsObjectsLoader.java | 12 ++++++---- .../ConfigurationController.java | 15 ++++++------ .../AgentConfigurationTest.java | 19 ++++++++------- .../DocsObjectsLoaderTest.java | 9 ++++--- .../ConfigurationControllerTest.java | 19 ++++++++------- 11 files changed, 79 insertions(+), 65 deletions(-) create mode 100644 components/inspectit-ocelot-configdocsgenerator/src/main/java/inspectit/ocelot/configdocsgenerator/model/AgentDocumentation.java delete mode 100644 components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentation.java diff --git a/components/inspectit-ocelot-configdocsgenerator/src/main/java/inspectit/ocelot/configdocsgenerator/ConfigDocsGenerator.java b/components/inspectit-ocelot-configdocsgenerator/src/main/java/inspectit/ocelot/configdocsgenerator/ConfigDocsGenerator.java index 44e35bb96d..1aba83e490 100644 --- a/components/inspectit-ocelot-configdocsgenerator/src/main/java/inspectit/ocelot/configdocsgenerator/ConfigDocsGenerator.java +++ b/components/inspectit-ocelot-configdocsgenerator/src/main/java/inspectit/ocelot/configdocsgenerator/ConfigDocsGenerator.java @@ -55,10 +55,10 @@ public class ConfigDocsGenerator { Map specialInputDescriptionsRegex = Collections.singletonMap("_arg\\d", "The _argN-th argument with which the instrumented method was called within which this action is getting executed."); /** - * Map of files, which contain a set of defined objects like actions, scopes, rules & metrics + * Set of documentations, which contain a set of documentable objects like actions, scopes, rules & metrics for every file */ @Setter - private Map> docsObjectsByFile = new HashMap<>(); + private Set agentDocumentations = new HashSet<>(); /** * Generates a ConfigDocumentation from a YAML String describing an {@link InspectitConfig}. @@ -233,9 +233,9 @@ private List generateActionDocs(Map a * @return a set of files, which contain the provided object */ private Set findFiles(String name) { - Set files = docsObjectsByFile.entrySet().stream() - .filter(entry -> entry.getValue().contains(name)) - .map(Map.Entry::getKey) + Set files = agentDocumentations.stream() + .filter(documentation -> documentation.getObjects().contains(name)) + .map(AgentDocumentation::getFilePath) .collect(Collectors.toSet()); if(files.isEmpty()) log.debug("No file found with definition of " + name); diff --git a/components/inspectit-ocelot-configdocsgenerator/src/main/java/inspectit/ocelot/configdocsgenerator/model/AgentDocumentation.java b/components/inspectit-ocelot-configdocsgenerator/src/main/java/inspectit/ocelot/configdocsgenerator/model/AgentDocumentation.java new file mode 100644 index 0000000000..51b371fa10 --- /dev/null +++ b/components/inspectit-ocelot-configdocsgenerator/src/main/java/inspectit/ocelot/configdocsgenerator/model/AgentDocumentation.java @@ -0,0 +1,24 @@ +package inspectit.ocelot.configdocsgenerator.model; + +import lombok.Value; + +import java.util.Set; + +/** + * Model, to store documentable objects of a specific yaml file. + * Documentable objects can be actions, scopes, rules & metrics. + * + */ +@Value +public class AgentDocumentation { + + /** + * file, which contains the documentable objects + */ + private String filePath; + + /** + * documentable objects of the file + */ + private Set objects; +} diff --git a/components/inspectit-ocelot-configdocsgenerator/src/test/java/inspectit/ocelot/configdocsgenerator/ConfigDocsGeneratorTest.java b/components/inspectit-ocelot-configdocsgenerator/src/test/java/inspectit/ocelot/configdocsgenerator/ConfigDocsGeneratorTest.java index bf9acebb0e..ab0364b420 100644 --- a/components/inspectit-ocelot-configdocsgenerator/src/test/java/inspectit/ocelot/configdocsgenerator/ConfigDocsGeneratorTest.java +++ b/components/inspectit-ocelot-configdocsgenerator/src/test/java/inspectit/ocelot/configdocsgenerator/ConfigDocsGeneratorTest.java @@ -371,8 +371,10 @@ void verifyFindFiles() throws IOException { docsObjects.add(ruleDocParent.getName()); docsObjects.add(metricDoc.getName()); - Map> docsObjectsByFile = Collections.singletonMap(file, docsObjects); - configDocsGenerator.setDocsObjectsByFile(docsObjectsByFile); + AgentDocumentation documentation = new AgentDocumentation(file, docsObjects); + Set agentDocumentations = Collections.singleton(documentation); + + configDocsGenerator.setAgentDocumentations(agentDocumentations); when(actionWithDocInYaml.getFiles()).thenReturn(Collections.singleton(file)); when(actionWithoutDocInYaml.getFiles()).thenReturn(Collections.singleton(file)); when(scopeDoc.getFiles()).thenReturn(Collections.singleton(file)); diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java index eb2fb3a39b..4c0c4f5bde 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java @@ -1,5 +1,6 @@ package rocks.inspectit.ocelot.agentconfiguration; +import inspectit.ocelot.configdocsgenerator.model.AgentDocumentation; import lombok.Value; import lombok.extern.slf4j.Slf4j; import org.springframework.util.DigestUtils; @@ -115,18 +116,12 @@ private static AgentConfiguration createDefault() { } /** - * Get the current agent documentations as a map. The documentation will be loaded lazy.
- * - Key: the file path
- * - Value: the set of objects, like actions, scopes, rules & metrics + * Get the current agent documentations. The documentations will be loaded lazy. * - * @return The agent documentations as a map + * @return The loaded agent documentations */ - public Map> getDocumentationsAsMap() { - return documentationSuppliers.stream() - .collect(Collectors.toMap( - supplier -> supplier.get().filePath(), - supplier -> supplier.get().objects() - )); + public Set getDocumentations() { + return documentationSuppliers.stream().map(AgentDocumentationSupplier::get).collect(Collectors.toSet()); } /** diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentation.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentation.java deleted file mode 100644 index 49cee90fdb..0000000000 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentation.java +++ /dev/null @@ -1,13 +0,0 @@ -package rocks.inspectit.ocelot.agentconfiguration; - -import java.util.Set; - -/** - * Model, to store documentable objects of a specific yaml file. - * Documentable objects can be actions, scopes, rules & metrics. - * - * @param filePath file, which contains the documentable objects - * @param objects documentable objects of the file - */ -public record AgentDocumentation(String filePath, Set objects) { -} diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentationSupplier.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentationSupplier.java index eb572f99cd..0d2a5636ea 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentationSupplier.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentationSupplier.java @@ -1,5 +1,7 @@ package rocks.inspectit.ocelot.agentconfiguration; +import inspectit.ocelot.configdocsgenerator.model.AgentDocumentation; + import java.util.function.Supplier; /** diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoader.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoader.java index 0a14eb261b..81435936aa 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoader.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoader.java @@ -1,5 +1,6 @@ package rocks.inspectit.ocelot.agentconfiguration; +import inspectit.ocelot.configdocsgenerator.model.AgentDocumentation; import inspectit.ocelot.configdocsgenerator.parsing.ConfigParser; import lombok.extern.slf4j.Slf4j; import org.yaml.snakeyaml.Yaml; @@ -22,6 +23,7 @@ public class DocsObjectsLoader { /** * Loads all documentable objects, like actions, scopes, rules & metrics from the provided inspectIT yaml + * * @param src the source yaml * @return a set of defined objects in this yaml */ @@ -49,11 +51,12 @@ public static Set loadObjects(String src) throws IOException { /** * Loads all documentable objects of each file for the current agent + * * @param defaultYamls A map of the default file paths and their yaml content * @return A set of defined objects for each file */ - public static Map> loadDefaultDocsObjectsByFile(Map defaultYamls) { - Map> defaultDocsObjectsByFile = new HashMap<>(); + public static Set loadDefaultAgentDocumentations(Map defaultYamls) { + Set defaultDocumentations = new HashSet<>(); for(Map.Entry entry : defaultYamls.entrySet()) { String path = OCELOT_DEFAULT_CONFIG_PREFIX + entry.getKey(); String src = entry.getValue(); @@ -64,9 +67,10 @@ public static Map> loadDefaultDocsObjectsByFile(Map getConfigDocumentation( AgentConfiguration configuration = configManager.getConfigurationForMapping(agentMapping.get()); String configYaml = configuration.getConfigYaml(); - Map> docsObjectsByFile = configuration.getDocumentationsAsMap(); + Set agentDocumentations = configuration.getDocumentations(); try { if (includeDefault) { @@ -125,11 +124,11 @@ public ResponseEntity getConfigDocumentation( combined = ObjectStructureMerger.merge(combined, loadedYaml); } configYaml = yaml.dump(combined); - Map> defaultObjectsByFile = DocsObjectsLoader.loadDefaultDocsObjectsByFile(defaultYamls); - docsObjectsByFile.putAll(defaultObjectsByFile); + Set defaultAgentDocumentations = DocsObjectsLoader.loadDefaultAgentDocumentations(defaultYamls); + agentDocumentations.addAll(defaultAgentDocumentations); } - configDocsGenerator.setDocsObjectsByFile(docsObjectsByFile); + configDocsGenerator.setAgentDocumentations(agentDocumentations); configDocumentation = configDocsGenerator.generateConfigDocs(configYaml); } catch (Exception e) { diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java index f252f5eb46..d6f4c63a42 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java @@ -1,5 +1,6 @@ package rocks.inspectit.ocelot.agentconfiguration; +import inspectit.ocelot.configdocsgenerator.model.AgentDocumentation; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -10,7 +11,6 @@ import rocks.inspectit.ocelot.mappings.model.AgentMapping; import java.util.*; -import java.util.function.Supplier; import java.util.stream.Stream; import static org.assertj.core.api.Assertions.*; @@ -175,29 +175,30 @@ void priorityRespected() { class getDocumentations { @Test - void verifyGetEmptyDocumentationsAsMap() { + void verifyGetEmptyDocumentations() { AgentConfiguration config = AgentConfiguration.NO_MATCHING_MAPPING; - Map> map = config.getDocumentationsAsMap(); + Set documentations = config.getDocumentations(); - assertThat(map).isEmpty(); + assertThat(documentations).isEmpty(); } @Test - void verifyGetDocumentationsAsMap() { + void verifyGetDocumentations() { String filePath = "test.yml"; - Map> docsObjectsByFile = new HashMap<>(); + Set agentDocumentations = new HashSet<>(); Set suppliers = new HashSet<>(); Set objects = Collections.singleton("yaml"); - docsObjectsByFile.put(filePath, objects); + AgentDocumentation documentation = new AgentDocumentation(filePath, objects); + agentDocumentations.add(documentation); AgentDocumentationSupplier supplier = new AgentDocumentationSupplier(() -> new AgentDocumentation(filePath, objects)); suppliers.add(supplier); AgentConfiguration config = AgentConfiguration.create(null, suppliers, ""); - Map> map = config.getDocumentationsAsMap(); + Set documentations = config.getDocumentations(); - assertThat(map).isEqualTo(docsObjectsByFile); + assertThat(documentations).isEqualTo(agentDocumentations); } } } diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoaderTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoaderTest.java index e2796da909..90e6487751 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoaderTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoaderTest.java @@ -1,12 +1,11 @@ package rocks.inspectit.ocelot.agentconfiguration; +import inspectit.ocelot.configdocsgenerator.model.AgentDocumentation; import org.junit.jupiter.api.Test; import java.io.IOException; import java.util.*; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static rocks.inspectit.ocelot.agentconfiguration.DocsObjectsLoader.OCELOT_DEFAULT_CONFIG_PREFIX; @@ -58,8 +57,8 @@ void verifyLoadDefaultDocsObjectsByFile() { Map configs = new HashMap<>(); configs.put(file, srcYaml); - Map> docsObjectsByFile = DocsObjectsLoader.loadDefaultDocsObjectsByFile(configs); - assertTrue(docsObjectsByFile.containsKey(fileWithPrefix)); - assertTrue(docsObjectsByFile.containsValue(Collections.singleton("s_jdbc_statement_execute"))); + Set documentations = DocsObjectsLoader.loadDefaultAgentDocumentations(configs); + assertTrue(documentations.stream().anyMatch(doc -> doc.getFilePath().equals(fileWithPrefix))); + assertTrue(documentations.stream().anyMatch(doc -> doc.getObjects().equals(Collections.singleton("s_jdbc_statement_execute")))); } } diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java index aa3716e32f..94c365b39e 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java @@ -2,10 +2,10 @@ import com.fasterxml.jackson.core.JsonProcessingException; import inspectit.ocelot.configdocsgenerator.ConfigDocsGenerator; +import inspectit.ocelot.configdocsgenerator.model.AgentDocumentation; import inspectit.ocelot.configdocsgenerator.model.ConfigDocumentation; import org.assertj.core.api.AssertionsForClassTypes; import org.eclipse.jgit.api.errors.GitAPIException; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -24,7 +24,6 @@ import java.io.IOException; import java.util.*; -import java.util.function.Supplier; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; @@ -94,7 +93,7 @@ public void initiatesCommit() throws GitAPIException { @Nested public class GetConfigDocumentationTest { - private Map> docsObjectsByFile; + private Set agentDocumentations; private Set suppliers; @@ -102,8 +101,9 @@ public class GetConfigDocumentationTest { void setUp() { String filePath = "test.yml"; Set objects = Collections.singleton("yaml"); - docsObjectsByFile = new HashMap<>(); - docsObjectsByFile.put(filePath, objects); + agentDocumentations = new HashSet<>(); + AgentDocumentation documentation = new AgentDocumentation(filePath, objects); + agentDocumentations.add(documentation); AgentDocumentationSupplier supplier = new AgentDocumentationSupplier(() -> new AgentDocumentation(filePath, objects)); suppliers = new HashSet<>(); @@ -122,7 +122,8 @@ void withDefaultConfig() throws IOException { Map defaultYamls = new HashMap<>(); final String defaultYamlContent = "defaultYaml"; defaultYamls.put("firstYaml", defaultYamlContent); - docsObjectsByFile.put(OCELOT_DEFAULT_CONFIG_PREFIX + "firstYaml", Collections.emptySet()); + AgentDocumentation documentation = new AgentDocumentation(OCELOT_DEFAULT_CONFIG_PREFIX + "firstYaml", Collections.emptySet()); + agentDocumentations.add(documentation); Map configYamlMap = new HashMap<>(); configYamlMap.put("entry", "value"); @@ -154,7 +155,7 @@ void withDefaultConfig() throws IOException { verify(yaml).load(eq(configYaml)); verify(yaml).dump(eq(combinedYamls)); verifyNoMoreInteractions(yaml); - verify(configDocsGenerator).setDocsObjectsByFile(eq(docsObjectsByFile)); + verify(configDocsGenerator).setAgentDocumentations(eq(agentDocumentations)); verify(configDocsGenerator).generateConfigDocs(eq(combinedYamlString)); verifyNoMoreInteractions(configDocsGenerator); @@ -218,7 +219,7 @@ void withoutDefaultConfig() throws IOException { verifyNoMoreInteractions(mappingManager); verify(agentConfigurationManager).getConfigurationForMapping(eq(agentMapping)); verifyNoMoreInteractions(agentConfigurationManager); - verify(configDocsGenerator).setDocsObjectsByFile(eq(docsObjectsByFile)); + verify(configDocsGenerator).setAgentDocumentations(eq(agentDocumentations)); verify(configDocsGenerator).generateConfigDocs(eq(configYaml)); verifyNoMoreInteractions(configDocsGenerator); @@ -269,7 +270,7 @@ void invalidYaml() throws IOException { verifyNoMoreInteractions(mappingManager); verify(agentConfigurationManager).getConfigurationForMapping(eq(agentMapping)); verifyNoMoreInteractions(agentConfigurationManager); - verify(configDocsGenerator).setDocsObjectsByFile(eq(docsObjectsByFile)); + verify(configDocsGenerator).setAgentDocumentations(eq(agentDocumentations)); verify(configDocsGenerator).generateConfigDocs(eq(configYaml)); verifyNoMoreInteractions(configDocsGenerator); From d1ae3bb0827a5cc28b4e5dc1241abcdc1d26617e Mon Sep 17 00:00:00 2001 From: EddeCCC Date: Tue, 20 Feb 2024 11:55:02 +0100 Subject: [PATCH 14/17] add tests --- .../AgentConfiguration.java | 36 +++--- .../agentconfiguration/DocsObjectsLoader.java | 4 + .../AgentConfigurationReloadTaskTest.java | 97 ++++++++++++++-- .../AgentConfigurationTest.java | 105 ++++++++++++++---- .../DocsObjectsLoaderTest.java | 90 +++++++++++---- .../agentstatus/AgentStatusManagerTest.java | 15 ++- .../rest/agent/AgentControllerTest.java | 23 ++-- .../ocelot/rest/agent/AgentServiceTest.java | 7 +- .../ConfigurationControllerTest.java | 51 ++------- 9 files changed, 294 insertions(+), 134 deletions(-) diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java index 4c0c4f5bde..c36032a165 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java @@ -72,17 +72,23 @@ private AgentConfiguration(AgentMapping mapping, Set * @return Created AgentConfiguration */ public static AgentConfiguration create(AgentMapping mapping, AbstractFileAccessor fileAccessor) { - LinkedHashSet allYamlFiles = getAllYamlFilesForMapping(fileAccessor, mapping); + if (mapping == null) throw new IllegalArgumentException("Cannot create AgentConfiguration for null mapping"); Set documentationSuppliers = new HashSet<>(); Object yamlResult = null; - for (String path : allYamlFiles) { - String src = fileAccessor.readConfigurationFile(path).orElse(""); - yamlResult = ObjectStructureMerger.loadAndMergeYaml(src, yamlResult, path); - - AgentDocumentationSupplier supplier = new AgentDocumentationSupplier(() -> loadDocumentation(path, src)); - documentationSuppliers.add(supplier); + if(fileAccessor == null) { + log.warn("No file accessor provided for mapping {}. Cannot read files", mapping); + } + else { + LinkedHashSet allYamlFiles = getAllYamlFilesForMapping(fileAccessor, mapping); + for (String path : allYamlFiles) { + String src = fileAccessor.readConfigurationFile(path).orElse(""); + yamlResult = ObjectStructureMerger.loadAndMergeYaml(src, yamlResult, path); + + AgentDocumentationSupplier supplier = new AgentDocumentationSupplier(() -> loadDocumentation(path, src)); + documentationSuppliers.add(supplier); + } } String configYaml = yamlResult == null ? "" : new Yaml().dump(yamlResult); @@ -91,19 +97,6 @@ public static AgentConfiguration create(AgentMapping mapping, AbstractFileAccess return new AgentConfiguration(mapping, documentationSuppliers, configYaml, hash); } - /** - * Factory method to create an AgentConfiguration. Also creates a cryptographic hash. - * - * @param mapping The agent mapping for which this instance represents the loaded configuration - * @param documentationSuppliers The set of suppliers for agent documentations - * @param configYaml The yaml string, which contains the configuration - * @return Created AgentConfiguration - */ - public static AgentConfiguration create(AgentMapping mapping, Set documentationSuppliers, String configYaml) { - String hash = DigestUtils.md5DigestAsHex(configYaml.getBytes(Charset.defaultCharset())); - return new AgentConfiguration(mapping, documentationSuppliers, configYaml, hash); - } - /** * Factory method to create a default AgentConfiguration. * @@ -120,7 +113,8 @@ private static AgentConfiguration createDefault() { * * @return The loaded agent documentations */ - public Set getDocumentations() { + public synchronized Set getDocumentations() { + if (documentationSuppliers == null) return Collections.emptySet(); return documentationSuppliers.stream().map(AgentDocumentationSupplier::get).collect(Collectors.toSet()); } diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoader.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoader.java index 81435936aa..77219610e1 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoader.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoader.java @@ -28,6 +28,8 @@ public class DocsObjectsLoader { * @return a set of defined objects in this yaml */ public static Set loadObjects(String src) throws IOException { + if(src == null) throw new IllegalArgumentException("Cannot load documentable objects from null"); + Yaml yaml = new Yaml(); Object rawYaml = yaml.load(src); Set objects = new HashSet<>(); @@ -56,6 +58,8 @@ public static Set loadObjects(String src) throws IOException { * @return A set of defined objects for each file */ public static Set loadDefaultAgentDocumentations(Map defaultYamls) { + if(defaultYamls == null) throw new IllegalArgumentException("Cannot load default documentable objects from null"); + Set defaultDocumentations = new HashSet<>(); for(Map.Entry entry : defaultYamls.entrySet()) { String path = OCELOT_DEFAULT_CONFIG_PREFIX + entry.getKey(); diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java index bdbc12562e..02f6988b57 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java @@ -1,5 +1,6 @@ package rocks.inspectit.ocelot.agentconfiguration; +import inspectit.ocelot.configdocsgenerator.model.AgentDocumentation; import org.apache.commons.lang3.mutable.MutableObject; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; @@ -48,10 +49,52 @@ void beforeEach() { @Nested class Run { + final String file = "/test.yml"; + + @Test + void verifyHappyPath() { + FileInfo fileInfo = mock(FileInfo.class); + when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of(file)); + when(workspaceAccessor.agentMappingsExist()).thenReturn(true); + when(workspaceAccessor.configurationFileExists(anyString())).thenReturn(true); + when(workspaceAccessor.configurationFileIsDirectory(anyString())).thenReturn(true); + when(workspaceAccessor.listConfigurationFiles(anyString())).thenReturn(Collections.singletonList(fileInfo)); + when(workspaceAccessor.readConfigurationFile(anyString())).thenReturn(Optional.of("key: valid")); + + AgentMapping mapping = AgentMapping.builder() + .name("test") + .source("/test") + .sourceBranch(WORKSPACE) + .build(); + doReturn(Collections.singletonList(mapping)).when(serializer).readAgentMappings(any()); + + AgentDocumentation documentation = new AgentDocumentation(file, Collections.emptySet()); + + MutableObject> configurations = new MutableObject<>(); + Consumer> consumer = configurations::setValue; + + AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, consumer); + + task.run(); + + List configurationList = configurations.getValue(); + assertThat(configurationList).hasSize(1); + + AgentConfiguration configuration = configurationList.get(0); + + assertThat(configuration.getMapping()).isEqualTo(mapping); + assertThat(configuration.getConfigYaml()).isEqualTo("{key: valid}\n"); + assertThat(configuration.getDocumentationSuppliers()).hasSize(1); + + AgentDocumentationSupplier supplier = configuration.getDocumentationSuppliers().iterator().next(); + + assertThat(supplier.get()).isEqualTo(documentation); + } + @Test void loadWithException() { FileInfo fileInfo = mock(FileInfo.class); - when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of("/test.yml"), Stream.of("/test.yml")); + when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of(file), Stream.of(file)); when(workspaceAccessor.agentMappingsExist()).thenReturn(true); when(workspaceAccessor.configurationFileExists(anyString())).thenReturn(true); when(workspaceAccessor.configurationFileIsDirectory(anyString())).thenReturn(true); @@ -71,6 +114,8 @@ void loadWithException() { .build(); doReturn(Arrays.asList(mapping, mapping2)).when(serializer).readAgentMappings(any()); + AgentDocumentation documentation = new AgentDocumentation(file, Collections.emptySet()); + MutableObject> configurations = new MutableObject<>(); Consumer> consumer = configurations::setValue; @@ -80,15 +125,22 @@ void loadWithException() { List configurationList = configurations.getValue(); assertThat(configurationList).hasSize(1); - assertThat(configurationList).element(0) - .extracting(AgentConfiguration::getConfigYaml) - .isEqualTo("{key: valid}\n"); + + AgentConfiguration configuration = configurationList.get(0); + + assertThat(configuration.getMapping()).isEqualTo(mapping2); + assertThat(configuration.getConfigYaml()).isEqualTo("{key: valid}\n"); + assertThat(configuration.getDocumentationSuppliers()).hasSize(1); + + AgentDocumentationSupplier supplier = configuration.getDocumentationSuppliers().iterator().next(); + + assertThat(supplier.get()).isEqualTo(documentation); } @Test void loadWithExceptionOnlyString() { FileInfo fileInfo = mock(FileInfo.class); - when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of("/test.yml"), Stream.of("/test.yml")); + when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of(file), Stream.of(file)); when(workspaceAccessor.agentMappingsExist()).thenReturn(true); when(workspaceAccessor.configurationFileExists(anyString())).thenReturn(true); when(workspaceAccessor.configurationFileIsDirectory(anyString())).thenReturn(true); @@ -108,6 +160,8 @@ void loadWithExceptionOnlyString() { .build(); doReturn(Arrays.asList(mapping, mapping2)).when(serializer).readAgentMappings(any()); + AgentDocumentation documentation = new AgentDocumentation(file, Collections.emptySet()); + MutableObject> configurations = new MutableObject<>(); Consumer> consumer = configurations::setValue; @@ -117,15 +171,22 @@ void loadWithExceptionOnlyString() { List configurationList = configurations.getValue(); assertThat(configurationList).hasSize(1); - assertThat(configurationList).element(0) - .extracting(AgentConfiguration::getConfigYaml) - .isEqualTo("{key: valid}\n"); + + AgentConfiguration configuration = configurationList.get(0); + + assertThat(configuration.getMapping()).isEqualTo(mapping2); + assertThat(configuration.getConfigYaml()).isEqualTo("{key: valid}\n"); + assertThat(configuration.getDocumentationSuppliers()).hasSize(1); + + AgentDocumentationSupplier supplier = configuration.getDocumentationSuppliers().iterator().next(); + + assertThat(supplier.get()).isEqualTo(documentation); } @Test void loadWithExceptionOnlyList() { FileInfo fileInfo = mock(FileInfo.class); - when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of("/test.yml"), Stream.of("/test.yml")); + when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of(file), Stream.of(file)); when(workspaceAccessor.agentMappingsExist()).thenReturn(true); when(workspaceAccessor.configurationFileExists(anyString())).thenReturn(true); when(workspaceAccessor.configurationFileIsDirectory(anyString())).thenReturn(true); @@ -145,6 +206,8 @@ void loadWithExceptionOnlyList() { .build(); doReturn(Arrays.asList(mapping, mapping2)).when(serializer).readAgentMappings(any()); + AgentDocumentation documentation = new AgentDocumentation(file, Collections.emptySet()); + MutableObject> configurations = new MutableObject<>(); Consumer> consumer = configurations::setValue; @@ -154,11 +217,21 @@ void loadWithExceptionOnlyList() { List configurationList = configurations.getValue(); assertThat(configurationList).hasSize(1); - assertThat(configurationList).element(0) - .extracting(AgentConfiguration::getConfigYaml) - .isEqualTo("{key: valid}\n"); + + AgentConfiguration configuration = configurationList.get(0); + + assertThat(configuration.getMapping()).isEqualTo(mapping2); + assertThat(configuration.getConfigYaml()).isEqualTo("{key: valid}\n"); + assertThat(configuration.getDocumentationSuppliers()).hasSize(1); + + AgentDocumentationSupplier supplier = configuration.getDocumentationSuppliers().iterator().next(); + + assertThat(supplier.get()).isEqualTo(documentation); } + } + @Nested + class MappingRevisionAccess { @Test void loadMappingFromWorkspace() { when(workspaceAccessor.agentMappingsExist()).thenReturn(true); diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java index d6f4c63a42..494587d00d 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java @@ -25,37 +25,74 @@ public class AgentConfigurationTest { @Mock RevisionAccess revisionAccess; - @Nested - class LoadAndMergeYaml { + final String file = "/test.yml"; + @Nested + class Create { @Test - void loadYaml() { + void verifyCreateHappyPath() { FileInfo fileInfo = mock(FileInfo.class); - when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of("/test.yml")); + when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of(file)); when(revisionAccess.configurationFileExists("test")).thenReturn(true); when(revisionAccess.configurationFileIsDirectory("test")).thenReturn(true); when(revisionAccess.listConfigurationFiles(anyString())).thenReturn(Collections.singletonList(fileInfo)); - when(revisionAccess.readConfigurationFile("/test.yml")).thenReturn(Optional.of("key: value")); + when(revisionAccess.readConfigurationFile(file)).thenReturn(Optional.of("key: value")); AgentMapping mapping = AgentMapping.builder() .name("test") .source("/test") .sourceBranch(WORKSPACE) .build(); + + AgentDocumentation documentation = new AgentDocumentation(file, Collections.emptySet()); + AgentConfiguration config = AgentConfiguration.create(mapping, revisionAccess); - String result = config.getConfigYaml(); - assertThat(result).isEqualTo("{key: value}\n"); + assertThat(config.getMapping()).isEqualTo(mapping); + assertThat(config.getConfigYaml()).isEqualTo("{key: value}\n"); + assertThat(config.getHash()).isNotBlank(); + assertThat(config.getDocumentationSuppliers()).isNotEmpty(); + + AgentDocumentationSupplier createdSupplier = config.getDocumentationSuppliers().stream().findFirst().get(); + + assertThat(createdSupplier.get()).isEqualTo(documentation); + } + + @Test + void verifyCreateNoMapping() { + assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy( + () -> AgentConfiguration.create(null, revisionAccess) + ).withMessage("Cannot create AgentConfiguration for null mapping"); } + @Test + void verifyCreateNoFileAccessor() { + AgentMapping mapping = AgentMapping.builder() + .name("test") + .source("/test") + .sourceBranch(WORKSPACE) + .build(); + + AgentConfiguration config = AgentConfiguration.create(mapping, null); + + assertThat(config.getMapping()).isEqualTo(mapping); + assertThat(config.getConfigYaml()).isBlank(); + assertThat(config.getHash()).isNotBlank(); + assertThat(config.getDocumentationSuppliers()).isEmpty(); + } + } + + @Nested + class LoadAndMergeYaml { + @Test void yamlWithTab() { FileInfo fileInfo = mock(FileInfo.class); - when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of("/test.yml")); + when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of(file)); when(revisionAccess.configurationFileExists("test")).thenReturn(true); when(revisionAccess.configurationFileIsDirectory("test")).thenReturn(true); when(revisionAccess.listConfigurationFiles(anyString())).thenReturn(Collections.singletonList(fileInfo)); - when(revisionAccess.readConfigurationFile("/test.yml")).thenReturn(Optional.of("key:\tvalue")); + when(revisionAccess.readConfigurationFile(file)).thenReturn(Optional.of("key:\tvalue")); AgentMapping mapping = AgentMapping.builder() .name("test") @@ -65,7 +102,7 @@ void yamlWithTab() { assertThatExceptionOfType(ObjectStructureMerger.InvalidConfigurationFileException.class).isThrownBy( () -> AgentConfiguration.create(mapping, revisionAccess) - ).withMessage("The configuration file '/test.yml' is invalid and cannot be parsed."); + ).withMessage("The configuration file '%s' is invalid and cannot be parsed.", file); } } @@ -174,31 +211,53 @@ void priorityRespected() { @Nested class getDocumentations { + private final String srcYaml = """ + inspectit: + instrumentation: + scopes: + s_jdbc_statement_execute: + docs: + description: 'Scope for executed JDBC statements.' + methods: + - name: execute + """; + @Test void verifyGetEmptyDocumentations() { AgentConfiguration config = AgentConfiguration.NO_MATCHING_MAPPING; - Set documentations = config.getDocumentations(); - assertThat(documentations).isEmpty(); + assertThat(config.getDocumentations()).isEmpty(); } @Test void verifyGetDocumentations() { - String filePath = "test.yml"; - Set agentDocumentations = new HashSet<>(); - Set suppliers = new HashSet<>(); + FileInfo fileInfo = mock(FileInfo.class); + when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of(file)); + when(revisionAccess.configurationFileExists("test")).thenReturn(true); + when(revisionAccess.configurationFileIsDirectory("test")).thenReturn(true); + when(revisionAccess.listConfigurationFiles(anyString())).thenReturn(Collections.singletonList(fileInfo)); + when(revisionAccess.readConfigurationFile(file)).thenReturn(Optional.of(srcYaml)); + + AgentDocumentation documentation = new AgentDocumentation(file, Collections.singleton("s_jdbc_statement_execute")); - Set objects = Collections.singleton("yaml"); - AgentDocumentation documentation = new AgentDocumentation(filePath, objects); - agentDocumentations.add(documentation); + AgentMapping mapping = AgentMapping.builder() + .name("test-mapping") + .source("/test") + .sourceBranch(WORKSPACE) + .build(); + + AgentConfiguration config = AgentConfiguration.create(mapping, revisionAccess); + + assertThat(config.getDocumentations()).containsExactly(documentation); + } - AgentDocumentationSupplier supplier = new AgentDocumentationSupplier(() -> new AgentDocumentation(filePath, objects)); - suppliers.add(supplier); + @Test + void verifyGetDocumentationsWithoutFileAccessor() { + AgentMapping mapping = AgentMapping.builder().build(); - AgentConfiguration config = AgentConfiguration.create(null, suppliers, ""); - Set documentations = config.getDocumentations(); + AgentConfiguration config = AgentConfiguration.create(mapping, null); - assertThat(documentations).isEqualTo(agentDocumentations); + assertThat(config.getDocumentations()).isEmpty(); } } } diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoaderTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoaderTest.java index 90e6487751..88f51c4b66 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoaderTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoaderTest.java @@ -1,6 +1,7 @@ package rocks.inspectit.ocelot.agentconfiguration; import inspectit.ocelot.configdocsgenerator.model.AgentDocumentation; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import java.io.IOException; @@ -12,7 +13,7 @@ public class DocsObjectsLoaderTest { - private final String srcYaml = """ + private final String srcYamlWithDocsObject = """ inspectit: instrumentation: scopes: @@ -31,34 +32,77 @@ public class DocsObjectsLoaderTest { free: true """; - @Test - void verifyLoadObjectsSuccessful() throws IOException { - Set objects = DocsObjectsLoader.loadObjects(srcYaml); - assertTrue(objects.contains("s_jdbc_statement_execute")); - } + @Nested + class LoadObjects { + @Test + void verifyLoadObjectsSuccessful() throws IOException { + Set objects = DocsObjectsLoader.loadObjects(srcYamlWithDocsObject); - @Test - void verifyLoadObjectsEmpty() throws IOException { - Set objects = DocsObjectsLoader.loadObjects(srcYamlWithoutDocsObjects); + assertTrue(objects.contains("s_jdbc_statement_execute")); + } - assertTrue(objects.isEmpty()); - } + @Test + void verifyLoadObjectsEmptyDocs() throws IOException { + Set objects = DocsObjectsLoader.loadObjects(srcYamlWithoutDocsObjects); + + assertTrue(objects.isEmpty()); + } + + @Test + void verifyLoadObjectsEmptyString() throws IOException { + Set objects = DocsObjectsLoader.loadObjects(""); + + assertTrue(objects.isEmpty()); + } + + @Test + void verifyLoadObjectsInvalidConfig() { + assertThrows(NoSuchElementException.class, () -> DocsObjectsLoader.loadObjects("invalid-config")); + } - @Test - void verifyLoadThrowsException() { - assertThrows(NoSuchElementException.class, () -> DocsObjectsLoader.loadObjects("invalid-config")); + @Test + void verifyLoadObjectsNull() { + assertThrows(IllegalArgumentException.class, () -> DocsObjectsLoader.loadObjects(null)); + } } - @Test - void verifyLoadDefaultDocsObjectsByFile() { - String file = "test.yml"; - String fileWithPrefix = OCELOT_DEFAULT_CONFIG_PREFIX + file; - Map configs = new HashMap<>(); - configs.put(file, srcYaml); + @Nested + class DefaultAgentDocumentation { + @Test + void verifyLoadDefaultAgentDocumentations() { + String file = "test.yml"; + String fileWithPrefix = OCELOT_DEFAULT_CONFIG_PREFIX + file; + Map configs = new HashMap<>(); + configs.put(file, srcYamlWithDocsObject); + + Set documentations = DocsObjectsLoader.loadDefaultAgentDocumentations(configs); + assertTrue(documentations.stream().anyMatch(doc -> doc.getFilePath().equals(fileWithPrefix))); + assertTrue(documentations.stream().anyMatch(doc -> doc.getObjects().equals(Collections.singleton("s_jdbc_statement_execute")))); + } + + @Test + void verifyLoadDefaultAgentDocumentationsEmptyDocs() { + String file = "test.yml"; + String fileWithPrefix = OCELOT_DEFAULT_CONFIG_PREFIX + file; + Map configs = new HashMap<>(); + configs.put(file, srcYamlWithoutDocsObjects); + + Set documentations = DocsObjectsLoader.loadDefaultAgentDocumentations(configs); + assertTrue(documentations.stream().anyMatch(doc -> doc.getFilePath().equals(fileWithPrefix))); + assertTrue(documentations.stream().anyMatch(doc -> doc.getObjects().equals(Collections.emptySet()))); + } + + @Test + void verifyLoadDefaultAgentDocumentationsEmptyMap() { + Set documentations = DocsObjectsLoader.loadDefaultAgentDocumentations(new HashMap<>()); + + assertTrue(documentations.isEmpty()); + } - Set documentations = DocsObjectsLoader.loadDefaultAgentDocumentations(configs); - assertTrue(documentations.stream().anyMatch(doc -> doc.getFilePath().equals(fileWithPrefix))); - assertTrue(documentations.stream().anyMatch(doc -> doc.getObjects().equals(Collections.singleton("s_jdbc_statement_execute")))); + @Test + void verifyLoadDefaultAgentDocumentationsNull() { + assertThrows(IllegalArgumentException.class, () -> DocsObjectsLoader.loadObjects(null)); + } } } diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentstatus/AgentStatusManagerTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentstatus/AgentStatusManagerTest.java index 4a08030bcf..976e101bd5 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentstatus/AgentStatusManagerTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentstatus/AgentStatusManagerTest.java @@ -6,6 +6,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; +import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import rocks.inspectit.ocelot.agentconfiguration.AgentConfiguration; import rocks.inspectit.ocelot.config.model.InspectitServerSettings; @@ -16,6 +17,7 @@ import java.util.*; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) public class AgentStatusManagerTest { @@ -25,6 +27,9 @@ public class AgentStatusManagerTest { @InjectMocks AgentStatusManager manager; + @Mock + AgentConfiguration agentConfiguration; + @BeforeEach void init() { manager.config = InspectitServerSettings.builder() @@ -41,10 +46,10 @@ class NotifyAgentConfigurationFetched { void testWithAgentIdHeader() { Branch testBranch = Branch.WORKSPACE; AgentMapping agentMapping = AgentMapping.builder().name("test-conf").sourceBranch(testBranch).build(); - AgentConfiguration config = AgentConfiguration.create(agentMapping, new HashSet<>(), ""); - Map attributes = ImmutableMap.of("service", "test"); + when(agentConfiguration.getMapping()).thenReturn(agentMapping); - manager.notifyAgentConfigurationFetched(attributes, Collections.singletonMap(HEADER_AGENT_ID, "aid"), config); + Map attributes = ImmutableMap.of("service", "test"); + manager.notifyAgentConfigurationFetched(attributes, Collections.singletonMap(HEADER_AGENT_ID, "aid"), agentConfiguration); assertThat(manager.getAgentStatuses()).hasSize(1).anySatisfy(status -> { assertThat(status.getAttributes()).isEqualTo(attributes); @@ -73,7 +78,7 @@ void testNoMappingFound() { void testMappingFound() { Branch testBranch = Branch.WORKSPACE; AgentMapping agentMapping = AgentMapping.builder().name("test-conf").sourceBranch(testBranch).build(); - AgentConfiguration config = AgentConfiguration.create(agentMapping, new HashSet<>(),"");; + AgentConfiguration config = AgentConfiguration.create(agentMapping, null); Map attributes = ImmutableMap.of("service", "test"); manager.notifyAgentConfigurationFetched(attributes, Collections.emptyMap(), config); @@ -90,7 +95,7 @@ void testMappingFound() { void testOverriding() throws Exception { Branch testBranch = Branch.WORKSPACE; AgentMapping agentMapping = AgentMapping.builder().name("test-conf").sourceBranch(testBranch).build(); - AgentConfiguration config = AgentConfiguration.create(agentMapping, new HashSet<>(),""); + AgentConfiguration config = AgentConfiguration.create(agentMapping, null); Map attributes = ImmutableMap.of("service", "test"); manager.notifyAgentConfigurationFetched(attributes, Collections.emptyMap(), null); diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/agent/AgentControllerTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/agent/AgentControllerTest.java index d6569eeb52..9695111dfd 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/agent/AgentControllerTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/agent/AgentControllerTest.java @@ -34,6 +34,9 @@ public class AgentControllerTest { @Mock AgentConfigurationManager configManager; + @Mock + AgentConfiguration agentConfiguration; + @Mock AgentStatusManager statusManager; @@ -46,6 +49,8 @@ public class AgentControllerTest { @Nested public class FetchConfiguration { + String srcYaml = "foo : bar"; + @Test public void noMappingFound() { doReturn(null).when(configManager).getConfiguration(anyMap()); @@ -59,29 +64,31 @@ public void noMappingFound() { @Test public void mappingFound() { - AgentConfiguration config = AgentConfiguration.create(null, new HashSet<>(), "foo : bar"); - doReturn(config).when(configManager).getConfiguration(anyMap()); + doReturn(agentConfiguration).when(configManager).getConfiguration(anyMap()); + doReturn(srcYaml).when(agentConfiguration).getConfigYaml(); HashMap attributes = new HashMap<>(); ResponseEntity result = controller.fetchConfiguration(attributes, Collections.emptyMap()); assertThat(result.getStatusCode()).isEqualTo(HttpStatus.OK); - assertThat(result.getBody()).isEqualTo("foo : bar"); - verify(statusManager).notifyAgentConfigurationFetched(same(attributes), eq(Collections.emptyMap()), same(config)); + assertThat(result.getBody()).isEqualTo(srcYaml); + verify(statusManager).notifyAgentConfigurationFetched(same(attributes), eq(Collections.emptyMap()), same(agentConfiguration)); } @Test public void etagPresent() { - AgentConfiguration config = AgentConfiguration.create(null, new HashSet<>(), "foo : bar"); - doReturn(config).when(configManager).getConfiguration(anyMap()); + String hash = "1234"; + doReturn(agentConfiguration).when(configManager).getConfiguration(anyMap()); + doReturn(srcYaml).when(agentConfiguration).getConfigYaml(); + doReturn(hash).when(agentConfiguration).getHash(); ResponseEntity firstResult = controller.fetchConfiguration(new HashMap<>(), Collections.emptyMap()); ResponseEntity secondResult = controller.fetchConfiguration(new HashMap<>(), Collections.emptyMap()); assertThat(firstResult.getStatusCode()).isEqualTo(HttpStatus.OK); - assertThat(firstResult.getBody()).isEqualTo("foo : bar"); + assertThat(firstResult.getBody()).isEqualTo(srcYaml); assertThat(secondResult.getStatusCode()).isEqualTo(HttpStatus.OK); - assertThat(secondResult.getBody()).isEqualTo("foo : bar"); + assertThat(secondResult.getBody()).isEqualTo(srcYaml); assertThat(firstResult.getHeaders().getFirst("ETag")).isNotBlank() .isEqualTo(secondResult.getHeaders().getFirst("ETag")); } diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/agent/AgentServiceTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/agent/AgentServiceTest.java index 5fe774ed03..ce7fe177b7 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/agent/AgentServiceTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/agent/AgentServiceTest.java @@ -38,6 +38,9 @@ public class AgentServiceTest { @Mock AgentConfigurationManager configManager; + @Mock + AgentConfiguration config; + @Mock InspectitServerSettings configuration; @@ -46,8 +49,6 @@ public class BuildSupportArchive { AgentService serviceSpy; - AgentConfiguration config; - Map attributes; String logs; @@ -61,7 +62,7 @@ public void setupTestData(boolean logPreloadingEnabled) { String configYaml = String.format("inspectit.log-preloading: {enabled: %b}", logPreloadingEnabled); //set config - config = AgentConfiguration.create(null, new HashSet<>(), configYaml); + when(config.getConfigYaml()).thenReturn(configYaml); //set attributes attributes = new HashMap() {{ diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java index 94c365b39e..18a7f794fd 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/rest/configuration/ConfigurationControllerTest.java @@ -40,6 +40,9 @@ public class ConfigurationControllerTest { @Mock AgentConfigurationManager agentConfigurationManager; + @Mock + AgentConfiguration agentConfiguration; + @Mock FileManager fileManager; @@ -60,8 +63,8 @@ public class FetchConfiguration { @Test public void returningConfiguration() { - AgentConfiguration configuration = AgentConfiguration.create(null, new HashSet<>(), "yaml"); - when(agentConfigurationManager.getConfiguration(any())).thenReturn(configuration); + when(agentConfiguration.getConfigYaml()).thenReturn("yaml"); + when(agentConfigurationManager.getConfiguration(any())).thenReturn(agentConfiguration); ResponseEntity output = configurationController.fetchConfiguration(null); @@ -93,9 +96,12 @@ public void initiatesCommit() throws GitAPIException { @Nested public class GetConfigDocumentationTest { + private final String mappingName = "name"; + private final AgentMapping agentMapping = AgentMapping.builder().build(); + private final String configYaml = "yaml"; private Set agentDocumentations; - private Set suppliers; + @BeforeEach void setUp() { @@ -105,20 +111,13 @@ void setUp() { AgentDocumentation documentation = new AgentDocumentation(filePath, objects); agentDocumentations.add(documentation); - AgentDocumentationSupplier supplier = new AgentDocumentationSupplier(() -> new AgentDocumentation(filePath, objects)); - suppliers = new HashSet<>(); - suppliers.add(supplier); + lenient().when(agentConfiguration.getMapping()).thenReturn(agentMapping); + lenient().when(agentConfiguration.getConfigYaml()).thenReturn(configYaml); + lenient().when(agentConfiguration.getDocumentations()).thenReturn(agentDocumentations); } @Test void withDefaultConfig() throws IOException { - - final String mappingName = "name"; - AgentMapping agentMapping = AgentMapping.builder().build(); - - final String configYaml = "yaml"; - AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, suppliers, configYaml); - Map defaultYamls = new HashMap<>(); final String defaultYamlContent = "defaultYaml"; defaultYamls.put("firstYaml", defaultYamlContent); @@ -165,17 +164,8 @@ void withDefaultConfig() throws IOException { @Test void errorWhenGettingDefaultConfig() throws IOException { - - final String mappingName = "name"; - AgentMapping agentMapping = AgentMapping.builder().build(); - - final String configYaml = "yaml"; - AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, suppliers, configYaml); - IOException exception = new IOException(); - ConfigDocumentation configDocumentationMock = mock(ConfigDocumentation.class); - when(mappingManager.getAgentMapping(mappingName)).thenReturn(Optional.of(agentMapping)); when(agentConfigurationManager.getConfigurationForMapping(agentMapping)).thenReturn(agentConfiguration); when(defaultConfigController.getDefaultConfigContent()).thenThrow(exception); @@ -198,13 +188,6 @@ void errorWhenGettingDefaultConfig() throws IOException { @Test void withoutDefaultConfig() throws IOException { - - final String mappingName = "name"; - AgentMapping agentMapping = AgentMapping.builder().build(); - - final String configYaml = "yaml"; - AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, suppliers, configYaml); - ConfigDocumentation configDocumentationMock = mock(ConfigDocumentation.class); when(mappingManager.getAgentMapping(mappingName)).thenReturn(Optional.of(agentMapping)); @@ -229,9 +212,6 @@ void withoutDefaultConfig() throws IOException { @Test void agentMappingNotFound() { - - final String mappingName = "name"; - when(mappingManager.getAgentMapping(mappingName)).thenReturn(Optional.empty()); ResponseEntity result = configurationController.getConfigDocumentation(mappingName, false); @@ -247,13 +227,6 @@ void agentMappingNotFound() { @Test void invalidYaml() throws IOException { - - final String mappingName = "name"; - AgentMapping agentMapping = AgentMapping.builder().build(); - - final String configYaml = "yaml"; - AgentConfiguration agentConfiguration = AgentConfiguration.create(agentMapping, suppliers, configYaml); - JsonProcessingException exception = mock(JsonProcessingException.class); final String errorMessage = "JsonProcessingException: Yaml could not be processed."; From 022d6ebc3f3e3d2bb7a06bf3deefb9b8ded30a7b Mon Sep 17 00:00:00 2001 From: EddeCCC Date: Thu, 22 Feb 2024 08:52:14 +0100 Subject: [PATCH 15/17] refactor tests --- .../AgentConfiguration.java | 21 ++--- .../AgentConfigurationReloadTask.java | 10 +++ .../agentconfiguration/DocsObjectsLoader.java | 4 - .../AgentConfigurationReloadTaskTest.java | 89 +++++++++++-------- .../AgentConfigurationTest.java | 23 ----- .../DocsObjectsLoaderTest.java | 5 -- 6 files changed, 70 insertions(+), 82 deletions(-) diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java index c36032a165..74cf0bcb21 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfiguration.java @@ -72,23 +72,16 @@ private AgentConfiguration(AgentMapping mapping, Set * @return Created AgentConfiguration */ public static AgentConfiguration create(AgentMapping mapping, AbstractFileAccessor fileAccessor) { - if (mapping == null) throw new IllegalArgumentException("Cannot create AgentConfiguration for null mapping"); - Set documentationSuppliers = new HashSet<>(); Object yamlResult = null; - if(fileAccessor == null) { - log.warn("No file accessor provided for mapping {}. Cannot read files", mapping); - } - else { - LinkedHashSet allYamlFiles = getAllYamlFilesForMapping(fileAccessor, mapping); - for (String path : allYamlFiles) { - String src = fileAccessor.readConfigurationFile(path).orElse(""); - yamlResult = ObjectStructureMerger.loadAndMergeYaml(src, yamlResult, path); - - AgentDocumentationSupplier supplier = new AgentDocumentationSupplier(() -> loadDocumentation(path, src)); - documentationSuppliers.add(supplier); - } + LinkedHashSet allYamlFiles = getAllYamlFilesForMapping(fileAccessor, mapping); + for (String path : allYamlFiles) { + String src = fileAccessor.readConfigurationFile(path).orElse(""); + yamlResult = ObjectStructureMerger.loadAndMergeYaml(src, yamlResult, path); + + AgentDocumentationSupplier supplier = new AgentDocumentationSupplier(() -> loadDocumentation(path, src)); + documentationSuppliers.add(supplier); } String configYaml = yamlResult == null ? "" : new Yaml().dump(yamlResult); diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java index 82a2a75238..3dbfce633d 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTask.java @@ -54,12 +54,22 @@ public void run() { List mappingsToLoad = mappingsSerializer.readAgentMappings(fileAccess); List newConfigurations = new ArrayList<>(); for (AgentMapping mapping : mappingsToLoad) { + if(mapping == null) { + log.debug("Could not load null agent mapping"); + continue; + } try { if (isCanceled()) { log.debug("Configuration reloading canceled"); return; } + AbstractFileAccessor fileAccessor = getFileAccessorForMapping(mapping); + if(fileAccessor == null) { + log.debug("No file accessor provided for mapping {}. Cannot read files", mapping); + continue; + } + AgentConfiguration agentConfiguration = AgentConfiguration.create(mapping, fileAccessor); newConfigurations.add(agentConfiguration); } catch (Exception e) { diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoader.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoader.java index 77219610e1..81435936aa 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoader.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoader.java @@ -28,8 +28,6 @@ public class DocsObjectsLoader { * @return a set of defined objects in this yaml */ public static Set loadObjects(String src) throws IOException { - if(src == null) throw new IllegalArgumentException("Cannot load documentable objects from null"); - Yaml yaml = new Yaml(); Object rawYaml = yaml.load(src); Set objects = new HashSet<>(); @@ -58,8 +56,6 @@ public static Set loadObjects(String src) throws IOException { * @return A set of defined objects for each file */ public static Set loadDefaultAgentDocumentations(Map defaultYamls) { - if(defaultYamls == null) throw new IllegalArgumentException("Cannot load default documentable objects from null"); - Set defaultDocumentations = new HashSet<>(); for(Map.Entry entry : defaultYamls.entrySet()) { String path = OCELOT_DEFAULT_CONFIG_PREFIX + entry.getKey(); diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java index 02f6988b57..bfbaa033c6 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationReloadTaskTest.java @@ -39,26 +39,27 @@ public class AgentConfigurationReloadTaskTest { @Mock RevisionAccess workspaceAccessor; + final String file = "/test.yml"; + @BeforeEach void beforeEach() { lenient().when(fileManager.getWorkspaceRevision()).thenReturn(workspaceAccessor); lenient().when(fileManager.getLiveRevision()).thenReturn(liveAccessor); lenient().when(serializer.getRevisionAccess()).thenReturn(workspaceAccessor); + lenient().when(workspaceAccessor.agentMappingsExist()).thenReturn(true); + + FileInfo fileInfo = mock(FileInfo.class); + lenient().when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of(file), Stream.of(file)); + lenient().when(workspaceAccessor.configurationFileExists(anyString())).thenReturn(true); + lenient().when(workspaceAccessor.configurationFileIsDirectory(anyString())).thenReturn(true); + lenient().when(workspaceAccessor.listConfigurationFiles(anyString())).thenReturn(Collections.singletonList(fileInfo)); } @Nested class Run { - final String file = "/test.yml"; - @Test - void verifyHappyPath() { - FileInfo fileInfo = mock(FileInfo.class); - when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of(file)); - when(workspaceAccessor.agentMappingsExist()).thenReturn(true); - when(workspaceAccessor.configurationFileExists(anyString())).thenReturn(true); - when(workspaceAccessor.configurationFileIsDirectory(anyString())).thenReturn(true); - when(workspaceAccessor.listConfigurationFiles(anyString())).thenReturn(Collections.singletonList(fileInfo)); + void runTaskWithValidOutput() throws Exception { when(workspaceAccessor.readConfigurationFile(anyString())).thenReturn(Optional.of("key: valid")); AgentMapping mapping = AgentMapping.builder() @@ -72,19 +73,18 @@ void verifyHappyPath() { MutableObject> configurations = new MutableObject<>(); Consumer> consumer = configurations::setValue; - AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, consumer); task.run(); List configurationList = configurations.getValue(); - assertThat(configurationList).hasSize(1); + assertThat(configurationList).isNotEmpty(); AgentConfiguration configuration = configurationList.get(0); assertThat(configuration.getMapping()).isEqualTo(mapping); assertThat(configuration.getConfigYaml()).isEqualTo("{key: valid}\n"); - assertThat(configuration.getDocumentationSuppliers()).hasSize(1); + assertThat(configuration.getDocumentationSuppliers()).isNotEmpty(); AgentDocumentationSupplier supplier = configuration.getDocumentationSuppliers().iterator().next(); @@ -92,13 +92,43 @@ void verifyHappyPath() { } @Test - void loadWithException() { - FileInfo fileInfo = mock(FileInfo.class); - when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of(file), Stream.of(file)); - when(workspaceAccessor.agentMappingsExist()).thenReturn(true); - when(workspaceAccessor.configurationFileExists(anyString())).thenReturn(true); - when(workspaceAccessor.configurationFileIsDirectory(anyString())).thenReturn(true); - when(workspaceAccessor.listConfigurationFiles(anyString())).thenReturn(Collections.singletonList(fileInfo)); + void runTaskWithNullMapping() { + doReturn(Collections.singletonList(null)).when(serializer).readAgentMappings(any()); + + MutableObject> configurations = new MutableObject<>(); + Consumer> consumer = configurations::setValue; + + AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, consumer); + + task.run(); + + List configurationList = configurations.getValue(); + assertThat(configurationList).isEmpty(); + } + + @Test + void runTaskWithoutFileAccessor() { + when(fileManager.getWorkspaceRevision()).thenReturn(null); + AgentMapping mapping = AgentMapping.builder() + .name("test") + .source("/test") + .sourceBranch(WORKSPACE) + .build(); + doReturn(Collections.singletonList(mapping)).when(serializer).readAgentMappings(any()); + + MutableObject> configurations = new MutableObject<>(); + Consumer> consumer = configurations::setValue; + + AgentConfigurationReloadTask task = new AgentConfigurationReloadTask(serializer, fileManager, consumer); + + task.run(); + + List configurationList = configurations.getValue(); + assertThat(configurationList).isEmpty(); + } + + @Test + void loadTabWithException() { // the first call will return a broken file when(workspaceAccessor.readConfigurationFile(anyString())).thenReturn(Optional.of("key:\tbroken"), Optional.of("key: valid")); @@ -138,13 +168,7 @@ void loadWithException() { } @Test - void loadWithExceptionOnlyString() { - FileInfo fileInfo = mock(FileInfo.class); - when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of(file), Stream.of(file)); - when(workspaceAccessor.agentMappingsExist()).thenReturn(true); - when(workspaceAccessor.configurationFileExists(anyString())).thenReturn(true); - when(workspaceAccessor.configurationFileIsDirectory(anyString())).thenReturn(true); - when(workspaceAccessor.listConfigurationFiles(anyString())).thenReturn(Collections.singletonList(fileInfo)); + void loadOnlyStringWithException() { // the first call will return an invalid file only containing a string when(workspaceAccessor.readConfigurationFile(anyString())).thenReturn(Optional.of("onlystring"), Optional.of("key: valid")); @@ -184,13 +208,7 @@ void loadWithExceptionOnlyString() { } @Test - void loadWithExceptionOnlyList() { - FileInfo fileInfo = mock(FileInfo.class); - when(fileInfo.getAbsoluteFilePaths(any())).thenReturn(Stream.of(file), Stream.of(file)); - when(workspaceAccessor.agentMappingsExist()).thenReturn(true); - when(workspaceAccessor.configurationFileExists(anyString())).thenReturn(true); - when(workspaceAccessor.configurationFileIsDirectory(anyString())).thenReturn(true); - when(workspaceAccessor.listConfigurationFiles(anyString())).thenReturn(Collections.singletonList(fileInfo)); + void loadOnlyListWithException() { // the first call will return an invalid file only containing a list when(workspaceAccessor.readConfigurationFile(anyString())).thenReturn(Optional.of("- listentry1\n listentry2"), Optional.of("key: valid")); @@ -232,10 +250,9 @@ void loadWithExceptionOnlyList() { @Nested class MappingRevisionAccess { + @Test void loadMappingFromWorkspace() { - when(workspaceAccessor.agentMappingsExist()).thenReturn(true); - AgentMapping mapping = AgentMapping.builder() .name("test") .source("/test") @@ -254,7 +271,7 @@ void loadMappingFromWorkspace() { @Test void loadMappingFromLive() { - lenient().when(serializer.getRevisionAccess()).thenReturn(liveAccessor); + when(serializer.getRevisionAccess()).thenReturn(liveAccessor); when(liveAccessor.agentMappingsExist()).thenReturn(true); AgentMapping mapping = AgentMapping.builder() diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java index 494587d00d..0bbb732385 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/AgentConfigurationTest.java @@ -57,29 +57,6 @@ void verifyCreateHappyPath() { assertThat(createdSupplier.get()).isEqualTo(documentation); } - - @Test - void verifyCreateNoMapping() { - assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy( - () -> AgentConfiguration.create(null, revisionAccess) - ).withMessage("Cannot create AgentConfiguration for null mapping"); - } - - @Test - void verifyCreateNoFileAccessor() { - AgentMapping mapping = AgentMapping.builder() - .name("test") - .source("/test") - .sourceBranch(WORKSPACE) - .build(); - - AgentConfiguration config = AgentConfiguration.create(mapping, null); - - assertThat(config.getMapping()).isEqualTo(mapping); - assertThat(config.getConfigYaml()).isBlank(); - assertThat(config.getHash()).isNotBlank(); - assertThat(config.getDocumentationSuppliers()).isEmpty(); - } } @Nested diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoaderTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoaderTest.java index 88f51c4b66..b66158b30f 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoaderTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoaderTest.java @@ -99,10 +99,5 @@ void verifyLoadDefaultAgentDocumentationsEmptyMap() { assertTrue(documentations.isEmpty()); } - - @Test - void verifyLoadDefaultAgentDocumentationsNull() { - assertThrows(IllegalArgumentException.class, () -> DocsObjectsLoader.loadObjects(null)); - } } } From 4948a8614531960072e6990ebfc20a4c491b65cf Mon Sep 17 00:00:00 2001 From: EddeCCC Date: Thu, 22 Feb 2024 09:06:14 +0100 Subject: [PATCH 16/17] remove deprecated test --- .../ocelot/agentconfiguration/DocsObjectsLoaderTest.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoaderTest.java b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoaderTest.java index b66158b30f..8c4c3245ce 100644 --- a/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoaderTest.java +++ b/components/inspectit-ocelot-configurationserver/src/test/java/rocks/inspectit/ocelot/agentconfiguration/DocsObjectsLoaderTest.java @@ -60,11 +60,6 @@ void verifyLoadObjectsEmptyString() throws IOException { void verifyLoadObjectsInvalidConfig() { assertThrows(NoSuchElementException.class, () -> DocsObjectsLoader.loadObjects("invalid-config")); } - - @Test - void verifyLoadObjectsNull() { - assertThrows(IllegalArgumentException.class, () -> DocsObjectsLoader.loadObjects(null)); - } } @Nested From bba05dafdf66e60b134f621d7a026aec8d172b74 Mon Sep 17 00:00:00 2001 From: EddeCCC Date: Mon, 26 Feb 2024 10:10:20 +0100 Subject: [PATCH 17/17] make documentation volatile --- .../ocelot/agentconfiguration/AgentDocumentationSupplier.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentationSupplier.java b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentationSupplier.java index 0d2a5636ea..44931d75c2 100644 --- a/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentationSupplier.java +++ b/components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/agentconfiguration/AgentDocumentationSupplier.java @@ -12,7 +12,7 @@ public class AgentDocumentationSupplier implements Supplier private final Supplier supplier; - private AgentDocumentation documentation; + private volatile AgentDocumentation documentation; public AgentDocumentationSupplier(Supplier supplier) { this.supplier = supplier;