Skip to content

RUM-3294 Add the Core Tracer tests #1924

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.

Sign up for GitHub

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

Merged
mariusc83 merged 2 commits into from
Mar 27, 2024
Merged

RUM-3294 Add the Core Tracer tests #1924

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d889a92
RUM-3268 Provide the Java CoreTracer unit tests
mariusc83 Mar 18, 2024
bac5936
Fix unit tests on CI
mariusc83 Mar 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions .gitlab-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,49 @@ test:release:
reports:
junit: "**/build/test-results/testReleaseUnitTest/*.xml"

# We need separated steps for tracing module tests as these cannot be run with the CI visibility agent enabled. Also they need to be executed with
# the daemon capability as the worker threads are running as a daemon.

test:trace-debug:
Comment thread
0xnm marked this conversation as resolved.
Outdated
tags: [ "arch:amd64" ]
image: $CI_IMAGE_DOCKER
stage: test
timeout: 1h
cache:
key: $CI_COMMIT_REF_SLUG
paths:
- cache/caches/
- cache/notifications/
policy: pull
script:
- rm -rf ~/.gradle/daemon/
- GRADLE_OPTS="-Xmx3072m" ./gradlew :features:dd-sdk-android-trace:testDebugUnitTest --stacktrace --build-cache --gradle-user-home cache/
artifacts:
when: always
expire_in: 1 week
reports:
junit: "**/build/test-results/testDebugUnitTest/*.xml"

test:trace-release:
tags: [ "arch:amd64" ]
image: $CI_IMAGE_DOCKER
stage: test
timeout: 1h
cache:
key: $CI_COMMIT_REF_SLUG
paths:
- cache/caches/
- cache/notifications/
policy: pull
script:
- rm -rf ~/.gradle/daemon/
- GRADLE_OPTS="-Xmx3072m" ./gradlew :features:dd-sdk-android-trace:testReleaseUnitTest --stacktrace --build-cache --gradle-user-home cache/
artifacts:
when: always
expire_in: 1 week
reports:
junit: "**/build/test-results/testReleaseUnitTest/*.xml"

test:tools:
tags: [ "arch:amd64" ]
image: $CI_IMAGE_DOCKER
Expand Down
1 change: 1 addition & 0 deletions LICENSE-3rdparty.csv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ import(test),org.objenesis,Apache-2.0,"Copyright (c) 2003-2013, Objenesis Team a
import(test),org.opentest4j,Apache-2.0,Copyright 2015-2018 the original author or authors
import(test),org.robolectric,Apache-2.0,Copyright 2015-2018 the original author or authors
import(test),org.sonatype.oss,Apache-2.0,"Copyright (c) 2008-present Sonatype, Inc."
import(test),uk.org.webcompere,MIT,Copyright (c) 2020 Ashley Frieze (c) 2017 Stefan Birkner
build,androidx.compose.compiler,Apache-2.0,Copyright 2019 The Android Open Source Project
build,ch.qos.logback,EPL-1.0,"Copyright (C) 1999-2015, QOS.ch"
build,com.android.tools.build,Apache-2.0,Copyright (C) 2013 The Android Open Source Project
Expand Down
4 changes: 2 additions & 2 deletions build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ tasks.register("checkAll") {
}

registerSubModuleAggregationTask("assembleLibraries", "assemble")
registerSubModuleAggregationTask("unitTestRelease", "testReleaseUnitTest")
registerSubModuleAggregationTask("unitTestDebug", "testDebugUnitTest")
registerSubModuleAggregationTask("unitTestRelease", "testReleaseUnitTest", exceptions = setOf("dd-sdk-android-trace"))
registerSubModuleAggregationTask("unitTestDebug", "testDebugUnitTest", exceptions = setOf("dd-sdk-android-trace"))

