Skip to content

Commit

Permalink
Remove LongAdder and ClassValue usages from instrumentation-api (open…
Browse files Browse the repository at this point in the history
  • Loading branch information
Mateusz Rzeszutek authored and RashmiRam committed May 23, 2022
1 parent 34793da commit cb52ddc
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@

import io.opentelemetry.instrumentation.api.caching.Cache;
import io.opentelemetry.instrumentation.api.field.VirtualField;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import javax.annotation.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -39,18 +37,17 @@ public static VirtualFieldSupplier get() {
return instance;
}

private static final class CacheBasedVirtualFieldSupplier
extends ClassValue<Map<Class<?>, VirtualField<?, ?>>> implements VirtualFieldSupplier {
private static final class CacheBasedVirtualFieldSupplier implements VirtualFieldSupplier {

private final Cache<Class<?>, Cache<Class<?>, VirtualField<?, ?>>>
ownerToFieldToImplementationMap = Cache.builder().setWeakKeys().build();

@Override
public <U extends T, T, F> VirtualField<U, F> find(Class<T> type, Class<F> fieldType) {
return (VirtualField<U, F>)
get(type).computeIfAbsent(fieldType, k -> new CacheBasedVirtualField<>());
}

@Override
protected Map<Class<?>, VirtualField<?, ?>> computeValue(Class<?> type) {
return new ConcurrentHashMap<>();
ownerToFieldToImplementationMap
.computeIfAbsent(type, c -> Cache.builder().setWeakKeys().build())
.computeIfAbsent(fieldType, c -> new CacheBasedVirtualField<>());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.LongAdder;
import java.util.concurrent.atomic.AtomicLong;
import java.util.function.Consumer;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -22,7 +22,7 @@ public final class SupportabilityMetrics {
private final Consumer<String> reporter;

private final ConcurrentMap<String, KindCounters> suppressionCounters = new ConcurrentHashMap<>();
private final ConcurrentMap<String, LongAdder> counters = new ConcurrentHashMap<>();
private final ConcurrentMap<String, AtomicLong> counters = new ConcurrentHashMap<>();

private static final SupportabilityMetrics INSTANCE =
new SupportabilityMetrics(Config.get(), logger::debug).start();
Expand Down Expand Up @@ -52,7 +52,7 @@ public void incrementCounter(String counterName) {
return;
}

counters.computeIfAbsent(counterName, k -> new LongAdder()).increment();
counters.computeIfAbsent(counterName, k -> new AtomicLong()).incrementAndGet();
}

// visible for testing
Expand All @@ -69,7 +69,7 @@ void report() {
});
counters.forEach(
(counterName, counter) -> {
long value = counter.sumThenReset();
long value = counter.getAndSet(0);
if (value > 0) {
reporter.accept("Counter '" + counterName + "' : " + value);
}
Expand Down Expand Up @@ -100,44 +100,44 @@ private CounterNames() {}

// this class is threadsafe.
private static class KindCounters {
private final LongAdder server = new LongAdder();
private final LongAdder client = new LongAdder();
private final LongAdder internal = new LongAdder();
private final LongAdder consumer = new LongAdder();
private final LongAdder producer = new LongAdder();
private final AtomicLong server = new AtomicLong();
private final AtomicLong client = new AtomicLong();
private final AtomicLong internal = new AtomicLong();
private final AtomicLong consumer = new AtomicLong();
private final AtomicLong producer = new AtomicLong();

void increment(SpanKind kind) {
switch (kind) {
case INTERNAL:
internal.increment();
internal.incrementAndGet();
break;
case SERVER:
server.increment();
server.incrementAndGet();
break;
case CLIENT:
client.increment();
client.incrementAndGet();
break;
case PRODUCER:
producer.increment();
producer.incrementAndGet();
break;
case CONSUMER:
consumer.increment();
consumer.incrementAndGet();
break;
}
}

long getAndReset(SpanKind kind) {
switch (kind) {
case INTERNAL:
return internal.sumThenReset();
return internal.getAndSet(0);
case SERVER:
return server.sumThenReset();
return server.getAndSet(0);
case CLIENT:
return client.sumThenReset();
return client.getAndSet(0);
case PRODUCER:
return producer.sumThenReset();
return producer.getAndSet(0);
case CONSUMER:
return consumer.sumThenReset();
return consumer.getAndSet(0);
}
return 0;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,32 +5,32 @@

package io.opentelemetry.instrumentation.api.tracer;

import io.opentelemetry.instrumentation.api.caching.Cache;

public final class ClassNames {

private static final ClassValue<String> simpleNames =
new ClassValue<String>() {
@Override
protected String computeValue(Class<?> type) {
if (!type.isAnonymousClass()) {
return type.getSimpleName();
}
String className = type.getName();
if (type.getPackage() != null) {
String pkgName = type.getPackage().getName();
if (!pkgName.isEmpty()) {
className = className.substring(pkgName.length() + 1);
}
}
return className;
}
};
private static final Cache<Class<?>, String> simpleNames = Cache.builder().setWeakKeys().build();

/**
* This method is used to generate a simple name based on a given class reference, e.g. for use in
* span names and span attributes. Anonymous classes are named based on their parent.
*/
public static String simpleName(Class<?> type) {
return simpleNames.get(type);
return simpleNames.computeIfAbsent(type, ClassNames::computeSimpleName);
}

private static String computeSimpleName(Class<?> type) {
if (!type.isAnonymousClass()) {
return type.getSimpleName();
}
String className = type.getName();
if (type.getPackage() != null) {
String pkgName = type.getPackage().getName();
if (!pkgName.isEmpty()) {
className = className.substring(pkgName.length() + 1);
}
}
return className;
}

private ClassNames() {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

package io.opentelemetry.instrumentation.api.tracer;

import io.opentelemetry.instrumentation.api.caching.Cache;
import io.opentelemetry.instrumentation.api.util.ClassAndMethod;
import java.lang.reflect.Method;
import java.util.Map;
Expand All @@ -13,14 +14,8 @@

public final class SpanNames {

private static final ClassValue<Map<String, String>> spanNameCaches =
new ClassValue<Map<String, String>>() {
@Override
protected Map<String, String> computeValue(Class<?> clazz) {
// the cache is naturally bounded by the number of methods in a class
return new ConcurrentHashMap<>();
}
};
private static final Cache<Class<?>, Map<String, String>> spanNameCaches =
Cache.builder().setWeakKeys().build();

/**
* This method is used to generate a span name based on a method. Anonymous classes are named
Expand Down Expand Up @@ -51,7 +46,10 @@ public static String fromMethod(ClassAndMethod classAndMethod) {
* based on their parent.
*/
public static String fromMethod(Class<?> clazz, String methodName) {
Map<String, String> spanNameCache = spanNameCaches.get(clazz);
// the cache (ConcurrentHashMap) is naturally bounded by the number of methods in a class
Map<String, String> spanNameCache =
spanNameCaches.computeIfAbsent(clazz, c -> new ConcurrentHashMap<>());

// not using computeIfAbsent, because it would require a capturing (allocating) lambda
String spanName = spanNameCache.get(methodName);
if (spanName != null) {
Expand Down

0 comments on commit cb52ddc

Please sign in to comment.