-
Notifications
You must be signed in to change notification settings - Fork 290
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
ZIO 2.0 instrumentation #4848
ZIO 2.0 instrumentation #4848
Conversation
619af03
to
64ffa27
Compare
With the work that @PerfectSlayer is doing on #4795 I think they need to be the one to review this. |
2186802
to
1de9440
Compare
Hi @dmytr, thank you for the contribution. I'm sorry that this seems to have fallen between the cracks. I'll try to review this today. |
Hey @dmytr, just wanted to let you know that I've been looking at this today, but I need to dig a bit deeper into it. Will continue on Monday. |
Thank you, @bantonsson! Looking forward to your feedback. In case if it could be useful - I've done something similar for OTEL JVM agent as well (link). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks once again @dmytr. Mostly some minor comments based on my limited understanding of the internals of ZIO. Please correct me if I've misunderstood anything.
.../zio-2.0/src/main/java/datadog/trace/instrumentation/zio/v2_0/ZioRuntimeInstrumentation.java
Outdated
Show resolved
Hide resolved
...mentation/zio/zio-2.0/src/main/java/datadog/trace/instrumentation/zio/v2_0/FiberContext.java
Outdated
Show resolved
Hide resolved
...mentation/zio/zio-2.0/src/main/java/datadog/trace/instrumentation/zio/v2_0/FiberContext.java
Outdated
Show resolved
Hide resolved
...mentation/zio/zio-2.0/src/main/java/datadog/trace/instrumentation/zio/v2_0/FiberContext.java
Outdated
Show resolved
Hide resolved
...mentation/zio/zio-2.0/src/main/java/datadog/trace/instrumentation/zio/v2_0/FiberContext.java
Outdated
Show resolved
Hide resolved
...mentation/zio/zio-2.0/src/main/java/datadog/trace/instrumentation/zio/v2_0/FiberContext.java
Outdated
Show resolved
Hide resolved
...mentation/zio/zio-2.0/src/main/java/datadog/trace/instrumentation/zio/v2_0/FiberContext.java
Outdated
Show resolved
Hide resolved
dd-java-agent/instrumentation/zio/zio-2.0/src/test/scala/ZioTestFixtures.scala
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your review, @bantonsson. I've made requested changes and answered on your comments.
...mentation/zio/zio-2.0/src/main/java/datadog/trace/instrumentation/zio/v2_0/FiberContext.java
Outdated
Show resolved
Hide resolved
dd-java-agent/instrumentation/zio/zio-2.0/src/test/scala/ZioTestFixtures.scala
Show resolved
Hide resolved
.../zio-2.0/src/main/java/datadog/trace/instrumentation/zio/v2_0/ZioRuntimeInstrumentation.java
Outdated
Show resolved
Hide resolved
...mentation/zio/zio-2.0/src/main/java/datadog/trace/instrumentation/zio/v2_0/FiberContext.java
Outdated
Show resolved
Hide resolved
dd-java-agent/instrumentation/zio/zio-2.0/src/test/groovy/ZioRuntimeInstrumentationTest.groovy
Outdated
Show resolved
Hide resolved
Hey @dmytr would you mind squashing your commits before we merge this PR? |
1c26bf8
to
4e609c1
Compare
Hi @bantonsson, I've squashed commits in this PR. And just in case re-tested it using my toy distributed app. Everything looks good! Thanks a lot for your help with this PR 🙇♂️ |
Thanks for the contribution @dmytr. I'll merge this as soon as I've nursed it through some CI flakiness. |
Does anyone know if it's still working? It doesn't add any traces in my Scala 3.4.2 / ZIO 2.0.22 project 🤔 |
What Does This Do
Adds instrumentation for ZIO 2.0.
Motivation
ZIO is a popular Scala library for asynchronous and concurrent programming. Its runtime uses green threads (fibers) which breaks instrumentation because scope state is stored in
ThreadLocal
variable. This PR takes care of synchronizing scope state between fibers and JVM threads on which they are executed.Implementation was tested on a non-trivial distributed application.
Details
This instrumentation is experimental and it's disabled by default. Use
-Ddd.integration.zio.experimental.enabled=true
to enable it.