tasks.register("unitTestTools") {
dependsOn(
Expand Down
4 changes: 3 additions & 1 deletion buildSrc/src/main/kotlin/com/datadog/gradle/config/TestPyramidConfig.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@ fun Project.registerSubModuleAggregationTask(
taskName: String,
subModuleTaskName: String,
subModuleNamePrefix: String = "dd-sdk-android-",
exceptions: Set<String> = emptySet(),
additionalConfiguration: Task.() -> Unit = {}
) {
tasks.register(taskName) {
project.subprojects.forEach { subProject ->
if (subProject.name.startsWith(subModuleNamePrefix)) {
val name = subProject.name
if (!exceptions.contains(name) && name.startsWith(subModuleNamePrefix)) {
dependsOn("${subProject.path}:$subModuleTaskName")
}
}
Expand Down
25 changes: 24 additions & 1 deletion features/dd-sdk-android-trace/api/dd-sdk-android-trace.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3848,6 +3848,10 @@ public class com/datadog/trace/api/naming/SpanNaming {
public fun version ()I
}

public class com/datadog/trace/api/naming/SpanNaming$Singleton {
public fun <init> ()V
}

public class com/datadog/trace/api/naming/v0/CacheNamingV0 : com/datadog/trace/api/naming/NamingSchema$ForCache {
public fun <init> (Z)V
public fun operation (Ljava/lang/String;)Ljava/lang/String;
Expand Down Expand Up @@ -5659,7 +5663,9 @@ public class com/datadog/trace/common/writer/ListWriter : java/util/concurrent/C
public fun close ()V
public fun firstTrace ()Ljava/util/List;
public fun flush ()Z
public fun getTraceCount ()Ljava/util/concurrent/atomic/AtomicInteger;
public fun incrementDropCounts (I)V
public fun secondTrace ()Ljava/util/List;
public fun setFilter (Lcom/datadog/trace/common/writer/ListWriter$Filter;)V
public fun start ()V
public fun toString ()Ljava/lang/String;
Expand All @@ -5684,6 +5690,15 @@ public class com/datadog/trace/common/writer/LoggingWriter : com/datadog/trace/c
public fun write (Ljava/util/List;)V
}

public class com/datadog/trace/common/writer/NoOpWriter : com/datadog/trace/common/writer/Writer {
public fun <init> ()V
public fun close ()V
public fun flush ()Z
public fun incrementDropCounts (I)V
public fun start ()V
public fun write (Ljava/util/List;)V
}

public abstract class com/datadog/trace/common/writer/Payload {
protected field body Ljava/nio/ByteBuffer;
public fun <init> ()V
Expand Down Expand Up @@ -5921,6 +5936,11 @@ public abstract interface class com/datadog/trace/core/CoreSpan {
}

public class com/datadog/trace/core/CoreTracer : com/datadog/trace/bootstrap/instrumentation/api/AgentTracer$TracerAPI {
public static final field LANG_INTERPRETER_STATSD_TAG Ljava/lang/String;
public static final field LANG_INTERPRETER_VENDOR_STATSD_TAG Ljava/lang/String;
public static final field LANG_STATSD_TAG Ljava/lang/String;
public static final field LANG_VERSION_STATSD_TAG Ljava/lang/String;
public static final field TRACER_VERSION_STATSD_TAG Ljava/lang/String;
public static final field TRACE_ID_MAX Ljava/math/BigInteger;
public fun activateNext (Lcom/datadog/trace/bootstrap/instrumentation/api/AgentSpan;)Lcom/datadog/trace/bootstrap/instrumentation/api/AgentScope;
public fun activateSpan (Lcom/datadog/trace/bootstrap/instrumentation/api/AgentSpan;)Lcom/datadog/trace/bootstrap/instrumentation/api/AgentScope;
Expand Down Expand Up @@ -6023,7 +6043,6 @@ public class com/datadog/trace/core/CoreTracer$CoreTracerBuilder {
public fun scopeManager (Lcom/datadog/trace/bootstrap/instrumentation/api/AgentScopeManager;)Lcom/datadog/trace/core/CoreTracer$CoreTracerBuilder;
public fun serviceName (Ljava/lang/String;)Lcom/datadog/trace/core/CoreTracer$CoreTracerBuilder;
public fun serviceNameMappings (Ljava/util/Map;)Lcom/datadog/trace/core/CoreTracer$CoreTracerBuilder;
public fun statsDClient (Lcom/datadog/trace/core/taginterceptor/TagInterceptor;)Lcom/datadog/trace/core/CoreTracer$CoreTracerBuilder;
public fun strictTraceWrites (Z)Lcom/datadog/trace/core/CoreTracer$CoreTracerBuilder;
public fun tagInterceptor (Lcom/datadog/trace/core/taginterceptor/TagInterceptor;)Lcom/datadog/trace/core/CoreTracer$CoreTracerBuilder;
public fun taggedHeaders (Ljava/util/Map;)Lcom/datadog/trace/core/CoreTracer$CoreTracerBuilder;
Expand All @@ -6034,6 +6053,7 @@ public class com/datadog/trace/core/CoreTracer$CoreTracerBuilder {

public class com/datadog/trace/core/DDSpan : com/datadog/trace/api/profiling/TransientProfilingContextHolder, com/datadog/trace/bootstrap/instrumentation/api/AgentSpan, com/datadog/trace/bootstrap/instrumentation/api/AttachableWrapper, com/datadog/trace/core/CoreSpan {
public static final field CHECKPOINTED_TAG Ljava/lang/String;
public fun <init> (Ljava/lang/String;JLcom/datadog/trace/core/DDSpanContext;Ljava/util/List;)V

@0xnm 0xnm Mar 26, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

minor: given that we have a method to call private constructor now, should we make this constructor public?

@mariusc83 mariusc83 Mar 26, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

actually not...

@mariusc83 mariusc83 Mar 26, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I am not really sure where this is coming from as I am just exposing the class not the constructors so I will leave it as is. I need the class exposed to cast to DDSpan in the unit tests.

@0xnm 0xnm Mar 26, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It is from this change https://github.com/DataDog/dd-sdk-android/pull/1924/files#diff-1c12b21f74915bf39d879d5d17374ad221d56bb67bed99743adc8a86dfbf944cR127 where private constructor was made public. But that is ok I believe, just was wondering about the consistency in the approach (since for PendingTrace we are using reflection to create an instance).

public fun addLink (Lcom/datadog/trace/bootstrap/instrumentation/api/AgentSpanLink;)V
public synthetic fun addThrowable (Ljava/lang/Throwable;)Lcom/datadog/trace/bootstrap/instrumentation/api/AgentSpan;
public synthetic fun addThrowable (Ljava/lang/Throwable;)Lcom/datadog/trace/core/CoreSpan;
Expand Down Expand Up @@ -6316,9 +6336,12 @@ public class com/datadog/trace/core/PendingTrace : com/datadog/trace/bootstrap/i
public static fun getDurationNano (Lcom/datadog/trace/core/CoreSpan;)J
public fun getEndToEndStartTime ()J
public fun getLastWriteTime ()J
public fun getPendingReferenceCount ()I
public fun getRootSpan ()Lcom/datadog/trace/core/DDSpan;
public fun getRunningTraceStartTime ()J
public fun getSpans ()Ljava/util/List;
public fun getTimeSource ()Lcom/datadog/trace/api/time/TimeSource;
public fun isRootSpanWritten ()Z
public fun lastReferencedNanosAgo (J)Z
public fun oldestFinishedTime ()J
public fun registerContinuation (Lcom/datadog/trace/bootstrap/instrumentation/api/AgentScope$Continuation;)V
Expand Down
2 changes: 2 additions & 0 deletions features/dd-sdk-android-trace/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ dependencies {
testImplementation(libs.okHttp)
testImplementation(libs.bundles.jUnit5)
testImplementation(libs.bundles.testTools)
testImplementation(libs.systemStubsJupiter)

unmock(libs.robolectric)
}

Expand Down
2 changes: 1 addition & 1 deletion features/dd-sdk-android-trace/src/main/java/com/datadog/trace/api/Config.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2610,7 +2610,7 @@ public boolean isIastStacktraceLeakSuppress() {
}

public boolean isCiVisibilityEnabled() {
return instrumenterConfig.isCiVisibilityEnabled();
return false;
}

public boolean isUsmEnabled() {
Expand Down
3 changes: 3 additions & 0 deletions ...res/dd-sdk-android-trace/src/main/java/com/datadog/trace/api/interceptor/MutableSpan.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.datadog.trace.api.interceptor;

import androidx.annotation.Nullable;

import com.datadog.trace.api.DDTags;

import java.util.Map;
Expand All @@ -24,6 +26,7 @@ public interface MutableSpan {

MutableSpan setResourceName(final CharSequence resourceName);

@Nullable
Integer getSamplingPriority();

/**
Expand Down
2 changes: 1 addition & 1 deletion features/dd-sdk-android-trace/src/main/java/com/datadog/trace/api/naming/SpanNaming.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public class SpanNaming {
public static final int SCHEMA_MIN_VERSION = 0;
public static final int SCHEMA_MAX_VERSION = 1;

private static class Singleton {
public static class Singleton {
private static SpanNaming INSTANCE = new SpanNaming();
}

Expand Down
7 changes: 7 additions & 0 deletions features/dd-sdk-android-trace/src/main/java/com/datadog/trace/common/writer/ListWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ public boolean accept(List<DDSpan> trace) {
public List<DDSpan> firstTrace() {
return get(0);
}
public List<DDSpan> secondTrace() {

@0xnm 0xnm Mar 25, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is adding this needed for the tests?

@mariusc83 mariusc83 Mar 25, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yup, they are using groovy in their tests and they are able to access these private properties directly.

@0xnm 0xnm Mar 25, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

let's maybe do the same? write reflection wrappers to access all this. Because otherwise we make quite a lot of internal things public just for the tests, not sure if it opens the door for the misuse on the client side.

@mariusc83 mariusc83 Mar 26, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

it shouldn't honestly, there are plenty of places like these in their api's.

@0xnm 0xnm Mar 26, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it makes sense if you think about this code as an agent - it is attached to the process and is not supposed to be used as a SDK.

But in our case it becomes a SDK, and we should care much more about things which should be open vs closed.

@mariusc83 mariusc83 Mar 26, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

actually this class is only used for testing purposes on their end, it blocks the thread so to make sure the traces are consumed. I think this one is safe.

return get(1);
}

@Override
public void write(List<DDSpan> trace) {
Expand Down Expand Up @@ -143,6 +146,10 @@ public boolean flush() {
return true;
}

public AtomicInteger getTraceCount() {
return traceCount;
}

@Override
public void close() {
clear();
Expand Down
39 changes: 39 additions & 0 deletions features/dd-sdk-android-trace/src/main/java/com/datadog/trace/common/writer/NoOpWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0.
* This product includes software developed at Datadog (https://www.datadoghq.com/).
* Copyright 2016-Present Datadog, Inc.
*/

package com.datadog.trace.common.writer;

import com.datadog.trace.core.DDSpan;

import java.util.List;

public class NoOpWriter implements Writer{

@Override
public void write(List<DDSpan> trace) {

}

@Override
public void start() {

}

@Override
public boolean flush() {
return true;
}

@Override
public void close() {

}

@Override
public void incrementDropCounts(int spanCount) {

}
}
2 changes: 0 additions & 2 deletions ...sdk-android-trace/src/main/java/com/datadog/trace/common/writer/TraceStructureWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ public void write(List<DDSpan> trace) {
+ ")"
+ "->"
+ span.getTraceId();
out.println("ERROR: " + message);
Comment thread
0xnm marked this conversation as resolved.
if (debugLog) {
log.error(message);
}
Expand All @@ -139,7 +138,6 @@ public void write(List<DDSpan> trace) {
+ ")"
+ "->"
+ span.getParentId();
out.println("ERROR: " + message);
if (debugLog) {
log.error(message);
}
Expand Down
18 changes: 7 additions & 11 deletions features/dd-sdk-android-trace/src/main/java/com/datadog/trace/core/CoreTracer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
import com.datadog.trace.common.sampling.Sampler;
import com.datadog.trace.common.sampling.SpanSamplingRules;
import com.datadog.trace.common.sampling.TraceSamplingRules;
import com.datadog.trace.common.writer.NoOpWriter;
import com.datadog.trace.common.writer.Writer;
import com.datadog.trace.core.datastreams.DataStreamsMonitoring;
import com.datadog.trace.core.datastreams.NoOpDataStreamMonitoring;
Expand Down Expand Up @@ -112,11 +113,11 @@ public static CoreTracerBuilder builder() {
return new CoreTracerBuilder();
}

private static final String LANG_STATSD_TAG = "lang";
private static final String LANG_VERSION_STATSD_TAG = "lang_version";
private static final String LANG_INTERPRETER_STATSD_TAG = "lang_interpreter";
private static final String LANG_INTERPRETER_VENDOR_STATSD_TAG = "lang_interpreter_vendor";
private static final String TRACER_VERSION_STATSD_TAG = "tracer_version";
public static final String LANG_STATSD_TAG = "lang";
public static final String LANG_VERSION_STATSD_TAG = "lang_version";
public static final String LANG_INTERPRETER_STATSD_TAG = "lang_interpreter";
public static final String LANG_INTERPRETER_VENDOR_STATSD_TAG = "lang_interpreter_vendor";
public static final String TRACER_VERSION_STATSD_TAG = "tracer_version";

/**
* Tracer start time in nanoseconds measured up to a millisecond accuracy
Expand Down Expand Up @@ -264,7 +265,7 @@ public static class CoreTracerBuilder {

private Config config;
private String serviceName;
private Writer writer;
private Writer writer = new NoOpWriter();
Comment thread
0xnm marked this conversation as resolved.
private IdGenerationStrategy idGenerationStrategy;
private Sampler sampler;
private HttpCodec.Extractor extractor;
Expand Down Expand Up @@ -348,11 +349,6 @@ public CoreTracerBuilder tagInterceptor(TagInterceptor tagInterceptor) {
return this;
}

public CoreTracerBuilder statsDClient(TagInterceptor tagInterceptor) {
this.tagInterceptor = tagInterceptor;
return this;
}

public CoreTracerBuilder strictTraceWrites(boolean strictTraceWrites) {
this.strictTraceWrites = strictTraceWrites;
return this;
Expand Down
4 changes: 3 additions & 1 deletion features/dd-sdk-android-trace/src/main/java/com/datadog/trace/core/DDSpan.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;

import com.datadog.trace.api.Config;
import com.datadog.trace.api.DDSpanId;
Expand Down Expand Up @@ -122,7 +123,8 @@ static DDSpan create(
* @param timestampMicro if greater than zero, use this time instead of the current time
* @param context the context used for the span
*/
private DDSpan(
@VisibleForTesting
public DDSpan(

@0xnm 0xnm Mar 26, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is it possible to put VisibleForTesting annotation to the elements we are making public just for testing?

@NonNull String instrumentationName,
final long timestampMicro,
@NonNull DDSpanContext context,
Expand Down
13 changes: 13 additions & 0 deletions features/dd-sdk-android-trace/src/main/java/com/datadog/trace/core/PendingTrace.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,19 @@ public void setLastWriteTime(long now) {
LAST_WRITE_TIME_NANO.set(this, now);
}

public int getPendingReferenceCount() {
return pendingReferenceCount;
}

public boolean isRootSpanWritten() {
return rootSpanWritten;
}

public List<DDSpan> getSpans() {
return spans.isEmpty() ? EMPTY : new ArrayList<>(spans);
}


@Override
public boolean setEnqueued(boolean enqueued) {
int expected = enqueued ? 0 : 1;
Expand Down
Loading