Skip to content

Commit

Permalink
remove more references
Browse files Browse the repository at this point in the history
  • Loading branch information
jaydeluca committed Dec 12, 2024
1 parent 362e6cc commit 188efd9
Show file tree
Hide file tree
Showing 25 changed files with 36 additions and 40 deletions.
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 @@ -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

0 comments on commit 188efd9

Please sign in to comment.