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

Instrumentation for Kotlin coroutines 1.5.0+ #4624

Merged

Conversation

monosoul
Copy link
Contributor

@monosoul monosoul commented Jan 31, 2023

What Does This Do

Introduces support for coroutines 1.5.0+.
Solves #931 , Solves #1123 .

Motivation

The instrumentation I introduced in #4405 only supports Kotlin coroutines up to 1.4.3.

Additional Notes

Updates project-wide Kotlin dependency version to 1.6.21 because latest versions of coroutines require higher API level version (unfortunately Kotlin plugin for Gradle has some breaking changes in version 1.7.0, so I didn't use it here).
Introduces 3 new Gradle modules: kotlin-coroutines-common, kotlin-coroutines-1.3 and kotlin-coroutines-1.5.
Also configures lastDep test (was missing before).

@monosoul monosoul requested a review from a team as a code owner January 31, 2023 03:26
@monosoul
Copy link
Contributor Author

@bantonsson would you mind to have a look since you reviewed the last one? Thanks!

@monosoul monosoul force-pushed the feat/support-kotlin-coroutines-1.5.0 branch from 5025b6a to 826bace Compare January 31, 2023 03:35
@smola smola added the inst: others All other instrumentations label Jan 31, 2023
@monosoul monosoul force-pushed the feat/support-kotlin-coroutines-1.5.0 branch from 84aac35 to b196d54 Compare February 1, 2023 20:20
@monosoul
Copy link
Contributor Author

monosoul commented Feb 2, 2023

Hey @bantonsson , sorry for pinging, but I'd appreciate it a lot if we can get this merged before the next release.
I even got lucky enough to get the required build green this time 😁

@bantonsson
Copy link
Contributor

Thanks for the contribution @monosoul. I was on vacation last week, but will give this a review today.

Copy link
Contributor

@bantonsson bantonsson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot @monosoul. Would you mind squashing this as well?

Only minor nitpick. Going forward we want to try to group related instrumentations in this fashion:

:dd-java-agent:instrumentation:kotlin-coroutines
:dd-java-agent:instrumentation:kotlin-coroutines:coroutines-1.3
:dd-java-agent:instrumentation:kotlin-coroutines:coroutines-1.5

Signed-off-by: monosoul <Kloz.Klaud@gmail.com>
@monosoul monosoul force-pushed the feat/support-kotlin-coroutines-1.5.0 branch from 2489951 to 41306a3 Compare February 6, 2023 12:04
@monosoul
Copy link
Contributor Author

monosoul commented Feb 6, 2023

Thanks a lot @monosoul. Would you mind squashing this as well?

Only minor nitpick. Going forward we want to try to group related instrumentations in this fashion:

:dd-java-agent:instrumentation:kotlin-coroutines
:dd-java-agent:instrumentation:kotlin-coroutines:coroutines-1.3
:dd-java-agent:instrumentation:kotlin-coroutines:coroutines-1.5

@bantonsson squashed and refactored the submodules structure 👍

@bantonsson bantonsson merged commit 1436486 into DataDog:master Feb 7, 2023
@bantonsson bantonsson added the tag: community Community contribution label Feb 7, 2023
@bantonsson bantonsson assigned bantonsson and unassigned bantonsson Feb 7, 2023
@bantonsson bantonsson added this to the 1.7.0 milestone Feb 7, 2023
@monosoul monosoul deleted the feat/support-kotlin-coroutines-1.5.0 branch December 18, 2023 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inst: others All other instrumentations tag: community Community contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants