From e2c642b5af43c45b8fbcb00294decad6b625b2fa Mon Sep 17 00:00:00 2001 From: jenniferWitzig Date: Thu, 11 Apr 2024 15:24:10 +0200 Subject: [PATCH 1/5] [VHVAPM-444] refactore code, add tests and writable check --- .../core/metrics/MeasureTagValueGuard.java | 227 +++++++----------- .../tagGuards/PersistedTagsReaderWriter.java | 74 ++++++ .../PersistedTagsReaderWriterTest.java | 160 ++++++++++++ 3 files changed, 322 insertions(+), 139 deletions(-) create mode 100644 inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/tagGuards/PersistedTagsReaderWriter.java create mode 100644 inspectit-ocelot-core/src/test/java/rocks/inspectit/ocelot/core/metrics/tagGuards/PersistedTagsReaderWriterTest.java diff --git a/inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/MeasureTagValueGuard.java b/inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/MeasureTagValueGuard.java index 319db02c16..a029305692 100644 --- a/inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/MeasureTagValueGuard.java +++ b/inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/MeasureTagValueGuard.java @@ -1,7 +1,5 @@ package rocks.inspectit.ocelot.core.metrics; -import com.fasterxml.jackson.core.type.TypeReference; -import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Maps; import com.google.common.collect.Sets; @@ -9,9 +7,7 @@ import io.opencensus.tags.TagContextBuilder; import io.opencensus.tags.TagKey; import io.opencensus.tags.Tags; -import lombok.AllArgsConstructor; import lombok.EqualsAndHashCode; -import lombok.NonNull; import lombok.Value; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.StringUtils; @@ -25,17 +21,13 @@ import rocks.inspectit.ocelot.core.instrumentation.context.InspectitContextImpl; import rocks.inspectit.ocelot.core.instrumentation.hook.actions.IHookAction; import rocks.inspectit.ocelot.core.instrumentation.hook.actions.model.MetricAccessor; +import rocks.inspectit.ocelot.core.metrics.tagGuards.PersistedTagsReaderWriter; import rocks.inspectit.ocelot.core.selfmonitoring.AgentHealthManager; import rocks.inspectit.ocelot.core.tags.CommonTagsManager; import rocks.inspectit.ocelot.core.tags.TagUtils; import javax.annotation.PostConstruct; import javax.annotation.PreDestroy; -import java.io.IOException; -import java.nio.charset.StandardCharsets; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.Paths; import java.time.Duration; import java.util.*; import java.util.concurrent.Future; @@ -46,35 +38,25 @@ @Slf4j public class MeasureTagValueGuard { private static final String tagOverFlowMessageTemplate = "Overflow in measure %s for tag key %s"; - + /** + * Map of measure names and their related set of tag keys, which are currently blocked. + */ + private final Map> blockedTagKeysByMeasure = Maps.newHashMap(); + PersistedTagsReaderWriter fileReaderWriter; @Autowired private InspectitEnvironment env; - @Autowired private AgentHealthManager agentHealthManager; - /** * Common tags manager needed for gathering common tags when recording metrics. */ @Autowired private CommonTagsManager commonTagsManager; - @Autowired private ScheduledExecutorService executor; - - private PersistedTagsReaderWriter fileReaderWriter; - private volatile boolean isShuttingDown = false; - private boolean hasTagValueOverflow = false; - - /** - * Map of measure names and their related set of tag keys, which are currently blocked. - */ - private final Map> blockedTagKeysByMeasure = Maps.newHashMap(); - private Set latestTags = Collections.synchronizedSet(new HashSet<>()); - private Future blockTagValuesFuture; @PostConstruct @@ -82,7 +64,12 @@ protected void init() { TagGuardSettings tagGuardSettings = env.getCurrentConfig().getMetrics().getTagGuard(); if (!tagGuardSettings.isEnabled()) return; - fileReaderWriter = new PersistedTagsReaderWriter(tagGuardSettings.getDatabaseFile(), new ObjectMapper()); + final String filename = tagGuardSettings.getDatabaseFile(); + if (StringUtils.isBlank(filename)) { + + } + fileReaderWriter = PersistedTagsReaderWriter.of(filename); + scheduleTagGuardJob(); log.info(String.format("TagValueGuard started with scheduleDelay %s and database file %s", tagGuardSettings.getScheduleDelay(), tagGuardSettings.getDatabaseFile())); @@ -101,78 +88,6 @@ protected void stop() { blockTagValuesFuture.cancel(true); } - /** - * Task, which reads the persisted tag values to determine, which tags should be blocked, because of exceeding - * the specific tag value limit. - * If new tags values have been created, they will be persisted. - */ - @VisibleForTesting - Runnable blockTagValuesTask = () -> { - if (!env.getCurrentConfig().getMetrics().getTagGuard().isEnabled()) return; - - // read current tag value database - Map>> availableTagsByMeasure = fileReaderWriter.read(); - - Set copy = latestTags; - latestTags = Collections.synchronizedSet(new HashSet<>()); - - // process new tags - copy.forEach(tagsHolder -> { - String measureName = tagsHolder.getMeasureName(); - Map newTags = tagsHolder.getTags(); - int maxValuesPerTag = getMaxValuesPerTag(measureName, env.getCurrentConfig()); - - Map> tagValuesByTagKey = availableTagsByMeasure.computeIfAbsent(measureName, k -> Maps.newHashMap()); - newTags.forEach((tagKey, tagValue) -> { - Set tagValues = tagValuesByTagKey.computeIfAbsent(tagKey, (x) -> new HashSet<>()); - // if tag value is new AND max values per tag is already reached - if (!tagValues.contains(tagValue) && tagValues.size() >= maxValuesPerTag) { - boolean isNewBlockedTag = blockedTagKeysByMeasure.computeIfAbsent(measureName, measure -> Sets.newHashSet()).add(tagKey); - if(isNewBlockedTag) { - agentHealthManager.handleInvalidatableHealth(AgentHealth.ERROR, this.getClass(), - String.format(tagOverFlowMessageTemplate, measureName, tagKey)); - hasTagValueOverflow = true; - } - } else { - tagValues.add(tagValue); - } - }); - - }); - - fileReaderWriter.write(availableTagsByMeasure); - - // remove all blocked tags, if no values are stored in the database file - if(availableTagsByMeasure.isEmpty()) blockedTagKeysByMeasure.clear(); - - // independent of processing new tags, check if tags should be blocked or unblocked due to their tag value limit - availableTagsByMeasure.forEach((measureName, tags) -> { - int maxValuesPerTag = getMaxValuesPerTag(measureName, env.getCurrentConfig()); - tags.forEach((tagKey, tagValues) -> { - if(tagValues.size() >= maxValuesPerTag) { - boolean isNewBlockedTag = blockedTagKeysByMeasure.computeIfAbsent(measureName, measure -> Sets.newHashSet()) - .add(tagKey); - if(isNewBlockedTag) { - agentHealthManager.handleInvalidatableHealth(AgentHealth.ERROR, this.getClass(), - String.format(tagOverFlowMessageTemplate, measureName, tagKey)); - hasTagValueOverflow = true; - } - } else { - blockedTagKeysByMeasure.getOrDefault(measureName, Sets.newHashSet()).remove(tagKey); - } - }); - }); - - // invalidate incident, if tag overflow was detected, but no more tags are blocked - boolean noBlockedTagKeys = blockedTagKeysByMeasure.values().stream().allMatch(Set::isEmpty); - if(hasTagValueOverflow && noBlockedTagKeys) { - agentHealthManager.invalidateIncident(this.getClass(), "Overflow for tags resolved"); - hasTagValueOverflow = false; - } - - if (!isShuttingDown) scheduleTagGuardJob(); - }; - /** * Gets the max value amount per tag for the given measure by hierarchically extracting * {@link MetricDefinitionSettings#maxValuesPerTag} (prio 1), @@ -198,7 +113,8 @@ int getMaxValuesPerTag(String measureName, InspectitConfig config) { /** * Creates the full tag context, including all specified tags, for the current measure - * @param context current context + * + * @param context current context * @param metricAccessor accessor for the measure as well as the particular tags * @return TagContext including all tags for the current measure */ @@ -247,7 +163,77 @@ public TagContext getTagContext(IHookAction.ExecutionContext context, MetricAcce return tagContextBuilder.build(); - } + } /** + * Task, which reads the persisted tag values to determine, which tags should be blocked, because of exceeding + * the specific tag value limit. + * If new tags values have been created, they will be persisted. + */ + @VisibleForTesting + Runnable blockTagValuesTask = () -> { + if (!env.getCurrentConfig().getMetrics().getTagGuard().isEnabled()) return; + + // read current tag value database + Map>> availableTagsByMeasure = fileReaderWriter.read(); + + Set copy = latestTags; + latestTags = Collections.synchronizedSet(new HashSet<>()); + + // process new tags + copy.forEach(tagsHolder -> { + String measureName = tagsHolder.getMeasureName(); + Map newTags = tagsHolder.getTags(); + int maxValuesPerTag = getMaxValuesPerTag(measureName, env.getCurrentConfig()); + + Map> tagValuesByTagKey = availableTagsByMeasure.computeIfAbsent(measureName, k -> Maps.newHashMap()); + newTags.forEach((tagKey, tagValue) -> { + Set tagValues = tagValuesByTagKey.computeIfAbsent(tagKey, (x) -> new HashSet<>()); + // if tag value is new AND max values per tag is already reached + if (!tagValues.contains(tagValue) && tagValues.size() >= maxValuesPerTag) { + boolean isNewBlockedTag = blockedTagKeysByMeasure.computeIfAbsent(measureName, measure -> Sets.newHashSet()).add(tagKey); + if (isNewBlockedTag) { + agentHealthManager.handleInvalidatableHealth(AgentHealth.ERROR, this.getClass(), + String.format(tagOverFlowMessageTemplate, measureName, tagKey)); + hasTagValueOverflow = true; + } + } else { + tagValues.add(tagValue); + } + }); + + }); + + fileReaderWriter.write(availableTagsByMeasure); + + // remove all blocked tags, if no values are stored in the database file + if (availableTagsByMeasure.isEmpty()) blockedTagKeysByMeasure.clear(); + + // independent of processing new tags, check if tags should be blocked or unblocked due to their tag value limit + availableTagsByMeasure.forEach((measureName, tags) -> { + int maxValuesPerTag = getMaxValuesPerTag(measureName, env.getCurrentConfig()); + tags.forEach((tagKey, tagValues) -> { + if (tagValues.size() >= maxValuesPerTag) { + boolean isNewBlockedTag = blockedTagKeysByMeasure.computeIfAbsent(measureName, measure -> Sets.newHashSet()) + .add(tagKey); + if (isNewBlockedTag) { + agentHealthManager.handleInvalidatableHealth(AgentHealth.ERROR, this.getClass(), + String.format(tagOverFlowMessageTemplate, measureName, tagKey)); + hasTagValueOverflow = true; + } + } else { + blockedTagKeysByMeasure.getOrDefault(measureName, Sets.newHashSet()).remove(tagKey); + } + }); + }); + + // invalidate incident, if tag overflow was detected, but no more tags are blocked + boolean noBlockedTagKeys = blockedTagKeysByMeasure.values().stream().allMatch(Set::isEmpty); + if (hasTagValueOverflow && noBlockedTagKeys) { + agentHealthManager.invalidateIncident(this.getClass(), "Overflow for tags resolved"); + hasTagValueOverflow = false; + } + + if (!isShuttingDown) scheduleTagGuardJob(); + }; @Value @EqualsAndHashCode @@ -259,46 +245,9 @@ private static class TagsHolder { } - @AllArgsConstructor - static class PersistedTagsReaderWriter { - - @NonNull - private String fileName; - - @NonNull - private ObjectMapper mapper; - - public Map>> read() { - if (!StringUtils.isBlank(fileName)) { - Path path = Paths.get(fileName); - if (Files.exists(path)) { - try { - byte[] content = Files.readAllBytes(path); - @SuppressWarnings("unchecked") Map>> tags = mapper.readValue(content, new TypeReference>>>() { - }); - return tags; - } catch (Exception e) { - log.error("Error loading tag-guard database from persistence file '{}'", fileName, e); - } - } else { - log.info("Could not find tag-guard database file. File will be created during next write"); - } - } - return Maps.newHashMap(); - } - public void write(Map>> tagValues) { - if (!StringUtils.isBlank(fileName)) { - try { - Path path = Paths.get(fileName); - Files.createDirectories(path.getParent()); - String tagValuesString = mapper.writeValueAsString(tagValues); - Files.write(path, tagValuesString.getBytes(StandardCharsets.UTF_8)); - } catch (IOException e) { - log.error("Error writing tag-guard database to file '{}'", fileName, e); - } - } - } - } + } + + diff --git a/inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/tagGuards/PersistedTagsReaderWriter.java b/inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/tagGuards/PersistedTagsReaderWriter.java new file mode 100644 index 0000000000..76bc9a0429 --- /dev/null +++ b/inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/tagGuards/PersistedTagsReaderWriter.java @@ -0,0 +1,74 @@ +package rocks.inspectit.ocelot.core.metrics.tagGuards; + +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; +import lombok.Getter; +import lombok.extern.slf4j.Slf4j; +import org.jetbrains.annotations.NotNull; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.Set; + + +@Getter +@Slf4j +public class PersistedTagsReaderWriter { + + private final ObjectMapper mapper; + @NotNull + private final Path path; + + private PersistedTagsReaderWriter(final ObjectMapper mapper, @NotNull final Path path) { + this.mapper = mapper; + this.path = path; + } + + public static PersistedTagsReaderWriter of(final String filenameInput) { + final Path path = Paths.get(filenameInput); + return new PersistedTagsReaderWriter(new ObjectMapper(), path); + } + + public Map>> read() { + if (!Files.exists(path)) { + log.info("Could not find tag-guard database file. File will be created during next write"); + return new HashMap<>(); + } + + try { + byte[] content = Files.readAllBytes(path); + return mapper.readValue(content, new TypeReference>>>() { + }); + } catch (final Exception e) { + log.error("Error loading tag-guard database from persistence file", e); + return new HashMap<>(); + } + } + + public void write(Map>> tagValues) { + try { + final Path parent = path.getParent(); + if (Objects.isNull(parent) || !Files.isWritable(parent) ) { + log.error("Cannot find write the file because of an invalid path."); + return; + } + + createFileDirectory(parent); + final String tagValuesString = mapper.writeValueAsString(tagValues); + Files.writeString(path, tagValuesString); + } catch (final IOException e) { + log.error("Error writing tag-guard database to file", e); + } + } + + private void createFileDirectory(final Path parent) throws IOException { + if (!Files.isDirectory(parent)) { + Files.createDirectories(parent); + } + } +} \ No newline at end of file diff --git a/inspectit-ocelot-core/src/test/java/rocks/inspectit/ocelot/core/metrics/tagGuards/PersistedTagsReaderWriterTest.java b/inspectit-ocelot-core/src/test/java/rocks/inspectit/ocelot/core/metrics/tagGuards/PersistedTagsReaderWriterTest.java new file mode 100644 index 0000000000..e0d607feb8 --- /dev/null +++ b/inspectit-ocelot-core/src/test/java/rocks/inspectit/ocelot/core/metrics/tagGuards/PersistedTagsReaderWriterTest.java @@ -0,0 +1,160 @@ +package rocks.inspectit.ocelot.core.metrics.tagGuards; + +import com.google.common.collect.Maps; +import org.apache.commons.lang3.StringUtils; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +import static org.assertj.core.api.Assertions.assertThat; + +class PersistedTagsReaderWriterTest { + String tempFileName; + + Map>> tagValues; + + @BeforeEach + public void setup() { + tempFileName = generateTempFilePath(); + tagValues = createTagValues(); + } + + @Test + void ofInvalidInput() { + } + + @Test + void ofWillReturnAValidInstanceIfFilenameIsValid() { + + //GIVEN || WHEN + PersistedTagsReaderWriter readerWriter = PersistedTagsReaderWriter.of(tempFileName); + readerWriter.write(tagValues); + + //THEN + Map>> loaded = readerWriter.read(); + + assertThat(loaded).flatExtracting("measure_1") + .flatExtracting("tagKey_1") + .containsExactlyInAnyOrder("value1", "value2", "value3"); + } + + @Test + void readWillReturnAnEmptyMapIfPathIsEmpty() { + + //GIVEN + PersistedTagsReaderWriter readerWriter = PersistedTagsReaderWriter.of(StringUtils.EMPTY); + + //WHEN + final Map>> result = readerWriter.read(); + + //THEN + Assertions.assertTrue(result.isEmpty()); + } + + @Test + void readWillReturnAnEmptyMapIfThereIsNoFileInThePath() { + + //GIVEN + PersistedTagsReaderWriter readerWriter = PersistedTagsReaderWriter.of("blubb"); + + //WHEN + final Map>> result = readerWriter.read(); + + //THEN + Assertions.assertTrue(result.isEmpty()); + } + + @Test + void writeWillAddTagsIfEverythingIsValid() { + + //GIVEN + PersistedTagsReaderWriter readerWriter = PersistedTagsReaderWriter.of(tempFileName); + + //WHEN + readerWriter.write(tagValues); + final Map>> result = readerWriter.read(); + + //THEN + Assertions.assertFalse(result.isEmpty()); + } + + @Test + void writeWillReturnAnEmptyMapIfPathIsEmpty() { + + //GIVEN + PersistedTagsReaderWriter readerWriter = PersistedTagsReaderWriter.of(StringUtils.EMPTY); + + //WHEN + readerWriter.write(tagValues); + final Map>> result = readerWriter.read(); + + //THEN + Assertions.assertTrue(result.isEmpty()); + } + + @Test + void write() { + + //GIVEN + PersistedTagsReaderWriter readerWriter = PersistedTagsReaderWriter.of(tempFileName); + + //WHEN + readerWriter.write(tagValues); + PersistedTagsReaderWriter readerWriter1 = PersistedTagsReaderWriter.of("blubb"); + final Map>> result = readerWriter1.read(); + + //THEN + Assertions.assertTrue(result.isEmpty()); + } + + + @Test + public void testReadWriteTagsFromDisk() { + + + PersistedTagsReaderWriter readerWriter = PersistedTagsReaderWriter.of(tempFileName); + readerWriter.write(tagValues); + + + Map>> loaded = readerWriter.read(); + + assertThat(loaded).flatExtracting("measure_1") + .flatExtracting("tagKey_1") + .containsExactlyInAnyOrder("value1", "value2", "value3"); + + } + + private String generateTempFilePath() { + try { + Path tempFile = Files.createTempFile("inspectit", ""); + System.out.println(tempFile); + Files.delete(tempFile); + tempFile.toFile().deleteOnExit(); + return tempFile.toString(); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + private Map>> createTagValues() { + Set tagValue = new HashSet<>(); + tagValue.add("value1"); + tagValue.add("value2"); + tagValue.add("value3"); + + Map> tagKeys2Values = Maps.newHashMap(); + tagKeys2Values.put("tagKey_1", tagValue); + + Map>> measure2TagKeys = Maps.newHashMap(); + measure2TagKeys.put("measure_1", tagKeys2Values); + + return measure2TagKeys; + } +} From 2565357283124b547ce212246824eac155b5d9d7 Mon Sep 17 00:00:00 2001 From: jenniferWitzig Date: Thu, 11 Apr 2024 16:00:10 +0200 Subject: [PATCH 2/5] [VHVAPM-444] remove empty test --- .../core/metrics/tagGuards/PersistedTagsReaderWriterTest.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/inspectit-ocelot-core/src/test/java/rocks/inspectit/ocelot/core/metrics/tagGuards/PersistedTagsReaderWriterTest.java b/inspectit-ocelot-core/src/test/java/rocks/inspectit/ocelot/core/metrics/tagGuards/PersistedTagsReaderWriterTest.java index e0d607feb8..56cfc9a3e6 100644 --- a/inspectit-ocelot-core/src/test/java/rocks/inspectit/ocelot/core/metrics/tagGuards/PersistedTagsReaderWriterTest.java +++ b/inspectit-ocelot-core/src/test/java/rocks/inspectit/ocelot/core/metrics/tagGuards/PersistedTagsReaderWriterTest.java @@ -26,9 +26,6 @@ public void setup() { tagValues = createTagValues(); } - @Test - void ofInvalidInput() { - } @Test void ofWillReturnAValidInstanceIfFilenameIsValid() { From 5c302db06cb8f012ad9506c234983ef1610f70c5 Mon Sep 17 00:00:00 2001 From: jenniferWitzig Date: Thu, 18 Apr 2024 09:48:59 +0200 Subject: [PATCH 3/5] [VHVAPM-444] apply review comments, find some potential NPE via tests and fix the logic, using reader and writer instead of bytes --- .../core/metrics/MeasureTagValueGuard.java | 48 ++++++++++++------- .../tagGuards/PersistedTagsReaderWriter.java | 33 ++++++++----- .../metrics/MeasureTagValueGuardTest.java | 7 +-- .../PersistedTagsReaderWriterTest.java | 34 +++++++++++-- 4 files changed, 85 insertions(+), 37 deletions(-) diff --git a/inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/MeasureTagValueGuard.java b/inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/MeasureTagValueGuard.java index a029305692..a0344203a1 100644 --- a/inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/MeasureTagValueGuard.java +++ b/inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/MeasureTagValueGuard.java @@ -63,16 +63,25 @@ public class MeasureTagValueGuard { protected void init() { TagGuardSettings tagGuardSettings = env.getCurrentConfig().getMetrics().getTagGuard(); if (!tagGuardSettings.isEnabled()) return; + initTagReaderWriter(tagGuardSettings); + scheduleTagGuardJob(); + log.info(String.format("TagValueGuard started with scheduleDelay %s and database file %s", tagGuardSettings.getScheduleDelay(), tagGuardSettings.getDatabaseFile())); + } + private void initTagReaderWriter(final TagGuardSettings tagGuardSettings) { + final String filename = getFilename(tagGuardSettings); + if (Objects.nonNull(filename)) { + fileReaderWriter = PersistedTagsReaderWriter.of(filename); + } + } + + private String getFilename(final TagGuardSettings tagGuardSettings) { final String filename = tagGuardSettings.getDatabaseFile(); if (StringUtils.isBlank(filename)) { - + log.error("Filename is empty. Not able writign tags."); + return null; } - fileReaderWriter = PersistedTagsReaderWriter.of(filename); - - scheduleTagGuardJob(); - - log.info(String.format("TagValueGuard started with scheduleDelay %s and database file %s", tagGuardSettings.getScheduleDelay(), tagGuardSettings.getDatabaseFile())); + return filename; } private void scheduleTagGuardJob() { @@ -80,6 +89,7 @@ private void scheduleTagGuardJob() { blockTagValuesFuture = executor.schedule(blockTagValuesTask, tagGuardScheduleDelay.toNanos(), TimeUnit.NANOSECONDS); } + @PreDestroy protected void stop() { if (!env.getCurrentConfig().getMetrics().getTagGuard().isEnabled()) return; @@ -163,6 +173,17 @@ public TagContext getTagContext(IHookAction.ExecutionContext context, MetricAcce return tagContextBuilder.build(); + } + + private boolean isMetrikSettingEnable() { + return env.getCurrentConfig().getMetrics().getTagGuard().isEnabled(); + } + + @Value + @EqualsAndHashCode + private static class TagsHolder { + String measureName; + Map tags; } /** * Task, which reads the persisted tag values to determine, which tags should be blocked, because of exceeding * the specific tag value limit. @@ -170,7 +191,9 @@ public TagContext getTagContext(IHookAction.ExecutionContext context, MetricAcce */ @VisibleForTesting Runnable blockTagValuesTask = () -> { - if (!env.getCurrentConfig().getMetrics().getTagGuard().isEnabled()) return; + if (!isMetrikSettingEnable()) { + return; + } // read current tag value database Map>> availableTagsByMeasure = fileReaderWriter.read(); @@ -235,17 +258,6 @@ public TagContext getTagContext(IHookAction.ExecutionContext context, MetricAcce if (!isShuttingDown) scheduleTagGuardJob(); }; - @Value - @EqualsAndHashCode - private static class TagsHolder { - - String measureName; - - Map tags; - - } - - } diff --git a/inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/tagGuards/PersistedTagsReaderWriter.java b/inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/tagGuards/PersistedTagsReaderWriter.java index 76bc9a0429..71f0c8f57e 100644 --- a/inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/tagGuards/PersistedTagsReaderWriter.java +++ b/inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/tagGuards/PersistedTagsReaderWriter.java @@ -6,7 +6,9 @@ import lombok.extern.slf4j.Slf4j; import org.jetbrains.annotations.NotNull; +import java.io.BufferedReader; import java.io.IOException; +import java.io.Writer; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -21,6 +23,7 @@ public class PersistedTagsReaderWriter { private final ObjectMapper mapper; + @NotNull private final Path path; @@ -34,16 +37,21 @@ public static PersistedTagsReaderWriter of(final String filenameInput) { return new PersistedTagsReaderWriter(new ObjectMapper(), path); } + public Map>> read() { if (!Files.exists(path)) { log.info("Could not find tag-guard database file. File will be created during next write"); return new HashMap<>(); } + return readTagsFromFile(); + } - try { - byte[] content = Files.readAllBytes(path); - return mapper.readValue(content, new TypeReference>>>() { + @NotNull + private Map>> readTagsFromFile() { + try (final BufferedReader content = Files.newBufferedReader(path)) { + final Map>> result = mapper.readValue(content, new TypeReference>>>() { }); + return Objects.nonNull(result) ? result : new HashMap<>(); } catch (final Exception e) { log.error("Error loading tag-guard database from persistence file", e); return new HashMap<>(); @@ -51,16 +59,13 @@ public Map>> read() { } public void write(Map>> tagValues) { - try { - final Path parent = path.getParent(); - if (Objects.isNull(parent) || !Files.isWritable(parent) ) { - log.error("Cannot find write the file because of an invalid path."); - return; - } + if(!isWritable(path)){ + return; + } - createFileDirectory(parent); - final String tagValuesString = mapper.writeValueAsString(tagValues); - Files.writeString(path, tagValuesString); + try (final Writer filesWriter = Files.newBufferedWriter(path)) { + createFileDirectory(path.getParent()); + mapper.writeValue(filesWriter, tagValues); } catch (final IOException e) { log.error("Error writing tag-guard database to file", e); } @@ -71,4 +76,8 @@ private void createFileDirectory(final Path parent) throws IOException { Files.createDirectories(parent); } } + + private boolean isWritable(final Path path) { + return Files.exists(path) ? Files.isWritable(path) : Files.isWritable(path.getParent()); + } } \ No newline at end of file diff --git a/inspectit-ocelot-core/src/test/java/rocks/inspectit/ocelot/core/metrics/MeasureTagValueGuardTest.java b/inspectit-ocelot-core/src/test/java/rocks/inspectit/ocelot/core/metrics/MeasureTagValueGuardTest.java index d26b4b00c9..8c24fbff73 100644 --- a/inspectit-ocelot-core/src/test/java/rocks/inspectit/ocelot/core/metrics/MeasureTagValueGuardTest.java +++ b/inspectit-ocelot-core/src/test/java/rocks/inspectit/ocelot/core/metrics/MeasureTagValueGuardTest.java @@ -24,6 +24,7 @@ import rocks.inspectit.ocelot.core.instrumentation.hook.actions.IHookAction.ExecutionContext; import rocks.inspectit.ocelot.core.instrumentation.hook.actions.model.MetricAccessor; +import rocks.inspectit.ocelot.core.metrics.tagGuards.PersistedTagsReaderWriter; import rocks.inspectit.ocelot.core.selfmonitoring.AgentHealthManager; import rocks.inspectit.ocelot.core.tags.CommonTagsManager; @@ -45,11 +46,11 @@ class MeasureTagValueGuardTest { @Mock private CommonTagsManager commonTagsManager; @Mock + PersistedTagsReaderWriter readerWriter; + @Mock private AgentHealthManager agentHealthManager; @Mock private ScheduledExecutorService executor; - @Mock - MeasureTagValueGuard.PersistedTagsReaderWriter readerWriter; @InjectMocks private MeasureTagValueGuard guard = new MeasureTagValueGuard(); @@ -114,7 +115,7 @@ private Map>> createTagValues() { public void testReadWriteTagsFromDisk() { String tempFileName = generateTempFilePath(); - MeasureTagValueGuard.PersistedTagsReaderWriter readerWriter = new MeasureTagValueGuard.PersistedTagsReaderWriter(tempFileName, new ObjectMapper()); + PersistedTagsReaderWriter readerWriter = PersistedTagsReaderWriter.of(tempFileName); Map>> tagValues = createTagValues(); readerWriter.write(tagValues); Map>> loaded = readerWriter.read(); diff --git a/inspectit-ocelot-core/src/test/java/rocks/inspectit/ocelot/core/metrics/tagGuards/PersistedTagsReaderWriterTest.java b/inspectit-ocelot-core/src/test/java/rocks/inspectit/ocelot/core/metrics/tagGuards/PersistedTagsReaderWriterTest.java index 56cfc9a3e6..7867b3b229 100644 --- a/inspectit-ocelot-core/src/test/java/rocks/inspectit/ocelot/core/metrics/tagGuards/PersistedTagsReaderWriterTest.java +++ b/inspectit-ocelot-core/src/test/java/rocks/inspectit/ocelot/core/metrics/tagGuards/PersistedTagsReaderWriterTest.java @@ -9,6 +9,7 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; @@ -97,25 +98,50 @@ void writeWillReturnAnEmptyMapIfPathIsEmpty() { } @Test - void write() { + void writeWillReturnAnEmptyMapIfPathIsWrong() { //GIVEN PersistedTagsReaderWriter readerWriter = PersistedTagsReaderWriter.of(tempFileName); //WHEN readerWriter.write(tagValues); - PersistedTagsReaderWriter readerWriter1 = PersistedTagsReaderWriter.of("blubb"); - final Map>> result = readerWriter1.read(); + PersistedTagsReaderWriter readerWriterWrongPath = PersistedTagsReaderWriter.of("blubb"); + final Map>> result = readerWriterWrongPath.read(); //THEN Assertions.assertTrue(result.isEmpty()); } + @Test + void wirteWillReturnAnEmptyMapIfTagMapIsNull() { + + //GIVEN + PersistedTagsReaderWriter readerWriter = PersistedTagsReaderWriter.of(tempFileName); + + //WHEN + readerWriter.write(null); + final Map>> result = readerWriter.read(); + //THEN + Assertions.assertTrue(result.isEmpty()); + } @Test - public void testReadWriteTagsFromDisk() { + void writeWillReturnAnEmptyIf() { + //GIVEN + PersistedTagsReaderWriter readerWriter = PersistedTagsReaderWriter.of(tempFileName); + + //WHEN + readerWriter.write(new HashMap<>()); + final Map>> result = readerWriter.read(); + //THEN + Assertions.assertTrue(result.isEmpty()); + } + + + @Test + public void testReadWriteTagsFromDisk() { PersistedTagsReaderWriter readerWriter = PersistedTagsReaderWriter.of(tempFileName); readerWriter.write(tagValues); From c819b59966e26ec1b489033afdecea2672e8b501 Mon Sep 17 00:00:00 2001 From: jenniferWitzig Date: Fri, 19 Apr 2024 11:06:51 +0200 Subject: [PATCH 4/5] [VHVAPM-444] remove creating tempfile manually and use junit standard instead --- .../PersistedTagsReaderWriterTest.java | 20 +++++++------------ 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/inspectit-ocelot-core/src/test/java/rocks/inspectit/ocelot/core/metrics/tagGuards/PersistedTagsReaderWriterTest.java b/inspectit-ocelot-core/src/test/java/rocks/inspectit/ocelot/core/metrics/tagGuards/PersistedTagsReaderWriterTest.java index 7867b3b229..5d9677b2b9 100644 --- a/inspectit-ocelot-core/src/test/java/rocks/inspectit/ocelot/core/metrics/tagGuards/PersistedTagsReaderWriterTest.java +++ b/inspectit-ocelot-core/src/test/java/rocks/inspectit/ocelot/core/metrics/tagGuards/PersistedTagsReaderWriterTest.java @@ -5,10 +5,9 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; +import java.io.File; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -17,8 +16,9 @@ import static org.assertj.core.api.Assertions.assertThat; class PersistedTagsReaderWriterTest { + @TempDir + File anotherTempDir; String tempFileName; - Map>> tagValues; @BeforeEach @@ -125,6 +125,7 @@ void wirteWillReturnAnEmptyMapIfTagMapIsNull() { //THEN Assertions.assertTrue(result.isEmpty()); } + @Test void writeWillReturnAnEmptyIf() { @@ -155,15 +156,8 @@ public void testReadWriteTagsFromDisk() { } private String generateTempFilePath() { - try { - Path tempFile = Files.createTempFile("inspectit", ""); - System.out.println(tempFile); - Files.delete(tempFile); - tempFile.toFile().deleteOnExit(); - return tempFile.toString(); - } catch (IOException e) { - throw new RuntimeException(e); - } + File inspectTempFile = new File(anotherTempDir, "inspectit.txt"); + return inspectTempFile.getPath(); } private Map>> createTagValues() { From cd6339581f8c3923b3d16dd5ecdac5cf2ab38b01 Mon Sep 17 00:00:00 2001 From: jenniferWitzig Date: Thu, 25 Apr 2024 09:26:55 +0200 Subject: [PATCH 5/5] [VHVAPM-444] change fileReaderWriter call --- .../core/metrics/MeasureTagValueGuard.java | 108 +++++++++++------- 1 file changed, 65 insertions(+), 43 deletions(-) diff --git a/inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/MeasureTagValueGuard.java b/inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/MeasureTagValueGuard.java index a0344203a1..15067e9aa3 100644 --- a/inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/MeasureTagValueGuard.java +++ b/inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/MeasureTagValueGuard.java @@ -61,27 +61,7 @@ public class MeasureTagValueGuard { @PostConstruct protected void init() { - TagGuardSettings tagGuardSettings = env.getCurrentConfig().getMetrics().getTagGuard(); - if (!tagGuardSettings.isEnabled()) return; - initTagReaderWriter(tagGuardSettings); scheduleTagGuardJob(); - log.info(String.format("TagValueGuard started with scheduleDelay %s and database file %s", tagGuardSettings.getScheduleDelay(), tagGuardSettings.getDatabaseFile())); - } - - private void initTagReaderWriter(final TagGuardSettings tagGuardSettings) { - final String filename = getFilename(tagGuardSettings); - if (Objects.nonNull(filename)) { - fileReaderWriter = PersistedTagsReaderWriter.of(filename); - } - } - - private String getFilename(final TagGuardSettings tagGuardSettings) { - final String filename = tagGuardSettings.getDatabaseFile(); - if (StringUtils.isBlank(filename)) { - log.error("Filename is empty. Not able writign tags."); - return null; - } - return filename; } private void scheduleTagGuardJob() { @@ -92,7 +72,9 @@ private void scheduleTagGuardJob() { @PreDestroy protected void stop() { - if (!env.getCurrentConfig().getMetrics().getTagGuard().isEnabled()) return; + if (isTagGuardDisabled()) { + return; + } isShuttingDown = true; blockTagValuesFuture.cancel(true); @@ -175,29 +157,70 @@ public TagContext getTagContext(IHookAction.ExecutionContext context, MetricAcce } - private boolean isMetrikSettingEnable() { - return env.getCurrentConfig().getMetrics().getTagGuard().isEnabled(); + private boolean isTagGuardDisabled() { + return !env.getCurrentConfig().getMetrics().getTagGuard().isEnabled(); } - @Value - @EqualsAndHashCode - private static class TagsHolder { - String measureName; - Map tags; - } /** + /** * Task, which reads the persisted tag values to determine, which tags should be blocked, because of exceeding * the specific tag value limit. * If new tags values have been created, they will be persisted. */ @VisibleForTesting Runnable blockTagValuesTask = () -> { - if (!isMetrikSettingEnable()) { + if (isNotWritable()) { return; } - // read current tag value database - Map>> availableTagsByMeasure = fileReaderWriter.read(); + Map>> storedTags = fileReaderWriter.read(); + processNewTags(storedTags); + fileReaderWriter.write(storedTags); + removeBlockedTags(storedTags); + + // invalidate incident, if tag overflow was detected, but no more tags are blocked + boolean noBlockedTagKeys = blockedTagKeysByMeasure.values().stream().allMatch(Set::isEmpty); + if (hasTagValueOverflow && noBlockedTagKeys) { + agentHealthManager.invalidateIncident(this.getClass(), "Overflow for tags resolved"); + hasTagValueOverflow = false; + } + + if (!isShuttingDown) scheduleTagGuardJob(); + }; + + private boolean isNotWritable() { + if (isTagGuardDisabled()) { + return true; + } + + initTagReaderWriter(); + return Objects.isNull(fileReaderWriter); + } + private void initTagReaderWriter() { + final String filename = getFilename(); + if (Objects.nonNull(filename)) { + fileReaderWriter = PersistedTagsReaderWriter.of(filename); + } + } + + private String getFilename() { + TagGuardSettings tagGuardSettings = env.getCurrentConfig().getMetrics().getTagGuard(); + if (!tagGuardSettings.isEnabled()) { + log.error("Filename is not set. Not able to be writing tags."); + return null; + } + + final String filename = tagGuardSettings.getDatabaseFile(); + if (StringUtils.isBlank(filename)) { + log.error("Filename is empty. Not able to be writing tags."); + return null; + } + + log.info(String.format("TagValueGuard started with scheduleDelay %s and database file %s", tagGuardSettings.getScheduleDelay(), tagGuardSettings.getDatabaseFile())); + return filename; + } + + private void processNewTags(Map>> storedTags) { Set copy = latestTags; latestTags = Collections.synchronizedSet(new HashSet<>()); @@ -207,7 +230,7 @@ private static class TagsHolder { Map newTags = tagsHolder.getTags(); int maxValuesPerTag = getMaxValuesPerTag(measureName, env.getCurrentConfig()); - Map> tagValuesByTagKey = availableTagsByMeasure.computeIfAbsent(measureName, k -> Maps.newHashMap()); + Map> tagValuesByTagKey = storedTags.computeIfAbsent(measureName, k -> Maps.newHashMap()); newTags.forEach((tagKey, tagValue) -> { Set tagValues = tagValuesByTagKey.computeIfAbsent(tagKey, (x) -> new HashSet<>()); // if tag value is new AND max values per tag is already reached @@ -224,9 +247,9 @@ private static class TagsHolder { }); }); + } - fileReaderWriter.write(availableTagsByMeasure); - + private void removeBlockedTags(Map>> availableTagsByMeasure) { // remove all blocked tags, if no values are stored in the database file if (availableTagsByMeasure.isEmpty()) blockedTagKeysByMeasure.clear(); @@ -247,16 +270,15 @@ private static class TagsHolder { } }); }); + } - // invalidate incident, if tag overflow was detected, but no more tags are blocked - boolean noBlockedTagKeys = blockedTagKeysByMeasure.values().stream().allMatch(Set::isEmpty); - if (hasTagValueOverflow && noBlockedTagKeys) { - agentHealthManager.invalidateIncident(this.getClass(), "Overflow for tags resolved"); - hasTagValueOverflow = false; - } + @Value + @EqualsAndHashCode + private static class TagsHolder { + String measureName; + Map tags; + } - if (!isShuttingDown) scheduleTagGuardJob(); - };