Skip to content

Commit

Permalink
ConnectionUtil refactoring (and splitting out a NetworkAttributesAppe… (
Browse files Browse the repository at this point in the history
#412)

* ConnectionUtil refactoring (and splitting out a NetworkAttributesAppender)

* code review comments
  • Loading branch information
Mateusz Rzeszutek authored Dec 1, 2022
1 parent e2745d3 commit e3cf6db
Show file tree
Hide file tree
Showing 10 changed files with 177 additions and 104 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@
import android.os.Build;
import android.util.Log;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import java.util.List;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.function.Supplier;

// note: based on ideas from stack overflow:
Expand All @@ -40,7 +41,7 @@ class ConnectionUtil {
private final NetworkDetector networkDetector;

private volatile CurrentNetwork currentNetwork = UNKNOWN_NETWORK;
@Nullable private volatile ConnectionStateListener connectionStateListener;
private final List<NetworkChangeListener> listeners = new CopyOnWriteArrayList<>();

ConnectionUtil(NetworkDetector networkDetector) {
this.networkDetector = networkDetector;
Expand Down Expand Up @@ -94,54 +95,41 @@ static NetworkRequest createNetworkMonitoringRequest() {
.build();
}

boolean isOnline() {
return currentNetwork.isOnline();
}

CurrentNetwork getActiveNetwork() {
return currentNetwork;
}

void setInternetStateListener(ConnectionStateListener listener) {
connectionStateListener = listener;
void addNetworkChangeListener(NetworkChangeListener listener) {
listeners.add(listener);
}

private void notifyListeners(CurrentNetwork activeNetwork) {
for (NetworkChangeListener listener : listeners) {
listener.onNetworkChange(activeNetwork);
}
}

private class ConnectionMonitor extends ConnectivityManager.NetworkCallback {

@Override
public void onAvailable(@NonNull Network network) {
Log.d(SplunkRum.LOG_TAG, "onAvailable: ");
CurrentNetwork activeNetwork = refreshNetworkStatus();
ConnectionStateListener connectionStateListener =
ConnectionUtil.this.connectionStateListener;
if (connectionStateListener != null) {
connectionStateListener.onAvailable(true, activeNetwork);
Log.d(
SplunkRum.LOG_TAG,
" onAvailable: isConnected:"
+ isOnline()
+ ", activeNetwork: "
+ activeNetwork);
}
Log.d(SplunkRum.LOG_TAG, " onAvailable: activeNetwork=" + activeNetwork);

notifyListeners(activeNetwork);
}

@Override
public void onLost(@NonNull Network network) {
Log.d(SplunkRum.LOG_TAG, "onLost: ");
// it seems that the "currentActiveNetwork" is still the one that is being lost, so for
// this method, we'll force it to be NO_NETWORK, rather than relying on the
// ConnectivityManager to have the right
// state at the right time during this event.
CurrentNetwork activeNetwork = NO_NETWORK;
currentNetwork = activeNetwork;
ConnectionStateListener connectionStateListener =
ConnectionUtil.this.connectionStateListener;
if (connectionStateListener != null) {
connectionStateListener.onAvailable(false, activeNetwork);
Log.d(
SplunkRum.LOG_TAG,
" onLost: isConnected:" + false + ", activeNetwork: " + activeNetwork);
}
Log.d(SplunkRum.LOG_TAG, " onLost: activeNetwork=" + activeNetwork);

notifyListeners(activeNetwork);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,23 +21,18 @@
import io.opentelemetry.sdk.trace.ReadableSpan;
import io.opentelemetry.sdk.trace.SpanProcessor;

class RumAttributeAppender implements SpanProcessor {
final class NetworkAttributesAppender implements SpanProcessor {

private final VisibleScreenTracker visibleScreenTracker;
private final ConnectionUtil connectionUtil;
private final CurrentNetworkAttributesExtractor networkAttributesExtractor =
new CurrentNetworkAttributesExtractor();

RumAttributeAppender(VisibleScreenTracker visibleScreenTracker, ConnectionUtil connectionUtil) {
this.visibleScreenTracker = visibleScreenTracker;
NetworkAttributesAppender(ConnectionUtil connectionUtil) {
this.connectionUtil = connectionUtil;
}

@Override
public void onStart(Context parentContext, ReadWriteSpan span) {
String currentScreen = visibleScreenTracker.getCurrentlyVisibleScreen();
span.setAttribute(SplunkRum.SCREEN_NAME_KEY, currentScreen);

CurrentNetwork currentNetwork = connectionUtil.getActiveNetwork();
span.setAllAttributes(networkAttributesExtractor.extract(currentNetwork));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.splunk.rum;

interface ConnectionStateListener {
void onAvailable(boolean deviceIsOnline, CurrentNetwork currentNetwork);
interface NetworkChangeListener {

void onNetworkChange(CurrentNetwork currentNetwork);
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ class NetworkMonitor implements ApplicationStateListener {
}

void addConnectivityListener(Tracer tracer) {
connectionUtil.setInternetStateListener(
new TracingConnectionStateListener(tracer, shouldEmitChangeEvents));
connectionUtil.addNetworkChangeListener(
new TracingNetworkChangeListener(tracer, shouldEmitChangeEvents));
}

@Override
Expand All @@ -51,20 +51,20 @@ public void onApplicationBackgrounded() {
}

// visibleForTesting
static class TracingConnectionStateListener implements ConnectionStateListener {
static class TracingNetworkChangeListener implements NetworkChangeListener {

private final Tracer tracer;
private final AtomicBoolean shouldEmitChangeEvents;
private final CurrentNetworkAttributesExtractor networkAttributesExtractor =
new CurrentNetworkAttributesExtractor();

TracingConnectionStateListener(Tracer tracer, AtomicBoolean shouldEmitChangeEvents) {
TracingNetworkChangeListener(Tracer tracer, AtomicBoolean shouldEmitChangeEvents) {
this.tracer = tracer;
this.shouldEmitChangeEvents = shouldEmitChangeEvents;
}

@Override
public void onAvailable(boolean deviceIsOnline, CurrentNetwork activeNetwork) {
public void onNetworkChange(CurrentNetwork activeNetwork) {
if (!shouldEmitChangeEvents.get()) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,14 @@ SplunkRum initialize(ConnectionUtil.Factory connectionUtilFactory, Looper mainLo
GlobalAttributesSpanAppender.create(builder.globalAttributes);
otelRumBuilder.addTracerProviderCustomizer(
(tracerProviderBuilder, app) -> {
NetworkAttributesAppender networkAttributesAppender =
new NetworkAttributesAppender(connectionUtil);
ScreenAttributesAppender screenAttributesAppender =
new ScreenAttributesAppender(visibleScreenTracker);
initializationEvents.add(
new RumInitializer.InitializationEvent(
"attributeAppenderInitialized", timingClock.now()));

SpanExporter zipkinExporter = buildFilteringExporter(connectionUtil);
initializationEvents.add(
new RumInitializer.InitializationEvent(
Expand All @@ -116,15 +124,10 @@ SplunkRum initialize(ConnectionUtil.Factory connectionUtilFactory, Looper mainLo
new RumInitializer.InitializationEvent(
"batchSpanProcessorInitialized", timingClock.now()));

RumAttributeAppender attributeAppender =
new RumAttributeAppender(visibleScreenTracker, connectionUtil);
initializationEvents.add(
new RumInitializer.InitializationEvent(
"attributeAppenderInitialized", timingClock.now()));

tracerProviderBuilder
.addSpanProcessor(attributeAppender)
.addSpanProcessor(globalAttributesSpanAppender)
.addSpanProcessor(networkAttributesAppender)
.addSpanProcessor(screenAttributesAppender)
.addSpanProcessor(batchSpanProcessor)
.setSpanLimits(
SpanLimits.builder()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* 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 io.opentelemetry.context.Context;
import io.opentelemetry.sdk.trace.ReadWriteSpan;
import io.opentelemetry.sdk.trace.ReadableSpan;
import io.opentelemetry.sdk.trace.SpanProcessor;

class ScreenAttributesAppender implements SpanProcessor {

private final VisibleScreenTracker visibleScreenTracker;

ScreenAttributesAppender(VisibleScreenTracker visibleScreenTracker) {
this.visibleScreenTracker = visibleScreenTracker;
}

@Override
public void onStart(Context parentContext, ReadWriteSpan span) {
String currentScreen = visibleScreenTracker.getCurrentlyVisibleScreen();
span.setAttribute(SplunkRum.SCREEN_NAME_KEY, currentScreen);
}

@Override
public boolean isStartRequired() {
return true;
}

@Override
public void onEnd(ReadableSpan span) {}

@Override
public boolean isEndRequired() {
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
package com.splunk.rum;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isA;
Expand Down Expand Up @@ -64,7 +62,6 @@ public void lollipop() {
ConnectionUtil connectionUtil = new ConnectionUtil(networkDetector);
connectionUtil.startMonitoring(() -> networkRequest, connectivityManager);

assertTrue(connectionUtil.isOnline());
assertEquals(
CurrentNetwork.builder(NetworkState.TRANSPORT_WIFI).build(),
connectionUtil.getActiveNetwork());
Expand All @@ -75,18 +72,16 @@ public void lollipop() {
.registerNetworkCallback(eq(networkRequest), monitorCaptor.capture());

AtomicInteger notified = new AtomicInteger(0);
connectionUtil.setInternetStateListener(
(deviceIsOnline, currentNetwork) -> {
connectionUtil.addNetworkChangeListener(
(currentNetwork) -> {
int timesCalled = notified.incrementAndGet();
if (timesCalled == 1) {
assertTrue(deviceIsOnline);
assertEquals(
CurrentNetwork.builder(NetworkState.TRANSPORT_CELLULAR)
.subType("LTE")
.build(),
currentNetwork);
} else {
assertFalse(deviceIsOnline);
assertEquals(
CurrentNetwork.builder(NetworkState.NO_NETWORK_AVAILABLE).build(),
currentNetwork);
Expand Down Expand Up @@ -117,7 +112,6 @@ public void modernSdks() {
ConnectionUtil connectionUtil = new ConnectionUtil(networkDetector);
connectionUtil.startMonitoring(() -> networkRequest, connectivityManager);

assertTrue(connectionUtil.isOnline());
assertEquals(
CurrentNetwork.builder(NetworkState.TRANSPORT_WIFI).build(),
connectionUtil.getActiveNetwork());
Expand All @@ -129,18 +123,16 @@ public void modernSdks() {
verify(connectivityManager).registerDefaultNetworkCallback(monitorCaptor.capture());

AtomicInteger notified = new AtomicInteger(0);
connectionUtil.setInternetStateListener(
(deviceIsOnline, currentNetwork) -> {
connectionUtil.addNetworkChangeListener(
(currentNetwork) -> {
int timesCalled = notified.incrementAndGet();
if (timesCalled == 1) {
assertTrue(deviceIsOnline);
assertEquals(
CurrentNetwork.builder(NetworkState.TRANSPORT_CELLULAR)
.subType("LTE")
.build(),
currentNetwork);
} else {
assertFalse(deviceIsOnline);
assertEquals(
CurrentNetwork.builder(NetworkState.NO_NETWORK_AVAILABLE).build(),
currentNetwork);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* 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 org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
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;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;

@ExtendWith(MockitoExtension.class)
class NetworkAttributesAppenderTest {

@Mock ConnectionUtil connectionUtil;
@Mock ReadWriteSpan span;

@InjectMocks NetworkAttributesAppender underTest;

@Test
void shouldAppendNetworkAttributes() {
when(connectionUtil.getActiveNetwork())
.thenReturn(
CurrentNetwork.builder(NetworkState.TRANSPORT_CELLULAR)
.subType("LTE")
.build());

assertTrue(underTest.isStartRequired());
underTest.onStart(Context.current(), span);

verify(span)
.setAllAttributes(
Attributes.of(
SemanticAttributes.NET_HOST_CONNECTION_TYPE, "cell",
SemanticAttributes.NET_HOST_CONNECTION_SUBTYPE, "LTE"));

assertFalse(underTest.isEndRequired());
}
}
Loading

0 comments on commit e3cf6db

Please sign in to comment.