Refactor CurrentNetwork: add a method that returns all network Attrib… (

#410)

* Refactor CurrentNetwork: add a method that returns all network Attributes

* code review comments

* Improved NetworkMonitor assertions

* fix crash reporter tests?

splunk-otel-android/src/main/java/com/splunk/rum/CurrentNetworkAttributesExtractor.java

@@ -0,0 +1,53 @@
+/*
+ * Copyright Splunk Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.splunk.rum;
+
+import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NET_HOST_CARRIER_ICC;
+import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NET_HOST_CARRIER_MCC;
+import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NET_HOST_CARRIER_MNC;
+import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NET_HOST_CARRIER_NAME;
+import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NET_HOST_CONNECTION_SUBTYPE;
+import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NET_HOST_CONNECTION_TYPE;
+
+import androidx.annotation.Nullable;
+import io.opentelemetry.api.common.AttributeKey;
+import io.opentelemetry.api.common.Attributes;
+import io.opentelemetry.api.common.AttributesBuilder;
+
+class CurrentNetworkAttributesExtractor {
+
+    Attributes extract(CurrentNetwork network) {
+        AttributesBuilder builder =
+                Attributes.builder()
+                        .put(NET_HOST_CONNECTION_TYPE, network.getState().getHumanName());
+
+        setIfNotNull(builder, NET_HOST_CONNECTION_SUBTYPE, network.getSubType());
+        setIfNotNull(builder, NET_HOST_CARRIER_NAME, network.getCarrierName());
+        setIfNotNull(builder, NET_HOST_CARRIER_MCC, network.getCarrierCountryCode());
+        setIfNotNull(builder, NET_HOST_CARRIER_MNC, network.getCarrierNetworkCode());
+        setIfNotNull(builder, NET_HOST_CARRIER_ICC, network.getCarrierIsoCountryCode());
+
+        return builder.build();
+    }
+
+    private static void setIfNotNull(
+            AttributesBuilder builder, AttributeKey<String> key, @Nullable String value) {
+        if (value != null) {
+            builder.put(key, value);
+        }
+    }
+}

splunk-otel-android/src/main/java/com/splunk/rum/NetworkMonitor.java

@@ -52,8 +52,11 @@ public void onApplicationBackgrounded() {
 
     // visibleForTesting
     static class TracingConnectionStateListener implements ConnectionStateListener {
+
         private final Tracer tracer;
         private final AtomicBoolean shouldEmitChangeEvents;
+        private final CurrentNetworkAttributesExtractor networkAttributesExtractor =
+                new CurrentNetworkAttributesExtractor();
 
         TracingConnectionStateListener(Tracer tracer, AtomicBoolean shouldEmitChangeEvents) {
             this.tracer = tracer;
@@ -81,7 +84,7 @@ public void onAvailable(boolean deviceIsOnline, CurrentNetwork activeNetwork) {
                                 .startSpan();
                 // put these after span start to override what might be set in the
                 // RumAttributeAppender.
-                RumAttributeAppender.appendNetworkAttributes(available, activeNetwork);
+                available.setAllAttributes(networkAttributesExtractor.extract(activeNetwork));
                 available.end();
             }
         }

splunk-otel-android/src/main/java/com/splunk/rum/RumAttributeAppender.java

@@ -16,28 +16,17 @@
 
 package com.splunk.rum;
 
-import static io.opentelemetry.api.common.AttributeKey.stringKey;
-import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NET_HOST_CARRIER_ICC;
-import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NET_HOST_CARRIER_MCC;
-import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NET_HOST_CARRIER_MNC;
-import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NET_HOST_CARRIER_NAME;
-import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NET_HOST_CONNECTION_SUBTYPE;
-import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NET_HOST_CONNECTION_TYPE;
-
-import androidx.annotation.Nullable;
-import io.opentelemetry.api.common.AttributeKey;
-import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.sdk.trace.ReadWriteSpan;
 import io.opentelemetry.sdk.trace.ReadableSpan;
 import io.opentelemetry.sdk.trace.SpanProcessor;
 
 class RumAttributeAppender implements SpanProcessor {
 
-    static final AttributeKey<String> SESSION_ID_KEY = stringKey("splunk.rumSessionId");
-
     private final VisibleScreenTracker visibleScreenTracker;
     private final ConnectionUtil connectionUtil;
+    private final CurrentNetworkAttributesExtractor networkAttributesExtractor =
+            new CurrentNetworkAttributesExtractor();
 
     RumAttributeAppender(VisibleScreenTracker visibleScreenTracker, ConnectionUtil connectionUtil) {
         this.visibleScreenTracker = visibleScreenTracker;
@@ -50,22 +39,7 @@ public void onStart(Context parentContext, ReadWriteSpan span) {
         span.setAttribute(SplunkRum.SCREEN_NAME_KEY, currentScreen);
 
         CurrentNetwork currentNetwork = connectionUtil.getActiveNetwork();
-        appendNetworkAttributes(span, currentNetwork);
-    }
-
-    static void appendNetworkAttributes(Span span, CurrentNetwork currentNetwork) {
-        setIfNotNull(span, NET_HOST_CONNECTION_TYPE, currentNetwork.getState().getHumanName());
-        setIfNotNull(span, NET_HOST_CONNECTION_SUBTYPE, currentNetwork.getSubType());
-        setIfNotNull(span, NET_HOST_CARRIER_NAME, currentNetwork.getCarrierName());
-        setIfNotNull(span, NET_HOST_CARRIER_MCC, currentNetwork.getCarrierCountryCode());
-        setIfNotNull(span, NET_HOST_CARRIER_MNC, currentNetwork.getCarrierNetworkCode());
-        setIfNotNull(span, NET_HOST_CARRIER_ICC, currentNetwork.getCarrierIsoCountryCode());
-    }
-
-    private static void setIfNotNull(Span span, AttributeKey<String> key, @Nullable String value) {
-        if (value != null) {
-            span.setAttribute(key, value);
-        }
+        span.setAllAttributes(networkAttributesExtractor.extract(currentNetwork));
     }
 
     @Override

splunk-otel-android/src/test/java/com/splunk/rum/CurrentNetworkAttributesExtractorTest.java

@@ -0,0 +1,74 @@
+/*
+ * Copyright Splunk Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.splunk.rum;
+
+import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
+import static org.assertj.core.api.Assertions.entry;
+
+import android.os.Build;
+import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.annotation.Config;
+
+@RunWith(RobolectricTestRunner.class)
+public class CurrentNetworkAttributesExtractorTest {
+
+    final CurrentNetworkAttributesExtractor underTest = new CurrentNetworkAttributesExtractor();
+
+    @Config(sdk = Build.VERSION_CODES.P)
+    @Test
+    public void getNetworkAttributes_withCarrier() {
+        CurrentNetwork currentNetwork =
+                CurrentNetwork.builder(NetworkState.TRANSPORT_CELLULAR)
+                        .subType("aaa")
+                        .carrier(
+                                Carrier.builder()
+                                        .id(206)
+                                        .name("ShadyTel")
+                                        .isoCountryCode("US")
+                                        .mobileCountryCode("usa")
+                                        .mobileNetworkCode("omg")
+                                        .build())
+                        .build();
+
+        assertThat(underTest.extract(currentNetwork))
+                .containsOnly(
+                        entry(SemanticAttributes.NET_HOST_CONNECTION_TYPE, "cell"),
+                        entry(SemanticAttributes.NET_HOST_CONNECTION_SUBTYPE, "aaa"),
+                        entry(SemanticAttributes.NET_HOST_CARRIER_NAME, "ShadyTel"),
+                        entry(SemanticAttributes.NET_HOST_CARRIER_ICC, "US"),
+                        entry(SemanticAttributes.NET_HOST_CARRIER_MCC, "usa"),
+                        entry(SemanticAttributes.NET_HOST_CARRIER_MNC, "omg"));
+    }
+
+    @Config(sdk = Build.VERSION_CODES.O)
+    @Test
+    public void getNetworkAttributes_withoutCarrier() {
+        CurrentNetwork currentNetwork =
+                CurrentNetwork.builder(NetworkState.TRANSPORT_CELLULAR)
+                        .subType("aaa")
+                        .carrier(Carrier.builder().id(42).name("ShadyTel").build())
+                        .build();
+
+        assertThat(underTest.extract(currentNetwork))
+                .containsOnly(
+                        entry(SemanticAttributes.NET_HOST_CONNECTION_TYPE, "cell"),
+                        entry(SemanticAttributes.NET_HOST_CONNECTION_SUBTYPE, "aaa"));
+    }
+}

splunk-otel-android/src/test/java/com/splunk/rum/NetworkMonitorTest.java

@@ -16,69 +16,90 @@
 
 package com.splunk.rum;
 
-import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NET_HOST_CONNECTION_SUBTYPE;
-import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NET_HOST_CONNECTION_TYPE;
+import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
+import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-import io.opentelemetry.api.common.Attributes;
+import android.os.Build;
 import io.opentelemetry.api.trace.Tracer;
-import io.opentelemetry.sdk.testing.junit5.OpenTelemetryExtension;
+import io.opentelemetry.sdk.testing.junit4.OpenTelemetryRule;
 import io.opentelemetry.sdk.trace.data.SpanData;
+import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicBoolean;
-import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.RegisterExtension;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.annotation.Config;
+
+@Config(sdk = Build.VERSION_CODES.P)
+@RunWith(RobolectricTestRunner.class)
+public class NetworkMonitorTest {
+
+    @Rule public OpenTelemetryRule otelTesting = OpenTelemetryRule.create();
 
-class NetworkMonitorTest {
-    @RegisterExtension final OpenTelemetryExtension otelTesting = OpenTelemetryExtension.create();
     private Tracer tracer;
 
-    @BeforeEach
-    void setup() {
+    @Before
+    public void setup() {
         tracer = otelTesting.getOpenTelemetry().getTracer("testTracer");
     }
 
     @Test
-    void networkAvailable_wifi() {
+    public void networkAvailable_wifi() {
         NetworkMonitor.TracingConnectionStateListener listener =
                 new NetworkMonitor.TracingConnectionStateListener(tracer, new AtomicBoolean(true));
 
         listener.onAvailable(true, CurrentNetwork.builder(NetworkState.TRANSPORT_WIFI).build());
 
         List<SpanData> spans = otelTesting.getSpans();
         assertEquals(1, spans.size());
-        SpanData spanData = spans.get(0);
-        assertEquals("network.change", spanData.getName());
-        Attributes attributes = spanData.getAttributes();
-        assertEquals("available", attributes.get(NetworkMonitor.NETWORK_STATUS_KEY));
-        assertEquals("wifi", attributes.get(NET_HOST_CONNECTION_TYPE));
-        assertNull(attributes.get(NET_HOST_CONNECTION_SUBTYPE));
+        assertThat(spans.get(0))
+                .hasName("network.change")
+                .hasAttributesSatisfyingExactly(
+                        equalTo(NetworkMonitor.NETWORK_STATUS_KEY, "available"),
+                        equalTo(SemanticAttributes.NET_HOST_CONNECTION_TYPE, "wifi"));
     }
 
     @Test
-    void networkAvailable_cellular() {
+    public void networkAvailable_cellular() {
         NetworkMonitor.TracingConnectionStateListener listener =
                 new NetworkMonitor.TracingConnectionStateListener(tracer, new AtomicBoolean(true));
 
-        listener.onAvailable(
-                true,
-                CurrentNetwork.builder(NetworkState.TRANSPORT_CELLULAR).subType("LTE").build());
+        CurrentNetwork network =
+                CurrentNetwork.builder(NetworkState.TRANSPORT_CELLULAR)
+                        .subType("LTE")
+                        .carrier(
+                                Carrier.builder()
+                                        .id(206)
+                                        .name("ShadyTel")
+                                        .isoCountryCode("US")
+                                        .mobileCountryCode("usa")
+                                        .mobileNetworkCode("omg")
+                                        .build())
+                        .build();
+
+        listener.onAvailable(true, network);
 
         List<SpanData> spans = otelTesting.getSpans();
         assertEquals(1, spans.size());
-        SpanData spanData = spans.get(0);
-        assertEquals("network.change", spanData.getName());
-        Attributes attributes = spanData.getAttributes();
-        assertEquals("available", attributes.get(NetworkMonitor.NETWORK_STATUS_KEY));
-        assertEquals("cell", attributes.get(NET_HOST_CONNECTION_TYPE));
-        assertEquals("LTE", attributes.get(NET_HOST_CONNECTION_SUBTYPE));
+        assertThat(spans.get(0))
+                .hasName("network.change")
+                .hasAttributesSatisfyingExactly(
+                        equalTo(NetworkMonitor.NETWORK_STATUS_KEY, "available"),
+                        equalTo(SemanticAttributes.NET_HOST_CONNECTION_TYPE, "cell"),
+                        equalTo(SemanticAttributes.NET_HOST_CONNECTION_SUBTYPE, "LTE"),
+                        equalTo(SemanticAttributes.NET_HOST_CARRIER_NAME, "ShadyTel"),
+                        equalTo(SemanticAttributes.NET_HOST_CARRIER_ICC, "US"),
+                        equalTo(SemanticAttributes.NET_HOST_CARRIER_MCC, "usa"),
+                        equalTo(SemanticAttributes.NET_HOST_CARRIER_MNC, "omg"));
     }
 
     @Test
-    void networkLost() {
+    public void networkLost() {
         NetworkMonitor.TracingConnectionStateListener listener =
                 new NetworkMonitor.TracingConnectionStateListener(tracer, new AtomicBoolean(true));
 
@@ -87,16 +108,15 @@ void networkLost() {
 
         List<SpanData> spans = otelTesting.getSpans();
         assertEquals(1, spans.size());
-        SpanData spanData = spans.get(0);
-        assertEquals("network.change", spanData.getName());
-        Attributes attributes = spanData.getAttributes();
-        assertEquals("lost", attributes.get(NetworkMonitor.NETWORK_STATUS_KEY));
-        assertEquals("unavailable", attributes.get(NET_HOST_CONNECTION_TYPE));
-        assertNull(attributes.get(NET_HOST_CONNECTION_SUBTYPE));
+        assertThat(spans.get(0))
+                .hasName("network.change")
+                .hasAttributesSatisfyingExactly(
+                        equalTo(NetworkMonitor.NETWORK_STATUS_KEY, "lost"),
+                        equalTo(SemanticAttributes.NET_HOST_CONNECTION_TYPE, "unavailable"));
     }
 
     @Test
-    void noEventsPlease() {
+    public void noEventsPlease() {
         AtomicBoolean shouldEmitChangeEvents = new AtomicBoolean(false);
 
         NetworkMonitor.TracingConnectionStateListener listener =

splunk-otel-android/src/test/java/com/splunk/rum/PostApi28NetworkDetectorTest.java

@@ -16,7 +16,7 @@
 
 package com.splunk.rum;
 
-import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;

splunk-otel-android/src/test/java/com/splunk/rum/RumAttributeAppenderTest.java

@@ -25,6 +25,7 @@
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.sdk.trace.ReadWriteSpan;
 import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
@@ -66,8 +67,11 @@ void appendAttributesOnStart() {
 
         rumAttributeAppender.onStart(Context.current(), span);
         verify(span).setAttribute(SplunkRum.SCREEN_NAME_KEY, "ScreenOne");
-        verify(span).setAttribute(SemanticAttributes.NET_HOST_CONNECTION_TYPE, "cell");
-        verify(span).setAttribute(SemanticAttributes.NET_HOST_CONNECTION_SUBTYPE, "LTE");
+        verify(span)
+                .setAllAttributes(
+                        Attributes.of(
+                                SemanticAttributes.NET_HOST_CONNECTION_TYPE, "cell",
+                                SemanticAttributes.NET_HOST_CONNECTION_SUBTYPE, "LTE"));
     }
 
     @Test

splunk-otel-android/src/test/java/io/opentelemetry/rum/internal/instrumentation/crash/CrashReporterTest.java

@@ -73,6 +73,7 @@ void integrationTest() {
                         () -> {
                             throw crash;
                         });
+        crashingThread.setDaemon(true);
         crashingThread.start();
 
         Attributes expectedAttributes =