Remove todo assignment (#12884)
This commit is contained in:
Jay DeLuca
GitHub
parent
ce01f00413
commit
712e0a7acc
|
|
@ -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>
|
||||
|
|
|
|||
|
|
@ -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))
|
||||
|
|
@ -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-")) {
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -158,8 +158,7 @@ abstract class SmokeTest {
|
|||
.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) {
|
||||
|
|
|
|||
|
|
@ -180,8 +180,8 @@ public abstract class AbstractArmeriaHttpServerTest extends AbstractHttpServerTe
|
|||
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);
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
||||
|
|
|
|||
|
|
@ -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"() {
|
||||
|
|
|
|||
|
|
@ -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"))
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ import javax.annotation.Nullable;
|
|||
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;
|
||||
|
||||
|
|
|
|||
|
|
@ -38,8 +38,8 @@ class LettuceReactiveClientTest extends AbstractLettuceReactiveClientTest {
|
|||
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() {
|
||||
|
|
|
|||
|
|
@ -112,7 +112,7 @@ class Netty38ClientTest extends AbstractHttpClientTest<Request> {
|
|||
|
||||
// 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>() {
|
||||
|
|
|
|||
|
|
@ -31,7 +31,7 @@ public abstract class AbstractProcessMetricsTest {
|
|||
metric ->
|
||||
assertThat(metric)
|
||||
.hasUnit("By")
|
||||
// TODO(anuraaga): Provide fuzzy value matching
|
||||
// TODO: Provide fuzzy value matching
|
||||
.hasLongSumSatisfying(
|
||||
sum ->
|
||||
assertThat(metric.getLongSumData().getPoints())
|
||||
|
|
@ -46,7 +46,7 @@ public abstract class AbstractProcessMetricsTest {
|
|||
metric ->
|
||||
assertThat(metric)
|
||||
.hasUnit("ms")
|
||||
// TODO(anuraaga): Provide fuzzy value matching
|
||||
// TODO: Provide fuzzy value matching
|
||||
.hasLongGaugeSatisfying(
|
||||
gauge ->
|
||||
assertThat(metric.getLongGaugeData().getPoints())
|
||||
|
|
|
|||
|
|
@ -31,7 +31,7 @@ public abstract class AbstractSystemMetricsTest {
|
|||
metric ->
|
||||
assertThat(metric)
|
||||
.hasUnit("By")
|
||||
// TODO(anuraaga): Provide fuzzy value matching
|
||||
// TODO: Provide fuzzy value matching
|
||||
.hasLongSumSatisfying(
|
||||
sum ->
|
||||
assertThat(metric.getLongSumData().getPoints())
|
||||
|
|
@ -46,7 +46,7 @@ public abstract class AbstractSystemMetricsTest {
|
|||
metric ->
|
||||
assertThat(metric)
|
||||
.hasUnit("1")
|
||||
// TODO(anuraaga): Provide fuzzy value matching
|
||||
// TODO: Provide fuzzy value matching
|
||||
.hasDoubleGaugeSatisfying(
|
||||
gauge ->
|
||||
assertThat(metric.getDoubleGaugeData().getPoints())
|
||||
|
|
|
|||
|
|
@ -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"))
|
||||
|
|
|
|||
|
|
@ -31,7 +31,7 @@ final class TracingJobListener implements JobListener {
|
|||
|
||||
@Override
|
||||
public void jobExecutionVetoed(JobExecutionContext jobExecutionContext) {
|
||||
// TODO(anuraaga): Consider adding an attribute for vetoed jobs.
|
||||
// TODO: Consider adding an attribute for vetoed jobs.
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
|
|||
|
|
@ -266,7 +266,7 @@ public abstract class AbstractRatpackHttpServerTest extends AbstractHttpServerTe
|
|||
});
|
||||
}
|
||||
|
||||
// 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 {
|
||||
|
|
|
|||
|
|
@ -23,7 +23,7 @@ class InitializationTest {
|
|||
|
||||
// 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(
|
||||
|
|
|
|||
|
|
@ -30,7 +30,7 @@ public class RxJavaPluginsInstrumentation implements TypeInstrumentation {
|
|||
@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() {
|
||||
|
|
|
|||
|
|
@ -30,7 +30,7 @@ public class RxJavaPluginsInstrumentation implements TypeInstrumentation {
|
|||
@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() {
|
||||
|
|
|
|||
|
|
@ -30,7 +30,7 @@ public class RxJavaPluginsInstrumentation implements TypeInstrumentation {
|
|||
@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() {
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -131,7 +131,7 @@ public abstract class InstrumentationTestRunner {
|
|||
// 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;
|
||||
|
|
|
|||
|
|
@ -350,7 +350,7 @@ public final class AgentTestingExporterAccess {
|
|||
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 {
|
||||
|
|
|
|||
Loading…
Reference in New Issue