Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

remove guava #482

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

remove guava #482

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 1 addition & 13 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/

plugins {
id 'me.champeau.jmh' version '0.6.8'
id "com.github.spotbugs" version "5.0.13" apply false
id 'com.netflix.nebula.dependency-recommender' version '12.0.0'
id 'com.netflix.nebula.netflixoss' version '11.1.1'
Expand All @@ -28,7 +27,6 @@ ext {
allprojects {
apply plugin: 'com.netflix.nebula.dependency-recommender'
apply plugin: 'com.netflix.nebula.netflixoss'
apply plugin: 'me.champeau.jmh'
}

subprojects {
Expand Down Expand Up @@ -68,20 +66,11 @@ subprojects {

dependencies {
api 'org.slf4j:slf4j-api'
api 'com.google.guava:guava'
implementation 'com.github.ben-manes.caffeine:caffeine:2.9.2'
api 'com.netflix.spectator:spectator-api'
testImplementation 'org.testng:testng:6.1.1'
testImplementation 'org.slf4j:slf4j-log4j12'
testImplementation 'log4j:log4j:1.2.17'
jmh 'org.slf4j:slf4j-simple'
}

jmh {
warmupIterations = 2
iterations = 10
fork = 5
profilers = ['STACK']
includes = ['.*']
}

checkstyle {
Expand All @@ -97,7 +86,6 @@ subprojects {
ignoreFailures = false
spotbugsMain.enabled = true
spotbugsTest.enabled = false
spotbugsJmh.enabled = false
}
spotbugsMain {
reports {
Expand Down
2 changes: 1 addition & 1 deletion dependencies.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ com.fasterxml.jackson.core:jackson-annotations = 2.12.1
com.fasterxml.jackson.core:jackson-core = 2.12.1
com.fasterxml.jackson.core:jackson-databind = 2.12.1
com.fasterxml.jackson.dataformat:jackson-dataformat-smile = 2.12.1
com.google.guava:guava = 19.0
com.google.guava:guava = 32.1.3-jre
com.netflix.archaius:archaius2-api = 2.3.16
com.netflix.archaius:archaius2-core = 2.3.16
com.netflix.awsobjectmapper:awsobjectmapper = 1.11.965
Expand Down
1 change: 0 additions & 1 deletion servo-atlas/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ dependencies {
api 'io.netty:netty-buffer'
api 'io.netty:netty-codec-http'
api 'io.reactivex:rxjava'
jmh project(':servo-core')
}

jar {
Expand Down
2 changes: 1 addition & 1 deletion servo-core/src/main/java/com/netflix/servo/monitor/AbstractContextualMonitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
package com.netflix.servo.monitor;

import com.google.common.base.Function;
import com.netflix.servo.tag.BasicTagList;
import com.netflix.servo.tag.TagList;
import com.netflix.servo.tag.TaggingContext;
Expand All @@ -24,6 +23,7 @@
import java.util.List;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.function.Function;

/**
* Base class used to simplify creation of contextual monitors.
Expand Down
3 changes: 2 additions & 1 deletion servo-core/src/main/java/com/netflix/servo/monitor/ContextualCounter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@
*/
package com.netflix.servo.monitor;

import com.google.common.base.Function;
import com.netflix.servo.tag.TaggingContext;

import java.util.function.Function;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue with this is that Function is part of the public API for this class. See previous discussions in #288 and #290. We would need to make sure it wouldn't break anything internally. Since servo is deprecated, we would probably focus more on moving users off instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah good point, I can look at fixing the breakages if they occur internally. I also took a quick look at who is still using servo and it's a bit intertwined into internal platform libraries which are also deprecated. I can also recommend full removal of servo but figured this would get some benefit in the interim.


/**
* Composite that maintains separate simple counters for each distinct set of tags returned by the
* tagging context.
Expand Down
2 changes: 1 addition & 1 deletion servo-core/src/main/java/com/netflix/servo/monitor/ContextualTimer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@
*/
package com.netflix.servo.monitor;

import com.google.common.base.Function;
import com.netflix.servo.tag.TaggingContext;

import java.util.concurrent.TimeUnit;
import java.util.function.Function;

/**
* Composite that maintains separate simple timers for each distinct set of tags returned by the
Expand Down
12 changes: 6 additions & 6 deletions servo-core/src/main/java/com/netflix/servo/monitor/DoubleGauge.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@
*/
package com.netflix.servo.monitor;

import com.google.common.util.concurrent.AtomicDouble;
import com.netflix.servo.SpectatorContext;
import com.netflix.servo.annotations.DataSourceType;
import com.netflix.servo.tag.TagList;
import com.netflix.spectator.api.Id;
import java.util.concurrent.atomic.AtomicReference;

/**
* A {@link Gauge} that reports a double value.
Expand All @@ -28,7 +28,7 @@ public class DoubleGauge extends AbstractMonitor<Double>
implements Gauge<Double>, SpectatorMonitor {

private final MonitorConfig baseConfig;
private final AtomicDouble number;
private final AtomicReference<Double> number;
private final SpectatorContext.LazyGauge spectatorGauge;

/**
Expand All @@ -39,7 +39,7 @@ public class DoubleGauge extends AbstractMonitor<Double>
public DoubleGauge(MonitorConfig config) {
super(config.withAdditionalTag(DataSourceType.GAUGE));
baseConfig = config;
number = new AtomicDouble(0.0);
number = new AtomicReference<>(0.0);
spectatorGauge = SpectatorContext.gauge(config);
}

Expand All @@ -52,9 +52,9 @@ public void set(Double n) {
}

/**
* Returns a reference to the {@link com.google.common.util.concurrent.AtomicDouble}.
* Returns a reference to the {@link AtomicReference}.
*/
public AtomicDouble getNumber() {
public AtomicReference<Double> getNumber() {
return number;
}

Expand Down Expand Up @@ -92,7 +92,7 @@ public boolean equals(Object o) {
@Override
public int hashCode() {
int result = config.hashCode();
final int n = Double.valueOf(number.get()).hashCode();
final int n = number.get().hashCode();
result = 31 * result + n;
return result;
}
Expand Down
35 changes: 10 additions & 25 deletions servo-core/src/main/java/com/netflix/servo/monitor/DynamicGauge.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,15 @@
*/
package com.netflix.servo.monitor;

import com.google.common.base.Throwables;
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.collect.ImmutableList;
import com.github.benmanes.caffeine.cache.Caffeine;
import com.netflix.servo.DefaultMonitorRegistry;
import com.github.benmanes.caffeine.cache.LoadingCache;
import com.netflix.servo.tag.TagList;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.ArrayList;
import java.util.Collections;

import java.util.List;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;

/**
Expand All @@ -36,7 +32,7 @@
* Gauges are automatically expired after 15 minutes of inactivity.
*/
public final class DynamicGauge implements CompositeMonitor<Long>, SpectatorMonitor {
private static final Logger LOGGER = LoggerFactory.getLogger(DynamicGauge.class);

private static final String DEFAULT_EXPIRATION = "15";
private static final String DEFAULT_EXPIRATION_UNIT = "MINUTES";
private static final String CLASS_NAME = DynamicGauge.class.getCanonicalName();
Expand All @@ -49,23 +45,17 @@ public final class DynamicGauge implements CompositeMonitor<Long>, SpectatorMoni
private static final DynamicGauge INSTANCE = new DynamicGauge();

private final LoadingCache<MonitorConfig, DoubleGauge> gauges;
private final CompositeMonitor<?> cacheMonitor;

private DynamicGauge() {
final String expiration = System.getProperty(EXPIRATION_PROP, DEFAULT_EXPIRATION);
final String expirationUnit = System.getProperty(EXPIRATION_PROP_UNIT, DEFAULT_EXPIRATION_UNIT);
final long expirationValue = Long.parseLong(expiration);
final TimeUnit expirationUnitValue = TimeUnit.valueOf(expirationUnit);

gauges = CacheBuilder.newBuilder()
gauges = Caffeine.newBuilder()
.expireAfterAccess(expirationValue, expirationUnitValue)
.build(new CacheLoader<MonitorConfig, DoubleGauge>() {
@Override
public DoubleGauge load(final MonitorConfig config) throws Exception {
return new DoubleGauge(config);
}
});
cacheMonitor = Monitors.newCacheMonitor(CACHE_MONITOR_ID, gauges);
.build(DoubleGauge::new);
Monitors.newCacheMonitor(CACHE_MONITOR_ID, gauges);
DefaultMonitorRegistry.getInstance().register(this);
}

Expand Down Expand Up @@ -102,12 +92,7 @@ public static void set(String name, TagList list, double value) {
}

private DoubleGauge get(MonitorConfig config) {
try {
return gauges.get(config);
} catch (ExecutionException e) {
LOGGER.error("Failed to get a gauge for {}: {}", config, e.getMessage());
throw Throwables.propagate(e);
}
return gauges.get(config);
}

/**
Expand All @@ -116,7 +101,7 @@ private DoubleGauge get(MonitorConfig config) {
@Override
public List<Monitor<?>> getMonitors() {
final ConcurrentMap<MonitorConfig, DoubleGauge> gaugesMap = gauges.asMap();
return ImmutableList.copyOf(gaugesMap.values());
return Collections.unmodifiableList(new ArrayList<>(gaugesMap.values()));
}

/**
Expand Down
9 changes: 2 additions & 7 deletions servo-core/src/main/java/com/netflix/servo/monitor/MonitoredCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
*/
package com.netflix.servo.monitor;

import com.google.common.cache.Cache;
import com.google.common.cache.CacheStats;
import com.github.benmanes.caffeine.cache.Cache;
import com.github.benmanes.caffeine.cache.stats.CacheStats;
import com.netflix.servo.util.Memoizer;

import java.util.concurrent.Callable;
Expand Down Expand Up @@ -65,11 +65,6 @@ long loadCount() {
return memoStats.get().loadCount();
}

@com.netflix.servo.annotations.Monitor(name = "loadExceptionCount", type = COUNTER)
long loadExceptionCount() {
return memoStats.get().loadExceptionCount();
}

@com.netflix.servo.annotations.Monitor(name = "loadSuccessCount", type = COUNTER)
long loadSuccessCount() {
return memoStats.get().loadSuccessCount();
Expand Down
4 changes: 2 additions & 2 deletions servo-core/src/main/java/com/netflix/servo/monitor/Monitors.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@
*/
package com.netflix.servo.monitor;

import com.google.common.base.Function;
import com.google.common.cache.Cache;
import com.github.benmanes.caffeine.cache.Cache;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is no usage of Monitors.newCacheMonitor internally at Netflix that would break, then I would prefer to just remove the method rather than move it over to Caffeine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me, will remove vs update to Caffeine 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like DynamicGauge uses it 🤔

import com.netflix.servo.DefaultMonitorRegistry;
import com.netflix.servo.annotations.DataSourceType;
import com.netflix.servo.annotations.MonitorTags;
Expand All @@ -33,6 +32,7 @@
import java.util.Set;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.function.Function;

import static com.netflix.servo.util.Reflection.getFieldsAnnotatedBy;
import static com.netflix.servo.util.Reflection.getMethodsAnnotatedBy;
Expand Down
2 changes: 1 addition & 1 deletion servo-core/src/main/java/com/netflix/servo/publish/MetricTransformObserver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@
*/
package com.netflix.servo.publish;

import com.google.common.base.Function;
import com.netflix.servo.Metric;

import java.util.List;
import java.util.function.Function;
import java.util.stream.Collectors;

/**
Expand Down
30 changes: 1 addition & 29 deletions servo-core/src/main/java/com/netflix/servo/tag/Tags.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,37 +15,10 @@
*/
package com.netflix.servo.tag;

import com.google.common.collect.Interner;
import com.google.common.collect.Interners;

/**
* Helper functions for working with tags and tag lists.
*/
public final class Tags {
/**
* Keep track of the strings that have been used for keys and values.
*/
private static final Interner<String> STR_CACHE = Interners.newWeakInterner();

/**
* Keep track of tags that have been seen before and reuse.
*/
private static final Interner<Tag> TAG_CACHE = Interners.newWeakInterner();

/**
* Intern strings used for tag keys or values.
*/
public static String intern(String v) {
return STR_CACHE.intern(v);
}

/**
* Returns the canonical representation of a tag.
*/
public static Tag intern(Tag t) {
return TAG_CACHE.intern(t);
}

/**
* Interns custom tag types, assumes that basic tags are already interned. This is used to
* ensure that we have a common view of tags internally. In particular, different subclasses of
Expand All @@ -60,8 +33,7 @@ static Tag internCustom(Tag t) {
* Create a new tag instance.
*/
public static Tag newTag(String key, String value) {
Tag newTag = new BasicTag(intern(key), intern(value));
return intern(newTag);
return new BasicTag(key, value);
}

/**
Expand Down
1 change: 1 addition & 0 deletions servo-example/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ dependencies {
api project(':servo-core')
api project(':servo-atlas')
api project(':servo-graphite')
api "com.google.guava:guava:32.1.3-jre"
}

task(run, dependsOn: 'classes', type: JavaExec) {
Expand Down
Loading