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

Register SpanLinkAdapter and SpanLinkJson for reflection #6892

Merged
merged 1 commit into from
Apr 11, 2024

Conversation

luneo7
Copy link
Contributor

@luneo7 luneo7 commented Apr 5, 2024

What Does This Do

Uses a initialization-on-demand holder idiom for the DDSpanLink encoder to make sure that it is not initialized more than once, so it is thread safe. During debugging we saw a bunch of

java.lang.IllegalArgumentException: Expected at least one @ToJson or @FromJson method on datadog.trace.agent.core.DDSpanLink$SpanLinkAdapter
	at com.squareup.moshi.AdapterMethodsFactory.get(AdapterMethodsFactory.java:153)
	at com.squareup.moshi.Moshi$Builder.add(Moshi.java:223)
	at datadog.trace.agent.core.DDSpanLink.getEncoder(DDSpanLink.java:99)
	at datadog.trace.agent.core.DDSpanLink.toTag(DDSpanLink.java:77)
	at datadog.trace.agent.core.DDSpanContext.processTagsAndBaggage(DDSpanContext.java:789)
	at datadog.trace.agent.core.DDSpan.processTagsAndBaggage(DDSpan.java:703)
	at datadog.trace.agent.common.writer.ddagent.TraceMapperV0_4.map(TraceMapperV0_4.java:219)
	at datadog.trace.agent.common.writer.ddagent.TraceMapperV0_4.map(TraceMapperV0_4.java:20)
	at datadog.communication.serialization.msgpack.MsgPackWriter.format(MsgPackWriter.java:84)
	at datadog.trace.agent.common.writer.PayloadDispatcherImpl.addTrace(PayloadDispatcherImpl.java:80)
	at datadog.trace.agent.common.writer.TraceProcessingWorker$TraceSerializingHandler.onEvent(TraceProcessingWorker.java:239)
	at datadog.trace.agent.common.writer.TraceProcessingWorker$TraceSerializingHandler.consumeFromPrimaryQueue(TraceProcessingWorker.java:258)
	at datadog.trace.agent.common.writer.TraceProcessingWorker$DaemonTraceSerializingHandler.runDutyCycle(TraceProcessingWorker.java:155)
	at datadog.trace.agent.common.writer.TraceProcessingWorker$DaemonTraceSerializingHandler.run(TraceProcessingWorker.java:145)
	at [email protected]/java.lang.Thread.runWith(Thread.java:1596)
	at [email protected]/java.lang.Thread.run(Thread.java:1583)
	at org.graalvm.nativeimage.builder/com.oracle.svm.core.thread.PlatformThreads.threadStartRoutine(PlatformThreads.java:838)
	at org.graalvm.nativeimage.builder/com.oracle.svm.core.posix.thread.PosixPlatformThreads.pthreadStartRoutine(PosixPlatformThreads.java:211)

which means that SpanLinkAdapter and SpanLinkJson were not registered for reflection to be used on GraalVM.

Registration for SpanLinkAdapter is required so Moshi is able to build the adapter during runtime, and for SpanLinkJson as well, since this is lazy init GraalVM can't auto infer reflections.

Motivation

SpanLinkAdapter was failing, thus producing invalid payloads (we also raised support request number 1631888)... there you can see the payload and logs for the auto instrumentation and DD side car.

We've have conducted load tests with the change in our environments and with the change theCannot decode v0.4 traces payload: msgp: attempted to decode type "array" with method for "str error message is gone, and we didn't see any error whatsoever.

Fixes

#6889

@luneo7 luneo7 requested a review from a team as a code owner April 5, 2024 18:40
@luneo7 luneo7 requested review from dougqh and ygree April 5, 2024 18:40
@luneo7
Copy link
Contributor Author

luneo7 commented Apr 5, 2024

Btw tests are failing because:

AWS infastructure is not configured correctly for auto-injection testing
Datadog agent is not configured correctly for auto-injection testing

Seems something wrong with CI?

@luneo7 luneo7 changed the title Use custom Moshi JsonAdapter Register SpanLinkAdapter and SpanLinkJson for reflection Apr 7, 2024
@luneo7
Copy link
Contributor Author

luneo7 commented Apr 8, 2024

@dougqh @ygree is there a way to re-trigger (re-run) the tests? last run failed with

Could not create service of type UserHomeScopedCompileCaches using UserHomeScopeServices.createCompileCaches().
   > Timeout waiting to lock Java compile cache (/home/circleci/.gradle/caches/8.4/javaCompile). It is currently in use by another Gradle instance.

@luneo7
Copy link
Contributor Author

luneo7 commented Apr 9, 2024

@ygree @mcculls are you able to do a review of this as well? 🥺

@bm1549 bm1549 added tag: community Community contribution comp: native-image GraalVM native-image labels Apr 11, 2024
@bm1549 bm1549 merged commit 8a3889a into DataDog:master Apr 11, 2024
53 of 67 checks passed
@bm1549
Copy link
Contributor

bm1549 commented Apr 11, 2024

Merged - thank you for your contribution!

Based on our normal release schedule, this is expected to be released the second week of May

@bm1549 bm1549 added this to the 1.33.0 milestone Apr 11, 2024
@luneo7
Copy link
Contributor Author

luneo7 commented Apr 11, 2024

Thank you! We are using a patched in-house built version of 1.32.0... since that "bug" was flooding our logs with decode error, and we were losing traces.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: native-image GraalVM native-image tag: community Community contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants