Can't we update the Kotlinter?

Originally posted by @cre8ivejp in #135 (comment)

Summary

We want to add and remove deprecated metric events from the SDK to improve our monitoring system.
Basically, we will make changes in the current implementation to support the new metrics. Nothing in particular.

We use Okhttp and Moshi for the API requests.

APIs

For get_evaluations, we will report all metrics events, Including the latency and size metrics events.
For register_events, we will only report error metrics, such as timeout, network, server errors, etc.

InternalErrorMetricsEvent

We will remove the InternalErrorMetricsEvent, and use InternalServerErrorMetricsEvent for 500 internal server errors and InternalSdkErrorMetricsEvent for SDK internal errors.

Labels

All the metrics events have a key/value map called labels.
In the labels, we will set the same tag that is used to initialize the SDK.

E.g

{
  "tag": "android"
}

Metric event list

Metric list
https://github.com/bucketeer-io/bucketeer/blob/main/proto/event/client/event.proto#L119-L184

enum ApiId {
  UNKNOWN_API = 0;
  GET_EVALUATION = 1;
  GET_EVALUATIONS = 2;
  REGISTER_EVENTS = 3;
}

message LatencyMetricsEvent {
  ApiId api_id = 1;
  map<string, string> labels = 2;
  google.protobuf.Duration duration = 3;
}

message SizeMetricsEvent {
  ApiId api_id = 1;
  map<string, string> labels = 2;
  int32 size_byte = 3;
}

// HTTP Mapping: 400 Bad Request
message BadRequestErrorMetricsEvent {
  ApiId api_id = 1;
  map<string, string> labels = 2;
}

// HTTP Mapping: 401 Unauthorized
message UnauthorizedErrorMetricsEvent {
  ApiId api_id = 1;
  map<string, string> labels = 2;
}

// HTTP Mapping: 403 Forbidden
message ForbiddenErrorMetricsEvent {
  ApiId api_id = 1;
  map<string, string> labels = 2;
}

// HTTP Mapping: 404 Not Found
message NotFoundErrorMetricsEvent {
  ApiId api_id = 1;
  map<string, string> labels = 2;
}

// HTTP Mapping: 499 Client Closed Request
message ClientClosedRequestErrorMetricsEvent {
  ApiId api_id = 1;
  map<string, string> labels = 2;
}

// HTTP Mapping: 500 Internal Server Error
message InternalServerErrorMetricsEvent {
  ApiId api_id = 1;
  map<string, string> labels = 2;
}

// HTTP Mapping: 503 Service Unavailable
message ServiceUnavailableErrorMetricsEvent {
  ApiId api_id = 1;
  map<string, string> labels = 2;
}

message TimeoutErrorMetricsEvent {
  ApiId api_id = 1;
  map<string, string> labels = 2;
}

message NetworkErrorMetricsEvent {
  ApiId api_id = 1;
  map<string, string> labels = 2;
}

message InternalSdkErrorMetricsEvent {
  ApiId api_id = 1;
  map<string, string> labels = 2;
}

message UnknownErrorMetricsEvent {
  ApiId api_id = 1;
  map<string, string> labels = 2;
}

TODO

• Track the 3xx status code as "RedirectRequestException".
  • add response_code via labels
• Track the 413 status code as "PayloadTooLargeException"
• 408 errors, and we should handle it as a timeout error.
• Make iOS error handling consistent with Android SDK. Android handles more error code than iOS SDK. That is ( 405, 502, 504)
• Implement adding more information via labels for Unknown errors, so we don't spend too much time debugging.
  • Add error_message and response_code to the ErrorMetricsEvent labels

Note

the current metrics on the server are the following.
https://github.com/bucketeer-io/bucketeer/blob/main/proto/event/client/event.proto#L99-L187

Overview

We should add more data to the SDK requests for future data analysis and monitoring.

• App version
• SDK Client version
• OS version
• Network (Cell/Wifi)
• Device model
• Mobile, Desktop, TV, or Tablet

Note: Some data will be required, such as the app version, and it will be added in the BKTConfig.

Model

Besides the sdk_version, all other additional data will be delivered using a map.

Note: For metrics events, we don't need the metadata.

  @JsonClass(generateAdapter = true)
  data class GoalEvent(
    val timestamp: Long,
    val goal_id: String,
    val user_id: String,
    val value: Double,
    val user: User,
    val tag: String,
    val source_id: SourceID,
    val sdk_version: String, // NEW
    val metadata: Map<String, String> // NEW
  ) : EventData()

  @JsonClass(generateAdapter = true)
  data class EvaluationEvent(
    val timestamp: Long,
    val feature_id: String,
    val feature_version: Int = 0,
    val user_id: String,
    val variation_id: String = "",
    val user: User,
    val reason: Reason,
    val tag: String,
    val source_id: SourceID,
    val sdk_version: String, // NEW
    val metadata: Map<String, String> // NEW
  ) : EventData()

  // we can't use codegen here
  // see MetricsEventAdapterFactory
  data class MetricsEvent(
    val timestamp: Long,
    val event: MetricsEventData,
    val type: MetricsEventType,
    val sdk_version: String, // NEW
  ) : EventData()

Because the developer can manually configure the timeout in the initialize interface, the server doesn't know if the request got the timeout error using the default 30 seconds or user setting. To detect it on the server, we need to pass the current setting used in the failed request.

Things to do

• Pass the current timeout setting in seconds via labels.

E.g.

labels = mapOf("tag" to "android", "timeout" to "5"),

https://github.com/bucketeer-io/android-client-sdk/blob/main/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/model/MetricsEventData.kt#L32

Problem

• We have 2 test cases fail when run test against Android Emulator ARM API 32.
  • getEvaluations - network error
  • registerEvents - network error
• But its not fail on API 29 (like in the CI/CD). It means on API 32 , the error "network error" is being reported as "timeout error"

expected instance of: io.bucketeer.sdk.android.BKTException$NetworkException
but was instance of : io.bucketeer.sdk.android.BKTException$TimeoutException
with value          : io.bucketeer.sdk.android.BKTException$TimeoutException: Request timeout error: Connect timed out
expected instance of: io.bucketeer.sdk.android.BKTException$NetworkException
but was instance of : io.bucketeer.sdk.android.BKTException$TimeoutException
with value          : io.bucketeer.sdk.android.BKTException$TimeoutException: Request timeout error: Connect timed out
	at app//io.bucketeer.sdk.android.internal.remote.ApiClientImplTest.registerEvents - network error(ApiClientImplTest.kt:344)
Caused by: io.bucketeer.sdk.android.BKTException$TimeoutException: Request timeout error: Connect timed out
	at app//io.bucketeer.sdk.android.internal.remote.ApiClientExtKt.toBKTException(ApiClientExt.kt:93)
	at app//io.bucketeer.sdk.android.internal.remote.ApiClientImpl.registerEvents(ApiClientImpl.kt:152)
	at app//io.bucketeer.sdk.android.internal.remote.ApiClientImplTest.registerEvents - network error(ApiClientImplTest.kt:339)
Caused by: java.net.SocketTimeoutException: Connect timed out
	at java.base/sun.nio.ch.NioSocketImpl.timedFinishConnect(Unknown Source)
	at java.base/sun.nio.ch.NioSocketImpl.connect(Unknown Source)
	at java.base/java.net.SocksSocketImpl.connect(Unknown Source)
	at java.base/java.net.Socket.connect(Unknown Source)
	at okhttp3.internal.platform.Platform.connectSocket(Platform.kt:128)
	at okhttp3.internal.connection.RealConnection.connectSocket(RealConnection.kt:295)
	at okhttp3.internal.connection.RealConnection.connect(RealConnection.kt:207)
	at okhttp3.internal.connection.ExchangeFinder.findConnection(ExchangeFinder.kt:226)
	at okhttp3.internal.connection.ExchangeFinder.findHealthyConnection(ExchangeFinder.kt:106)
	at okhttp3.internal.connection.ExchangeFinder.find(ExchangeFinder.kt:74)
	at okhttp3.internal.connection.RealCall.initExchange$okhttp(RealCall.kt:255)
	at okhttp3.internal.connection.ConnectInterceptor.intercept(ConnectInterceptor.kt:32)
	at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109)
	at okhttp3.internal.cache.CacheInterceptor.intercept(CacheInterceptor.kt:95)
	at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109)
	at okhttp3.internal.http.BridgeInterceptor.intercept(BridgeInterceptor.kt:83)
	at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109)
	at okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(RetryAndFollowUpInterceptor.kt:76)
	at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109)
	at okhttp3.internal.connection.RealCall.getResponseWithInterceptorChain$okhttp(RealCall.kt:201)
	at okhttp3.internal.connection.RealCall.execute(RealCall.kt:154)
	at io.bucketeer.sdk.android.internal.remote.ApiClientImpl.registerEvents(ApiClientImpl.kt:131)
	... 1 more

• We should check the hash of the event data before saving it to the database to prevent duplicate data. Refs :

https://github.com/bucketeer-io/ios-client-sdk/blob/af5709fd5fb171c605fe294bfecf9b26b512c4d2/Bucketeer/Sources/Internal/Event/EventDaoImpl.swift#L4

