Skip to content

Commit

Permalink
Closes #1048 - Extend the 'illegal tag value' error message, to speci…
Browse files Browse the repository at this point in the history 
…fy which tag is concerned (#1052)
  • Loading branch information
@Mahir-Isikli
Mahir-Isikli authored Apr 29, 2021
1 parent ed43709 commit baedd37
Show file tree
Hide file tree
Showing 10 changed files with 87 additions and 89 deletions.
47 changes: 21 additions & 26 deletions ...c/main/java/rocks/inspectit/ocelot/core/instrumentation/context/InspectitContextImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,7 @@ public class InspectitContextImpl implements InternalInspectitContext {
/**
* 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
));
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");

Expand Down Expand Up @@ -208,6 +205,7 @@ private InspectitContextImpl(InspectitContextImpl parent, PropagationMetaData de
* @param commonTags the common tags used to populate the data if this is a root context
* @param defaultPropagation the data propagation settings to use if this is a root context. Otherwise the parent context's settings will be inherited.
* @param interactWithApplicationTagContexts if true, data from the currently active {@link TagContext} will be inherited and makeActive will publish the data as a TagContext
*
* @return the newly created context
*/
public static InspectitContextImpl createFromCurrent(Map<String, String> commonTags, PropagationMetaData defaultPropagation, boolean interactWithApplicationTagContexts) {
Expand All @@ -225,7 +223,6 @@ public static InspectitContextImpl createFromCurrent(Map<String, String> commonT
return result;
}


public void setSpanScope(AutoCloseable spanScope) {
currentSpanScope = spanScope;
}
Expand Down Expand Up @@ -315,8 +312,7 @@ public boolean isInActiveOrExitPhase() {
* inherit the {@link #postEntryPhaseDownPropagatedData} from its parent.
*/
private boolean isInDifferentThreadThanParentOrIsParentClosed() {
return parent != null &&
(parent.openingThread != openingThread || !parent.isInActiveOrExitPhase());
return parent != null && (parent.openingThread != openingThread || !parent.isInActiveOrExitPhase());
}

/**
Expand All @@ -330,14 +326,13 @@ private boolean isInDifferentThreadThanParentOrIsParentClosed() {
*/
public Scope enterFullTagScope() {
TagContextBuilder builder = Tags.getTagger().emptyBuilder();
dataTagsStream()
.forEach(e -> builder.putLocal(TagKey.create(e.getKey()), TagUtils.createTagValue(e.getValue().toString())));
dataTagsStream().forEach(e -> builder.putLocal(TagKey.create(e.getKey()), TagUtils.createTagValue(e.getKey(), e.getValue()
.toString())));
return builder.buildScoped();
}

private Stream<Map.Entry<String, Object>> dataTagsStream() {
return getDataAsStream()
.filter(e -> propagation.isTag(e.getKey()))
return getDataAsStream().filter(e -> propagation.isTag(e.getKey()))
.filter(e -> ALLOWED_TAG_TYPES.contains(e.getValue().getClass()));
}

Expand All @@ -346,6 +341,7 @@ private Stream<Map.Entry<String, Object>> dataTagsStream() {
* set via {@link #setData(String, Object)} or changed due to an up-propagation.
*
* @param key the name of the data to query
*
* @return the data element which is related to the given key or `null` if it doesn't exist
*/
@Override
Expand All @@ -371,7 +367,6 @@ public void setData(String key, Object value) {
dataOverwrites.put(key, value);
}


/**
* Closes this context.
* If any {@link TagContext} was opened during {@link #makeActive()}, this context is also closed.
Expand Down Expand Up @@ -431,18 +426,14 @@ public Map<String, String> getDownPropagationHeaders() {
spanContext = null;
}
}
return ContextPropagationUtil.buildPropagationHeaderMap(
getDataAsStream()
.filter(e -> propagation.isPropagatedDownGlobally(e.getKey())),
spanContext
);
return ContextPropagationUtil.buildPropagationHeaderMap(getDataAsStream().filter(e -> propagation.isPropagatedDownGlobally(e
.getKey())), spanContext);
}

@Override
public Map<String, String> getUpPropagationHeaders() {
return ContextPropagationUtil.buildPropagationHeaderMap(
getDataAsStream()
.filter(e -> propagation.isPropagatedUpGlobally(e.getKey())));
return ContextPropagationUtil.buildPropagationHeaderMap(getDataAsStream().filter(e -> propagation.isPropagatedUpGlobally(e
.getKey())));
}

@Override
Expand Down Expand Up @@ -510,6 +501,7 @@ private void readOverridesFromCurrentTagContext() {
*
* @param tagKey the key of the found tag
* @param existingBuilder an existing builder to which the settings shall be added. If it is null, a builder is created using copy() on {@link #propagation}.
*
* @return exisitingBuilder or the newly created builder if it was null.
*/
private PropagationMetaData.Builder configureTagPropagation(String tagKey, PropagationMetaData.Builder existingBuilder) {
Expand All @@ -529,10 +521,11 @@ private PropagationMetaData.Builder configureTagPropagation(String tagKey, Propa
}

private Stream<Map.Entry<String, Object>> getDataAsStream() {
return Stream.concat(
postEntryPhaseDownPropagatedData.entrySet().stream().filter(e -> !dataOverwrites.containsKey(e.getKey())),
dataOverwrites.entrySet().stream().filter(e -> e.getValue() != null)
);
return Stream.concat(postEntryPhaseDownPropagatedData.entrySet()
.stream()
.filter(e -> !dataOverwrites.containsKey(e.getKey())), dataOverwrites.entrySet()
.stream()
.filter(e -> e.getValue() != null));
}

private Map<String, Object> getOrComputeActivePhaseDownPropagatedData() {
Expand Down Expand Up @@ -561,10 +554,12 @@ private HashMap<String, Object> getDownPropagatedDataAsNewMap() {
}

private Iterator<Tag> getPostEntryPhaseTags() {
return postEntryPhaseDownPropagatedData.entrySet().stream()
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()), TagUtils.createTagValue(e.getValue().toString()), TagMetadata.create(TagMetadata.TagTtl.UNLIMITED_PROPAGATION)))
.map(e -> Tag.create(TagKey.create(e.getKey()), TagUtils.createTagValue(e.getKey(), e.getValue()
.toString()), TagMetadata.create(TagMetadata.TagTtl.UNLIMITED_PROPAGATION)))
.iterator();
}

Expand Down
9 changes: 5 additions & 4 deletions ...c/main/java/rocks/inspectit/ocelot/core/instrumentation/hook/actions/MetricsRecorder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,18 @@ 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, TagUtils.createTagValue(value.toString())))
);
.ifPresent(value -> builder.putLocal(commonTagKey, TagUtils.createTagValue(commonTagKey.getName(), value
.toString()))));

// then constant tags to allow overwrite by data
metricAccessor.getConstantTags()
.forEach((key, value) -> builder.putLocal(TagKey.create(key), TagUtils.createTagValue(value)));
.forEach((key, value) -> builder.putLocal(TagKey.create(key), TagUtils.createTagValue(key, 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), TagUtils.createTagValue(tagValue.toString()))));
.ifPresent(tagValue -> builder.putLocal(TagKey.create(key), TagUtils.createTagValue(key, tagValue
.toString()))));

