Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Force update to Kotlin 1.9 #6026

Closed
TomasChladekSL opened this issue Nov 29, 2023 · 14 comments
Closed

Force update to Kotlin 1.9 #6026

TomasChladekSL opened this issue Nov 29, 2023 · 14 comments
Labels
Feature Request Suggest an idea for this project

Comments

@TomasChladekSL
Copy link

Is your feature request related to a problem? Please describe.
We're utilizing the opentelemetry-sdk, opentelemetry-exporter-otlp, and opentelemetry-exporter-otlp-common modules within our Android SDK. Since upgrading to Otel version 1.32.0, we are now required to use Kotlin version 1.9.0 or higher. However, this necessity could potentially pose a challenge for several large clients who are operating on older Gradle and Kotlin versions. Updating these dependencies could potentially initiate a long process, involving compatibility issues with Gradle plugins and Kotlin KAPT/KSP, stretching over several months.

Describe the solution you'd like
There's no requirement to update to Kotlin 1.9.0.

@TomasChladekSL TomasChladekSL added the Feature Request Suggest an idea for this project label Nov 29, 2023
@jack-berg
Copy link
Member

we are now required to use Kotlin version 1.9.0 or higher

That shouldn't be the case. The android / kotlin requirements we have a listed here. Android API version 21+ is required with desugaring enabled. Kotlin 1.6 is required, but only for the opentelemetry-extension-kotlin artifact.

Can you provide more info on where the kotlin 1.9.0 requirement is coming from?

@TomasChladekSL
Copy link
Author

We're using Kotlin 1.7.20, AGP 7.3.1, and desugar 1.2.3. After upgrading to Otel 1.32.0, we encountered this error message.

/Users/chladek/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-common/1.9.10/dafaf2c27f27c09220cee312df10917d9a5d97ce/kotlin-stdlib-common-1.9.10.jar!/META-INF/kotlin-stdlib-common.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.9.0, expected version is 1.7.1.

If I downgrade to Otel version 1.31.0, everything is fine.

@jack-berg
Copy link
Member

Maybe the @open-telemetry/android-maintainers could comment on this? I'm not sure which component would have been compiled with kotlin 1.9.0.

@trask
Copy link
Member

trask commented Nov 29, 2023

@TomasChladekSL can you let us know where exactly the kotlin-stdlib-common-1.9.10 transitive dependency is coming from (e.g. ./gradlew dependencies)?

@TomasChladekSL
Copy link
Author

It seems that Kotlin 1.9.10 is required due to the dependency on com.squareup.okhttp3:okhttp.

+--- io.opentelemetry:opentelemetry-exporter-otlp:1.32.0
|    +--- io.opentelemetry:opentelemetry-exporter-otlp-common:1.32.0
|    |    \--- io.opentelemetry:opentelemetry-exporter-common:1.32.0
|    |         \--- io.opentelemetry:opentelemetry-api:1.32.0 (*)
|    +--- io.opentelemetry:opentelemetry-exporter-sender-okhttp:1.32.0
|    |    +--- io.opentelemetry:opentelemetry-exporter-common:1.32.0 (*)
|    |    +--- io.opentelemetry:opentelemetry-sdk-common:1.32.0 (*)
|    |    \--- com.squareup.okhttp3:okhttp:4.12.0
|    |         +--- com.squareup.okio:okio:3.6.0
|    |         |    \--- com.squareup.okio:okio-jvm:3.6.0
|    |         |         +--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.9.10
|    |         |         |    +--- org.jetbrains.kotlin:kotlin-stdlib:1.9.10
|    |         |         |    |    +--- org.jetbrains.kotlin:kotlin-stdlib-common:1.9.10
|    |         |         |    |    \--- org.jetbrains:annotations:13.0
|    |         |         |    \--- org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.9.10
|    |         |         |         \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.10 (*)
|    |         |         \--- org.jetbrains.kotlin:kotlin-stdlib-common:1.9.10
|    |         \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.21 -> 1.9.10 (*)

@trask
Copy link
Member

trask commented Nov 29, 2023

thanks @TomasChladekSL. what http client library (if any) are you using in your Android application? we sort of thought most Android apps would be using okhttp already and so thought the dependency on it would be helpful (rather than harmful) for Android

@breedx-splk breedx-splk added the needs author feedback Waiting for additional feedback from the author label Nov 29, 2023
@TomasChladekSL
Copy link
Author

No, we are using our own HTTP client, and the OkHttp dependency presents another issue due to potential version incompatibility.

@github-actions github-actions bot removed the needs author feedback Waiting for additional feedback from the author label Nov 30, 2023
@SenNeonoveNoci
Copy link

Usage of OkHttp

We sort of thought most Android apps would be using OkHttp.

Although this might be true for a large number of applications, there are definitely applications that don't use OkHttp. Some typical examples are:

  • Kotlin multiplatform applications (typically use Ktor).
  • Unreal and Unity games.
  • Applications using Volley.
  • Applications that only use the native Java API HttpUrlConnection.

In all these cases, the applications are now "forced" to depend on OkHttp.

Possible Problems

The usage of a third-party library (OkHttp) transitionally forces every application or library that uses opentelemetry-java to update Kotlin to the latest version. This is harmful for the client applications and developers:

  • Applications developed using React Native are now unable to use opentelemetry-java because the latest version of Kotlin currently supported is 1.7.22 (source).
  • Many applications use older or obsolete versions of Gradle and libraries. This is very common in projects that are in "maintenance" mode.
  • This dependency also increases the size of the application, even if the application itself does not use OkHttp.
  • Possible compatibility issues might occur if the application uses another OkHttp version with breaking changes.