This test fails randomly due to slow CI.

io.bucketeer.sdk.android.internal.remote.ApiClientImplTest > getEvaluations - custom timeout FAILED
    com.google.common.truth.AssertionErrorWithFacts at ApiClientImplTest.kt:194

https://github.com/bucketeer-io/android-client-sdk/actions/runs/8261127634/job/22597824194

          I mean we should rename EventDao to EventSQLDao.

Also, we might want to add EventStorage just for consistency.

Originally posted by @yshrsmz in #89 (comment)

When the client SDK is being destroyed and there are ongoing requests to the server, it will continue running and may try to write to the closed database.
Currently, the DAO will simply ignore this.
We aim to implement request cancellation, similar to the iOS SDK, to enhance overall safety.

Things to do

• Change all the fields in the request and the model data objects from snake_case to camelCase.
• Change the reason type from int to string (reference)
• Add protobuf @type (reference)
• Change the APIs

JSON object structure

In the new endpoint's response, there is no data object.

Before:

{
  data: {
    ...
  },
}

After:

{
  ...
}

New APIs

• get_evaluations
• register_events

E.g.
https:/api.example.com/get_evaluations
https:/api.example.com/register_events

PR Reference:
ca-dp/bucketeer-ios-sdk#91

Summary

We are improving the evaluation process on the server to evaluate only the flags that have been changed.
This will improve the load on the server and the response time since it will only return the flags that have been changed.

Currently, when a flag changes on the server, it evaluates all flags again and sends the latest evaluations to the client SDK.
The SDK checks the response, and it does nothing if the response is empty. Otherwise, it deletes all the data in the DB and saves the latest evaluations.

Implementation

When the SDK is initialized

1- Change the featureTag setting to be optional in the BKTConfig
2- Save the featureTag in the SharedPreferences if it is configured in the BKTConfig
3- Clear the userEvaluationsID in the SharedPreferences if the featureTag changes

Sending requests

The following new fields will be added to get_evaluations request.

• evaluatedAt: the last time the user was evaluated. The server will return in the get_evaluations response (UserEvaluations.CreatedAt), and it must be saved in the client
• userAttributesUpdated: when the user attributes change via the customAttributes interface, the userAttributesUpdated field must be set to true in the next request.

Update conditions when getting the response

The server will return the new following fields in the response.

• forceUpdate: a boolean that tells the SDK to delete all the current data and save the latest evaluations from the response
• archivedFeatureIds: a list of feature flag ids that were archived on the console. All those feature flags must be deleted from the DB.

forceUpdate is True

1- Delete all the evaluations from DB, and save the latest evaluations from the response into the DB
2- Save the UserEvaluations.CreatedAt in the response as evaluatedAt in the SharedPreferences

forceUpdate is false

1- Check the evaluation list in the response and upsert them in the DB if the list is not empty
2- Check the list of the feature flags that were archived on the console and delete them from the DB
3- Save the UserEvaluations.CreatedAt in the response as evaluatedAt in the SharedPreferences

To help monitor the APIs by sdkVersion, we need to add it to the GetEvaluations and RegisterEvents APIs.

Things to do

• Add the sdkVersion in the API request.

GetEvaluations

https://github.com/bucketeer-io/android-client-sdk/blob/main/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/model/request/GetEvaluationsRequest.kt#L8-L13

RegisterEvents

https://github.com/bucketeer-io/android-client-sdk/blob/main/bucketeer/src/main/kotlin/io/bucketeer/sdk/android/internal/model/request/RegisterEventsRequest.kt#L7-L9

This issue lists Renovate updates and detected dependencies. Read the Dependency Dashboard docs to learn more.

Open

These updates have all been created already. Click a checkbox below to force a retry/rebase of any.

• build(deps): update all non-major dependencies (benc-uk/workflow-dispatch, com.google.devtools.ksp, com.google.android.material:material, org.jetbrains.kotlinx:kotlinx-coroutines-android, androidx.lifecycle:lifecycle-process, androidx.lifecycle:lifecycle-runtime, org.jetbrains.kotlin.kapt, org.jetbrains.kotlin.android, com.android.application, com.android.library)
• test(deps): update all non-major test dependencies (com.google.testparameterinjector:test-parameter-injector, org.robolectric:robolectric)
• build(deps): update all major dependencies (major) (actions/setup-java, actions/upload-artifact, google-github-actions/release-please-action, gradle/gradle-build-action, macos)
• Click on this checkbox to rebase all open PRs at once

Detected dependencies

• Check this box to trigger a request for Renovate to run again on this repository

cd0fbde#r130152451