From fd7e830c3fff1bf2ef659b6bf31c8d40914eb302 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Wed, 29 Sep 2021 10:48:48 +0200 Subject: [PATCH] code review comments --- .../opentelemetry/instrumentation/api/field/VirtualField.java | 4 ++-- .../api/internal/RuntimeVirtualFieldSupplier.java | 4 ++-- .../hibernate/v3_3/SessionFactoryInstrumentation.java | 4 ++-- .../hibernate/v4_0/SessionFactoryInstrumentation.java | 2 +- .../instrumentation/hibernate/SessionMethodUtils.java | 2 +- .../netty/v3_8/ChannelFutureListenerInstrumentation.java | 2 +- .../netty/v3_8/NettyChannelInstrumentation.java | 2 +- .../netty/v3_8/client/HttpClientRequestTracingHandler.java | 2 +- .../netty/v3_8/client/HttpClientResponseTracingHandler.java | 2 +- .../netty/v3_8/server/HttpServerRequestTracingHandler.java | 2 +- .../netty/v3_8/server/HttpServerResponseTracingHandler.java | 2 +- .../netty/v4_0/NettyChannelPipelineInstrumentation.java | 2 +- .../netty/v4_1/NettyChannelPipelineInstrumentation.java | 2 +- .../servlet/v3_0/Servlet3FilterInitAdvice.java | 2 +- .../instrumentation/servlet/v3_0/Servlet3InitAdvice.java | 2 +- .../servlet/v5_0/service/JakartaServletFilterInitAdvice.java | 2 +- .../servlet/v5_0/service/JakartaServletInitAdvice.java | 2 +- .../instrumentation/api/concurrent/ExecutorAdviceHelper.java | 2 +- .../javaagent/tooling/context/FieldBackedProvider.java | 4 ++-- .../tooling/muzzle/InstrumentationClassPredicateTest.groovy | 2 +- .../src/main/java/context/ContextTestInstrumentation.java | 4 ++-- 21 files changed, 26 insertions(+), 26 deletions(-) diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/field/VirtualField.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/field/VirtualField.java index 7c447a1c98fe..6b33de9fbff3 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/field/VirtualField.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/field/VirtualField.java @@ -53,7 +53,7 @@ public static VirtualField find(Class type, Class VirtualField find(Class type, Class fieldValueSupplier); + public abstract F setIfNullAndGet(T object, Supplier fieldValueSupplier); } diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/RuntimeVirtualFieldSupplier.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/RuntimeVirtualFieldSupplier.java index 6c472b831a79..36e1ae27e2cd 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/RuntimeVirtualFieldSupplier.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/RuntimeVirtualFieldSupplier.java @@ -73,12 +73,12 @@ public void set(T object, @Nullable F fieldValue) { } @Override - public F setIfAbsentAndGet(T object, F fieldValue) { + public F setIfNullAndGet(T object, F fieldValue) { return cache.computeIfAbsent(object, k -> fieldValue); } @Override - public F setIfAbsentAndGet(T object, Supplier fieldValueSupplier) { + public F setIfNullAndGet(T object, Supplier fieldValueSupplier) { return cache.computeIfAbsent(object, k -> fieldValueSupplier.get()); } } diff --git a/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/SessionFactoryInstrumentation.java b/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/SessionFactoryInstrumentation.java index eb018d894045..e9d8625ad884 100644 --- a/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/SessionFactoryInstrumentation.java +++ b/instrumentation/hibernate/hibernate-3.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v3_3/SessionFactoryInstrumentation.java @@ -62,11 +62,11 @@ public static void openSession(@Advice.Return Object session) { if (session instanceof Session) { VirtualField virtualField = VirtualField.find(Session.class, Context.class); - virtualField.setIfAbsentAndGet((Session) session, context); + virtualField.setIfNullAndGet((Session) session, context); } else if (session instanceof StatelessSession) { VirtualField virtualField = VirtualField.find(StatelessSession.class, Context.class); - virtualField.setIfAbsentAndGet((StatelessSession) session, context); + virtualField.setIfNullAndGet((StatelessSession) session, context); } } } diff --git a/instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/SessionFactoryInstrumentation.java b/instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/SessionFactoryInstrumentation.java index 43e0a2ccfb79..4117fbfb29e5 100644 --- a/instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/SessionFactoryInstrumentation.java +++ b/instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/SessionFactoryInstrumentation.java @@ -58,7 +58,7 @@ public static void openSession(@Advice.Return SharedSessionContract session) { VirtualField virtualField = VirtualField.find(SharedSessionContract.class, Context.class); - virtualField.setIfAbsentAndGet(session, context); + virtualField.setIfNullAndGet(session, context); } } } diff --git a/instrumentation/hibernate/hibernate-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/SessionMethodUtils.java b/instrumentation/hibernate/hibernate-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/SessionMethodUtils.java index 0515eb4efa23..aa44df514fce 100644 --- a/instrumentation/hibernate/hibernate-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/SessionMethodUtils.java +++ b/instrumentation/hibernate/hibernate-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/SessionMethodUtils.java @@ -90,7 +90,7 @@ public static void attachSpanFromStore( return; } - targetVirtualField.setIfAbsentAndGet(target, sessionContext); + targetVirtualField.setIfNullAndGet(target, sessionContext); } public static String getSessionMethodSpanName(String methodName) { diff --git a/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/ChannelFutureListenerInstrumentation.java b/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/ChannelFutureListenerInstrumentation.java index 44699d1f2c66..321f513cfb48 100644 --- a/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/ChannelFutureListenerInstrumentation.java +++ b/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/ChannelFutureListenerInstrumentation.java @@ -63,7 +63,7 @@ public static Scope activateScope(@Advice.Argument(0) ChannelFuture future) { VirtualField.find(Channel.class, ChannelTraceContext.class); ChannelTraceContext channelTraceContext = - virtualField.setIfAbsentAndGet(future.getChannel(), ChannelTraceContext.FACTORY); + virtualField.setIfNullAndGet(future.getChannel(), ChannelTraceContext.FACTORY); Context parentContext = channelTraceContext.getConnectionContext(); if (parentContext == null) { return null; diff --git a/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/NettyChannelInstrumentation.java b/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/NettyChannelInstrumentation.java index 65754e710f4a..8ad58f1adca7 100644 --- a/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/NettyChannelInstrumentation.java +++ b/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/NettyChannelInstrumentation.java @@ -55,7 +55,7 @@ public static void onEnter(@Advice.This Channel channel) { VirtualField.find(Channel.class, ChannelTraceContext.class); if (virtualField - .setIfAbsentAndGet(channel, ChannelTraceContext.FACTORY) + .setIfNullAndGet(channel, ChannelTraceContext.FACTORY) .getConnectionContext() == null) { virtualField.get(channel).setConnectionContext(context); diff --git a/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/HttpClientRequestTracingHandler.java b/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/HttpClientRequestTracingHandler.java index 6907d8aed9fa..c6e9670adb44 100644 --- a/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/HttpClientRequestTracingHandler.java +++ b/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/HttpClientRequestTracingHandler.java @@ -34,7 +34,7 @@ public void writeRequested(ChannelHandlerContext ctx, MessageEvent event) { } ChannelTraceContext channelTraceContext = - virtualField.setIfAbsentAndGet(ctx.getChannel(), ChannelTraceContext.FACTORY); + virtualField.setIfNullAndGet(ctx.getChannel(), ChannelTraceContext.FACTORY); Context parentContext = channelTraceContext.getConnectionContext(); if (parentContext != null) { diff --git a/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/HttpClientResponseTracingHandler.java b/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/HttpClientResponseTracingHandler.java index 48d87ef7beb8..94d09b7e4961 100644 --- a/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/HttpClientResponseTracingHandler.java +++ b/instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/HttpClientResponseTracingHandler.java @@ -28,7 +28,7 @@ public HttpClientResponseTracingHandler(VirtualField PropagatedContext attachContextToTask( Context context, VirtualField virtualField, T task) { PropagatedContext propagatedContext = - virtualField.setIfAbsentAndGet(task, PropagatedContext.FACTORY); + virtualField.setIfNullAndGet(task, PropagatedContext.FACTORY); if (ContextPropagationDebug.isThreadPropagationDebuggerEnabled()) { context = ContextPropagationDebug.appendLocations(context, new Exception().getStackTrace(), task); diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/context/FieldBackedProvider.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/context/FieldBackedProvider.java index a54ac7ed4ff5..57a0469b2c7e 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/context/FieldBackedProvider.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/context/FieldBackedProvider.java @@ -882,7 +882,7 @@ public Object get(Object object) { } @Override - public Object setIfAbsentAndGet(Object object, Object fieldValue) { + public Object setIfNullAndGet(Object object, Object fieldValue) { Object oldFieldValue = realGet(object); if (oldFieldValue != null) { return oldFieldValue; @@ -898,7 +898,7 @@ public Object setIfAbsentAndGet(Object object, Object fieldValue) { } @Override - public Object setIfAbsentAndGet(Object object, Supplier fieldValueSupplier) { + public Object setIfNullAndGet(Object object, Supplier fieldValueSupplier) { Object existingContext = realGet(object); if (null != existingContext) { return existingContext; diff --git a/javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/tooling/muzzle/InstrumentationClassPredicateTest.groovy b/javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/tooling/muzzle/InstrumentationClassPredicateTest.groovy index fc1364e3ca6b..f0c3982eabfb 100644 --- a/javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/tooling/muzzle/InstrumentationClassPredicateTest.groovy +++ b/javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/tooling/muzzle/InstrumentationClassPredicateTest.groovy @@ -37,6 +37,6 @@ class InstrumentationClassPredicateTest extends Specification { "Java SDK class" | "java.util.ArrayList" "javaagent-tooling class" | "io.opentelemetry.javaagent.tooling.Constants" "instrumentation-api class" | "io.opentelemetry.instrumentation.api.InstrumentationVersion" - "javaagent-instrumentation-api class" | "io.opentelemetry.instrumentation.api.field.ContextStore" + "javaagent-instrumentation-api class" | "io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge" } } diff --git a/testing-common/integration-tests/src/main/java/context/ContextTestInstrumentation.java b/testing-common/integration-tests/src/main/java/context/ContextTestInstrumentation.java index 947d86db4c53..91eecdecc014 100644 --- a/testing-common/integration-tests/src/main/java/context/ContextTestInstrumentation.java +++ b/testing-common/integration-tests/src/main/java/context/ContextTestInstrumentation.java @@ -52,7 +52,7 @@ public static void methodExit( @Advice.This KeyClass thiz, @Advice.Return(readOnly = false) int contextCount) { VirtualField virtualField = VirtualField.find(KeyClass.class, Context.class); - Context context = virtualField.setIfAbsentAndGet(thiz, new Context()); + Context context = virtualField.setIfNullAndGet(thiz, new Context()); contextCount = ++context.count; } } @@ -64,7 +64,7 @@ public static void methodExit( @Advice.This KeyClass thiz, @Advice.Return(readOnly = false) int contextCount) { VirtualField virtualField = VirtualField.find(KeyClass.class, Context.class); - Context context = virtualField.setIfAbsentAndGet(thiz, Context.FACTORY); + Context context = virtualField.setIfNullAndGet(thiz, Context.FACTORY); contextCount = ++context.count; } }