-
Notifications
You must be signed in to change notification settings - Fork 61
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
RUM-3389 Add the Otel API feature integration tests #1995
RUM-3389 Add the Otel API feature integration tests #1995
Conversation
f45e21c
to
76ae6a7
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## feature/otel-support #1995 +/- ##
========================================================
+ Coverage 60.26% 60.38% +0.12%
========================================================
Files 808 808
Lines 30184 30224 +40
Branches 4945 4951 +6
========================================================
+ Hits 18188 18248 +60
+ Misses 10772 10760 -12
+ Partials 1224 1216 -8
|
76ae6a7
to
8c37b96
Compare
8c37b96
to
6a1b938
Compare
40ec44a
to
f28a8cf
Compare
6a1b938
to
989c4e0
Compare
import com.datadog.trace.bootstrap.instrumentation.api.AgentSpan | ||
import io.opentelemetry.api.trace.Span | ||
|
||
internal fun Span.lessSignificantTraceIdAsHexString(): String { |
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.
internal fun Span.lessSignificantTraceIdAsHexString(): String { | |
internal fun Span.leastSignificantTraceIdAsHexString(): String { |
internal class BlockingWriterWrapper(wrapped: Writer) : Writer { | ||
private val latches: LinkedList<CountDownLatch> = LinkedList() | ||
private val currentTraceCount: AtomicInteger = AtomicInteger(0) | ||
|
||
private val wrappedWriter = wrapped |
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.
internal class BlockingWriterWrapper(wrapped: Writer) : Writer { | |
private val latches: LinkedList<CountDownLatch> = LinkedList() | |
private val currentTraceCount: AtomicInteger = AtomicInteger(0) | |
private val wrappedWriter = wrapped | |
internal class BlockingWriterWrapper(private val wrappedWriter: Writer) : Writer { | |
private val latches: LinkedList<CountDownLatch> = LinkedList() | |
private val currentTraceCount: AtomicInteger = AtomicInteger(0) |
val latch = CountDownLatch(toWait) | ||
synchronized(latches) { | ||
latches.add(latch) | ||
} | ||
return latch.await(seconds.toLong(), TimeUnit.SECONDS) |
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.
In theory just a ConditionWatcher
could be used here which polls the value of currentTraceCount
, then in this case there is no need to create multiple latches. But fine as it is as well.
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.
I see, I might change that if I have time but maybe in another ticket. I forgot about that ConditionWatcher
fun `set up`(forge: Forge) { | ||
val datadogContextStorageClass = | ||
Class.forName("com.datadog.android.trace.opentelemetry.DatadogContextStorage") | ||
val constructor = datadogContextStorageClass.constructors.first { it.parameterCount == 1 } |
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.
constructor
is a reserved keyword in Kotlin, even github highlights this word differently. To avoid confusion it is better to rename it to something like ctor
or contructorRef
.
989c4e0
to
0a423fe
Compare
What does this PR do?
A brief description of the change being made with this pull request.
Motivation
What inspired you to submit this pull request?
Additional Notes
Anything else we should know when reviewing?
Review checklist (to be filled by reviewers)