Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion buildscripts/checkstyle.xml
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@
<!-- skip jflex generated files -->
<module name="SuppressionSingleFilter">
<property name="checks" value=".*"/>
<!-- TODO(anuraaga): Fix path after https://github.com/jprante/gradle-plugin-jflex/issues/20 -->
<!-- TODO: Fix path after https://github.com/jprante/gradle-plugin-jflex/issues/20 -->
<property name="files"
value="instrumentation-api[/\\]build[/\\]generated[/\\]sources[/\\]main"/>
</module>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ afterEvaluate {
val agentShadowJar = agentForTesting.resolve().first()

dependsOn(shadowJar)
// TODO(anuraaga): Figure out why dependsOn override is still needed in otel.javaagent-testing
// despite this dependency.
// TODO: Figure out why dependsOn override is still needed in otel.javaagent-testing despite
// this dependency.
dependsOn(agentForTesting.buildDependencies)

jvmArgumentProviders.add(JavaagentTestArgumentsProvider(agentShadowJar, shadowJar.archiveFile.get().asFile))
Expand All @@ -119,8 +119,8 @@ afterEvaluate {
return@filter false
}

// TODO(anuraaga): Better not to have this naming constraint, we can likely use
// plugin identification instead.
// TODO: Better not to have this naming constraint, we can likely use plugin identification
// instead.

val lib = it.absoluteFile
if (lib.name.startsWith("opentelemetry-javaagent-")) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ tasks {
disable("TypeParameterNaming")

// In bytecode instrumentation it's very common to separate across onEnter / onExit
// TODO(anuraaga): Only disable for javaagent instrumentation modules.
// TODO: Only disable for javaagent instrumentation modules.
disable("MustBeClosedChecker")

// Common to avoid an allocation. Revisit if it's worth opt-in suppressing instead of
Expand All @@ -101,16 +101,15 @@ tasks {
disable("JdkObsolete")
disable("JavaUtilDate")

// TODO(anuraaga): Remove this, we use this pattern in several tests and it will mean
// some moving.
// TODO: Remove this, we use this pattern in several tests and it will mean some moving.
disable("DefaultPackage")

// we use modified Otel* checks which ignore *Advice classes
disable("PrivateConstructorForUtilityClass")
disable("CanIgnoreReturnValueSuggester")

// TODO(anuraaga): Remove this, probably after instrumenter API migration instead of dealing
// with older APIs.
// TODO: Remove this, probably after instrumenter API migration instead of dealing with
// older APIs.
disable("InconsistentOverloads")

// lots of low level APIs use arrays
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ tasks.withType<Test>().configureEach {
// There's no real harm in setting this for all tests even if any happen to not be using context
// propagation.
jvmArgs("-Dio.opentelemetry.context.enableStrictContext=${rootProject.findProperty("enableStrictContext") ?: true}")
// TODO(anuraaga): Have agent map unshaded to shaded.
// TODO: Have agent map unshaded to shaded.
if (project.findProperty("disableShadowRelocate") != "true") {
jvmArgs("-Dio.opentelemetry.javaagent.shaded.io.opentelemetry.context.enableStrictContext=${rootProject.findProperty("enableStrictContext") ?: true}")
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,7 @@ protected Collection<ExportTraceServiceRequest> waitForTraces()
.map(
it -> {
ExportTraceServiceRequest.Builder builder = ExportTraceServiceRequest.newBuilder();
// TODO(anuraaga): Register parser into object mapper to avoid de -> re ->
// deserialize.
// TODO: Register parser into object mapper to avoid de -> re -> deserialize.
try {
JsonFormat.parser().merge(OBJECT_MAPPER.writeValueAsString(it), builder);
} catch (InvalidProtocolBufferException | JsonProcessingException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,8 @@ protected void configure(HttpServerTestOptions options) {
options.setExpectedHttpRoute(
(endpoint, method) -> {
if (endpoint == ServerEndpoint.NOT_FOUND) {
// TODO(anuraaga): Revisit this when applying instrumenters to more libraries, Armeria
// currently reports '/*' which is a fallback route.
// TODO: Revisit this when applying instrumenters to more libraries, Armeria currently
// reports '/*' which is a fallback route.
return "/*";
}
return expectedHttpRoute(endpoint, method);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ public abstract class ApiGatewayProxyRequest {

private static final Logger logger = Logger.getLogger(ApiGatewayProxyRequest.class.getName());

// TODO(anuraaga): We should create a RequestFactory type of class instead of evaluating this
// for every request.
// TODO: We should create a RequestFactory type of class instead of evaluating this for every
// request.
private static boolean noHttpPropagationNeeded() {
Collection<String> fields =
GlobalOpenTelemetry.getPropagators().getTextMapPropagator().fields();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ dependencies {
// We need Jackson for wrappers to reproduce the serialization does when Lambda invokes a RequestHandler with event
// since Lambda will only be able to invoke the wrapper itself with a generic Object.
// Note that Lambda itself uses Jackson, but does not expose it to the function so we need to include it here.
// TODO(anuraaga): Switch to aws-lambda-java-serialization to more robustly follow Lambda's serialization logic.
// TODO: Switch to aws-lambda-java-serialization to more robustly follow Lambda's serialization logic.
implementation("com.fasterxml.jackson.core:jackson-databind")
implementation("io.opentelemetry.contrib:opentelemetry-aws-xray-propagator")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ abstract class AbstractAws2ClientTest extends AbstractAws2ClientCoreTest {
"""
}

// TODO(anuraaga): Without AOP instrumentation of the HTTP client, we cannot model retries as
// TODO: Without AOP instrumentation of the HTTP client, we cannot model retries as
// spans because of https://github.com/aws/aws-sdk-java-v2/issues/1741. We should at least tweak
// the instrumentation to add Events for retries instead.
def "timeout and retry errors not captured"() {
Expand Down
3 changes: 1 addition & 2 deletions instrumentation/finatra-2.9/javaagent/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@ val finatraLatest by configurations.creating {
}

dependencies {
// TODO(anuraaga): Something about library configuration doesn't work well with scala compilation
// here.
// TODO: Something about library configuration doesn't work well with scala compilation here.
compileOnly("com.twitter:finatra-http_2.11:2.9.0")

testInstrumentation(project(":instrumentation:netty:netty-4.1:javaagent"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import okhttp3.Request;

/** Helper class to inject span context into request headers. */
// TODO(anuraaga): Figure out a way to avoid copying this from okhttp instrumentation.
// TODO: Figure out a way to avoid copying this from okhttp instrumentation.
enum RequestBuilderHeaderSetter implements TextMapSetter<Request.Builder> {
INSTANCE;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ protected RedisClient createClient(String uri) {
return RedisClient.create(uri);
}

// TODO(anuraaga): reactor library instrumentation doesn't seem to handle this case, figure out if
// it should and if so move back to base class.
// TODO: reactor library instrumentation doesn't seem to handle this case, figure out if it
// should and if so move back to base class.
@SuppressWarnings("deprecation") // using deprecated semconv
@Test
void testAsyncSubscriberWithSpecificThreadPool() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public void sendRequestWithCallback(

// TODO: context is not automatically propagated into callbacks
Context context = Context.current();
// TODO(anuraaga): Do we also need to test ListenableFuture callback?
// TODO: Do we also need to test ListenableFuture callback?
client.executeRequest(
request,
new AsyncCompletionHandler<Void>() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ void test() {
metric ->
assertThat(metric)
.hasUnit("By")
// TODO(anuraaga): Provide fuzzy value matching
// TODO: Provide fuzzy value matching
.hasLongSumSatisfying(
sum ->
assertThat(metric.getLongSumData().getPoints())
Expand All @@ -46,7 +46,7 @@ void test() {
metric ->
assertThat(metric)
.hasUnit("ms")
// TODO(anuraaga): Provide fuzzy value matching
// TODO: Provide fuzzy value matching
.hasLongGaugeSatisfying(
gauge ->
assertThat(metric.getLongGaugeData().getPoints())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public void test() {
metric ->
assertThat(metric)
.hasUnit("By")
// TODO(anuraaga): Provide fuzzy value matching
// TODO: Provide fuzzy value matching
.hasLongSumSatisfying(
sum ->
assertThat(metric.getLongSumData().getPoints())
Expand All @@ -46,7 +46,7 @@ public void test() {
metric ->
assertThat(metric)
.hasUnit("1")
// TODO(anuraaga): Provide fuzzy value matching
// TODO: Provide fuzzy value matching
.hasDoubleGaugeSatisfying(
gauge ->
assertThat(metric.getDoubleGaugeData().getPoints())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ otelJava {
}

dependencies {
// TODO(anuraaga): Something about library configuration doesn't work well with scala compilation
// here.
// TODO: Something about library configuration doesn't work well with scala compilation here.
compileOnly("com.typesafe.play:play_$scalaVersion:$playVersion")

testInstrumentation(project(":instrumentation:netty:netty-4.0:javaagent"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public String getName() {

@Override
public void jobExecutionVetoed(JobExecutionContext jobExecutionContext) {
// TODO(anuraaga): Consider adding an attribute for vetoed jobs.
// TODO: Consider adding an attribute for vetoed jobs.
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ public void onComplete() {
});
}

// TODO(anuraaga): The default Ratpack error handler also returns a 500 which is all we test, so
// TODO: The default Ratpack error handler also returns a 500 which is all we test, so
// we don't actually have test coverage ensuring our instrumentation correctly delegates to this
// user registered handler.
private static class TestErrorHandler implements ServerErrorHandler {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ void contextPropagated() {

// If reactor augmentation of WithSpan is working correctly, we will end up with these
// implementation details.
// TODO(anuraaga): This should just check actual context propagation instead of implementation
// TODO: This should just check actual context propagation instead of implementation
// but couldn't figure out how.
assertThat(((Scannable) mono).parents().collect(Collectors.toList()))
.anySatisfy(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public void transform(TypeTransformer transformer) {
@SuppressWarnings("unused")
public static class MethodAdvice {

// TODO(anuraaga): Replace with adding a type initializer to RxJavaPlugins
// TODO: Replace with adding a type initializer to RxJavaPlugins
// https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/2685
@Advice.OnMethodEnter(suppress = Throwable.class)
public static void activateOncePerClassloader() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public void transform(TypeTransformer transformer) {
@SuppressWarnings("unused")
public static class MethodAdvice {

// TODO(anuraaga): Replace with adding a type initializer to RxJavaPlugins
// TODO: Replace with adding a type initializer to RxJavaPlugins
// https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/2685
@Advice.OnMethodEnter(suppress = Throwable.class)
public static void activateOncePerClassloader() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public void transform(TypeTransformer transformer) {
@SuppressWarnings("unused")
public static class MethodAdvice {

// TODO(anuraaga): Replace with adding a type initializer to RxJavaPlugins
// TODO: Replace with adding a type initializer to RxJavaPlugins
// https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/2685
@Advice.OnMethodEnter(suppress = Throwable.class)
public static void activateOncePerClassloader() {
Expand Down
2 changes: 1 addition & 1 deletion javaagent-tooling/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ tasks {

// Mockito inline mocking uses byte-buddy but agent tooling currently uses byte-buddy-dep, which cannot be on the same
// classpath. Disable inline mocking to prevent conflicts.
// TODO(anuraaga): Find a better solution
// TODO: Find a better solution
configurations {
testRuntimeClasspath {
dependencies {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class TelemetryRetriever {

return OBJECT_MAPPER.readTree(content).collect {
def builder = builderConstructor.get()
// TODO(anuraaga): Register parser into object mapper to avoid de -> re -> deserialize.
// TODO: Register parser into object mapper to avoid de -> re -> deserialize.
JsonFormat.parser().merge(OBJECT_MAPPER.writeValueAsString(it), builder)
return (T) builder.build()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ private <T extends Consumer<TraceAssert>> void waitAndAssertTraces(
// Don't throw this failure since the stack is the awaitility thread, causing confusion.
// Instead, just assert one more time on the test thread, which will fail with a better
// stack trace.
// TODO(anuraaga): There is probably a better way to do this.
// TODO: There is probably a better way to do this.
doAssertTraces(traceComparator, assertionsList, verifyScopeVersion);
} else {
throw t;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ private static MetricData createMetricData(
metric.getName(),
metric.getDescription(),
metric.getUnit(),
// TODO(anuraaga): Remove usages of internal types.
// TODO: Remove usages of internal types.
ImmutableGaugeData.create(
getDoublePointDatas(metric.getGauge().getDataPointsList())));
} else {
Expand Down
Loading