Update to Errorprone 2.7 (#3181)

* Update to errorprone 2.7

* ToString

* Finish
This commit is contained in:
Anuraag Agrawal 2021-06-04 10:40:55 +09:00 committed by GitHub
parent c8dc9d52a6
commit 2436499a09
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
34 changed files with 140 additions and 88 deletions

View File

@ -111,19 +111,18 @@ public class E2EAgentBenchmark {
}
static void printContainerMapping(GenericContainer<?> container) {
System.out.println(
String.format(
"Container %s ports exposed at %s",
container.getDockerImageName(),
container.getExposedPorts().stream()
.map(
port ->
new AbstractMap.SimpleImmutableEntry<>(
port,
"http://"
+ container.getContainerIpAddress()
+ ":"
+ container.getMappedPort(port)))
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue))));
LOG.info(
"Container {} ports exposed at {}",
container.getDockerImageName(),
container.getExposedPorts().stream()
.map(
port ->
new AbstractMap.SimpleImmutableEntry<>(
port,
"http://"
+ container.getContainerIpAddress()
+ ":"
+ container.getMappedPort(port)))
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)));
}
}

View File

@ -86,6 +86,7 @@ allprojects {
// Require Guava
disable("AutoValueImmutableFields")
disable("StringSplitter")
disable("ImmutableMemberCollection")
// Don't currently use this (to indicate a local variable that's mutated) but could
// consider for future.
@ -107,6 +108,7 @@ allprojects {
// We end up using obsolete types if a library we're instrumenting uses them.
disable("JdkObsolete")
disable("JavaUtilDate")
// Limits API possibilities
disable("NoFunctionalReturnType")
@ -128,8 +130,13 @@ allprojects {
disable("InconsistentOverloads")
disable("TypeParameterNaming")
// We don't use tools that recognize.
disable("InlineMeSuggester")
disable("DoNotCallSuggester")
if (name.contains("Jmh") || name.contains("Test")) {
disable("FieldMissingNullable")
disable("HashCodeToString")
disable("MemberName")
}
}
}

View File

@ -43,7 +43,7 @@ val DEPENDENCY_SETS = listOf(
),
DependencySet(
"com.google.errorprone",
"2.4.0",
"2.7.1",
listOf("error_prone_annotations", "error_prone_core")
),
DependencySet(

View File

@ -48,8 +48,8 @@ enum Jdk8AsyncSpanEndStrategy implements AsyncSpanEndStrategy {
// so that it can be recorded to the span
try {
future.join();
} catch (Exception exception) {
tracer.endExceptionally(context, exception);
} catch (Throwable t) {
tracer.endExceptionally(context, t);
return true;
}
}

View File

@ -20,6 +20,7 @@ import io.opentelemetry.javaagent.instrumentation.api.concurrent.RunnableWrapper
import io.opentelemetry.javaagent.instrumentation.api.concurrent.State;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.concurrent.Callable;
import java.util.concurrent.ForkJoinTask;
import java.util.concurrent.Future;
@ -185,7 +186,7 @@ public class JavaExecutorInstrumentation extends AbstractExecutorInstrumentation
tasks = wrappedTasks;
return tasks;
}
return null;
return Collections.emptyList();
}
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
@ -201,7 +202,7 @@ public class JavaExecutorInstrumentation extends AbstractExecutorInstrumentation
any parent spans in case of an error.
(according to ExecutorService docs and AbstractExecutorService code)
*/
if (null != throwable && wrappedTasks != null) {
if (null != throwable) {
for (Callable<?> task : wrappedTasks) {
if (task != null) {
ContextStore<Callable, State> contextStore =

View File

@ -164,7 +164,6 @@ public class JaxRsAnnotationsTracer extends BaseTracer {
}
private static String buildSpanName(Path classPath, Path methodPath) {
String spanName;
StringBuilder spanNameBuilder = new StringBuilder();
boolean skipSlash = false;
if (classPath != null) {
@ -187,8 +186,7 @@ public class JaxRsAnnotationsTracer extends BaseTracer {
spanNameBuilder.append(path);
}
spanName = spanNameBuilder.toString().trim();
return spanName;
return spanNameBuilder.toString().trim();
}
@Override

View File

@ -196,7 +196,6 @@ public class JaxRsAnnotationsTracer extends BaseTracer {
}
private static String buildSpanName(Path classPath, Path methodPath) {
String spanName;
StringBuilder spanNameBuilder = new StringBuilder();
boolean skipSlash = false;
if (classPath != null) {
@ -220,9 +219,7 @@ public class JaxRsAnnotationsTracer extends BaseTracer {
spanNameBuilder.append(path);
}
spanName = spanNameBuilder.toString().trim();
return spanName;
return spanNameBuilder.toString().trim();
}
@Override

View File

@ -144,7 +144,6 @@ public enum JdbcConnectionUrlParser {
@Override
DbInfo.Builder doParse(String jdbcUrl, DbInfo.Builder builder) {
String type;
String serverName = "";
Integer port = null;
String name = null;
@ -155,7 +154,7 @@ public enum JdbcConnectionUrlParser {
return builder;
}
type = jdbcUrl.substring(0, hostIndex);
String type = jdbcUrl.substring(0, hostIndex);
String[] split;
if (type.equals("db2") || type.equals("as400")) {
@ -845,7 +844,7 @@ public enum JdbcConnectionUrlParser {
return withUrl(typeParsers.get(type).doParse(jdbcUrl, parsedProps), type);
}
return withUrl(GENERIC_URL_LIKE.doParse(jdbcUrl, parsedProps), type);
} catch (Exception e) {
} catch (RuntimeException e) {
log.debug("Error parsing URL", e);
return parsedProps.build();
}

View File

@ -47,13 +47,9 @@ public class TracingIterator implements Iterator<ConsumerRecord<?, ?>> {
ConsumerRecord<?, ?> next = delegateIterator.next();
try {
if (next != null) {
currentContext = tracer.startSpan(next);
currentScope = currentContext.makeCurrent();
}
} catch (Exception e) {
log.debug("Error during decoration", e);
if (next != null) {
currentContext = tracer.startSpan(next);
currentScope = currentContext.makeCurrent();
}
return next;
}

View File

@ -10,6 +10,7 @@ import io.opentelemetry.javaagent.instrumentation.api.ContextStore;
import java.util.Objects;
public class ChannelTraceContext {
public static class Factory implements ContextStore.Factory<ChannelTraceContext> {
public static final Factory INSTANCE = new Factory();
@ -74,4 +75,16 @@ public class ChannelTraceContext {
public int hashCode() {
return Objects.hash(connectionContext, clientParentContext, context);
}
@Override
public String toString() {
return "ChannelTraceContext{"
+ "connectionContext="
+ connectionContext
+ ", clientParentContext="
+ clientParentContext
+ ", context="
+ context
+ '}';
}
}

View File

@ -44,6 +44,7 @@ public class Servlet3FilterMappingResolverFactory
}
@Override
@SuppressWarnings("ReturnsNullCollection")
protected Collection<String> getServletMappings(String servletName) {
ServletRegistration servletRegistration =
filterConfig.getServletContext().getServletRegistration(servletName);

View File

@ -22,6 +22,7 @@ public class Servlet3MappingResolverFactory extends ServletMappingResolverFactor
}
@Override
@SuppressWarnings("ReturnsNullCollection")
public Collection<String> getMappings() {
String servletName = servletConfig.getServletName();
ServletContext servletContext = servletConfig.getServletContext();

View File

@ -44,6 +44,7 @@ public class JakartaServletFilterMappingResolverFactory
}
@Override
@SuppressWarnings("ReturnsNullCollection")
protected Collection<String> getServletMappings(String servletName) {
ServletRegistration servletRegistration =
filterConfig.getServletContext().getServletRegistration(servletName);

View File

@ -22,6 +22,7 @@ public class JakartaServletMappingResolverFactory extends ServletMappingResolver
}
@Override
@SuppressWarnings("ReturnsNullCollection")
public Collection<String> getMappings() {
String servletName = servletConfig.getServletName();
ServletContext servletContext = servletConfig.getServletContext();

View File

@ -25,6 +25,9 @@ public abstract class ServletFilterMappingResolverFactory<FILTERREGISTRATION> {
protected abstract Collection<String> getServletMappings(String servletName);
// TODO(anuraaga): We currently treat null as no mappings, and empty as having a default mapping.
// Error prone is correctly flagging this behavior as error prone.
@SuppressWarnings("ReturnsNullCollection")
private Collection<String> getMappings() {
FILTERREGISTRATION filterRegistration = getFilterRegistration();
if (filterRegistration == null) {

View File

@ -55,9 +55,9 @@ public abstract class CompletionListener<T> {
// Avoid swallowing InterruptedException
tracer().endExceptionally(context, e);
Thread.currentThread().interrupt();
} catch (Exception e) {
} catch (Throwable t) {
// This should never happen, just in case to make sure we cover all unexpected exceptions
tracer().endExceptionally(context, e);
tracer().endExceptionally(context, t);
} finally {
tracer().end(context);
}

View File

@ -126,7 +126,6 @@ public class VertxReactiveWebServer extends AbstractVerticle {
.rxQuery("SELECT id" + queryInfix + ", name, price, weight FROM products")
.flatMap(
result -> {
Thread.dumpStack();
JsonArray arr = new JsonArray();
result.getRows().forEach(arr::add);
return Single.just(arr);

View File

@ -37,7 +37,7 @@ public final class RoutingContextHandlerWrapper implements Handler<RoutingContex
// https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/465
serverSpan.updateName(context.currentRoute().getPath());
}
} catch (Exception ex) {
} catch (RuntimeException ex) {
log.error("Failed to update server span name with vert.x route", ex);
}
try {

View File

@ -42,6 +42,8 @@ import java.util.regex.Pattern;
* <li>Do dot touch any logging facilities here so we can configure them later
* </ul>
*/
// Too early for logging
@SuppressWarnings("SystemOut")
public final class OpenTelemetryAgent {
private static final Class<?> thisClass = OpenTelemetryAgent.class;
@ -197,7 +199,7 @@ public final class OpenTelemetryAgent {
public static void main(String... args) {
try {
System.out.println(OpenTelemetryAgent.class.getPackage().getImplementationVersion());
} catch (Exception e) {
} catch (RuntimeException e) {
System.out.println("Failed to parse agent version");
e.printStackTrace();
}

View File

@ -50,7 +50,7 @@ class LoggingFailSafeMatcher<T> extends ElementMatcher.Junction.AbstractBase<T>
public boolean matches(T target) {
try {
return matcher.matches(target);
} catch (Exception e) {
} catch (Throwable e) {
log.debug(description, e);
return fallback;
}

View File

@ -53,7 +53,7 @@ class SafeErasureMatcher<T extends TypeDefinition> extends ElementMatcher.Juncti
static TypeDescription safeAsErasure(TypeDefinition typeDefinition) {
try {
return typeDefinition.asErasure();
} catch (Exception e) {
} catch (Throwable e) {
if (log.isDebugEnabled()) {
log.debug(
"{} trying to get erasure for target {}: {}",

View File

@ -101,7 +101,7 @@ class SafeHasSuperTypeMatcher extends ElementMatcher.Junction.AbstractBase<TypeD
static TypeDefinition safeGetSuperClass(TypeDefinition typeDefinition) {
try {
return typeDefinition.getSuperClass();
} catch (Exception e) {
} catch (Throwable e) {
if (log.isDebugEnabled()) {
log.debug(
"{} trying to get super class for target {}: {}",
@ -156,7 +156,7 @@ class SafeHasSuperTypeMatcher extends ElementMatcher.Junction.AbstractBase<TypeD
Iterator<TypeDescription.Generic> it = null;
try {
it = typeDefinition.getInterfaces().iterator();
} catch (Exception e) {
} catch (Throwable e) {
logException(typeDefinition, e);
}
this.it = it;
@ -168,7 +168,7 @@ class SafeHasSuperTypeMatcher extends ElementMatcher.Junction.AbstractBase<TypeD
try {
next = it.next();
return true;
} catch (Exception e) {
} catch (Throwable e) {
logException(typeDefinition, e);
return false;
}
@ -191,7 +191,7 @@ class SafeHasSuperTypeMatcher extends ElementMatcher.Junction.AbstractBase<TypeD
return this;
}
private static void logException(TypeDefinition typeDefinition, Exception e) {
private static void logException(TypeDefinition typeDefinition, Throwable e) {
if (log.isDebugEnabled()) {
log.debug(
"{} trying to get interfaces for target {}: {}",

View File

@ -421,7 +421,7 @@ public class AgentInstaller {
for (ComponentInstaller componentInstaller : componentInstallers) {
try {
componentInstaller.afterByteBuddyAgent(config);
} catch (Exception e) {
} catch (RuntimeException e) {
log.error("Failed to execute {}", componentInstaller.getClass().getName(), e);
}
}

View File

@ -19,7 +19,7 @@ import java.net.URLClassLoader;
// TODO find a way to initialize logging before using this class
// TODO support scanning a folder for several extension jars and keep them isolated from each other
// Used by AgentInitializer
@SuppressWarnings("unused")
@SuppressWarnings({"unused", "SystemOut"})
public class ExtensionClassLoader extends URLClassLoader {
// NOTE it's important not to use slf4j in this class, because this class is used before slf4j is
// configured, and so using slf4j here would initialize slf4j-simple before we have a chance to

View File

@ -199,6 +199,19 @@ public class AgentCachingPoolStrategy implements AgentBuilder.PoolStrategy {
public final int hashCode() {
return hashCode;
}
@Override
public String toString() {
return "TypeCacheKey{"
+ "loaderHash="
+ loaderHash
+ ", loaderRef="
+ loaderRef
+ ", className='"
+ className
+ '\''
+ '}';
}
}
static final class SharedResolutionCacheAdapter implements TypePool.CacheProvider {

View File

@ -27,7 +27,7 @@ public final class ExceptionHandlers {
new ExceptionHandler.Simple(
new StackManipulation() {
// Pops one Throwable off the stack. Maxes the stack to at least 3.
private final Size size = new StackManipulation.Size(-1, 3);
private final StackManipulation.Size size = new StackManipulation.Size(-1, 3);
@Override
public boolean isValid() {
@ -35,7 +35,7 @@ public final class ExceptionHandlers {
}
@Override
public Size apply(MethodVisitor mv, Implementation.Context context) {
public StackManipulation.Size apply(MethodVisitor mv, Implementation.Context context) {
String name = context.getInstrumentedType().getName();
ClassLoader classLoader = Thread.currentThread().getContextClassLoader();

View File

@ -89,6 +89,23 @@ interface HelperReferenceWrapper {
public int hashCode() {
return Objects.hash(name, descriptor);
}
@Override
public String toString() {
return "Method{"
+ "isAbstract="
+ isAbstract
+ ", declaringClass='"
+ declaringClass
+ '\''
+ ", name='"
+ name
+ '\''
+ ", descriptor='"
+ descriptor
+ '\''
+ '}';
}
}
final class Field {
@ -124,6 +141,11 @@ interface HelperReferenceWrapper {
public int hashCode() {
return Objects.hash(name, descriptor);
}
@Override
public String toString() {
return "Field{" + "name='" + name + '\'' + ", descriptor='" + descriptor + '\'' + '}';
}
}
class Factory {

View File

@ -22,6 +22,8 @@ import java.util.ServiceLoader;
import net.bytebuddy.dynamic.ClassFileLocator;
/** Entry point for the muzzle gradle plugin. */
// Runs in special classloader so tedious to provide access to the Gradle logger.
@SuppressWarnings("SystemOut")
public final class MuzzleGradlePluginUtil {
private static final String INDENT = " ";
@ -101,7 +103,7 @@ public final class MuzzleGradlePluginUtil {
createHelperMap(allHelperClasses, agentClassLoader))
.transform(null, null, userClassLoader, null);
}
} catch (Exception e) {
} catch (RuntimeException e) {
System.err.println(
"FAILED HELPER INJECTION. Are Helpers being injected in the correct order?");
throw e;
@ -140,7 +142,7 @@ public final class MuzzleGradlePluginUtil {
for (ClassRef ref : instrumentationModule.getMuzzleReferences().values()) {
System.out.print(prettyPrint(ref));
}
} catch (Exception e) {
} catch (RuntimeException e) {
String message =
"Unexpected exception printing references for "
+ instrumentationModule.getClass().getName();

View File

@ -17,7 +17,6 @@ import io.opentelemetry.javaagent.extension.muzzle.MethodRef;
import io.opentelemetry.javaagent.tooling.AgentTooling;
import io.opentelemetry.javaagent.tooling.Utils;
import io.opentelemetry.javaagent.tooling.muzzle.InstrumentationClassPredicate;
import io.opentelemetry.javaagent.tooling.muzzle.matcher.HelperReferenceWrapper.Method;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
@ -121,7 +120,7 @@ public final class ReferenceMatcher {
}
return checkThirdPartyTypeMatch(reference, resolution.resolve());
}
} catch (Exception e) {
} catch (RuntimeException e) {
if (e.getMessage().startsWith("Cannot resolve type description for ")) {
// bytebuddy throws an illegal state exception with this message if it cannot resolve types
// TODO: handle missing type resolutions without catching bytebuddy's exceptions
@ -188,7 +187,9 @@ public final class ReferenceMatcher {
}
private static void collectMethodsFromTypeHierarchy(
HelperReferenceWrapper type, Set<Method> abstractMethods, Set<Method> plainMethods) {
HelperReferenceWrapper type,
Set<HelperReferenceWrapper.Method> abstractMethods,
Set<HelperReferenceWrapper.Method> plainMethods) {
type.getMethods()
.forEach(method -> (method.isAbstract() ? abstractMethods : plainMethods).add(method));

View File

@ -251,7 +251,7 @@ public class IntegrationTestUtils {
String line = null;
while ((line = reader.readLine()) != null) {
if (print) {
System.out.println(type + "> " + line);
logger.info("{}> {}", type, line);
}
}
} catch (IOException e) {

View File

@ -22,6 +22,7 @@ import picocli.CommandLine.Option;
@Command(
mixinStandardHelpOptions = true,
description = "Generates traces and spans at a specified rate")
@SuppressWarnings("SystemOut")
public class LoadGenerator implements Callable<Integer> {
private static final Tracer tracer = GlobalOpenTelemetry.getTracer("test");

View File

@ -230,7 +230,7 @@ public class WindowsTestContainerManager extends AbstractTestContainerManager {
try {
client.inspectImageCmd(imageName).exec();
return true;
} catch (Exception e) {
} catch (RuntimeException e) {
return false;
}
}

View File

@ -119,7 +119,7 @@ class AgentTestRunnerTest extends AgentInstrumentationSpecification {
if (!(testClassLoader instanceof URLClassLoader)) {
// java9's system loader does not extend URLClassLoader
// which breaks Guava ClassPath lookup
testClassLoader = ClasspathUtils.buildJavaClassPathClassLoader()
testClassLoader = buildJavaClassPathClassLoader()
}
try {
return ClassPath.from(testClassLoader)
@ -127,4 +127,27 @@ class AgentTestRunnerTest extends AgentInstrumentationSpecification {
throw new IllegalStateException(e)
}
}
/**
* Parse JVM classpath and return ClassLoader containing all classpath entries. Inspired by Guava.
*/
private static ClassLoader buildJavaClassPathClassLoader() {
List<URL> urls = new ArrayList<>()
for (String entry : getClasspath()) {
try {
try {
urls.add(new File(entry).toURI().toURL())
} catch (SecurityException e) { // File.toURI checks to see if the file is a directory
urls.add(new URL("file", null, new File(entry).getAbsolutePath()))
}
} catch (MalformedURLException e) {
throw new IllegalStateException(e)
}
}
return new URLClassLoader(urls.toArray(new URL[0]), (ClassLoader) null)
}
private static String[] getClasspath() {
return System.getProperty("java.class.path").split(System.getProperty("path.separator"))
}
}

View File

@ -11,11 +11,7 @@ import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.lang.reflect.Method;
import java.net.MalformedURLException;
import java.net.URL;
import java.net.URLClassLoader;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
import java.util.jar.JarEntry;
import java.util.jar.JarOutputStream;
@ -108,30 +104,6 @@ public final class ClasspathUtils {
jarOutputStream.closeEntry();
}
/**
* Parse JVM classpath and return ClassLoader containing all classpath entries. Inspired by Guava.
*/
public static ClassLoader buildJavaClassPathClassLoader() {
List<URL> urls = new ArrayList<>();
for (String entry : getClasspath()) {
try {
try {
urls.add(new File(entry).toURI().toURL());
} catch (SecurityException e) { // File.toURI checks to see if the file is a directory
urls.add(new URL("file", null, new File(entry).getAbsolutePath()));
}
} catch (MalformedURLException e) {
System.err.printf(
"Error injecting bootstrap jar: Malformed classpath entry: %s. %s%n", entry, e);
}
}
return new URLClassLoader(urls.toArray(new URL[0]), null);
}
private static String[] getClasspath() {
return System.getProperty("java.class.path").split(System.getProperty("path.separator"));
}
// Moved this to a java class because groovy was adding a hard ref to classLoader
public static boolean isClassLoaded(String className, ClassLoader classLoader) {
try {