// build and return
return builder.build();
Expand Down
17 changes: 8 additions & 9 deletions ...ocelot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/jmx/JmxMetricsRecorder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,17 +124,20 @@ public void recordBean(String domain, LinkedHashMap<String, String> beanProperti
metricValue(value).ifPresent(metricValue -> {
String metricName = metricName(domain, beanProperties, attrKeys, attrName);
Measure.MeasureDouble measure = measureManager.getMeasureDouble(metricName).orElseGet(() -> {
Map<String, Boolean> tags = beanProperties.keySet().stream()
Map<String, Boolean> tags = beanProperties.keySet()
.stream()
.skip(1)
.collect(Collectors.toMap(Function.identity(), k -> true));

return registerMeasure(metricName, attrDescription, tags);
});

TagContextBuilder tagContextBuilder = tagger.currentBuilder();
beanProperties.entrySet().stream()
beanProperties.entrySet()
.stream()
.skip(1)
.forEach(entry -> tagContextBuilder.putLocal(TagKey.create(entry.getKey()), TagUtils.createTagValue(entry.getValue())));
.forEach(entry -> tagContextBuilder.putLocal(TagKey.create(entry.getKey()), TagUtils.createTagValue(entry
.getKey(), entry.getValue())));

measureManager.tryRecordingMeasurement(measure.getName(), metricValue, tagContextBuilder.build());
});
Expand All @@ -145,10 +148,7 @@ private Measure.MeasureDouble registerMeasure(String metricName, String attrDesc
MetricDefinitionSettings definitionSettingsWithLastValueView = MetricDefinitionSettings.builder()
.description(attrDescription)
.unit("na")
.view(metricName, ViewDefinitionSettings.builder()
.tags(tags)
.build()
)
.view(metricName, ViewDefinitionSettings.builder().tags(tags).build())
.build()
.getCopyWithDefaultsPopulated(metricName);

Expand All @@ -165,8 +165,7 @@ private Measure.MeasureDouble registerMeasure(String metricName, String attrDesc
*/
private Optional<Double> metricValue(Object value) {
if (value instanceof Number) {
return Optional.of(((Number) value).doubleValue())
.filter(d -> d >= 0d);
return Optional.of(((Number) value).doubleValue()).filter(d -> d >= 0d);
} else if (value instanceof Boolean) {
return Optional.of(((Boolean) value) ? 1d : 0d);
} else {
Expand Down
22 changes: 15 additions & 7 deletions ...elot-core/src/main/java/rocks/inspectit/ocelot/core/metrics/system/GCMetricsRecorder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,33 +34,43 @@ public class GCMetricsRecorder extends AbstractMetricsRecorder {
private static final String METRIC_NAME_PREFIX = "jvm/gc/";

private static final String CONCURRENT_PHASE_TIME_METRIC_NAME = "concurrent.phase.time";

private static final String CONCURRENT_PHASE_TIME_METRIC_FULL_NAME = METRIC_NAME_PREFIX + "concurrent/phase/time";

private static final String PAUSE_METRIC_NAME = "pause";

private static final String PAUSE_METRIC_FULL_NAME = METRIC_NAME_PREFIX + "pause";

private static final String MEMORY_PROMOTED_METRIC_NAME = "memory.promoted";

private static final String MEMORY_PROMOTED_METRIC_FULL_NAME = METRIC_NAME_PREFIX + "memory/promoted";

private static final String MAX_DATA_SIZE_METRIC_NAME = "max.data.size";

private static final String MAX_DATA_SIZE_METRIC_FULL_NAME = METRIC_NAME_PREFIX + "max/data/size";

private static final String LIVE_DATA_SIZE_METRIC_NAME = "live.data.size";

private static final String LIVE_DATA_SIZE_METRIC_FULL_NAME = METRIC_NAME_PREFIX + "live/data/size";

private static final String MEMORY_ALLOCATED_METRIC_NAME = "memory.allocated";

private static final String MEMORY_ALLOCATED_METRIC_FULL_NAME = METRIC_NAME_PREFIX + "memory/allocated";

private static final boolean MANAGEMENT_EXTENSIONS_PRESENT = isManagementExtensionsPresent();

private final NotificationListener notificationListener = this::handleNotification;

private StandardMetricsSettings config;

private final TagKey actionTagKey = TagKey.create("action");

private final TagKey causeTagKey = TagKey.create("cause");

private String youngGenPoolName;

private String oldGenPoolName;

private long youngGenSizeAfter = 0L;

@Autowired
Expand Down Expand Up @@ -201,17 +211,17 @@ private void recordYoungGenSizes(Map<String, MemoryUsage> before, Map<String, Me

private void recordConcurrentPhaseTime(GarbageCollectionNotificationInfo notificationInfo) {
TagContext tags = tagger.toBuilder(commonTags.getCommonTagContext())
.putLocal(actionTagKey, TagUtils.createTagValue(notificationInfo.getGcAction()))
.putLocal(causeTagKey, TagUtils.createTagValue(notificationInfo.getGcCause()))
.putLocal(actionTagKey, TagUtils.createTagValue(actionTagKey.getName(), notificationInfo.getGcAction()))
.putLocal(causeTagKey, TagUtils.createTagValue(causeTagKey.getName(), notificationInfo.getGcCause()))
.build();
measureManager.tryRecordingMeasurement(CONCURRENT_PHASE_TIME_METRIC_FULL_NAME, notificationInfo.getGcInfo()
.getDuration(), tags);
}

private void recordGCPause(GarbageCollectionNotificationInfo notificationInfo) {
TagContext tags = tagger.toBuilder(commonTags.getCommonTagContext())
.putLocal(actionTagKey, TagUtils.createTagValue(notificationInfo.getGcAction()))
.putLocal(causeTagKey, TagUtils.createTagValue(notificationInfo.getGcCause()))
.putLocal(actionTagKey, TagUtils.createTagValue(actionTagKey.getName(), notificationInfo.getGcAction()))
.putLocal(causeTagKey, TagUtils.createTagValue(causeTagKey.getName(), notificationInfo.getGcCause()))
.build();
measureManager.tryRecordingMeasurement(PAUSE_METRIC_FULL_NAME, notificationInfo.getGcInfo()
.getDuration(), tags);
Expand Down Expand Up @@ -259,9 +269,7 @@ private boolean isYoungGenPool(String name) {
* Generalization of which parts of the heap are considered "young" or "old" for multiple GC implementations
*/
enum GcGenerationAge {
OLD,
YOUNG,
UNKNOWN;
OLD, YOUNG, UNKNOWN;

private static Map<String, GcGenerationAge> knownCollectors = new HashMap<String, GcGenerationAge>() {{
put("ConcurrentMarkSweep", OLD);
Expand Down
6 changes: 3 additions & 3 deletions ...-core/src/main/java/rocks/inspectit/ocelot/core/metrics/system/MemoryMetricsRecorder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,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, TagUtils.createTagValue(memoryPoolBean.getName()))
.putLocal(areaTagKey, TagUtils.createTagValue(area))
.putLocal(idTagKey, TagUtils.createTagValue(idTagKey.getName(), memoryPoolBean.getName()))
.putLocal(areaTagKey, TagUtils.createTagValue(areaTagKey.getName(), area))
.build();

if (usedEnabled) {
Expand Down Expand Up @@ -110,7 +110,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, TagUtils.createTagValue(bufferPoolBean.getName()))
.putLocal(idTagKey, TagUtils.createTagValue(idTagKey.getName(), bufferPoolBean.getName()))
.build();

if (bufferCountEnabled) {
Expand Down
13 changes: 6 additions & 7 deletions ...-core/src/main/java/rocks/inspectit/ocelot/core/metrics/system/ThreadMetricsRecorder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,13 @@ public ThreadMetricsRecorder() {
protected void takeMeasurement(MetricsSettings config) {
val enabled = config.getThreads().getEnabled();
if (enabled.getOrDefault(PEAK_METRIC_NAME, false)) {
measureManager.tryRecordingMeasurement(METRIC_NAME_PREFIX + PEAK_METRIC_NAME,
threadBean.getPeakThreadCount());
measureManager.tryRecordingMeasurement(METRIC_NAME_PREFIX + PEAK_METRIC_NAME, threadBean.getPeakThreadCount());
}
if (enabled.getOrDefault(DAEMON_METRIC_NAME, false)) {
measureManager.tryRecordingMeasurement(METRIC_NAME_PREFIX + DAEMON_METRIC_NAME,
threadBean.getDaemonThreadCount());
measureManager.tryRecordingMeasurement(METRIC_NAME_PREFIX + DAEMON_METRIC_NAME, threadBean.getDaemonThreadCount());
}
if (enabled.getOrDefault(LIVE_METRIC_NAME, false)) {
measureManager.tryRecordingMeasurement(METRIC_NAME_PREFIX + LIVE_METRIC_NAME,
threadBean.getThreadCount());
measureManager.tryRecordingMeasurement(METRIC_NAME_PREFIX + LIVE_METRIC_NAME, threadBean.getThreadCount());
}
if (enabled.getOrDefault(STATE_METRIC_NAME, false)) {
recordStateMetric();
Expand All @@ -77,7 +74,9 @@ protected boolean checkEnabledForConfig(MetricsSettings ms) {
private void recordStateMetric() {
String stateMeasureName = METRIC_NAME_PREFIX + STATE_METRIC_NAME;
for (Thread.State state : Thread.State.values()) {
TagContext tags = tagger.currentBuilder().putLocal(stateTag, TagUtils.createTagValue(state.name())).build();
TagContext tags = tagger.currentBuilder()
.putLocal(stateTag, TagUtils.createTagValue(stateTag.getName(), state.name()))
.build();
long count = Arrays.stream(threadBean.getThreadInfo(threadBean.getAllThreadIds()))
.filter(Objects::nonNull)
.map(ThreadInfo::getThreadState)
Expand Down
Loading

0 comments on commit baedd37

Please # to comment.