Target kotlin 1.6 in kotlin extension #5910

laurit · 2023-10-12T11:57:53Z

Resolves #5903
1.4 should be the earliest supported version for 1.9 compiler, probably we'll have to bump this with the next kotlin version

laurit · 2023-10-12T12:17:37Z

extensions/kotlin/build.gradle.kts

@@ -48,6 +48,8 @@ tasks {
  withType(KotlinCompile::class) {
    kotlinOptions {
      jvmTarget = "1.8"
+      // earliest version supported by kotlin 1.9
+      languageVersion = "1.4"


I think this also applies to the tests. Could also use

compileKotlin { kotlinOptions { languageVersion = "1.4" } }

codecov · 2023-10-12T12:18:33Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (531898c) 91.18% compared to head (256f4c7) 91.18%.

Additional details and impacted files

@@            Coverage Diff            @@
##               main    #5910   +/-   ##
=========================================
  Coverage     91.18%   91.18%           
  Complexity     5613     5613           
=========================================
  Files           616      616           
  Lines         16566    16566           
  Branches       1642     1642           
=========================================
  Hits          15105    15105           
  Misses         1013     1013           
  Partials        448      448

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jack-berg

This looks good to me, but wondering if folks with more kotlin experience like @breedx-splk / @jkwatson / @LikeTheSalad can chime in to confirm.

LikeTheSalad · 2023-10-13T07:47:57Z

This looks good to me, but wondering if folks with more kotlin experience like @breedx-splk / @jkwatson / @LikeTheSalad can chime in to confirm.

I was actually having issues with this lib forcing me to upgrade Kotlin a couple of days ago though I could sort it out by upgrading to Kotlin 1.8, I tried 1.9 first but that caused a bunch of other issues with other dependencies and it was a mess.. Luckily 1.8 worked out well, still I'm glad this change is being added because I understand that upgrading Kotlin can be painful sometimes. Tbh I've never used the config languageVersion before, but it seems the right approach without having to downgrade the Kotlin version used in this project.

1.4 should be the earliest supported version for 1.9 compiler, probably we'll have to bump this with the next kotlin version

I think 1.4 is a great option. In the Android gradle plugin they made 1.5 a requirement a while ago, so I wouldn't mind bumping it up in the next kotlin version in case 1.4 is no longer supported by then.

jkwatson · 2023-10-13T15:18:47Z

This looks good to me, but wondering if folks with more kotlin experience like @breedx-splk / @jkwatson / @LikeTheSalad can chime in to confirm.

I have very little kotlin experience, so I'll leave this to the experts.

trask · 2023-10-13T16:34:11Z

I think 1.4 is a great option. In the Android gradle plugin they made 1.5 a requirement a while ago, so I wouldn't mind bumping it up in the next kotlin version in case 1.4 is no longer supported by then.

since this artifact is marked stable, would dropping support for older kotlin version be considered a breaking change? (in the instrumentation repo this is why we encode the base version into the artifact name)

LikeTheSalad · 2023-10-13T16:52:36Z

since this artifact is marked stable, would dropping support for older kotlin version be considered a breaking change?

I think so, because in my case my project stopped compiling until I bumped its Kotlin version. However, it seems like Kotlin is dropping support for older language versions with every new minor version bump release, so it seems unavoidable to drop support for 1.4 at some point, unless we pin this project's Kotlin version to the last one that supports 1.4.

jack-berg · 2023-10-18T14:57:26Z

@trask are you suggesting that this PR is a breaking change or that upgrading to 1.5 after merging this PR would be a breaking change?

trask · 2023-10-18T15:52:25Z

@trask are you suggesting that this PR is a breaking change or that upgrading to 1.5 after merging this PR would be a breaking change?

the latter, and this was what got my attention:

probably we'll have to bump this with the next kotlin version

jack-berg · 2023-10-19T19:26:55Z

Hmm.. so the question is should we do something now do accommodate upgrading the minimum kotlin version (or at least have a plan for how we would handle it), or should we codify that we don't consider upgrading the kotlin version a breaking change.

Was looking at the okhttp changelog the other day and noticed that they upgrade the kotlin version in minor release versions.

jack-berg · 2023-11-03T19:45:45Z

This is related to the minimum android conversation in #5936. I think we should codify that upgrading the minimum android version should not require a new major version, and that the same advice applies to kotlin. Would be good to define some logic for how we choose the minimum kotlin version and document it.

LikeTheSalad · 2023-11-06T10:07:41Z

Would be good to define some logic for how we choose the minimum kotlin version and document it.

For Android purposes, I believe a good measure could be the AndroidX libs. They are the official Android tools outside of the Android SDK.

They are also the reason why many times I've had to upgrade my kotlin version after upgrading one of them due to compilation issues caused by kotlin version conflicts.

It seems like they've recently upgraded to kotlin 1.9, and set the languageVersion from 1.5 to 1.8. I'm not sure why they've jumped straight to 1.8 though, probably there were not many breaking changes from 1.5 up to 1.8... Though I'm not fully sure. In any case, I think this repo should be a good guide on what kotlin version to use when it comes to Android projects, not sure about non-android projects that use kotlin though.

One thing to keep an eye on is that this repo seems to set the languageVersion in two places, one for their build tools here and one for their deliverable projects here (which is later set to subprojects here). They're both set to 1.8 atm so it's not a problem, but it's probably worth taking a look at both once in a while.

jack-berg · 2023-11-09T15:27:02Z

@LikeTheSalad I think upgrading to 1.9 would conflict with the original issue #5903 in which @laurit describes that Spring boot 2.7.x users require 1.6.21.

Although, after writing that, I'm not sure why a spring boot 2.7.x user couldn't upgrade kotlin to a later version than 1.6.21. Is kotlin 1.9 not compatible with 1.6.21?

LikeTheSalad · 2023-11-09T16:14:07Z

@LikeTheSalad I think upgrading to 1.9 would conflict with the original issue #5903 in which @laurit describes that Spring boot 2.7.x users require 1.6.21.

Yeah definitely, I also think we shouldn't force 1.9 compatibility (which is the case right now), and the AndroidX lib I mentioned earlier doesn't force it either because they set their languageVersion compilation param to 1.8 which ensures compatibility with versions older than 1.9. So my comment above was more about what's the lowest languageVersion we could set to keep compatibility with older kotlin versions, and at least for Android, it seems like it could be 1.8 right now based on AndroidX libs, although that still seems pretty high.

Maybe just a simpler option is to always keep the languageVersion to the minimum non-deprecated one, which seems to be 1.6 right now. It's probably what @laurit was trying to do anyway, to use the minimum compat version supported.

Is kotlin 1.9 not compatible with 1.6.21?

No, I just tried it locally and, if I set my kotlin version to 1.6.21 while I have a dependency built with kotlin 1.9.x, I get this compilation error message:

Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.9.0, expected version is 1.6.0.

I also tried with 1.7.x with the same results. Only 1.8.x seems to be compatible with 1.9.x for some reason.

laurit · 2023-11-09T16:31:25Z

Only 1.8.x seems to be compatible with 1.9.x for some reason.

As far as I understand kotlin compiler accepts metadata that is at most one version ahead of its version.

Although, after writing that, I'm not sure why a spring boot 2.7.x user couldn't upgrade kotlin to a later version than 1.6.21. Is kotlin 1.9 not compatible with 1.6.21?

#5903 (comment) states that

Another alternative could be to use a newer Kotlin version than 1.6.21 myself but I don't know which consequences that would have.

Our choices are to choose the minimum kotlin version which we wish to support and avoid updating kotlin dependency for this module or keep updating the kotlin dependency and set the target to earliest supported version. Either is better than what we do now.

jack-berg · 2023-11-09T16:36:59Z

or keep updating the kotlin dependency and set the target to earliest supported version

Let's do this, and document this strategy in versioning. Avoiding updating the kotlin dependency is too restrictive without any strong justification. I'd like to retain the flexibility to update the kotlin version in the future.

extensions/kotlin/build.gradle.kts

laurit requested a review from a team October 12, 2023 11:57

laurit commented Oct 12, 2023

View reviewed changes

jack-berg approved these changes Oct 12, 2023

View reviewed changes

jack-berg mentioned this pull request Nov 13, 2023

Define language version compatibility requirements #5983

Merged

jack-berg reviewed Nov 16, 2023

View reviewed changes

extensions/kotlin/build.gradle.kts Outdated Show resolved Hide resolved

jack-berg mentioned this pull request Nov 16, 2023

Should opentelemetry-extension-kotlin have a lower languageVersion? #5903

Closed

laurit added 4 commits November 17, 2023 08:48

Target kotlin 1.4 in kotlin extension

518e048

add comment

b7f7f86

add apidiff

2811fbf

change kotlin language version to 1.6

5cecfe7

laurit force-pushed the kotlin-extension-kotlin-version branch from 17a9fae to 5cecfe7 Compare November 17, 2023 06:48

update apidiff

256f4c7

jack-berg approved these changes Nov 17, 2023

View reviewed changes

jack-berg merged commit c87852c into open-telemetry:main Nov 17, 2023
18 checks passed

jack-berg changed the title ~~Target kotlin 1.4 in kotlin extension~~ Target kotlin 1.6 in kotlin extension Nov 29, 2023

LikeTheSalad mentioned this pull request Dec 14, 2023

Adding Kotlin compile support open-telemetry/opentelemetry-android#189

Merged

LikeTheSalad mentioned this pull request Dec 15, 2023

Kotlin older versions support square/okhttp#8139

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Target kotlin 1.6 in kotlin extension #5910

Target kotlin 1.6 in kotlin extension #5910

laurit commented Oct 12, 2023 •

edited

Loading

laurit Oct 12, 2023

codecov bot commented Oct 12, 2023 •

edited

Loading

jack-berg left a comment

LikeTheSalad commented Oct 13, 2023

jkwatson commented Oct 13, 2023

trask commented Oct 13, 2023

LikeTheSalad commented Oct 13, 2023

jack-berg commented Oct 18, 2023

trask commented Oct 18, 2023

jack-berg commented Oct 19, 2023

jack-berg commented Nov 3, 2023

LikeTheSalad commented Nov 6, 2023 •

edited

Loading

jack-berg commented Nov 9, 2023

LikeTheSalad commented Nov 9, 2023

laurit commented Nov 9, 2023

jack-berg commented Nov 9, 2023

Target kotlin 1.6 in kotlin extension #5910

Target kotlin 1.6 in kotlin extension #5910

Conversation

laurit commented Oct 12, 2023 • edited Loading

laurit Oct 12, 2023

Choose a reason for hiding this comment

codecov bot commented Oct 12, 2023 • edited Loading

Codecov Report

jack-berg left a comment

Choose a reason for hiding this comment

LikeTheSalad commented Oct 13, 2023

jkwatson commented Oct 13, 2023

trask commented Oct 13, 2023

LikeTheSalad commented Oct 13, 2023

jack-berg commented Oct 18, 2023

trask commented Oct 18, 2023

jack-berg commented Oct 19, 2023

jack-berg commented Nov 3, 2023

LikeTheSalad commented Nov 6, 2023 • edited Loading

jack-berg commented Nov 9, 2023

LikeTheSalad commented Nov 9, 2023

laurit commented Nov 9, 2023

jack-berg commented Nov 9, 2023

laurit commented Oct 12, 2023 •

edited

Loading

codecov bot commented Oct 12, 2023 •

edited

Loading

LikeTheSalad commented Nov 6, 2023 •

edited

Loading