Closes inspectIT#391 handle invalid metric tags

util method in CommonTagsManager to create TagValue
for all invalid TagValues is <invalid> generated

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/instrumentation/context/InspectitContextImpl.java

@@ -12,6 +12,7 @@
 import rocks.inspectit.ocelot.bootstrap.context.InternalInspectitContext;
 import rocks.inspectit.ocelot.config.model.instrumentation.data.PropagationMode;
 import rocks.inspectit.ocelot.core.instrumentation.config.model.propagation.PropagationMetaData;
+import rocks.inspectit.ocelot.core.tags.CommonTagsManager;
 
 import java.util.*;
 import java.util.stream.Stream;
@@ -71,51 +72,54 @@
 @Slf4j
 public class InspectitContextImpl implements InternalInspectitContext {
 
+    static final Context.Key<InspectitContextImpl> INSPECTIT_KEY = Context.key("inspectit-context");
     /**
      * We only allow "data" of the following types to be used as tags
      */
     private static final Set<Class<?>> ALLOWED_TAG_TYPES = new HashSet<>(Arrays.asList(
             String.class, Character.class, Long.class, Integer.class, Short.class, Byte.class,
             Double.class, Float.class, Boolean.class
     ));
-
-    static final Context.Key<InspectitContextImpl> INSPECTIT_KEY = Context.key("inspectit-context");
-
-    /**
-     * Points to the parent from which this context inherits its data and to which potential up-propagation is performed.
-     * Is effectively final and never changes, except that it is set to null in {@link #close()} to prevent memory leaks.
-     */
-    private InspectitContextImpl parent;
-
-    /**
-     * Defines for each data key its propagation behaviour as well as if it is a tag.
-     */
-    private PropagationMetaData propagation;
-
     /**
      * Defines whether the context should interact with TagContexts opened by the instrumented application.
      * <p>
      * If this is true, the context will inherit all values from the current {@link TagContext} which was opened by the target application.
      * In addition if this value is true makeActive will open a TagContext containing all down propagated tags stored in this InspectIT context.
      */
     private final boolean interactWithApplicationTagContexts;
-
     /**
      * Contains the thread in which this context was created.
      * This is used to identify async traces by comparing their thread against the thread of their parent.
      */
     private final Thread openingThread;
-
+    /**
+     * Contains all writes performed via {@link #setData(String, Object)} during any life-cycle phase of the context.
+     * This means that this map represents all data which has been altered during the lifetime of this context.
+     * This also includes any writes performed due to the up-propagation of children.
+     * <p>
+     * The combination of {@link #postEntryPhaseDownPropagatedData} overwritten by this map therefore presents all current data.
+     * <p>
+     * Note that this map may contain null values: a null value indicates that the corresponding value has been cleared.
+     * This is required for example to ensure clearing data is propagated up correctly.
+     */
+    private final Map<String, Object> dataOverwrites;
+    /**
+     * Points to the parent from which this context inherits its data and to which potential up-propagation is performed.
+     * Is effectively final and never changes, except that it is set to null in {@link #close()} to prevent memory leaks.
+     */
+    private InspectitContextImpl parent;
+    /**
+     * Defines for each data key its propagation behaviour as well as if it is a tag.
+     */
+    private PropagationMetaData propagation;
     /**
      * Holds the previous GRPC context which was overridden when attaching this context as active in GRPC.
      */
     private Context overriddenGrpcContext;
-
     /**
      * The span which was (potentially) opened by invoking {@link #enterSpan(Span)}
      */
     private AutoCloseable currentSpanScope;
-
     /**
      * Holds the tag context which was opened by this context with the call to {@link #makeActive()}.
      * If none was opened, this variable is null.
@@ -125,21 +129,18 @@ public class InspectitContextImpl implements InternalInspectitContext {
      * The tag context is guaranteed to contain the same tags as returned by {@link #getPostEntryPhaseTags()}
      */
     private TagContext activePhaseDownPropagationTagContext;
-
     /**
      * Marker variable to indicate that {@link #activePhaseDownPropagationTagContext} is stale.
      * The tag context can become stale due to up-propagation when a child context up-propagates a new value for a tag which is present in the context.
      * This variable is used to indicate for child contexts that they should not reuse activePhaseDownPropagationTagContext
      * but instead should open a new tag context.
      */
     private boolean isActivePhaseDownPropagationTagContextStale;
-
     /**
      * If this context opened a new {@link #activePhaseDownPropagationTagContext} during {@link #makeActive()},
      * the corresponding scope is stored in this variable and will be used in {@link #close()} to clean up the context.
      */
     private Scope openedDownPropagationScope;
-
     /**
      * When a new context is created, this map contains the down-propagated data it inherited from its parent context.
      * During the entry phase, data updates are written to {@link #dataOverwrites}
@@ -153,19 +154,6 @@ public class InspectitContextImpl implements InternalInspectitContext {
      * When a data key is assigned the value "null", the key will simply be not present in this map.
      */
     private Map<String, Object> postEntryPhaseDownPropagatedData;
-
-    /**
-     * Contains all writes performed via {@link #setData(String, Object)} during any life-cycle phase of the context.
-     * This means that this map represents all data which has been altered during the lifetime of this context.
-     * This also includes any writes performed due to the up-propagation of children.
-     * <p>
-     * The combination of {@link #postEntryPhaseDownPropagatedData} overwritten by this map therefore presents all current data.
-     * <p>
-     * Note that this map may contain null values: a null value indicates that the corresponding value has been cleared.
-     * This is required for example to ensure clearing data is propagated up correctly.
-     */
-    private final Map<String, Object> dataOverwrites;
-
     /**
      * When a synchronous child context is opened during the active phase of its parent,
      * it inherits all {@link #postEntryPhaseDownPropagatedData} in combination with all down-propagated data from {@link #dataOverwrites}
@@ -346,7 +334,7 @@ private boolean isInDifferentThreadThanParentOrIsParentClosed() {
     public Scope enterFullTagScope() {
         TagContextBuilder builder = Tags.getTagger().emptyBuilder();
         dataTagsStream()
-                .forEach(e -> builder.putLocal(TagKey.create(e.getKey()), TagValue.create(e.getValue().toString())));
+                .forEach(e -> builder.putLocal(TagKey.create(e.getKey()), CommonTagsManager.createTagValue(e.getValue().toString())));
         return builder.buildScoped();
     }
 
@@ -579,7 +567,7 @@ private Iterator<Tag> getPostEntryPhaseTags() {
         return postEntryPhaseDownPropagatedData.entrySet().stream()
                 .filter(e -> propagation.isTag(e.getKey()))
                 .filter(e -> ALLOWED_TAG_TYPES.contains(e.getValue().getClass()))
-                .map(e -> Tag.create(TagKey.create(e.getKey()), TagValue.create(e.getValue().toString())))
+                .map(e -> Tag.create(TagKey.create(e.getKey()), CommonTagsManager.createTagValue(e.getValue().toString())))
                 .iterator();
     }
 

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/instrumentation/hook/actions/MetricsRecorder.java

@@ -2,7 +2,10 @@
 
 import io.opencensus.stats.MeasureMap;
 import io.opencensus.stats.StatsRecorder;
-import io.opencensus.tags.*;
+import io.opencensus.tags.TagContext;
+import io.opencensus.tags.TagContextBuilder;
+import io.opencensus.tags.TagKey;
+import io.opencensus.tags.Tags;
 import lombok.Value;
 import lombok.extern.slf4j.Slf4j;
 import rocks.inspectit.ocelot.core.instrumentation.context.InspectitContextImpl;
@@ -67,17 +70,17 @@ private TagContext getTagContext(ExecutionContext context, MetricAccessor metric
         // first common tags to allow overwrite by constant or data tags
         commonTagsManager.getCommonTagKeys()
                 .forEach(commonTagKey -> Optional.ofNullable(inspectitContext.getData(commonTagKey.getName()))
-                        .ifPresent(value -> builder.putLocal(commonTagKey, TagValue.create(value.toString())))
+                        .ifPresent(value -> builder.putLocal(commonTagKey, CommonTagsManager.createTagValue(value.toString())))
                 );
 
         // then constant tags to allow overwrite by data
         metricAccessor.getConstantTags()
-                .forEach((key, value) -> builder.putLocal(TagKey.create(key), TagValue.create(value)));
+                .forEach((key, value) -> builder.putLocal(TagKey.create(key), CommonTagsManager.createTagValue(value)));
 
         // go over data tags and match the value to the key from the contextTags (if available)
         metricAccessor.getDataTagAccessors()
                 .forEach((key, accessor) -> Optional.ofNullable(accessor.get(context))
-                        .ifPresent(tagValue -> builder.putLocal(TagKey.create(key), TagValue.create(tagValue.toString()))));
+                        .ifPresent(tagValue -> builder.putLocal(TagKey.create(key), CommonTagsManager.createTagValue(tagValue.toString()))));
 
         // build and return
         return builder.build();

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/jmx/JmxMetricsRecorder.java

@@ -7,7 +7,6 @@
 import io.opencensus.stats.StatsRecorder;
 import io.opencensus.tags.TagContextBuilder;
 import io.opencensus.tags.TagKey;
-import io.opencensus.tags.TagValue;
 import io.opencensus.tags.Tagger;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -72,9 +71,9 @@ public JmxMetricsRecorder(Tagger tagger) {
     JmxMetricsRecorder(Tagger tagger, MeasuresAndViewsManager measuresAndViewsManager, StatsRecorder statsRecorder, CommonTagsManager commonTagsManager) {
         super("metrics.jmx");
         this.tagger = tagger;
-        this.measureManager = measuresAndViewsManager;
-        this.recorder = statsRecorder;
-        this.commonTags = commonTagsManager;
+        measureManager = measuresAndViewsManager;
+        recorder = statsRecorder;
+        commonTags = commonTagsManager;
     }
 
     /**
@@ -85,8 +84,8 @@ public JmxMetricsRecorder(Tagger tagger) {
     @Override
     protected boolean doEnable(InspectitConfig configuration) {
         // create a new scraper, called on every update of every jmx setting
-        this.jmxScraper = createScraper(configuration.getMetrics().getJmx(), this);
-        this.lowerCaseMetricName = configuration.getMetrics().getJmx().isLowerCaseMetricName();
+        jmxScraper = createScraper(configuration.getMetrics().getJmx(), this);
+        lowerCaseMetricName = configuration.getMetrics().getJmx().isLowerCaseMetricName();
 
         // call super to handle scheduling
         return super.doEnable(configuration);
@@ -137,7 +136,7 @@ public void recordBean(String domain, LinkedHashMap<String, String> beanProperti
             TagContextBuilder tagContextBuilder = tagger.currentBuilder();
             beanProperties.entrySet().stream()
                     .skip(1)
-                    .forEach(entry -> tagContextBuilder.putLocal(TagKey.create(entry.getKey()), TagValue.create(entry.getValue())));
+                    .forEach(entry -> tagContextBuilder.putLocal(TagKey.create(entry.getKey()), CommonTagsManager.createTagValue(entry.getValue())));
 
             MeasureMap measureMap = recorder.newMeasureMap();
             measureMap.put(measure, metricValue);

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/system/GCMetricsRecorder.java

@@ -4,7 +4,6 @@
 import com.sun.management.GcInfo;
 import io.opencensus.tags.TagContext;
 import io.opencensus.tags.TagKey;
-import io.opencensus.tags.TagValue;
 import io.opencensus.tags.Tagger;
 import lombok.extern.slf4j.Slf4j;
 import lombok.val;
@@ -14,6 +13,7 @@
 import rocks.inspectit.ocelot.config.model.metrics.MetricsSettings;
 import rocks.inspectit.ocelot.config.model.metrics.StandardMetricsSettings;
 import rocks.inspectit.ocelot.core.selfmonitoring.SelfMonitoringService;
+import rocks.inspectit.ocelot.core.tags.CommonTagsManager;
 
 import javax.management.ListenerNotFoundException;
 import javax.management.Notification;
@@ -203,8 +203,8 @@ private void recordConcurrentPhaseTime(GarbageCollectionNotificationInfo notific
         measureManager.getMeasureLong(CONCURRENT_PHASE_TIME_METRIC_FULL_NAME)
                 .ifPresent(measure -> {
                     TagContext tags = tagger.toBuilder(commonTags.getCommonTagContext())
-                            .putLocal(actionTagKey, TagValue.create(notificationInfo.getGcAction()))
-                            .putLocal(causeTagKey, TagValue.create(notificationInfo.getGcCause()))
+                            .putLocal(actionTagKey, CommonTagsManager.createTagValue(notificationInfo.getGcAction()))
+                            .putLocal(causeTagKey, CommonTagsManager.createTagValue(notificationInfo.getGcCause()))
                             .build();
 
                     recorder.newMeasureMap()
@@ -217,8 +217,8 @@ private void recordGCPause(GarbageCollectionNotificationInfo notificationInfo) {
         measureManager.getMeasureLong(PAUSE_METRIC_FULL_NAME)
                 .ifPresent(measure -> {
                     TagContext tags = tagger.toBuilder(commonTags.getCommonTagContext())
-                            .putLocal(actionTagKey, TagValue.create(notificationInfo.getGcAction()))
-                            .putLocal(causeTagKey, TagValue.create(notificationInfo.getGcCause()))
+                            .putLocal(actionTagKey, CommonTagsManager.createTagValue(notificationInfo.getGcAction()))
+                            .putLocal(causeTagKey, CommonTagsManager.createTagValue(notificationInfo.getGcCause()))
                             .build();
 
                     recorder.newMeasureMap()

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/system/MemoryMetricsRecorder.java

@@ -2,12 +2,12 @@
 
 import io.opencensus.tags.TagContext;
 import io.opencensus.tags.TagKey;
-import io.opencensus.tags.TagValue;
 import io.opencensus.tags.Tagger;
 import lombok.val;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
 import rocks.inspectit.ocelot.config.model.metrics.MetricsSettings;
+import rocks.inspectit.ocelot.core.tags.CommonTagsManager;
 
 import java.lang.management.BufferPoolMXBean;
 import java.lang.management.ManagementFactory;
@@ -73,8 +73,8 @@ private void recordMemoryMetrics(Map<String, Boolean> enabledMetrics) {
             for (MemoryPoolMXBean memoryPoolBean : ManagementFactory.getPlatformMXBeans(MemoryPoolMXBean.class)) {
                 String area = MemoryType.HEAP.equals(memoryPoolBean.getType()) ? "heap" : "nonheap";
                 TagContext tags = tagger.currentBuilder()
-                        .putLocal(idTagKey, TagValue.create(memoryPoolBean.getName()))
-                        .putLocal(areaTagKey, TagValue.create(area))
+                        .putLocal(idTagKey, CommonTagsManager.createTagValue(memoryPoolBean.getName()))
+                        .putLocal(areaTagKey, CommonTagsManager.createTagValue(area))
                         .build();
 
                 val mm = recorder.newMeasureMap();
@@ -104,7 +104,7 @@ private void recordBufferMetrics(Map<String, Boolean> enabledMetrics) {
         if (bufferCountEnabled || bufferUsedEnabled || bufferCapacityEnabled) {
             for (BufferPoolMXBean bufferPoolBean : ManagementFactory.getPlatformMXBeans(BufferPoolMXBean.class)) {
                 TagContext tags = tagger.currentBuilder()
-                        .putLocal(idTagKey, TagValue.create(bufferPoolBean.getName()))
+                        .putLocal(idTagKey, CommonTagsManager.createTagValue(bufferPoolBean.getName()))
                         .build();
 
                 val mm = recorder.newMeasureMap();

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/system/ThreadMetricsRecorder.java

@@ -2,12 +2,12 @@
 
 import io.opencensus.tags.TagContextBuilder;
 import io.opencensus.tags.TagKey;
-import io.opencensus.tags.TagValue;
 import io.opencensus.tags.Tagger;
 import lombok.val;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
 import rocks.inspectit.ocelot.config.model.metrics.MetricsSettings;
+import rocks.inspectit.ocelot.core.tags.CommonTagsManager;
 
 import java.lang.management.ManagementFactory;
 import java.lang.management.ThreadInfo;
@@ -80,7 +80,7 @@ private void recordStateMetric() {
         measureManager.getMeasureLong(METRIC_NAME_PREFIX + STATE_METRIC_NAME)
                 .ifPresent(measure -> {
                     for (val state : Thread.State.values()) {
-                        TagContextBuilder contextBuilder = tagger.currentBuilder().putLocal(stateTag, TagValue.create(state.name()));
+                        TagContextBuilder contextBuilder = tagger.currentBuilder().putLocal(stateTag, CommonTagsManager.createTagValue(state.name()));
                         try (val scope = contextBuilder.buildScoped()) {
                             val mm = recorder.newMeasureMap();
                             val count = Arrays.stream(threadBean.getThreadInfo(threadBean.getAllThreadIds()))

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/selfmonitoring/SelfMonitoringService.java

@@ -3,7 +3,6 @@
 import io.opencensus.common.Scope;
 import io.opencensus.stats.StatsRecorder;
 import io.opencensus.tags.TagKey;
-import io.opencensus.tags.TagValue;
 import io.opencensus.tags.Tags;
 import lombok.Data;
 import lombok.extern.slf4j.Slf4j;
@@ -161,7 +160,7 @@ public void close() {
                     statsRecorder.newMeasureMap()
                             .put(m, durationInMicros)
                             .record(Tags.getTagger().toBuilder(commonTags.getCommonTagContext())
-                                    .putLocal(COMPONENT_TAG_KEY, TagValue.create(componentName)).build())
+                                    .putLocal(COMPONENT_TAG_KEY, CommonTagsManager.createTagValue(componentName)).build())
             );
 
             if (log.isTraceEnabled()) {

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/tags/CommonTagsManager.java

@@ -3,6 +3,7 @@
 import io.opencensus.common.Scope;
 import io.opencensus.tags.*;
 import lombok.Getter;
+import lombok.extern.slf4j.Slf4j;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.event.EventListener;
 import org.springframework.core.Ordered;
@@ -20,6 +21,7 @@
  * Component that provides tags that should be considered as common and used when ever a metric is recorded.
  */
 @Component
+@Slf4j
 public class CommonTagsManager {
 
     /**
@@ -97,7 +99,7 @@ public Scope withCommonTagScope(Map<String, String> customTagMap) {
         HashMap<String, String> tags = new HashMap<>(commonTagValueMap);
         tags.putAll(customTagMap);
         tags.entrySet().stream()
-                .forEach(entry -> tagContextBuilder.putLocal(TagKey.create(entry.getKey()), TagValue.create(entry.getValue())));
+                .forEach(entry -> tagContextBuilder.putLocal(TagKey.create(entry.getKey()), createTagValue(entry.getValue())));
         return tagContextBuilder.buildScoped();
     }
 
@@ -123,11 +125,26 @@ private void update() {
         newCommonTagValueMap.forEach((k, v) -> {
             TagKey key = TagKey.create(k);
             newCommonTagKeys.add(key);
-            tagContextBuilder.putLocal(key, TagValue.create(v));
+            tagContextBuilder.putLocal(key, createTagValue(v));
         });
         commonTagKeys = newCommonTagKeys;
         commonTagValueMap = newCommonTagValueMap;
         commonTagContext = tagContextBuilder.build();
     }
 
+    /**
+     * Constructs a {@code io.opencensus.tags.TagValue} from the given string.
+     * If String is not valid it an <code>&lt;invalid&gt;</code> TagName is created.
+     *
+     * @param v the tag value
+     * @return the created TagValue with 'v' or '&lt;invalid&gt;'
+     */
+    public static TagValue createTagValue(String v) {
+        try {
+            return TagValue.create(v);
+        } catch (IllegalArgumentException e) {
+            log.warn("illegal tag value {} converted to <invalid>", v);
+            return TagValue.create("<invalid>");
+        }
+    }
 }