Skip to content

Commit

Permalink
Closes #391 - Handle invalid metric tags (#668)
Browse files Browse the repository at this point in the history
  • Loading branch information
arolfes authored Apr 21, 2020
1 parent 8061017 commit 4481d0b
Show file tree
Hide file tree
Showing 15 changed files with 204 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import io.opencensus.common.Scope;
import io.opencensus.tags.TagContextBuilder;
import io.opencensus.tags.TagKey;
import io.opencensus.tags.TagValue;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;
Expand All @@ -14,6 +13,7 @@
import rocks.inspectit.oce.eum.server.configuration.model.BeaconMetricDefinitionSettings;
import rocks.inspectit.oce.eum.server.configuration.model.BeaconRequirement;
import rocks.inspectit.oce.eum.server.configuration.model.EumServerConfiguration;
import rocks.inspectit.oce.eum.server.utils.TagUtils;

import java.util.HashMap;
import java.util.List;
Expand Down Expand Up @@ -104,7 +104,7 @@ private TagContextBuilder getTagContextForBeacon(Beacon beacon) {
TagContextBuilder tagContextBuilder = measuresAndViewsManager.getTagContext();
for (String key : configuration.getTags().getBeacon().keySet()) {
if (beacon.contains(key)) {
tagContextBuilder.putLocal(TagKey.create(key), TagValue.create(beacon.get(key)));
tagContextBuilder.putLocal(TagKey.create(key), TagUtils.createTagValue(beacon.get(key)));
}
}
return tagContextBuilder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
import io.opencensus.stats.*;
import io.opencensus.tags.TagContextBuilder;
import io.opencensus.tags.TagKey;
import io.opencensus.tags.TagValue;
import io.opencensus.tags.Tags;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;
import rocks.inspectit.oce.eum.server.configuration.model.EumServerConfiguration;
import rocks.inspectit.oce.eum.server.utils.TagUtils;
import rocks.inspectit.ocelot.config.model.metrics.definition.MetricDefinitionSettings;
import rocks.inspectit.ocelot.config.model.metrics.definition.ViewDefinitionSettings;

Expand Down Expand Up @@ -105,7 +105,6 @@ private void updateViews(String metricName, MetricDefinitionSettings metricDefin

/**
* Returns all tags, which are exposed for the given metricDefinition
*
*/
private Set<String> getTagsForView(ViewDefinitionSettings viewDefinitionSettings) {
Set<String> tags = new HashSet<>(configuration.getTags().getDefineAsGlobal());
Expand All @@ -123,7 +122,7 @@ public TagContextBuilder getTagContext() {
TagContextBuilder tagContextBuilder = Tags.getTagger().currentBuilder();

for (Map.Entry<String, String> extraTag : configuration.getTags().getExtra().entrySet()) {
tagContextBuilder.putLocal(TagKey.create(extraTag.getKey()), TagValue.create(extraTag.getValue()));
tagContextBuilder.putLocal(TagKey.create(extraTag.getKey()), TagUtils.createTagValue(extraTag.getValue()));
}

return tagContextBuilder;
Expand All @@ -139,15 +138,14 @@ public TagContextBuilder getTagContext(Map<String, String> customTags) {
TagContextBuilder tagContextBuilder = getTagContext();

for (Map.Entry<String, String> customTag : customTags.entrySet()) {
tagContextBuilder.putLocal(TagKey.create(customTag.getKey()), TagValue.create(customTag.getValue()));
tagContextBuilder.putLocal(TagKey.create(customTag.getKey()), TagUtils.createTagValue(customTag.getValue()));
}

return tagContextBuilder;
}

/**
* Creates an aggregation depending on the given {@link Aggregation}
*
*/
private static Aggregation createAggregation(ViewDefinitionSettings viewDefinitionSettings) {
switch (viewDefinitionSettings.getAggregation()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package rocks.inspectit.oce.eum.server.utils;

import io.opencensus.internal.StringUtils;
import io.opencensus.tags.TagValue;
import lombok.extern.slf4j.Slf4j;

@Slf4j
public final class TagUtils {

private static boolean isWarningPrinted = false;

private TagUtils() {
// empty private default constructor for util class
}

/**
* Constructs a {@code io.opencensus.tags.TagValue} from the given string.
* If String is not valid 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) {
if (isTagValueValid(v)) {
return TagValue.create(v);
}
printWarningOnce();
return TagValue.create("<invalid>");
}

private static boolean isTagValueValid(String value) {
return value.length() <= TagValue.MAX_LENGTH && StringUtils.isPrintableString(value);
}

private static void printWarningOnce() {
if (!isWarningPrinted) {
log.warn("illegal tag value converted to <invalid>");
isWarningPrinted = true;
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package rocks.inspectit.oce.eum.server.utils;

import io.opencensus.tags.TagValue;
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;

public class TagUtilsTest {

@Test
public void createTagValue() {
assertThat(TagUtils.createTagValue("my-tag-value")).isEqualTo(TagValue.create("my-tag-value"));
}

@Test
public void createTagValue_tooLong() {
assertThat(TagUtils.createTagValue("this-value-is-over-255-characters-long ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------"))
.isEqualTo(TagValue.create("<invalid>"));
}

@Test
public void createTagValue_nonPrintableCharacter() {
assertThat(TagUtils.createTagValue("non-printable-character-\u007f"))
.isEqualTo(TagValue.create("<invalid>"));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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.TagUtils;

import java.util.*;
import java.util.stream.Stream;
Expand Down Expand Up @@ -346,7 +347,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()), TagUtils.createTagValue(e.getValue().toString())));
return builder.buildScoped();
}

Expand Down Expand Up @@ -579,7 +580,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()), TagMetadata.create(TagMetadata.TagTtl.UNLIMITED_PROPAGATION)))
.map(e -> Tag.create(TagKey.create(e.getKey()), TagUtils.createTagValue(e.getValue().toString()), TagMetadata.create(TagMetadata.TagTtl.UNLIMITED_PROPAGATION)))
.iterator();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@

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;
import rocks.inspectit.ocelot.core.instrumentation.hook.actions.model.MetricAccessor;
import rocks.inspectit.ocelot.core.metrics.MeasuresAndViewsManager;
import rocks.inspectit.ocelot.core.tags.CommonTagsManager;
import rocks.inspectit.ocelot.core.tags.TagUtils;

import java.util.List;
import java.util.Optional;
Expand Down Expand Up @@ -67,17 +71,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, TagUtils.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), TagUtils.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), TagUtils.createTagValue(tagValue.toString()))));

// build and return
return builder.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -20,6 +19,7 @@
import rocks.inspectit.ocelot.core.metrics.MeasuresAndViewsManager;
import rocks.inspectit.ocelot.core.metrics.system.AbstractPollingMetricsRecorder;
import rocks.inspectit.ocelot.core.tags.CommonTagsManager;
import rocks.inspectit.ocelot.core.tags.TagUtils;

import javax.management.ObjectName;
import java.time.Duration;
Expand Down Expand Up @@ -72,9 +72,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;
}

/**
Expand All @@ -85,8 +85,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);
Expand Down Expand Up @@ -137,7 +137,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()), TagUtils.createTagValue(entry.getValue())));

MeasureMap measureMap = recorder.newMeasureMap();
measureMap.put(measure, metricValue);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.TagUtils;

import javax.management.ListenerNotFoundException;
import javax.management.Notification;
Expand Down Expand Up @@ -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, TagUtils.createTagValue(notificationInfo.getGcAction()))
.putLocal(causeTagKey, TagUtils.createTagValue(notificationInfo.getGcCause()))
.build();

recorder.newMeasureMap()
Expand All @@ -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, TagUtils.createTagValue(notificationInfo.getGcAction()))
.putLocal(causeTagKey, TagUtils.createTagValue(notificationInfo.getGcCause()))
.build();

recorder.newMeasureMap()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.TagUtils;

import java.lang.management.BufferPoolMXBean;
import java.lang.management.ManagementFactory;
Expand Down Expand Up @@ -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, TagUtils.createTagValue(memoryPoolBean.getName()))
.putLocal(areaTagKey, TagUtils.createTagValue(area))
.build();

val mm = recorder.newMeasureMap();
Expand Down Expand Up @@ -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, TagUtils.createTagValue(bufferPoolBean.getName()))
.build();

val mm = recorder.newMeasureMap();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.TagUtils;

import java.lang.management.ManagementFactory;
import java.lang.management.ThreadInfo;
Expand Down Expand Up @@ -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, TagUtils.createTagValue(state.name()));
try (val scope = contextBuilder.buildScoped()) {
val mm = recorder.newMeasureMap();
val count = Arrays.stream(threadBean.getThreadInfo(threadBean.getAllThreadIds()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -17,6 +16,7 @@
import rocks.inspectit.ocelot.core.config.InspectitEnvironment;
import rocks.inspectit.ocelot.core.metrics.MeasuresAndViewsManager;
import rocks.inspectit.ocelot.core.tags.CommonTagsManager;
import rocks.inspectit.ocelot.core.tags.TagUtils;

import java.util.Collections;
import java.util.Map;
Expand Down Expand Up @@ -161,7 +161,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, TagUtils.createTagValue(componentName)).build())
);

if (log.isTraceEnabled()) {
Expand Down
Loading

0 comments on commit 4481d0b

Please sign in to comment.