From d7b082d438fed7ff059cb1b5528d0a8720118136 Mon Sep 17 00:00:00 2001 From: Andrey Li Date: Fri, 5 Apr 2019 14:50:57 -0700 Subject: [PATCH 01/12] Collect network usage metrics --- .../metrics/AbstractCompositeMetrics.java | 53 +++++++++++++++++++ .../android/telemetry/metrics/Metrics.java | 11 ++++ .../telemetry/metrics/MetricsImpl.java | 39 ++++++++++++++ .../telemetry/metrics/TelemetryMetrics.java | 30 +++++++++++ .../network/NetworkErrorInterceptor.java | 30 +++++++++++ .../network/NetworkUsageInterceptor.java | 42 +++++++++++++++ .../network/NetworkUsageMetricsCollector.java | 32 +++++++++++ 7 files changed, 237 insertions(+) create mode 100644 libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/AbstractCompositeMetrics.java create mode 100644 libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/Metrics.java create mode 100644 libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/MetricsImpl.java create mode 100644 libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/TelemetryMetrics.java create mode 100644 libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/network/NetworkErrorInterceptor.java create mode 100644 libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageInterceptor.java create mode 100644 libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageMetricsCollector.java diff --git a/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/AbstractCompositeMetrics.java b/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/AbstractCompositeMetrics.java new file mode 100644 index 000000000..e4663e7ec --- /dev/null +++ b/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/AbstractCompositeMetrics.java @@ -0,0 +1,53 @@ +package com.mapbox.android.telemetry.metrics; + +import android.os.SystemClock; +import android.support.annotation.NonNull; +import android.support.annotation.Nullable; + +import java.util.ArrayDeque; +import java.util.Deque; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +public abstract class AbstractCompositeMetrics { + private final Map> metricsMap = new ConcurrentHashMap<>(); + private final long maxLength; + + public AbstractCompositeMetrics(long maxLength) { + this.maxLength = maxLength; + } + + protected abstract Metrics nextMetrics(long start, long end); + + public void add(String name, long delta) { + long now = SystemClock.uptimeMillis(); + Deque metrics = getOrCreateMetrics(name.trim()); + + Metrics last; + synchronized (this) { + if (now >= metrics.getLast().getEnd()) { + metrics.add(nextMetrics(now, now + maxLength)); + } + last = metrics.getLast(); + } + last.add(delta); + } + + @Nullable + public Metrics getMetrics(@NonNull String name) { + Deque metrics = metricsMap.get(name.trim()); + return metrics != null && !metrics.isEmpty() ? metrics.pop() : null; + } + + @NonNull + private synchronized Deque getOrCreateMetrics(@NonNull String name) { + Deque metrics; + if ((metrics = metricsMap.get(name)) == null) { + metrics = new ArrayDeque<>(); + long now = SystemClock.uptimeMillis(); + metrics.add(nextMetrics(now, now + maxLength)); + metricsMap.put(name, metrics); + } + return metrics; + } +} diff --git a/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/Metrics.java b/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/Metrics.java new file mode 100644 index 000000000..600a34045 --- /dev/null +++ b/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/Metrics.java @@ -0,0 +1,11 @@ +package com.mapbox.android.telemetry.metrics; + +/** + * Metrics object that maintains counter over a time span + */ +public interface Metrics { + void add(long delta); + long getValue(); + long getStart(); + long getEnd(); +} \ No newline at end of file diff --git a/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/MetricsImpl.java b/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/MetricsImpl.java new file mode 100644 index 000000000..acf428fcb --- /dev/null +++ b/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/MetricsImpl.java @@ -0,0 +1,39 @@ +package com.mapbox.android.telemetry.metrics; + +import java.util.concurrent.atomic.AtomicLong; + +class MetricsImpl implements Metrics { + private final long start; + private final long end; + private final AtomicLong value; + + MetricsImpl(long start, long end, long initialValue) { + this.start = start; + this.end = end; + this.value = new AtomicLong(initialValue); + } + + public MetricsImpl(long start, long end) { + this(start, end, 0L); + } + + @Override + public void add(long delta) { + value.addAndGet(delta); + } + + @Override + public long getValue() { + return value.get(); + } + + @Override + public long getStart() { + return start; + } + + @Override + public long getEnd() { + return end; + } +} diff --git a/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/TelemetryMetrics.java b/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/TelemetryMetrics.java new file mode 100644 index 000000000..038f9f291 --- /dev/null +++ b/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/TelemetryMetrics.java @@ -0,0 +1,30 @@ +package com.mapbox.android.telemetry.metrics; + +import static android.net.ConnectivityManager.TYPE_WIFI; + +public class TelemetryMetrics extends AbstractCompositeMetrics { + public static final String EVENTS_TOTAL = "eventCountTotal"; + public static final String EVENTS_FAILED = "eventCountFailed"; + + private static final String MOBILE_BYTES_TX = "cellDataSent"; + private static final String WIFI_BYTES_TX = "wifiDataSent"; + private static final String MOBILE_BYTES_RX = "cellDataReceived"; + private static final String WIFI_BYTES_RX = "wifiDataReceived"; + + public TelemetryMetrics(long maxLength) { + super(maxLength); + } + + public void addRxBytesForType(int networkType, long bytes) { + add(networkType == TYPE_WIFI ? WIFI_BYTES_RX : MOBILE_BYTES_RX, bytes); + } + + public void addTxBytesForType(int networkType, long bytes) { + add(networkType == TYPE_WIFI ? WIFI_BYTES_TX : MOBILE_BYTES_TX, bytes); + } + + @Override + protected Metrics nextMetrics(long start, long end) { + return new MetricsImpl(start, end); + } +} diff --git a/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/network/NetworkErrorInterceptor.java b/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/network/NetworkErrorInterceptor.java new file mode 100644 index 000000000..16d088ca1 --- /dev/null +++ b/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/network/NetworkErrorInterceptor.java @@ -0,0 +1,30 @@ +package com.mapbox.android.telemetry.metrics.network; + +import com.mapbox.android.telemetry.metrics.TelemetryMetrics; +import okhttp3.Interceptor; +import okhttp3.Response; + +import java.io.IOException; + +import static com.mapbox.android.telemetry.metrics.TelemetryMetrics.EVENTS_FAILED; +import static com.mapbox.android.telemetry.metrics.TelemetryMetrics.EVENTS_TOTAL; + +public class NetworkErrorInterceptor implements Interceptor { + private final TelemetryMetrics metrics; + private final int eventCount; + + public NetworkErrorInterceptor(TelemetryMetrics metrics, int eventCount) { + this.metrics = metrics; + this.eventCount = eventCount; + } + + @Override + public Response intercept(Chain chain) throws IOException { + Response response = chain.proceed(chain.request()); + metrics.add(EVENTS_TOTAL, eventCount); + if (!response.isSuccessful()) { + metrics.add(EVENTS_FAILED, eventCount); + } + return response; + } +} diff --git a/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageInterceptor.java b/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageInterceptor.java new file mode 100644 index 000000000..3dbfe180a --- /dev/null +++ b/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageInterceptor.java @@ -0,0 +1,42 @@ +package com.mapbox.android.telemetry.metrics.network; + +import okhttp3.Interceptor; +import okhttp3.Request; +import okhttp3.RequestBody; +import okhttp3.Response; +import okhttp3.ResponseBody; + +import java.io.IOException; + +public class NetworkUsageInterceptor implements Interceptor { + private final NetworkUsageMetricsCollector metricsCollector; + + public NetworkUsageInterceptor(NetworkUsageMetricsCollector metricsCollector) { + this.metricsCollector = metricsCollector; + } + + @Override + public Response intercept(Chain chain) throws IOException { + Request request = chain.request(); + RequestBody requestBody = request.body(); + if (requestBody == null) { + return chain.proceed(request); + } + + Response response; + try { + response = chain.proceed(request); + } catch (IOException ioe) { + throw ioe; + } + + metricsCollector.addTxBytes(requestBody.contentLength()); + ResponseBody responseBody = response.body(); + if (responseBody == null) { + return response; + } + + metricsCollector.addRxBytes(responseBody.contentLength()); + return response; + } +} diff --git a/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageMetricsCollector.java b/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageMetricsCollector.java new file mode 100644 index 000000000..5b69534b2 --- /dev/null +++ b/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageMetricsCollector.java @@ -0,0 +1,32 @@ +package com.mapbox.android.telemetry.metrics.network; + +import android.content.Context; +import android.net.ConnectivityManager; +import android.net.NetworkInfo; +import com.mapbox.android.telemetry.metrics.TelemetryMetrics; + +public class NetworkUsageMetricsCollector { + private static final int TYPE_NONE = -1; + private final ConnectivityManager connectivityManager; + private final TelemetryMetrics metrics; + + public NetworkUsageMetricsCollector(Context context, TelemetryMetrics metrics) { + Context applicationContext = context.getApplicationContext(); + context = applicationContext != null ? applicationContext : context; + this.connectivityManager = (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE); + this.metrics = metrics; + } + + void addRxBytes(long bytes) { + metrics.addRxBytesForType(getActiveNetworkType(), bytes); + } + + void addTxBytes(long bytes) { + metrics.addTxBytesForType(getActiveNetworkType(), bytes); + } + + private int getActiveNetworkType() { + NetworkInfo activeNetwork = connectivityManager.getActiveNetworkInfo(); + return activeNetwork == null ? TYPE_NONE : activeNetwork.getType(); + } +} From c037bfb75a6d5d1ce2563759a3393281002867ba Mon Sep 17 00:00:00 2001 From: Andrey Li Date: Mon, 13 May 2019 13:43:10 -0700 Subject: [PATCH 02/12] Integrate network metrics with telemetry client --- .../android/telemetry/TelemetryClient.java | 2 +- .../telemetry/TelemetryClientBuild.java | 6 -- .../telemetry/TelemetryClientFactory.java | 76 +++++++------------ .../telemetry/TelemetryClientSettings.java | 29 +++++-- 4 files changed, 49 insertions(+), 64 deletions(-) delete mode 100644 libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClientBuild.java diff --git a/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClient.java b/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClient.java index 7cce84d6b..fa20b9c7d 100644 --- a/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClient.java +++ b/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClient.java @@ -150,7 +150,7 @@ private void sendBatch(List batch, Callback callback, boolean serializeNu .post(body) .build(); - OkHttpClient client = setting.getClient(certificateBlacklist); + OkHttpClient client = setting.getClient(certificateBlacklist, batch.size()); client.newCall(request).enqueue(callback); } diff --git a/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClientBuild.java b/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClientBuild.java deleted file mode 100644 index b521a2a39..000000000 --- a/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClientBuild.java +++ /dev/null @@ -1,6 +0,0 @@ -package com.mapbox.android.telemetry; - -interface TelemetryClientBuild { - - TelemetryClient build(ServerInformation serverInformation); -} diff --git a/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClientFactory.java b/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClientFactory.java index 0a97457b3..81a1c0094 100644 --- a/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClientFactory.java +++ b/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClientFactory.java @@ -1,13 +1,9 @@ package com.mapbox.android.telemetry; - import android.content.Context; import android.content.pm.ApplicationInfo; import android.content.pm.PackageManager; -import java.util.HashMap; -import java.util.Map; - class TelemetryClientFactory { private static final String LOG_TAG = "TelemetryClientFactory"; private static final String RETRIEVING_APP_META_DATA_ERROR_MESSAGE = "Failed when retrieving app meta-data: %s"; @@ -15,29 +11,6 @@ class TelemetryClientFactory { private final String userAgent; private final Logger logger; private final CertificateBlacklist certificateBlacklist; - private final Map BUILD_TELEMETRY_CLIENT = new HashMap() { - { - put(Environment.CHINA, new TelemetryClientBuild() { - @Override - public TelemetryClient build(ServerInformation serverInformation) { - return buildTelemetryClient(Environment.CHINA, certificateBlacklist); - } - }); - put(Environment.STAGING, new TelemetryClientBuild() { - @Override - public TelemetryClient build(ServerInformation serverInformation) { - return buildTelemetryClientCustom(serverInformation, certificateBlacklist); - } - }); - put(Environment.COM, new TelemetryClientBuild() { - @Override - public TelemetryClient build(ServerInformation serverInformation) { - return buildTelemetryClient(Environment.COM, certificateBlacklist); - } - }); - } - }; TelemetryClientFactory(String accessToken, String userAgent, Logger logger, CertificateBlacklist certificateBlacklist) { @@ -48,44 +21,47 @@ public TelemetryClient build(ServerInformation serverInformation) { } TelemetryClient obtainTelemetryClient(Context context) { - EnvironmentChain environmentChain = new EnvironmentChain(); - EnvironmentResolver setupChain = environmentChain.setup(); - ServerInformation serverInformation; try { ApplicationInfo appInformation = context.getPackageManager().getApplicationInfo(context.getPackageName(), PackageManager.GET_META_DATA); if (appInformation != null && appInformation.metaData != null) { - serverInformation = setupChain.obtainServerInformation(appInformation.metaData); - return BUILD_TELEMETRY_CLIENT.get(serverInformation.getEnvironment()).build(serverInformation); + EnvironmentChain environmentChain = new EnvironmentChain(); + return buildClientFrom(environmentChain.setup().obtainServerInformation(appInformation.metaData), context); } } catch (Exception exception) { logger.error(LOG_TAG, String.format(RETRIEVING_APP_META_DATA_ERROR_MESSAGE, exception.getMessage())); } - return buildTelemetryClient(Environment.COM, certificateBlacklist); + return buildTelemetryClient(Environment.COM, certificateBlacklist, context); } - private TelemetryClient buildTelemetryClient(Environment environment, CertificateBlacklist certificateBlacklist) { - TelemetryClientSettings telemetryClientSettings = new TelemetryClientSettings.Builder() - .environment(environment) - .build(); - TelemetryClient telemetryClient = new TelemetryClient(accessToken, userAgent, telemetryClientSettings, logger, - certificateBlacklist); - - return telemetryClient; + private TelemetryClient buildTelemetryClient(Environment environment, + CertificateBlacklist certificateBlacklist, + Context context) { + return new TelemetryClient(accessToken, userAgent, + new TelemetryClientSettings.Builder(context) + .environment(environment) + .build(), + logger, certificateBlacklist); } private TelemetryClient buildTelemetryClientCustom(ServerInformation serverInformation, - CertificateBlacklist certificateBlacklist) { - Environment environment = serverInformation.getEnvironment(); - String hostname = serverInformation.getHostname(); - String accessToken = serverInformation.getAccessToken(); - TelemetryClientSettings telemetryClientSettings = new TelemetryClientSettings.Builder() - .environment(environment) - .baseUrl(TelemetryClientSettings.configureUrlHostname(hostname)) + CertificateBlacklist certificateBlacklist, + Context context) { + TelemetryClientSettings telemetryClientSettings = new TelemetryClientSettings.Builder(context) + .environment(serverInformation.getEnvironment()) + .baseUrl(TelemetryClientSettings.configureUrlHostname(serverInformation.getHostname())) .build(); - TelemetryClient telemetryClient = new TelemetryClient(accessToken, userAgent, telemetryClientSettings, logger, + return new TelemetryClient(serverInformation.getAccessToken(), userAgent, telemetryClientSettings, logger, certificateBlacklist); + } - return telemetryClient; + private TelemetryClient buildClientFrom(ServerInformation serverInformation, Context context) { + Environment environment = serverInformation.getEnvironment(); + switch(environment) { + case STAGING: + return buildTelemetryClientCustom(serverInformation, certificateBlacklist, context); + default: + return buildTelemetryClient(environment, certificateBlacklist, context); + } } } diff --git a/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClientSettings.java b/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClientSettings.java index 7a383c76e..f897d9810 100644 --- a/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClientSettings.java +++ b/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClientSettings.java @@ -8,6 +8,10 @@ import javax.net.ssl.SSLSocketFactory; import javax.net.ssl.X509TrustManager; +import android.content.Context; +import com.mapbox.android.telemetry.metrics.network.NetworkErrorInterceptor; +import com.mapbox.android.telemetry.metrics.network.NetworkUsageInterceptor; +import com.mapbox.android.telemetry.metrics.network.NetworkUsageMetricsCollector; import okhttp3.ConnectionSpec; import okhttp3.HttpUrl; import okhttp3.Interceptor; @@ -25,6 +29,7 @@ class TelemetryClientSettings { } }; private static final String HTTPS_SCHEME = "https"; + private final Context context; private Environment environment; private final OkHttpClient client; private final HttpUrl baseUrl; @@ -34,6 +39,7 @@ class TelemetryClientSettings { private boolean debugLoggingEnabled; TelemetryClientSettings(Builder builder) { + this.context = builder.context; this.environment = builder.environment; this.client = builder.client; this.baseUrl = builder.baseUrl; @@ -47,8 +53,13 @@ Environment getEnvironment() { return environment; } - OkHttpClient getClient(CertificateBlacklist certificateBlacklist) { - return configureHttpClient(certificateBlacklist, new GzipRequestInterceptor()); + OkHttpClient getClient(CertificateBlacklist certificateBlacklist, int eventCount) { + // Order in which interceptors are added matter! + Interceptor[] interceptors = { + new GzipRequestInterceptor(), + new NetworkUsageInterceptor(new NetworkUsageMetricsCollector(context, null)), + new NetworkErrorInterceptor(null, eventCount) }; + return configureHttpClient(certificateBlacklist, interceptors); } OkHttpClient getAttachmentClient(CertificateBlacklist certificateBlacklist) { @@ -64,7 +75,7 @@ boolean isDebugLoggingEnabled() { } Builder toBuilder() { - return new Builder() + return new Builder(context) .environment(environment) .client(client) .baseUrl(baseUrl) @@ -81,6 +92,7 @@ static HttpUrl configureUrlHostname(String eventsHost) { } static final class Builder { + Context context; Environment environment = Environment.COM; OkHttpClient client = new OkHttpClient(); HttpUrl baseUrl = null; @@ -89,7 +101,8 @@ static final class Builder { HostnameVerifier hostnameVerifier = null; boolean debugLoggingEnabled = false; - Builder() { + Builder(Context context) { + this.context = context; } Builder environment(Environment environment) { @@ -141,15 +154,17 @@ TelemetryClientSettings build() { } private OkHttpClient configureHttpClient(CertificateBlacklist certificateBlacklist, - Interceptor interceptor) { + Interceptor[] interceptors) { CertificatePinnerFactory factory = new CertificatePinnerFactory(); OkHttpClient.Builder builder = client.newBuilder() .retryOnConnectionFailure(true) .certificatePinner(factory.provideCertificatePinnerFor(environment, certificateBlacklist)) .connectionSpecs(Arrays.asList(ConnectionSpec.MODERN_TLS, ConnectionSpec.COMPATIBLE_TLS)); - if (interceptor != null) { - builder.addInterceptor(interceptor); + if (interceptors != null) { + for (Interceptor interceptor: interceptors) { + builder.addInterceptor(interceptor); + } } if (isSocketFactoryUnset(sslSocketFactory, x509TrustManager)) { From 6fb7a78a677b2f988cc5aa84a1a261b0cf8a375f Mon Sep 17 00:00:00 2001 From: Andrey Li Date: Tue, 14 May 2019 11:16:47 -0700 Subject: [PATCH 03/12] Fix checkstyle warnings --- .../java/com/mapbox/android/telemetry/metrics/Metrics.java | 3 +++ .../com/mapbox/android/telemetry/TelemetryClientFactory.java | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/Metrics.java b/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/Metrics.java index 600a34045..32243ebe7 100644 --- a/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/Metrics.java +++ b/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/Metrics.java @@ -5,7 +5,10 @@ */ public interface Metrics { void add(long delta); + long getValue(); + long getStart(); + long getEnd(); } \ No newline at end of file diff --git a/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClientFactory.java b/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClientFactory.java index 81a1c0094..2c70cc22a 100644 --- a/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClientFactory.java +++ b/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClientFactory.java @@ -57,7 +57,7 @@ private TelemetryClient buildTelemetryClientCustom(ServerInformation serverInfor private TelemetryClient buildClientFrom(ServerInformation serverInformation, Context context) { Environment environment = serverInformation.getEnvironment(); - switch(environment) { + switch (environment) { case STAGING: return buildTelemetryClientCustom(serverInformation, certificateBlacklist, context); default: From 659081293a63f6f75e588ba9340f38c4a293af7f Mon Sep 17 00:00:00 2001 From: Andrey Li Date: Tue, 14 May 2019 11:27:48 -0700 Subject: [PATCH 04/12] Moving metrics to main --- .../android/telemetry/metrics/AbstractCompositeMetrics.java | 0 .../java/com/mapbox/android/telemetry/metrics/Metrics.java | 0 .../java/com/mapbox/android/telemetry/metrics/MetricsImpl.java | 0 .../com/mapbox/android/telemetry/metrics/TelemetryMetrics.java | 0 .../telemetry/metrics/network/NetworkErrorInterceptor.java | 0 .../telemetry/metrics/network/NetworkUsageInterceptor.java | 0 .../telemetry/metrics/network/NetworkUsageMetricsCollector.java | 0 7 files changed, 0 insertions(+), 0 deletions(-) rename libtelemetry/src/{full => main}/java/com/mapbox/android/telemetry/metrics/AbstractCompositeMetrics.java (100%) rename libtelemetry/src/{full => main}/java/com/mapbox/android/telemetry/metrics/Metrics.java (100%) rename libtelemetry/src/{full => main}/java/com/mapbox/android/telemetry/metrics/MetricsImpl.java (100%) rename libtelemetry/src/{full => main}/java/com/mapbox/android/telemetry/metrics/TelemetryMetrics.java (100%) rename libtelemetry/src/{full => main}/java/com/mapbox/android/telemetry/metrics/network/NetworkErrorInterceptor.java (100%) rename libtelemetry/src/{full => main}/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageInterceptor.java (100%) rename libtelemetry/src/{full => main}/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageMetricsCollector.java (100%) diff --git a/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/AbstractCompositeMetrics.java b/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/AbstractCompositeMetrics.java similarity index 100% rename from libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/AbstractCompositeMetrics.java rename to libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/AbstractCompositeMetrics.java diff --git a/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/Metrics.java b/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/Metrics.java similarity index 100% rename from libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/Metrics.java rename to libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/Metrics.java diff --git a/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/MetricsImpl.java b/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/MetricsImpl.java similarity index 100% rename from libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/MetricsImpl.java rename to libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/MetricsImpl.java diff --git a/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/TelemetryMetrics.java b/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/TelemetryMetrics.java similarity index 100% rename from libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/TelemetryMetrics.java rename to libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/TelemetryMetrics.java diff --git a/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/network/NetworkErrorInterceptor.java b/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/network/NetworkErrorInterceptor.java similarity index 100% rename from libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/network/NetworkErrorInterceptor.java rename to libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/network/NetworkErrorInterceptor.java diff --git a/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageInterceptor.java b/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageInterceptor.java similarity index 100% rename from libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageInterceptor.java rename to libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageInterceptor.java diff --git a/libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageMetricsCollector.java b/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageMetricsCollector.java similarity index 100% rename from libtelemetry/src/full/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageMetricsCollector.java rename to libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageMetricsCollector.java From 76f7cc093200e7a2aa94c9ffe080bc9d85d9e781 Mon Sep 17 00:00:00 2001 From: Andrey Li Date: Tue, 14 May 2019 11:28:01 -0700 Subject: [PATCH 05/12] Fix unit tests --- .../com/mapbox/android/telemetry/ConfigurationClientTest.java | 4 ++-- .../java/com/mapbox/android/telemetry/MockWebServerTest.java | 2 +- .../com/mapbox/android/telemetry/TelemetryClientTest.java | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libtelemetry/src/test/java/com/mapbox/android/telemetry/ConfigurationClientTest.java b/libtelemetry/src/test/java/com/mapbox/android/telemetry/ConfigurationClientTest.java index aec904e8c..457a04984 100644 --- a/libtelemetry/src/test/java/com/mapbox/android/telemetry/ConfigurationClientTest.java +++ b/libtelemetry/src/test/java/com/mapbox/android/telemetry/ConfigurationClientTest.java @@ -48,7 +48,7 @@ public void setUp() throws Exception { TelemetryClientSettings settings = provideDefaultTelemetryClientSettings(); CertificateBlacklist mockedBlacklist = mock(CertificateBlacklist.class); - OkHttpClient client = settings.getClient(mockedBlacklist); + OkHttpClient client = settings.getClient(mockedBlacklist, 0); Context mockedContext = getConfigContext(); File mockedFile = mock(File.class); @@ -115,7 +115,7 @@ private TelemetryClientSettings provideDefaultTelemetryClientSettings() { HttpUrl localUrl = obtainBaseEndpointUrl(); SslClient sslClient = SslClient.localhost(); - return new TelemetryClientSettings.Builder() + return new TelemetryClientSettings.Builder(mock(Context.class)) .baseUrl(localUrl) .sslSocketFactory(sslClient.socketFactory) .x509TrustManager(sslClient.trustManager) diff --git a/libtelemetry/src/test/java/com/mapbox/android/telemetry/MockWebServerTest.java b/libtelemetry/src/test/java/com/mapbox/android/telemetry/MockWebServerTest.java index f7ede2e98..9045c52d8 100644 --- a/libtelemetry/src/test/java/com/mapbox/android/telemetry/MockWebServerTest.java +++ b/libtelemetry/src/test/java/com/mapbox/android/telemetry/MockWebServerTest.java @@ -177,7 +177,7 @@ TelemetryClientSettings provideDefaultTelemetryClientSettings() { HttpUrl localUrl = obtainBaseEndpointUrl(); SslClient sslClient = SslClient.localhost(); - return new TelemetryClientSettings.Builder() + return new TelemetryClientSettings.Builder(mock(Context.class)) .baseUrl(localUrl) .sslSocketFactory(sslClient.socketFactory) .x509TrustManager(sslClient.trustManager) diff --git a/libtelemetry/src/test/java/com/mapbox/android/telemetry/TelemetryClientTest.java b/libtelemetry/src/test/java/com/mapbox/android/telemetry/TelemetryClientTest.java index 40eb81949..c7c3257b2 100644 --- a/libtelemetry/src/test/java/com/mapbox/android/telemetry/TelemetryClientTest.java +++ b/libtelemetry/src/test/java/com/mapbox/android/telemetry/TelemetryClientTest.java @@ -186,7 +186,7 @@ public void checksRequestTimeoutFailure() throws Exception { .build(); HttpUrl localUrl = obtainBaseEndpointUrl(); SslClient sslClient = SslClient.localhost(); - TelemetryClientSettings telemetryClientSettings = new TelemetryClientSettings.Builder() + TelemetryClientSettings telemetryClientSettings = new TelemetryClientSettings.Builder(mockedContext) .client(localOkHttpClientWithShortTimeout) .baseUrl(localUrl) .sslSocketFactory(sslClient.socketFactory) From 0d516b9a5742b9fde7339fbcd019924ad73f980c Mon Sep 17 00:00:00 2001 From: Andrey Li Date: Wed, 15 May 2019 09:07:50 -0700 Subject: [PATCH 06/12] Fix unit tests --- .../telemetry/TelemetryClientSettings.java | 8 +- .../network/NetworkUsageMetricsCollector.java | 2 - .../android/telemetry/MockWebServerTest.java | 8 +- ...emetryClientAppUserTurnstileEventTest.java | 6 +- .../TelemetryClientLocationEventTest.java | 5 +- .../telemetry/TelemetryClientTest.java | 79 +++++++++++-------- 6 files changed, 63 insertions(+), 45 deletions(-) diff --git a/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClientSettings.java b/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClientSettings.java index f897d9810..8dbc2f561 100644 --- a/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClientSettings.java +++ b/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClientSettings.java @@ -3,12 +3,14 @@ import java.util.Arrays; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.TimeUnit; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLSocketFactory; import javax.net.ssl.X509TrustManager; import android.content.Context; +import com.mapbox.android.telemetry.metrics.TelemetryMetrics; import com.mapbox.android.telemetry.metrics.network.NetworkErrorInterceptor; import com.mapbox.android.telemetry.metrics.network.NetworkUsageInterceptor; import com.mapbox.android.telemetry.metrics.network.NetworkUsageMetricsCollector; @@ -54,11 +56,13 @@ Environment getEnvironment() { } OkHttpClient getClient(CertificateBlacklist certificateBlacklist, int eventCount) { + // FIXME: access global reference + TelemetryMetrics metrics = new TelemetryMetrics(TimeUnit.HOURS.toMillis(24)); // Order in which interceptors are added matter! Interceptor[] interceptors = { new GzipRequestInterceptor(), - new NetworkUsageInterceptor(new NetworkUsageMetricsCollector(context, null)), - new NetworkErrorInterceptor(null, eventCount) }; + new NetworkUsageInterceptor(new NetworkUsageMetricsCollector(context, metrics)), + new NetworkErrorInterceptor(metrics, eventCount) }; return configureHttpClient(certificateBlacklist, interceptors); } diff --git a/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageMetricsCollector.java b/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageMetricsCollector.java index 5b69534b2..bb1864f6d 100644 --- a/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageMetricsCollector.java +++ b/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageMetricsCollector.java @@ -11,8 +11,6 @@ public class NetworkUsageMetricsCollector { private final TelemetryMetrics metrics; public NetworkUsageMetricsCollector(Context context, TelemetryMetrics metrics) { - Context applicationContext = context.getApplicationContext(); - context = applicationContext != null ? applicationContext : context; this.connectivityManager = (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE); this.metrics = metrics; } diff --git a/libtelemetry/src/test/java/com/mapbox/android/telemetry/MockWebServerTest.java b/libtelemetry/src/test/java/com/mapbox/android/telemetry/MockWebServerTest.java index 9045c52d8..d7e49521c 100644 --- a/libtelemetry/src/test/java/com/mapbox/android/telemetry/MockWebServerTest.java +++ b/libtelemetry/src/test/java/com/mapbox/android/telemetry/MockWebServerTest.java @@ -132,8 +132,8 @@ String obtainContentFromFile(String fileName) throws IOException { return stringBuilder.toString(); } - TelemetryClient obtainATelemetryClient(String accessToken, String userAgent) { - TelemetryClientSettings telemetryClientSettings = provideDefaultTelemetryClientSettings(); + TelemetryClient obtainATelemetryClient(String accessToken, String userAgent, Context context) { + TelemetryClientSettings telemetryClientSettings = provideDefaultTelemetryClientSettings(context); Logger mockedLogger = mock(Logger.class); CertificateBlacklist mockedBlacklist = mock(CertificateBlacklist.class); return new TelemetryClient(accessToken, userAgent, telemetryClientSettings, mockedLogger, mockedBlacklist); @@ -173,11 +173,11 @@ private Buffer gunzip(Buffer gzipped) throws IOException { return result; } - TelemetryClientSettings provideDefaultTelemetryClientSettings() { + TelemetryClientSettings provideDefaultTelemetryClientSettings(Context context) { HttpUrl localUrl = obtainBaseEndpointUrl(); SslClient sslClient = SslClient.localhost(); - return new TelemetryClientSettings.Builder(mock(Context.class)) + return new TelemetryClientSettings.Builder(context) .baseUrl(localUrl) .sslSocketFactory(sslClient.socketFactory) .x509TrustManager(sslClient.trustManager) diff --git a/libtelemetry/src/test/java/com/mapbox/android/telemetry/TelemetryClientAppUserTurnstileEventTest.java b/libtelemetry/src/test/java/com/mapbox/android/telemetry/TelemetryClientAppUserTurnstileEventTest.java index f741aa80b..f905d4a89 100644 --- a/libtelemetry/src/test/java/com/mapbox/android/telemetry/TelemetryClientAppUserTurnstileEventTest.java +++ b/libtelemetry/src/test/java/com/mapbox/android/telemetry/TelemetryClientAppUserTurnstileEventTest.java @@ -11,16 +11,16 @@ import okhttp3.Callback; -import static org.mockito.Mockito.RETURNS_DEEP_STUBS; import static org.mockito.Mockito.mock; public class TelemetryClientAppUserTurnstileEventTest extends MockWebServerTest { @Test public void sendsTheCorrectBodyPostingAppUserTurnstileEvent() throws Exception { - Context mockedContext = mock(Context.class, RETURNS_DEEP_STUBS); + Context mockedContext = TelemetryClientTest.getMockedContext(); MapboxTelemetry.applicationContext = mockedContext; - TelemetryClient telemetryClient = obtainATelemetryClient("anyAccessToken", "anyUserAgent"); + TelemetryClient telemetryClient = obtainATelemetryClient("anyAccessToken", "anyUserAgent", + mockedContext); Event anAppUserTurnstile = new AppUserTurnstile("anySdkIdentifier", "anySdkVersion", false); List theAppUserTurnstile = obtainEvents(anAppUserTurnstile); Callback mockedCallback = mock(Callback.class); diff --git a/libtelemetry/src/test/java/com/mapbox/android/telemetry/TelemetryClientLocationEventTest.java b/libtelemetry/src/test/java/com/mapbox/android/telemetry/TelemetryClientLocationEventTest.java index 373d4a8b6..edf6e405c 100644 --- a/libtelemetry/src/test/java/com/mapbox/android/telemetry/TelemetryClientLocationEventTest.java +++ b/libtelemetry/src/test/java/com/mapbox/android/telemetry/TelemetryClientLocationEventTest.java @@ -20,12 +20,13 @@ public class TelemetryClientLocationEventTest extends MockWebServerTest { @Test public void sendsTheCorrectBodyPostingLocationEvent() throws Exception { - Context mockedContext = mock(Context.class, RETURNS_DEEP_STUBS); + Context mockedContext = TelemetryClientTest.getMockedContext(); MapboxTelemetry.applicationContext = mockedContext; ActivityManager mockedActivityManager = mock(ActivityManager.class, RETURNS_DEEP_STUBS); when(mockedContext.getSystemService(Context.ACTIVITY_SERVICE)).thenReturn(mockedActivityManager); - TelemetryClient telemetryClient = obtainATelemetryClient("anyAccessToken", "anyUserAgent"); + TelemetryClient telemetryClient = obtainATelemetryClient("anyAccessToken", "anyUserAgent", + mockedContext); double aLatitude = 40.416775; double aLongitude = -3.703790; Event aLocationEvent = new LocationEvent("aSessionId", aLatitude, aLongitude, ""); diff --git a/libtelemetry/src/test/java/com/mapbox/android/telemetry/TelemetryClientTest.java b/libtelemetry/src/test/java/com/mapbox/android/telemetry/TelemetryClientTest.java index c7c3257b2..d5aba8445 100644 --- a/libtelemetry/src/test/java/com/mapbox/android/telemetry/TelemetryClientTest.java +++ b/libtelemetry/src/test/java/com/mapbox/android/telemetry/TelemetryClientTest.java @@ -2,6 +2,8 @@ import android.content.Context; +import android.net.ConnectivityManager; +import android.net.NetworkInfo; import com.google.gson.Gson; import org.junit.Test; @@ -25,23 +27,26 @@ import okhttp3.Response; import okhttp3.mockwebserver.internal.tls.SslClient; +import static android.net.ConnectivityManager.TYPE_WIFI; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.contains; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.RETURNS_DEEP_STUBS; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; public class TelemetryClientTest extends MockWebServerTest { @Test public void sendsContentTypeHeader() throws Exception { - Context mockedContext = mock(Context.class, RETURNS_DEEP_STUBS); - MapboxTelemetry.applicationContext = mockedContext; - TelemetryClient telemetryClient = obtainATelemetryClient("anyAccessToken", "anyUserAgent"); + MapboxTelemetry.applicationContext = getMockedContext(); + TelemetryClient telemetryClient = obtainATelemetryClient("anyAccessToken", "anyUserAgent", + MapboxTelemetry.applicationContext); List mockedEvent = obtainAnEvent(); Callback mockedCallback = mock(Callback.class); enqueueMockResponse(); @@ -53,9 +58,9 @@ public void sendsContentTypeHeader() throws Exception { @Test public void sendsContentEncodingHeader() throws Exception { - Context mockedContext = mock(Context.class, RETURNS_DEEP_STUBS); - MapboxTelemetry.applicationContext = mockedContext; - TelemetryClient telemetryClient = obtainATelemetryClient("anyAccessToken", "anyUserAgent"); + MapboxTelemetry.applicationContext = getMockedContext(); + TelemetryClient telemetryClient = obtainATelemetryClient("anyAccessToken", "anyUserAgent", + MapboxTelemetry.applicationContext); List mockedEvent = obtainAnEvent(); Callback mockedCallback = mock(Callback.class); enqueueMockResponse(); @@ -67,9 +72,9 @@ public void sendsContentEncodingHeader() throws Exception { @Test public void sendsPostEventRequestWithTheCorrectAccessTokenParameter() throws Exception { - Context mockedContext = mock(Context.class, RETURNS_DEEP_STUBS); - MapboxTelemetry.applicationContext = mockedContext; - TelemetryClient telemetryClient = obtainATelemetryClient("theAccessToken", "anyUserAgent"); + MapboxTelemetry.applicationContext = getMockedContext(); + TelemetryClient telemetryClient = obtainATelemetryClient("theAccessToken", "anyUserAgent", + MapboxTelemetry.applicationContext); List mockedEvent = obtainAnEvent(); Callback mockedCallback = mock(Callback.class); enqueueMockResponse(); @@ -81,9 +86,9 @@ public void sendsPostEventRequestWithTheCorrectAccessTokenParameter() throws Exc @Test public void sendsUserAgentHeader() throws Exception { - Context mockedContext = mock(Context.class, RETURNS_DEEP_STUBS); - MapboxTelemetry.applicationContext = mockedContext; - TelemetryClient telemetryClient = obtainATelemetryClient("anyAccessToken", "theUserAgent"); + MapboxTelemetry.applicationContext = getMockedContext(); + TelemetryClient telemetryClient = obtainATelemetryClient("anyAccessToken", "theUserAgent", + MapboxTelemetry.applicationContext); List mockedEvent = obtainAnEvent(); Callback mockedCallback = mock(Callback.class); enqueueMockResponse(); @@ -95,9 +100,9 @@ public void sendsUserAgentHeader() throws Exception { @Test public void sendsPostEventRequestToTheCorrectEndpoint() throws Exception { - Context mockedContext = mock(Context.class, RETURNS_DEEP_STUBS); - MapboxTelemetry.applicationContext = mockedContext; - TelemetryClient telemetryClient = obtainATelemetryClient("anyAccessToken", "anyUserAgent"); + MapboxTelemetry.applicationContext = getMockedContext(); + TelemetryClient telemetryClient = obtainATelemetryClient("anyAccessToken", "anyUserAgent", + MapboxTelemetry.applicationContext); List mockedEvent = obtainAnEvent(); Callback mockedCallback = mock(Callback.class); enqueueMockResponse(); @@ -109,9 +114,9 @@ public void sendsPostEventRequestToTheCorrectEndpoint() throws Exception { @Test public void sendsTheCorrectBodyPostingAnEvent() throws Exception { - Context mockedContext = mock(Context.class, RETURNS_DEEP_STUBS); - MapboxTelemetry.applicationContext = mockedContext; - TelemetryClient telemetryClient = obtainATelemetryClient("anyAccessToken", "anyUserAgent"); + MapboxTelemetry.applicationContext = getMockedContext(); + TelemetryClient telemetryClient = obtainATelemetryClient("anyAccessToken", "anyUserAgent", + MapboxTelemetry.applicationContext); List theEvent = obtainAnEvent(); Callback mockedCallback = mock(Callback.class); enqueueMockResponse(); @@ -125,9 +130,9 @@ public void sendsTheCorrectBodyPostingAnEvent() throws Exception { @Test public void receivesNoBodyPostingAnEventSuccessfully() throws Exception { - Context mockedContext = mock(Context.class, RETURNS_DEEP_STUBS); - MapboxTelemetry.applicationContext = mockedContext; - TelemetryClient telemetryClient = obtainATelemetryClient("anyAccessToken", "anyUserAgent"); + MapboxTelemetry.applicationContext = getMockedContext(); + TelemetryClient telemetryClient = obtainATelemetryClient("anyAccessToken", "anyUserAgent", + MapboxTelemetry.applicationContext); List theEvent = obtainAnEvent(); Callback mockedCallback = mock(Callback.class); enqueueMockNoResponse(204); @@ -139,9 +144,9 @@ public void receivesNoBodyPostingAnEventSuccessfully() throws Exception { @Test public void parsesUnauthorizedRequestResponseProperlyPostingAnEvent() throws Exception { - Context mockedContext = mock(Context.class, RETURNS_DEEP_STUBS); - MapboxTelemetry.applicationContext = mockedContext; - TelemetryClient telemetryClient = obtainATelemetryClient("anyAccessToken", "anyUserAgent"); + MapboxTelemetry.applicationContext = getMockedContext(); + TelemetryClient telemetryClient = obtainATelemetryClient("anyAccessToken", "anyUserAgent", + MapboxTelemetry.applicationContext); List theEvent = obtainAnEvent(); final CountDownLatch latch = new CountDownLatch(1); final AtomicReference bodyRef = new AtomicReference<>(); @@ -159,9 +164,9 @@ public void parsesUnauthorizedRequestResponseProperlyPostingAnEvent() throws Exc @Test public void parsesInvalidMessageBodyResponseProperlyPostingAnEvent() throws Exception { - Context mockedContext = mock(Context.class, RETURNS_DEEP_STUBS); - MapboxTelemetry.applicationContext = mockedContext; - TelemetryClient telemetryClient = obtainATelemetryClient("anyAccessToken", "anyUserAgent"); + MapboxTelemetry.applicationContext = getMockedContext();; + TelemetryClient telemetryClient = obtainATelemetryClient("anyAccessToken", "anyUserAgent", + MapboxTelemetry.applicationContext); List theEvent = obtainAnEvent(); final CountDownLatch latch = new CountDownLatch(1); final AtomicReference bodyRef = new AtomicReference<>(); @@ -179,7 +184,7 @@ public void parsesInvalidMessageBodyResponseProperlyPostingAnEvent() throws Exce @Test public void checksRequestTimeoutFailure() throws Exception { - Context mockedContext = mock(Context.class, RETURNS_DEEP_STUBS); + Context mockedContext = getMockedContext(); MapboxTelemetry.applicationContext = mockedContext; OkHttpClient localOkHttpClientWithShortTimeout = new OkHttpClient.Builder() .readTimeout(100, TimeUnit.MILLISECONDS) @@ -215,9 +220,9 @@ public boolean verify(String hostname, SSLSession session) { @Test public void checksDebugLoggingEnabledBatch() throws Exception { - Context mockedContext = mock(Context.class, RETURNS_DEEP_STUBS); + Context mockedContext = getMockedContext(); MapboxTelemetry.applicationContext = mockedContext; - TelemetryClientSettings clientSettings = provideDefaultTelemetryClientSettings(); + TelemetryClientSettings clientSettings = provideDefaultTelemetryClientSettings(mockedContext); Logger mockedLogger = mock(Logger.class); List mockedEvent = obtainAnEvent(); Callback mockedCallback = mock(Callback.class); @@ -234,9 +239,9 @@ public void checksDebugLoggingEnabledBatch() throws Exception { @Test public void checksDebugLoggingEnabledAttachment() throws Exception { - Context mockedContext = mock(Context.class, RETURNS_DEEP_STUBS); + Context mockedContext = getMockedContext(); MapboxTelemetry.applicationContext = mockedContext; - TelemetryClientSettings clientSettings = provideDefaultTelemetryClientSettings(); + TelemetryClientSettings clientSettings = provideDefaultTelemetryClientSettings(mockedContext); Logger mockedLogger = mock(Logger.class); TelemetryClient telemetryClient = new TelemetryClient("anyAccessToken", "anyUserAgent", clientSettings, @@ -283,4 +288,14 @@ private void assertTelemetryResponseEquals(String responseBody, String expectedM assertEquals(expectedTelemetryResponse, actualTelemetryResponse); } + + static Context getMockedContext() { + Context mockedContext = mock(Context.class, RETURNS_DEEP_STUBS); + ConnectivityManager mockedCm = mock(ConnectivityManager.class, RETURNS_DEEP_STUBS); + NetworkInfo mockedNetworkInfo = mock(NetworkInfo.class, RETURNS_DEEP_STUBS); + when(mockedNetworkInfo.getType()).thenReturn(TYPE_WIFI); + when(mockedCm.getActiveNetworkInfo()).thenReturn(mockedNetworkInfo); + when(mockedContext.getSystemService(anyString())).thenReturn(mockedCm); + return mockedContext; + } } \ No newline at end of file From 8ae2a928ce87a4235b7664a39f2eb9fb5bba21b0 Mon Sep 17 00:00:00 2001 From: Andrey Li Date: Fri, 7 Jun 2019 17:34:14 -0700 Subject: [PATCH 07/12] Move metric core classes to libcore --- .../metrics/AbstractCompositeMetrics.java | 28 +++++++++++++-- .../mapbox/android/core/metrics/Metrics.java | 35 +++++++++++++++++++ .../android/core}/metrics/MetricsImpl.java | 7 ++-- .../android/telemetry/metrics/Metrics.java | 14 -------- .../telemetry/metrics/TelemetryMetrics.java | 4 +++ 5 files changed, 70 insertions(+), 18 deletions(-) rename {libtelemetry/src/main/java/com/mapbox/android/telemetry => libcore/src/main/java/com/mapbox/android/core}/metrics/AbstractCompositeMetrics.java (65%) create mode 100644 libcore/src/main/java/com/mapbox/android/core/metrics/Metrics.java rename {libtelemetry/src/main/java/com/mapbox/android/telemetry => libcore/src/main/java/com/mapbox/android/core}/metrics/MetricsImpl.java (82%) delete mode 100644 libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/Metrics.java diff --git a/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/AbstractCompositeMetrics.java b/libcore/src/main/java/com/mapbox/android/core/metrics/AbstractCompositeMetrics.java similarity index 65% rename from libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/AbstractCompositeMetrics.java rename to libcore/src/main/java/com/mapbox/android/core/metrics/AbstractCompositeMetrics.java index e4663e7ec..56487fac2 100644 --- a/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/AbstractCompositeMetrics.java +++ b/libcore/src/main/java/com/mapbox/android/core/metrics/AbstractCompositeMetrics.java @@ -1,4 +1,4 @@ -package com.mapbox.android.telemetry.metrics; +package com.mapbox.android.core.metrics; import android.os.SystemClock; import android.support.annotation.NonNull; @@ -9,16 +9,40 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +/** + * Tracks stats over rolling time window of the max length. + * Optimized for write heavy metrics. + */ public abstract class AbstractCompositeMetrics { private final Map> metricsMap = new ConcurrentHashMap<>(); private final long maxLength; + /** + * Create instance of the composite metric. + * + * @param maxLength max time window in milliseconds. + */ public AbstractCompositeMetrics(long maxLength) { this.maxLength = maxLength; } + /** + * Called by child class when new metrics is needed. + * Concrete implementation of the metric is delegated to child class. + * + * @param start of the time span. + * @param end of the time span. + * @return reference to the new metric object. + */ protected abstract Metrics nextMetrics(long start, long end); + /** + * Adds value to the metric and occasionally creates new metric + * if the delta is out of the exiting metric span. + * + * @param name name of the metric. + * @param delta value to increment. + */ public void add(String name, long delta) { long now = SystemClock.uptimeMillis(); Deque metrics = getOrCreateMetrics(name.trim()); @@ -34,7 +58,7 @@ public void add(String name, long delta) { } @Nullable - public Metrics getMetrics(@NonNull String name) { + Metrics getMetrics(@NonNull String name) { Deque metrics = metricsMap.get(name.trim()); return metrics != null && !metrics.isEmpty() ? metrics.pop() : null; } diff --git a/libcore/src/main/java/com/mapbox/android/core/metrics/Metrics.java b/libcore/src/main/java/com/mapbox/android/core/metrics/Metrics.java new file mode 100644 index 000000000..65c0b9f45 --- /dev/null +++ b/libcore/src/main/java/com/mapbox/android/core/metrics/Metrics.java @@ -0,0 +1,35 @@ +package com.mapbox.android.core.metrics; + +/** + * Metrics object counter over a time span + */ +public interface Metrics { + + /** + * Increment metric + * + * @param delta value + */ + void add(long delta); + + /** + * Return current metric value + * + * @return current state of the metric + */ + long getValue(); + + /** + * Return start of the time span [start, end] + * + * @return timestamp in milliseconds. + */ + long getStart(); + + /** + * Return end of the time span [start, end] + * + * @return timestamp in milliseconds. + */ + long getEnd(); +} \ No newline at end of file diff --git a/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/MetricsImpl.java b/libcore/src/main/java/com/mapbox/android/core/metrics/MetricsImpl.java similarity index 82% rename from libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/MetricsImpl.java rename to libcore/src/main/java/com/mapbox/android/core/metrics/MetricsImpl.java index acf428fcb..1a54c65d2 100644 --- a/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/MetricsImpl.java +++ b/libcore/src/main/java/com/mapbox/android/core/metrics/MetricsImpl.java @@ -1,8 +1,11 @@ -package com.mapbox.android.telemetry.metrics; +package com.mapbox.android.core.metrics; import java.util.concurrent.atomic.AtomicLong; -class MetricsImpl implements Metrics { +/** + * Default implementation of the metric. + */ +public class MetricsImpl implements Metrics { private final long start; private final long end; private final AtomicLong value; diff --git a/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/Metrics.java b/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/Metrics.java deleted file mode 100644 index 32243ebe7..000000000 --- a/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/Metrics.java +++ /dev/null @@ -1,14 +0,0 @@ -package com.mapbox.android.telemetry.metrics; - -/** - * Metrics object that maintains counter over a time span - */ -public interface Metrics { - void add(long delta); - - long getValue(); - - long getStart(); - - long getEnd(); -} \ No newline at end of file diff --git a/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/TelemetryMetrics.java b/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/TelemetryMetrics.java index 038f9f291..aebe97676 100644 --- a/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/TelemetryMetrics.java +++ b/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/TelemetryMetrics.java @@ -1,5 +1,9 @@ package com.mapbox.android.telemetry.metrics; +import com.mapbox.android.core.metrics.AbstractCompositeMetrics; +import com.mapbox.android.core.metrics.Metrics; +import com.mapbox.android.core.metrics.MetricsImpl; + import static android.net.ConnectivityManager.TYPE_WIFI; public class TelemetryMetrics extends AbstractCompositeMetrics { From 33f058e0ac5ef81c4deb24d0faf45d9204538fcd Mon Sep 17 00:00:00 2001 From: Andrey Li Date: Fri, 7 Jun 2019 17:46:39 -0700 Subject: [PATCH 08/12] Add telemetry client and cleanup integration point --- .../telemetry/TelemetryClientSettings.java | 15 ++---- .../metrics/TelemetryMetricsClient.java | 54 +++++++++++++++++++ .../telemetry/TelemetryClientTest.java | 2 +- 3 files changed, 59 insertions(+), 12 deletions(-) create mode 100644 libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/TelemetryMetricsClient.java diff --git a/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClientSettings.java b/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClientSettings.java index 8dbc2f561..81097733e 100644 --- a/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClientSettings.java +++ b/libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClientSettings.java @@ -3,17 +3,11 @@ import java.util.Arrays; import java.util.HashMap; import java.util.Map; -import java.util.concurrent.TimeUnit; - import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLSocketFactory; import javax.net.ssl.X509TrustManager; import android.content.Context; -import com.mapbox.android.telemetry.metrics.TelemetryMetrics; -import com.mapbox.android.telemetry.metrics.network.NetworkErrorInterceptor; -import com.mapbox.android.telemetry.metrics.network.NetworkUsageInterceptor; -import com.mapbox.android.telemetry.metrics.network.NetworkUsageMetricsCollector; import okhttp3.ConnectionSpec; import okhttp3.HttpUrl; import okhttp3.Interceptor; @@ -56,13 +50,12 @@ Environment getEnvironment() { } OkHttpClient getClient(CertificateBlacklist certificateBlacklist, int eventCount) { - // FIXME: access global reference - TelemetryMetrics metrics = new TelemetryMetrics(TimeUnit.HOURS.toMillis(24)); // Order in which interceptors are added matter! Interceptor[] interceptors = { - new GzipRequestInterceptor(), - new NetworkUsageInterceptor(new NetworkUsageMetricsCollector(context, metrics)), - new NetworkErrorInterceptor(metrics, eventCount) }; + new GzipRequestInterceptor() }; + // TODO: add network interceptors in the following order + // new NetworkUsageInterceptor(new NetworkUsageMetricsCollector(context, metrics)), + // new NetworkErrorInterceptor(metrics, eventCount) }; return configureHttpClient(certificateBlacklist, interceptors); } diff --git a/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/TelemetryMetricsClient.java b/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/TelemetryMetricsClient.java new file mode 100644 index 000000000..408281536 --- /dev/null +++ b/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/TelemetryMetricsClient.java @@ -0,0 +1,54 @@ +package com.mapbox.android.telemetry.metrics; + +import android.content.Context; +import android.support.annotation.NonNull; +import android.support.annotation.VisibleForTesting; + +import java.util.concurrent.TimeUnit; + +public class TelemetryMetricsClient { + private static final String TELEMETRY_METRICS_USER_AGENT = "mapbox-android-metrics"; + private static TelemetryMetricsClient telemetryMetricsClient; + private static final Object lock = new Object(); + private final TelemetryMetrics telemetryMetrics; + + @VisibleForTesting + TelemetryMetricsClient(@NonNull TelemetryMetrics telemetryMetrics) { + this.telemetryMetrics = telemetryMetrics; + } + + public static TelemetryMetricsClient install(@NonNull Context context) { + Context applicationContext; + if (context.getApplicationContext() == null) { + // In shared processes content providers getApplicationContext() can return null. + applicationContext = context; + } else { + applicationContext = context.getApplicationContext(); + } + + // TODO: fetch metrics state file + TelemetryMetrics metrics = new TelemetryMetrics(TimeUnit.HOURS.toMillis(24)); + synchronized (lock) { + if (telemetryMetricsClient == null) { + telemetryMetricsClient = new TelemetryMetricsClient(metrics); + } + } + return telemetryMetricsClient; + } + + @NonNull + public static TelemetryMetricsClient getInstance() { + synchronized (lock) { + if (telemetryMetricsClient != null) { + return telemetryMetricsClient; + } else { + throw new IllegalStateException("TelemetryMetricsClient is not installed."); + } + } + } + + @NonNull + public TelemetryMetrics getMetrics() { + return telemetryMetrics; + } +} diff --git a/libtelemetry/src/test/java/com/mapbox/android/telemetry/TelemetryClientTest.java b/libtelemetry/src/test/java/com/mapbox/android/telemetry/TelemetryClientTest.java index 3f3dc8741..c5792393d 100644 --- a/libtelemetry/src/test/java/com/mapbox/android/telemetry/TelemetryClientTest.java +++ b/libtelemetry/src/test/java/com/mapbox/android/telemetry/TelemetryClientTest.java @@ -189,7 +189,7 @@ public void checksRequestTimeoutFailure() throws Exception { .readTimeout(100, TimeUnit.MILLISECONDS) .build(); HttpUrl localUrl = obtainBaseEndpointUrl(); - TelemetryClientSettings telemetryClientSettings = new TelemetryClientSettings.Builder() + TelemetryClientSettings telemetryClientSettings = new TelemetryClientSettings.Builder(mockedContext) .client(localOkHttpClientWithShortTimeout) .baseUrl(localUrl) .sslSocketFactory(clientCertificates.sslSocketFactory()) From 4865bbb55634b4c11c00ab0524253f5c275fe995 Mon Sep 17 00:00:00 2001 From: Andrey Li Date: Mon, 10 Jun 2019 15:44:35 -0700 Subject: [PATCH 09/12] Add unit tests and document public apis --- ...tractCompositeMetricsInstrumentedTest.java | 32 ++++++++++ .../android/core/metrics/MetricsImpl.java | 37 +++++++++++- .../metrics/AbstractCompositeMetricsTest.java | 58 ++++++++++++++++++ .../android/core/metrics/MetricsImplTest.java | 59 +++++++++++++++++++ 4 files changed, 183 insertions(+), 3 deletions(-) create mode 100644 libcore/src/androidTest/java/com/mapbox/android/core/metrics/AbstractCompositeMetricsInstrumentedTest.java create mode 100644 libcore/src/test/java/com/mapbox/android/core/metrics/AbstractCompositeMetricsTest.java create mode 100644 libcore/src/test/java/com/mapbox/android/core/metrics/MetricsImplTest.java diff --git a/libcore/src/androidTest/java/com/mapbox/android/core/metrics/AbstractCompositeMetricsInstrumentedTest.java b/libcore/src/androidTest/java/com/mapbox/android/core/metrics/AbstractCompositeMetricsInstrumentedTest.java new file mode 100644 index 000000000..cc411dff1 --- /dev/null +++ b/libcore/src/androidTest/java/com/mapbox/android/core/metrics/AbstractCompositeMetricsInstrumentedTest.java @@ -0,0 +1,32 @@ +package com.mapbox.android.core.metrics; + +import org.junit.Test; + +import java.util.concurrent.TimeUnit; + +import static org.junit.Assert.assertEquals; + +public class AbstractCompositeMetricsInstrumentedTest { + @Test + public void addShortSpanMetric() throws InterruptedException { + TestCompositeMetrics metrics = new TestCompositeMetrics(TimeUnit.SECONDS.toMillis(1)); + metrics.add("test", 100L); + Thread.sleep(1000L); + metrics.add("test", 10L); + Metrics firstMetric = metrics.getMetrics("test"); + Metrics secondMetric = metrics.getMetrics("test"); + assertEquals(100L, firstMetric.getValue()); + assertEquals(10L, secondMetric.getValue()); + } + + private static final class TestCompositeMetrics extends AbstractCompositeMetrics { + TestCompositeMetrics(long maxLength) { + super(maxLength); + } + + @Override + protected Metrics nextMetrics(long start, long end) { + return new MetricsImpl(start, end); + } + } +} \ No newline at end of file diff --git a/libcore/src/main/java/com/mapbox/android/core/metrics/MetricsImpl.java b/libcore/src/main/java/com/mapbox/android/core/metrics/MetricsImpl.java index 1a54c65d2..79f715695 100644 --- a/libcore/src/main/java/com/mapbox/android/core/metrics/MetricsImpl.java +++ b/libcore/src/main/java/com/mapbox/android/core/metrics/MetricsImpl.java @@ -3,7 +3,8 @@ import java.util.concurrent.atomic.AtomicLong; /** - * Default implementation of the metric. + * Default implementation of the thread safe + * metric with time span. */ public class MetricsImpl implements Metrics { private final long start; @@ -11,30 +12,60 @@ public class MetricsImpl implements Metrics { private final AtomicLong value; MetricsImpl(long start, long end, long initialValue) { - this.start = start; - this.end = end; + if (start > end) { + this.start = end; + this.end = start; + } else { + this.start = start; + this.end = end; + } this.value = new AtomicLong(initialValue); } + /** + * Intantiate new metric with a span. + * @param start timestamp + * @param end timestamp + */ public MetricsImpl(long start, long end) { this(start, end, 0L); } + /** + * Increment metric by delta. (thread safe) + * + * @param delta value + */ @Override public void add(long delta) { value.addAndGet(delta); } + /** + * Return metric value. (thread safe) + * + * @return metric value + */ @Override public long getValue() { return value.get(); } + /** + * Return span start timestamp. + * + * @return timestamp in milliseconds + */ @Override public long getStart() { return start; } + /** + * Return span end timestamp. + * + * @return timestamp in milliseconds + */ @Override public long getEnd() { return end; diff --git a/libcore/src/test/java/com/mapbox/android/core/metrics/AbstractCompositeMetricsTest.java b/libcore/src/test/java/com/mapbox/android/core/metrics/AbstractCompositeMetricsTest.java new file mode 100644 index 000000000..34e3b4a35 --- /dev/null +++ b/libcore/src/test/java/com/mapbox/android/core/metrics/AbstractCompositeMetricsTest.java @@ -0,0 +1,58 @@ +package com.mapbox.android.core.metrics; + +import org.junit.Before; +import org.junit.Test; + +import java.util.concurrent.TimeUnit; + +import static org.assertj.core.api.Assertions.assertThat; + +public class AbstractCompositeMetricsTest { + private TestCompositeMetrics metrics; + + @Before + public void setUp() { + metrics = new TestCompositeMetrics(TimeUnit.HOURS.toMillis(1)); + } + + @Test + public void addLongSpanMetric() { + metrics.add("test", 100L); + metrics.add("test", 10L); + Metrics firstMetric = metrics.getMetrics("test"); + Metrics secondMetric = metrics.getMetrics("test"); + assertThat(firstMetric.getValue()).isEqualTo(110L); + assertThat(secondMetric).isNull(); + } + + @Test + public void addMultipleMetrics() { + metrics.add("test", 100L); + metrics.add("foo", 10L); + Metrics test = metrics.getMetrics("test"); + Metrics foo = metrics.getMetrics("foo"); + assertThat(test.getValue()).isEqualTo(100L); + assertThat(foo.getValue()).isEqualTo(10L); + } + + @Test + public void getEmptyMetric() { + assertThat(metrics.getMetrics("test")).isNull(); + } + + @Test + public void getMetricsEmptyString() { + assertThat(metrics.getMetrics("")).isNull(); + } + + private static final class TestCompositeMetrics extends AbstractCompositeMetrics { + TestCompositeMetrics(long maxLength) { + super(maxLength); + } + + @Override + protected Metrics nextMetrics(long start, long end) { + return new MetricsImpl(start, end); + } + } +} \ No newline at end of file diff --git a/libcore/src/test/java/com/mapbox/android/core/metrics/MetricsImplTest.java b/libcore/src/test/java/com/mapbox/android/core/metrics/MetricsImplTest.java new file mode 100644 index 000000000..3e01d766e --- /dev/null +++ b/libcore/src/test/java/com/mapbox/android/core/metrics/MetricsImplTest.java @@ -0,0 +1,59 @@ +package com.mapbox.android.core.metrics; + +import android.os.SystemClock; +import org.junit.Test; + +import java.util.concurrent.TimeUnit; + +import static org.assertj.core.api.Assertions.assertThat; + +public class MetricsImplTest { + + @Test + public void wrongSpan() { + long startTime = SystemClock.elapsedRealtime(); + long endTime = startTime + TimeUnit.HOURS.toMillis(24); + Metrics metrics = new MetricsImpl(endTime, startTime); + assertThat(metrics.getStart()).isEqualTo(startTime); + assertThat(metrics.getEnd()).isEqualTo(endTime); + } + + @Test + public void add() { + Metrics metrics = getMetrics(); + metrics.add(100L); + assertThat(metrics.getValue()).isEqualTo(100L); + } + + @Test + public void subtract() { + Metrics metrics = getMetrics(); + metrics.add(-100L); + assertThat(metrics.getValue()).isEqualTo(-100L); + } + + @Test + public void getValue() { + Metrics metrics = getMetrics(); + assertThat(metrics.getValue()).isEqualTo(0L); + } + + @Test + public void getStart() { + long startTime = SystemClock.elapsedRealtime(); + Metrics metrics = new MetricsImpl(startTime, startTime + TimeUnit.HOURS.toMillis(24)); + assertThat(metrics.getStart()).isEqualTo(startTime); + } + + @Test + public void getEnd() { + long endTime = SystemClock.elapsedRealtime(); + Metrics metrics = new MetricsImpl(0, endTime); + assertThat(metrics.getStart()).isEqualTo(endTime); + } + + private static Metrics getMetrics() { + long startTime = SystemClock.elapsedRealtime(); + return new MetricsImpl(startTime, startTime + TimeUnit.HOURS.toMillis(24)); + } +} \ No newline at end of file From 83698ed57e61c50c6640a33dfbc5ab2bc717acad Mon Sep 17 00:00:00 2001 From: Andrey Li Date: Mon, 10 Jun 2019 20:50:16 -0700 Subject: [PATCH 10/12] Address code review comments --- .../android/core/metrics/AbstractCompositeMetrics.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/libcore/src/main/java/com/mapbox/android/core/metrics/AbstractCompositeMetrics.java b/libcore/src/main/java/com/mapbox/android/core/metrics/AbstractCompositeMetrics.java index 56487fac2..17ad3cc97 100644 --- a/libcore/src/main/java/com/mapbox/android/core/metrics/AbstractCompositeMetrics.java +++ b/libcore/src/main/java/com/mapbox/android/core/metrics/AbstractCompositeMetrics.java @@ -45,10 +45,10 @@ public AbstractCompositeMetrics(long maxLength) { */ public void add(String name, long delta) { long now = SystemClock.uptimeMillis(); - Deque metrics = getOrCreateMetrics(name.trim()); Metrics last; synchronized (this) { + Deque metrics = getOrCreateMetrics(name.trim()); if (now >= metrics.getLast().getEnd()) { metrics.add(nextMetrics(now, now + maxLength)); } @@ -60,11 +60,13 @@ public void add(String name, long delta) { @Nullable Metrics getMetrics(@NonNull String name) { Deque metrics = metricsMap.get(name.trim()); - return metrics != null && !metrics.isEmpty() ? metrics.pop() : null; + synchronized (this) { + return metrics != null && !metrics.isEmpty() ? metrics.pop() : null; + } } @NonNull - private synchronized Deque getOrCreateMetrics(@NonNull String name) { + private Deque getOrCreateMetrics(@NonNull String name) { Deque metrics; if ((metrics = metricsMap.get(name)) == null) { metrics = new ArrayDeque<>(); From 920c7a62ec80ee1df041ff611a77b8ae077fa601 Mon Sep 17 00:00:00 2001 From: Andrey Li Date: Tue, 11 Jun 2019 09:28:11 -0700 Subject: [PATCH 11/12] Address code review comment --- .../android/core/metrics/AbstractCompositeMetrics.java | 5 ++++- .../core/metrics/AbstractCompositeMetricsTest.java | 8 ++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/libcore/src/main/java/com/mapbox/android/core/metrics/AbstractCompositeMetrics.java b/libcore/src/main/java/com/mapbox/android/core/metrics/AbstractCompositeMetrics.java index 17ad3cc97..9ecfe4a08 100644 --- a/libcore/src/main/java/com/mapbox/android/core/metrics/AbstractCompositeMetrics.java +++ b/libcore/src/main/java/com/mapbox/android/core/metrics/AbstractCompositeMetrics.java @@ -70,9 +70,12 @@ private Deque getOrCreateMetrics(@NonNull String name) { Deque metrics; if ((metrics = metricsMap.get(name)) == null) { metrics = new ArrayDeque<>(); + metricsMap.put(name, metrics); + } + + if (metrics.isEmpty()) { long now = SystemClock.uptimeMillis(); metrics.add(nextMetrics(now, now + maxLength)); - metricsMap.put(name, metrics); } return metrics; } diff --git a/libcore/src/test/java/com/mapbox/android/core/metrics/AbstractCompositeMetricsTest.java b/libcore/src/test/java/com/mapbox/android/core/metrics/AbstractCompositeMetricsTest.java index 34e3b4a35..eec397e6f 100644 --- a/libcore/src/test/java/com/mapbox/android/core/metrics/AbstractCompositeMetricsTest.java +++ b/libcore/src/test/java/com/mapbox/android/core/metrics/AbstractCompositeMetricsTest.java @@ -35,6 +35,14 @@ public void addMultipleMetrics() { assertThat(foo.getValue()).isEqualTo(10L); } + @Test + public void addRemoveMetric() { + metrics.add("test", 100L); + metrics.getMetrics("test"); + metrics.add("test", 10L); + assertThat(metrics.getMetrics("test").getValue()).isEqualTo(10L); + } + @Test public void getEmptyMetric() { assertThat(metrics.getMetrics("test")).isNull(); From ebd0bf13b314372d125a81a23b8f7fc88d8209cf Mon Sep 17 00:00:00 2001 From: Andrey Li Date: Tue, 11 Jun 2019 15:28:40 -0700 Subject: [PATCH 12/12] Add unit tests for telem --- .../metrics/AbstractCompositeMetrics.java | 2 +- .../telemetry/metrics/TelemetryMetrics.java | 29 +++++++--- .../metrics/TelemetryMetricsTest.java | 57 +++++++++++++++++++ .../NetworkUsageMetricsCollectorTest.java | 56 ++++++++++++++++++ 4 files changed, 135 insertions(+), 9 deletions(-) create mode 100644 libtelemetry/src/test/java/com/mapbox/android/telemetry/metrics/TelemetryMetricsTest.java create mode 100644 libtelemetry/src/test/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageMetricsCollectorTest.java diff --git a/libcore/src/main/java/com/mapbox/android/core/metrics/AbstractCompositeMetrics.java b/libcore/src/main/java/com/mapbox/android/core/metrics/AbstractCompositeMetrics.java index 9ecfe4a08..e51bc987e 100644 --- a/libcore/src/main/java/com/mapbox/android/core/metrics/AbstractCompositeMetrics.java +++ b/libcore/src/main/java/com/mapbox/android/core/metrics/AbstractCompositeMetrics.java @@ -58,7 +58,7 @@ public void add(String name, long delta) { } @Nullable - Metrics getMetrics(@NonNull String name) { + public Metrics getMetrics(@NonNull String name) { Deque metrics = metricsMap.get(name.trim()); synchronized (this) { return metrics != null && !metrics.isEmpty() ? metrics.pop() : null; diff --git a/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/TelemetryMetrics.java b/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/TelemetryMetrics.java index aebe97676..a861abf27 100644 --- a/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/TelemetryMetrics.java +++ b/libtelemetry/src/main/java/com/mapbox/android/telemetry/metrics/TelemetryMetrics.java @@ -1,34 +1,47 @@ package com.mapbox.android.telemetry.metrics; +import android.support.annotation.IntRange; +import android.support.annotation.VisibleForTesting; import com.mapbox.android.core.metrics.AbstractCompositeMetrics; import com.mapbox.android.core.metrics.Metrics; import com.mapbox.android.core.metrics.MetricsImpl; import static android.net.ConnectivityManager.TYPE_WIFI; +import static android.net.ConnectivityManager.TYPE_MOBILE; +import static android.net.ConnectivityManager.TYPE_VPN; public class TelemetryMetrics extends AbstractCompositeMetrics { public static final String EVENTS_TOTAL = "eventCountTotal"; public static final String EVENTS_FAILED = "eventCountFailed"; - private static final String MOBILE_BYTES_TX = "cellDataSent"; - private static final String WIFI_BYTES_TX = "wifiDataSent"; - private static final String MOBILE_BYTES_RX = "cellDataReceived"; - private static final String WIFI_BYTES_RX = "wifiDataReceived"; + @VisibleForTesting + static final String MOBILE_BYTES_TX = "cellDataSent"; + static final String WIFI_BYTES_TX = "wifiDataSent"; + static final String MOBILE_BYTES_RX = "cellDataReceived"; + static final String WIFI_BYTES_RX = "wifiDataReceived"; public TelemetryMetrics(long maxLength) { super(maxLength); } - public void addRxBytesForType(int networkType, long bytes) { - add(networkType == TYPE_WIFI ? WIFI_BYTES_RX : MOBILE_BYTES_RX, bytes); + public void addRxBytesForType(@IntRange(from = TYPE_MOBILE, to = TYPE_VPN) int networkType, long bytes) { + if (isValidNetworkType(networkType)) { + add(networkType == TYPE_WIFI ? WIFI_BYTES_RX : MOBILE_BYTES_RX, bytes); + } } - public void addTxBytesForType(int networkType, long bytes) { - add(networkType == TYPE_WIFI ? WIFI_BYTES_TX : MOBILE_BYTES_TX, bytes); + public void addTxBytesForType(@IntRange(from = TYPE_MOBILE, to = TYPE_VPN) int networkType, long bytes) { + if (isValidNetworkType(networkType)) { + add(networkType == TYPE_WIFI ? WIFI_BYTES_TX : MOBILE_BYTES_TX, bytes); + } } @Override protected Metrics nextMetrics(long start, long end) { return new MetricsImpl(start, end); } + + private static boolean isValidNetworkType(int type) { + return type >= TYPE_MOBILE && type <= TYPE_VPN; + } } diff --git a/libtelemetry/src/test/java/com/mapbox/android/telemetry/metrics/TelemetryMetricsTest.java b/libtelemetry/src/test/java/com/mapbox/android/telemetry/metrics/TelemetryMetricsTest.java new file mode 100644 index 000000000..c31156927 --- /dev/null +++ b/libtelemetry/src/test/java/com/mapbox/android/telemetry/metrics/TelemetryMetricsTest.java @@ -0,0 +1,57 @@ +package com.mapbox.android.telemetry.metrics; + +import android.net.ConnectivityManager; +import com.mapbox.android.core.metrics.Metrics; +import org.junit.Before; +import org.junit.Test; + +import java.util.concurrent.TimeUnit; + +import static org.assertj.core.api.Assertions.assertThat; + +public class TelemetryMetricsTest { + private TelemetryMetrics telemetryMetrics; + + @Before + public void setUp() { + telemetryMetrics = new TelemetryMetrics(TimeUnit.MINUTES.toMillis(10)); + } + + @Test + public void addRxBytesForMobileType() { + telemetryMetrics.addRxBytesForType(ConnectivityManager.TYPE_MOBILE, 10L); + Metrics metrics = telemetryMetrics.getMetrics(TelemetryMetrics.MOBILE_BYTES_RX); + assertThat(metrics.getValue()).isEqualTo(10L); + } + + @Test + public void addRxBytesForWifiType() { + telemetryMetrics.addRxBytesForType(ConnectivityManager.TYPE_WIFI, 10L); + Metrics metrics = telemetryMetrics.getMetrics(TelemetryMetrics.WIFI_BYTES_RX); + assertThat(metrics.getValue()).isEqualTo(10L); + } + + @Test + public void addTxBytesForWifiType() { + telemetryMetrics.addTxBytesForType(ConnectivityManager.TYPE_WIFI, 10L); + Metrics metrics = telemetryMetrics.getMetrics(TelemetryMetrics.WIFI_BYTES_TX); + assertThat(metrics.getValue()).isEqualTo(10L); + } + + @Test + public void addTxBytesForMobileType() { + telemetryMetrics.addTxBytesForType(ConnectivityManager.TYPE_MOBILE, 10L); + Metrics metrics = telemetryMetrics.getMetrics(TelemetryMetrics.MOBILE_BYTES_TX); + assertThat(metrics.getValue()).isEqualTo(10L); + } + + @Test + public void addBytesForUnsupportedType() { + telemetryMetrics.addTxBytesForType(1000, 10L); + telemetryMetrics.addRxBytesForType(1000, 10L); + assertThat(telemetryMetrics.getMetrics(TelemetryMetrics.MOBILE_BYTES_TX)).isNull(); + assertThat(telemetryMetrics.getMetrics(TelemetryMetrics.WIFI_BYTES_TX)).isNull(); + assertThat(telemetryMetrics.getMetrics(TelemetryMetrics.MOBILE_BYTES_RX)).isNull(); + assertThat(telemetryMetrics.getMetrics(TelemetryMetrics.WIFI_BYTES_RX)).isNull(); + } +} \ No newline at end of file diff --git a/libtelemetry/src/test/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageMetricsCollectorTest.java b/libtelemetry/src/test/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageMetricsCollectorTest.java new file mode 100644 index 000000000..4425553fc --- /dev/null +++ b/libtelemetry/src/test/java/com/mapbox/android/telemetry/metrics/network/NetworkUsageMetricsCollectorTest.java @@ -0,0 +1,56 @@ +package com.mapbox.android.telemetry.metrics.network; + +import android.content.Context; +import android.net.ConnectivityManager; +import android.net.NetworkInfo; +import com.mapbox.android.telemetry.metrics.TelemetryMetrics; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import static android.net.ConnectivityManager.TYPE_WIFI; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.RETURNS_DEEP_STUBS; + +@RunWith(MockitoJUnitRunner.class) +public class NetworkUsageMetricsCollectorTest { + + @Mock + private TelemetryMetrics metrics; + + private NetworkUsageMetricsCollector networkUsageMetricsCollector; + + @Before + public void setUp() { + networkUsageMetricsCollector = new NetworkUsageMetricsCollector(getMockedContext(TYPE_WIFI), metrics); + } + + @Test + public void addRxBytes() { + networkUsageMetricsCollector.addRxBytes(30); + verify(metrics).addRxBytesForType(anyInt(), anyLong()); + } + + @Test + public void addTxBytes() { + networkUsageMetricsCollector.addTxBytes(30); + verify(metrics).addTxBytesForType(anyInt(), anyLong()); + } + + private static Context getMockedContext(int networkType) { + Context mockedContext = mock(Context.class, RETURNS_DEEP_STUBS); + ConnectivityManager mockedCm = mock(ConnectivityManager.class, RETURNS_DEEP_STUBS); + NetworkInfo mockedNetworkInfo = mock(NetworkInfo.class, RETURNS_DEEP_STUBS); + when(mockedNetworkInfo.getType()).thenReturn(networkType); + when(mockedCm.getActiveNetworkInfo()).thenReturn(mockedNetworkInfo); + when(mockedContext.getSystemService(anyString())).thenReturn(mockedCm); + return mockedContext; + } +} \ No newline at end of file