Our Use Case

We (Cisco) are developing an Agent for mobile analytics/session recording that should be "lightweight" and are trying to use as few third-party dependencies as possible (based on the above-mentioned reasons). We really love the idea of Open Telemetry and want to use it, but changes like this make it problematic because now all our clients need to update Kotlin, rendering our solution "unusable" for them.

Possible Solutions

  • Not the optimal one, but a possible temporary solution is to use an older OkHttp version that will not enforce the latest Kotlin.
  • The best solution will be to not have a dependency on OkHttp -> the HTTP "logic" will be written using a Java API.

@jack-berg
Copy link
Member

Some thoughts:

  • My understanding is that OkHttp is pretty popular in the Android ecosystem, and yet I don't see any issues in the okhttp repo related to forcing the upgrade to kotlin 1.9.1. Not sure why that is.. 🤔
  • Forgive my naivety on the subject, but why can't applications upgrade to the latest version of kotlin? Isn't it supposed to have strong backwards compatibility guarantees? I get that the rective native gradle plugin uses kotlin 1.7.22, but seems like the plugin is designed poorly if it forces all applications which use it to be pinned to the same kotlin version.
  • The problem with using an older version of OkHttp is that there are CVEs (e.g. CVE-2023-3635 com.squareup.okio:okio-jvm:jar:3.0.0  #5637) and staying on the latest helps mitigate risk.
  • You should be able to add a dependency constraint that forces a lower version of com.squareup.okhttp3:okhttp, or com.squareup.okio:okio which uses an older version of kotlin. Not ideal because you don't know for sure that the OTLP exporter doesn't depend on new features of okhttp. Maybe we could add some tests to ensure that OTLP exporter works with certain older versions of okhttp?
  • With respect to the solution of implementing the OTLP exporter using a pure Java API - that's the idea behind the io.opentelemetry:opentelemetry-exporter-sender-jdk sender described here. Note that there is also the option to use the io.opentelemetry:opentelemetry-exporter-sender-grpc-managed-channel implementation which allows you to select your own gRPC transport implementation. Theoretically, we could add another sender implementation for http/protobuf based on HttpURLConnection, but I don't have the bandwidth / interest to maintain a 4th sender implementation.

@SenNeonoveNoci
Copy link

My understanding is that OkHttp is pretty popular in the Android ecosystem, and yet I don't see any issues in the okhttp repo related to forcing the upgrade to kotlin 1.9.1. Not sure why that is.. 🤔

We are in a pretty unique situation here. We need the latest opentelemetry-java (support for binary data in log body), which will bump the OkHttp version for our clients, and they are going to report issues to us, not the OkHttp repository (because they did nothing wrong). The problem is the client does not expect version changes of other libraries when using ours; we are introducing a side effect.

Forgive my naivety on the subject, but why can't applications upgrade to the latest version of kotlin? Isn't it supposed to have strong backwards compatibility guarantees? I get that the rective native gradle plugin uses kotlin 1.7.22, but seems like the plugin is designed poorly if it forces all applications which use it to be pinned to the same kotlin version.

Sadly, here we are speaking from experience with hundreds of clients that have various reasons to not update certain dependencies:

  • In outsourced development, updating is not included in the budget. Clients don't see a "business value" in updating.
  • Usage of obsolete libraries that would break after updating dependencies.
  • Framework restrictions typical of React Native, Flutter, Cordova, Ionic, Unity, ...

The React Native Gradle plugin might be poorly designed, but it's still used by a large number of applications/developers. Additionally, 'opentelemetry-java' requires applications to use specific versions of Kotlin and OkHttp.

The problem with using an older version of OkHttp is that there are CVEs (e.g. CVE-2023-3635 com.squareup.okio:okio-jvm:jar:3.0.0 #5637) and staying on the latest helps mitigate risk.

Although the suggestion appears logical, the removal of the third-party dependency (OkHttp) from the equation likely won't compromise security.

Resolution

We completely understand that the requested change (replace/remove OkHttp) is not trivial, and the open-source community currently does not have the bandwidth for this.

Thanks to your suggestion regarding "dependency constraint," we have likely found a solution for our case. We will exclude the dependency on OkHttp and will use our own HttpClient. While we still firmly believe that this is an issue and makes it more complicated to use OpenTelemetry for Android development, we have found a solution for our specific case. Therefore, this issue can be considered closed.

@marandaneto
Copy link
Member

Added a comment here as a possible alternative (it has to be tested).

@swankjesse
Copy link

Hi, I’m an OkHttp maintainer. We’re quite strict about compatibility. We have a strict rule to not break binary compatibility in our library or in Okio.

@LikeTheSalad
Copy link
Contributor

Just for the record, I've created this issue in the OkHttp repo where we can see what's OkHttp's stance regarding Kotlin's older versions' compilation and security support.

@jack-berg
Copy link
Member

With #6045 merged we now have testing in place that confirm that certain versions of OkHttp besides the latest work as intended. This should allow users to confidently downgrade to select older versions with confidence, and avoid any update kotlin updates that come with the latest versions.

If folks are interested in testing additional OkHttp versions, we can discuss in separate issues / PRs, but I'm going to close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request Suggest an idea for this project
Projects
None yet
Development

No branches or pull requests

8 participants