Fix up errorprone and warnings that fail the check build (#623)

* fix up errorprone and warnings that fail the check build * more fixups * use the instrumentation bom * fix import * spotless
signalfx · Aug 29, 2023 · c2a67d5 · c2a67d5
1 parent 3bf0c62
commit c2a67d5
Show file tree

Hide file tree

Showing 12 changed files with 89 additions and 50 deletions.
diff --git a/buildSrc/src/main/kotlin/splunk.errorprone-conventions.gradle.kts b/buildSrc/src/main/kotlin/splunk.errorprone-conventions.gradle.kts
@@ -1,28 +1,48 @@
+import com.android.build.api.variant.AndroidComponentsExtension
 import net.ltgt.gradle.errorprone.CheckSeverity
+import net.ltgt.gradle.errorprone.ErrorPronePlugin
 import net.ltgt.gradle.errorprone.errorprone
 import net.ltgt.gradle.nullaway.nullaway
+import java.util.Locale
 
 plugins {
     id("net.ltgt.errorprone")
     id("net.ltgt.nullaway")
 }
 
+
+val isAndroidProject = extensions.findByName("android") != null
+
+if (isAndroidProject) {
+    val errorProneConfig = configurations.getByName(ErrorPronePlugin.CONFIGURATION_NAME)
+    extensions.getByType(AndroidComponentsExtension::class.java).onVariants {
+        it.annotationProcessorConfiguration.extendsFrom(errorProneConfig)
+    }
+}
+
 dependencies {
-    errorprone("com.uber.nullaway:nullaway:0.9.9")
-    errorprone("com.google.errorprone:error_prone_core:2.15.0")
+    errorprone("com.uber.nullaway:nullaway:0.10.12")
+    errorprone("com.google.errorprone:error_prone_core:2.21.1")
     errorproneJavac("com.google.errorprone:javac:9+181-r4173-1")
 }
 
 nullaway {
+    annotatedPackages.add("io.opentelemetry.rum.internal")
     annotatedPackages.add("com.splunk.rum")
 }
 
 tasks {
     withType<JavaCompile>().configureEach {
         options.errorprone {
-            if (name.toLowerCase().contains("test")) {
+            if (name.lowercase(Locale.getDefault()).contains("test")) {
                 // just disable all error prone checks for test
                 isEnabled.set(false);
+                isCompilingTestOnlyCode.set(true)
+            } else {
+                if (isAndroidProject) {
+                    isEnabled.set(true)
+                    isCompilingTestOnlyCode.set(false)
+                }
             }
 
             nullaway {
@@ -35,4 +55,4 @@ tasks {
             disable("MixedMutabilityReturnType")
         }
     }
-}
+}
diff --git a/...strumentation/src/main/java/io/opentelemetry/rum/internal/SessionIdRatioBasedSampler.java b/...strumentation/src/main/java/io/opentelemetry/rum/internal/SessionIdRatioBasedSampler.java
@@ -33,10 +33,10 @@
  */
 public class SessionIdRatioBasedSampler implements Sampler {
     private final Sampler ratioBasedSampler;
-    private final SessionId sessionid;
+    private final SessionId sessionId;
 
     public SessionIdRatioBasedSampler(double ratio, SessionId sessionId) {
-        this.sessionid = sessionId;
+        this.sessionId = sessionId;
         // SessionId uses the same format as TraceId, so we can reuse trace ID ratio sampler.
         this.ratioBasedSampler = Sampler.traceIdRatioBased(ratio);
     }
@@ -51,7 +51,7 @@ public SamplingResult shouldSample(
             List<LinkData> parentLinks) {
         // Replace traceId with sessionId
         return ratioBasedSampler.shouldSample(
-                parentContext, sessionid.getSessionId(), name, spanKind, attributes, parentLinks);
+                parentContext, sessionId.getSessionId(), name, spanKind, attributes, parentLinks);
     }
 
     @Override

diff --git a/...tion/src/main/java/io/opentelemetry/rum/internal/instrumentation/ScreenNameExtractor.java b/...tion/src/main/java/io/opentelemetry/rum/internal/instrumentation/ScreenNameExtractor.java
@@ -17,26 +17,21 @@
 package io.opentelemetry.rum.internal.instrumentation;
 
 import android.app.Activity;
-import androidx.annotation.Nullable;
 import androidx.fragment.app.Fragment;
 
 public interface ScreenNameExtractor {
 
-    @Nullable
     String extract(Activity activity);
 
-    @Nullable
     String extract(Fragment fragment);
 
     ScreenNameExtractor DEFAULT =
             new ScreenNameExtractor() {
-                @Nullable
                 @Override
                 public String extract(Activity activity) {
                     return useAnnotationOrClassName(activity.getClass());
                 }
 
-                @Nullable
                 @Override
                 public String extract(Fragment fragment) {
                     return useAnnotationOrClassName(fragment.getClass());

diff --git a/.../src/main/java/io/opentelemetry/rum/internal/instrumentation/activity/ActivityTracer.java b/.../src/main/java/io/opentelemetry/rum/internal/instrumentation/activity/ActivityTracer.java
@@ -150,12 +150,15 @@ public static Builder builder(Activity activity) {
     }
 
     static class Builder {
+        private static final ActiveSpan INVALID_ACTIVE_SPAN = new ActiveSpan(() -> null);
+        private static final Tracer INVALID_TRACER = spanName -> null;
+        private static final AppStartupTimer INVALID_TIMER = new AppStartupTimer();
         private final Activity activity;
-        public String screenName;
+        public String screenName = "unknown_screen";
         private AtomicReference<String> initialAppActivity = new AtomicReference<>();
-        private Tracer tracer;
-        private AppStartupTimer appStartupTimer;
-        private ActiveSpan activeSpan;
+        private Tracer tracer = INVALID_TRACER;
+        private AppStartupTimer appStartupTimer = INVALID_TIMER;
+        private ActiveSpan activeSpan = INVALID_ACTIVE_SPAN;
 
         public Builder(Activity activity) {
             this.activity = activity;
@@ -195,13 +198,22 @@ private String getActivityName() {
             return activity.getClass().getSimpleName();
         }
 
-        public ActivityTracer build() {
-            return new ActivityTracer(this);
-        }
-
         public Builder setScreenName(String screenName) {
             this.screenName = screenName;
             return this;
         }
+
+        public ActivityTracer build() {
+            if (activeSpan == INVALID_ACTIVE_SPAN) {
+                throw new IllegalStateException("activeSpan must be configured.");
+            }
+            if (tracer == INVALID_TRACER) {
+                throw new IllegalStateException("tracer must be configured.");
+            }
+            if (appStartupTimer == INVALID_TIMER) {
+                throw new IllegalStateException("appStartupTimer must be configured.");
+            }
+            return new ActivityTracer(this);
+        }
     }
 }
diff --git a/.../src/main/java/io/opentelemetry/rum/internal/instrumentation/fragment/FragmentTracer.java b/.../src/main/java/io/opentelemetry/rum/internal/instrumentation/fragment/FragmentTracer.java
@@ -81,10 +81,12 @@ static Builder builder(Fragment fragment) {
     }
 
     static class Builder {
+        private static final ActiveSpan INVALID_ACTIVE_SPAN = new ActiveSpan(() -> null);
+        private static final Tracer INVALID_TRACER = spanName -> null;
         private final Fragment fragment;
-        public String screenName;
-        private Tracer tracer;
-        private ActiveSpan activeSpan;
+        public String screenName = "";
+        private Tracer tracer = INVALID_TRACER;
+        private ActiveSpan activeSpan = INVALID_ACTIVE_SPAN;
 
         public Builder(Fragment fragment) {
             this.fragment = fragment;
@@ -110,6 +112,12 @@ public String getFragmentName() {
         }
 
         FragmentTracer build() {
+            if (activeSpan == INVALID_ACTIVE_SPAN) {
+                throw new IllegalStateException("activeSpan must be configured.");
+            }
+            if (tracer == INVALID_TRACER) {
+                throw new IllegalStateException("tracer must be configured.");
+            }
             return new FragmentTracer(this);
         }
     }

diff --git a/...emetry/rum/internal/instrumentation/lifecycle/AndroidLifecycleInstrumentationBuilder.java b/...emetry/rum/internal/instrumentation/lifecycle/AndroidLifecycleInstrumentationBuilder.java
@@ -23,9 +23,11 @@
 import java.util.function.Function;
 
 public class AndroidLifecycleInstrumentationBuilder {
+    private static final VisibleScreenTracker INVALID_SCREEN_TRACKER = new VisibleScreenTracker();
+    private static final AppStartupTimer INVALID_TIMER = new AppStartupTimer();
     ScreenNameExtractor screenNameExtractor = ScreenNameExtractor.DEFAULT;
-    AppStartupTimer startupTimer;
-    VisibleScreenTracker visibleScreenTracker;
+    AppStartupTimer startupTimer = INVALID_TIMER;
+    VisibleScreenTracker visibleScreenTracker = INVALID_SCREEN_TRACKER;
     Function<Tracer, Tracer> tracerCustomizer = Function.identity();
 
     public AndroidLifecycleInstrumentationBuilder setStartupTimer(AppStartupTimer timer) {
@@ -52,6 +54,12 @@ public AndroidLifecycleInstrumentationBuilder setScreenNameExtractor(
     }
 
     public AndroidLifecycleInstrumentation build() {
+        if (visibleScreenTracker == INVALID_SCREEN_TRACKER) {
+            throw new IllegalStateException("visibleScreenTracker must be configured.");
+        }
+        if (startupTimer == INVALID_TIMER) {
+            throw new IllegalStateException("startupTimer must be configured.");
+        }
         return new AndroidLifecycleInstrumentation(this);
     }
 }
diff --git a/.../src/main/java/io/opentelemetry/rum/internal/instrumentation/startup/AppStartupTimer.java b/.../src/main/java/io/opentelemetry/rum/internal/instrumentation/startup/AppStartupTimer.java
@@ -65,9 +65,7 @@ public Span start(Tracer tracer) {
         return appStart;
     }
 
-    /**
-     * @return epoch timestamp in nanos calculated by the startupClock.
-     */
+    /** Returns the epoch timestamp in nanos calculated by the startupClock. */
     public long clockNow() {
         return startupClock.now();
     }

diff --git a/...nk-otel-android-volley/src/main/java/com/splunk/rum/VolleyHttpClientAttributesGetter.java b/...nk-otel-android-volley/src/main/java/com/splunk/rum/VolleyHttpClientAttributesGetter.java
@@ -34,15 +34,18 @@ enum VolleyHttpClientAttributesGetter
     INSTANCE;
 
     @Override
+    @Nullable
     public String getUrlFull(RequestWrapper requestWrapper) {
         return requestWrapper.getRequest().getUrl();
     }
 
+    @Nullable
     @Override
     public String getServerAddress(RequestWrapper requestWrapper) {
         return UrlParser.getHost(requestWrapper.getRequest().getUrl());
     }
 
+    @Nullable
     @Override
     public Integer getServerPort(RequestWrapper requestWrapper) {
         return UrlParser.getPort(requestWrapper.getRequest().getUrl());

diff --git a/splunk-otel-android/build.gradle.kts b/splunk-otel-android/build.gradle.kts
@@ -42,10 +42,9 @@ android {
 }
 
 val otelVersion = "1.29.0"
-val otelAlphaVersion = "$otelVersion-alpha"
-val otelInstrumentationAlphaVersion = "1.29.0-alpha-SNAPSHOT"
 
 dependencies {
+    api(platform("io.opentelemetry.instrumentation:opentelemetry-instrumentation-bom-alpha:$otelVersion-alpha"))
     implementation(project(":opentelemetry-android-instrumentation"))
     implementation(platform("org.jetbrains.kotlin:kotlin-bom:1.9.10"))
     implementation("androidx.appcompat:appcompat:1.6.1")
@@ -58,9 +57,9 @@ dependencies {
     implementation("io.zipkin.reporter2:zipkin-sender-okhttp3")
     implementation("io.opentelemetry:opentelemetry-exporter-logging")
 
-    implementation(platform("io.opentelemetry:opentelemetry-bom-alpha:$otelAlphaVersion"))
+    implementation(platform("io.opentelemetry:opentelemetry-bom-alpha"))
     implementation("io.opentelemetry:opentelemetry-semconv")
-    implementation("io.opentelemetry.instrumentation:opentelemetry-okhttp-3.0:$otelInstrumentationAlphaVersion")
+    implementation("io.opentelemetry.instrumentation:opentelemetry-okhttp-3.0")
 
     api("io.opentelemetry:opentelemetry-api")
     api("com.squareup.okhttp3:okhttp:4.11.0")

diff --git a/splunk-otel-android/src/main/java/com/splunk/rum/BackgroundProcessDetector.java b/splunk-otel-android/src/main/java/com/splunk/rum/BackgroundProcessDetector.java
@@ -22,29 +22,28 @@
 import java.lang.reflect.Method;
 import java.util.Objects;
 
-public final class BackgroundProcessDetector {
+final class BackgroundProcessDetector {
 
     private BackgroundProcessDetector() {}
 
-    public static Boolean isBackgroundProcess(String applicationId) {
+    static Boolean isBackgroundProcess(String applicationId) {
         String applicationProcessName = getApplicationProcessName();
         return Objects.equals(applicationProcessName, applicationId);
     }
 
     private static String getApplicationProcessName() {
         if (Build.VERSION.SDK_INT >= 28) {
             return Application.getProcessName();
-        } else {
-            try {
-                @SuppressLint("PrivateApi")
-                Class<?> activityThread = Class.forName("android.app.ActivityThread");
-                String methodName = "currentProcessName";
-                @SuppressLint("PrivateApi")
-                Method getProcessName = activityThread.getDeclaredMethod(methodName);
-                return (String) getProcessName.invoke(null);
-            } catch (Exception e) {
-                return null;
-            }
+        }
+        try {
+            @SuppressLint("PrivateApi")
+            Class<?> activityThread = Class.forName("android.app.ActivityThread");
+            String methodName = "currentProcessName";
+            @SuppressLint("PrivateApi")
+            Method getProcessName = activityThread.getDeclaredMethod(methodName);
+            return (String) getProcessName.invoke(null);
+        } catch (Exception e) {
+            return "";
         }
     }
 }
diff --git a/splunk-otel-android/src/main/java/com/splunk/rum/LogToSpanBridge.java b/splunk-otel-android/src/main/java/com/splunk/rum/LogToSpanBridge.java
@@ -19,6 +19,7 @@
 import static io.opentelemetry.api.common.AttributeKey.longKey;
 import static io.opentelemetry.api.common.AttributeKey.stringKey;
 
+import androidx.annotation.Nullable;
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.logs.Severity;
 import io.opentelemetry.api.trace.Span;
@@ -44,7 +45,7 @@ final class LogToSpanBridge implements LogRecordProcessor {
     static final AttributeKey<String> LOG_SEVERITY_TEXT = stringKey("log.severity_text");
     static final AttributeKey<String> LOG_BODY = stringKey("log.body");
 
-    private volatile TracerProvider tracerProvider;
+    @Nullable private volatile TracerProvider tracerProvider = null;
 
     void setTracerProvider(TracerProvider tracerProvider) {
         this.tracerProvider = tracerProvider;

diff --git a/splunk-otel-android/src/main/java/com/splunk/rum/SplunkScreenNameExtractor.java b/splunk-otel-android/src/main/java/com/splunk/rum/SplunkScreenNameExtractor.java
@@ -17,7 +17,6 @@
 package com.splunk.rum;
 
 import android.app.Activity;
-import androidx.annotation.Nullable;
 import androidx.fragment.app.Fragment;
 import io.opentelemetry.rum.internal.instrumentation.ScreenNameExtractor;
 import java.util.function.Function;
@@ -32,19 +31,16 @@ class SplunkScreenNameExtractor implements ScreenNameExtractor {
 
     private SplunkScreenNameExtractor() {}
 
-    @Nullable
     @Override
     public String extract(Activity activity) {
         return getOrDefault(activity, DEFAULT::extract);
     }
 
-    @Nullable
     @Override
     public String extract(Fragment fragment) {
         return getOrDefault(fragment, DEFAULT::extract);
     }
 
-    @Nullable
     private <T> String getOrDefault(T obj, Function<T, String> defaultMethod) {
         RumScreenName rumScreenName = obj.getClass().getAnnotation(RumScreenName.class);
         if (rumScreenName != null) {