code review comments

open-telemetry · Sep 29, 2021 · fd7e830 · fd7e830
1 parent 16ae9fb
commit fd7e830
Show file tree

Hide file tree

Showing 21 changed files with 26 additions and 26 deletions.
diff --git a/...umentation-api/src/main/java/io/opentelemetry/instrumentation/api/field/VirtualField.java b/...umentation-api/src/main/java/io/opentelemetry/instrumentation/api/field/VirtualField.java
@@ -53,13 +53,13 @@ public static <U extends T, T, F> VirtualField<U, F> find(Class<T> type, Class<F
    *
    * @return The old field value if it was present, or the passed {@code fieldValue}.
    */
-  public abstract F setIfAbsentAndGet(T object, F fieldValue);
+  public abstract F setIfNullAndGet(T object, F fieldValue);
 
   /**
    * Sets the new value of this virtual field if the current value is {@code null}.
    *
    * @return The old field value if it was present, or the result of evaluating passed {@code
    *     fieldValueSupplier}.
    */
-  public abstract F setIfAbsentAndGet(T object, Supplier<F> fieldValueSupplier);
+  public abstract F setIfNullAndGet(T object, Supplier<F> fieldValueSupplier);
 }
diff --git a/.../main/java/io/opentelemetry/instrumentation/api/internal/RuntimeVirtualFieldSupplier.java b/.../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<F> fieldValueSupplier) {
+    public F setIfNullAndGet(T object, Supplier<F> fieldValueSupplier) {
       return cache.computeIfAbsent(object, k -> fieldValueSupplier.get());
     }
   }

diff --git a/...opentelemetry/javaagent/instrumentation/hibernate/v3_3/SessionFactoryInstrumentation.java b/...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<Session, Context> virtualField =
             VirtualField.find(Session.class, Context.class);
-        virtualField.setIfAbsentAndGet((Session) session, context);
+        virtualField.setIfNullAndGet((Session) session, context);
       } else if (session instanceof StatelessSession) {
         VirtualField<StatelessSession, Context> virtualField =
             VirtualField.find(StatelessSession.class, Context.class);
-        virtualField.setIfAbsentAndGet((StatelessSession) session, context);
+        virtualField.setIfNullAndGet((StatelessSession) session, context);
       }
     }
   }

diff --git a/...opentelemetry/javaagent/instrumentation/hibernate/v4_0/SessionFactoryInstrumentation.java b/...opentelemetry/javaagent/instrumentation/hibernate/v4_0/SessionFactoryInstrumentation.java
@@ -58,7 +58,7 @@ public static void openSession(@Advice.Return SharedSessionContract session) {
 
       VirtualField<SharedSessionContract, Context> virtualField =
           VirtualField.find(SharedSessionContract.class, Context.class);
-      virtualField.setIfAbsentAndGet(session, context);
+      virtualField.setIfNullAndGet(session, context);
     }
   }
 }
diff --git a/...rc/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/SessionMethodUtils.java b/...rc/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/SessionMethodUtils.java
@@ -90,7 +90,7 @@ public static <S, T> void attachSpanFromStore(
       return;
     }
 
-    targetVirtualField.setIfAbsentAndGet(target, sessionContext);
+    targetVirtualField.setIfNullAndGet(target, sessionContext);
   }
 
   public static String getSessionMethodSpanName(String methodName) {

diff --git a/...ntelemetry/javaagent/instrumentation/netty/v3_8/ChannelFutureListenerInstrumentation.java b/...ntelemetry/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/...va/io/opentelemetry/javaagent/instrumentation/netty/v3_8/NettyChannelInstrumentation.java b/...va/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/...elemetry/javaagent/instrumentation/netty/v3_8/client/HttpClientRequestTracingHandler.java b/...elemetry/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/...lemetry/javaagent/instrumentation/netty/v3_8/client/HttpClientResponseTracingHandler.java b/...lemetry/javaagent/instrumentation/netty/v3_8/client/HttpClientResponseTracingHandler.java
@@ -28,7 +28,7 @@ public HttpClientResponseTracingHandler(VirtualField<Channel, ChannelTraceContex
   @Override
   public void messageReceived(ChannelHandlerContext ctx, MessageEvent msg) {
     ChannelTraceContext channelTraceContext =
-        virtualField.setIfAbsentAndGet(ctx.getChannel(), ChannelTraceContext.FACTORY);
+        virtualField.setIfNullAndGet(ctx.getChannel(), ChannelTraceContext.FACTORY);
 
     Context context = channelTraceContext.getContext();
     if (context == null) {

diff --git a/...elemetry/javaagent/instrumentation/netty/v3_8/server/HttpServerRequestTracingHandler.java b/...elemetry/javaagent/instrumentation/netty/v3_8/server/HttpServerRequestTracingHandler.java
@@ -28,7 +28,7 @@ public HttpServerRequestTracingHandler(VirtualField<Channel, ChannelTraceContext
   @Override
   public void messageReceived(ChannelHandlerContext ctx, MessageEvent event) {
     ChannelTraceContext channelTraceContext =
-        virtualField.setIfAbsentAndGet(ctx.getChannel(), ChannelTraceContext.FACTORY);
+        virtualField.setIfNullAndGet(ctx.getChannel(), ChannelTraceContext.FACTORY);
 
     Object message = event.getMessage();
     if (!(message instanceof HttpRequest)) {

diff --git a/...lemetry/javaagent/instrumentation/netty/v3_8/server/HttpServerResponseTracingHandler.java b/...lemetry/javaagent/instrumentation/netty/v3_8/server/HttpServerResponseTracingHandler.java
@@ -28,7 +28,7 @@ public HttpServerResponseTracingHandler(VirtualField<Channel, ChannelTraceContex
   @Override
   public void writeRequested(ChannelHandlerContext ctx, MessageEvent msg) {
     ChannelTraceContext channelTraceContext =
-        virtualField.setIfAbsentAndGet(ctx.getChannel(), ChannelTraceContext.FACTORY);
+        virtualField.setIfNullAndGet(ctx.getChannel(), ChannelTraceContext.FACTORY);
 
     Context context = tracer().getServerContext(channelTraceContext);
     if (context == null || !(msg.getMessage() instanceof HttpResponse)) {

diff --git a/...entelemetry/javaagent/instrumentation/netty/v4_0/NettyChannelPipelineInstrumentation.java b/...entelemetry/javaagent/instrumentation/netty/v4_0/NettyChannelPipelineInstrumentation.java
@@ -89,7 +89,7 @@ public static void addHandler(
           pipeline.addLast(ourHandler.getClass().getName(), ourHandler);
           // associate our handle with original handler so they could be removed together
           VirtualField.find(ChannelHandler.class, ChannelHandler.class)
-              .setIfAbsentAndGet(handler, ourHandler);
+              .setIfNullAndGet(handler, ourHandler);
         } catch (IllegalArgumentException e) {
           // Prevented adding duplicate handlers.
         }

diff --git a/...entelemetry/javaagent/instrumentation/netty/v4_1/NettyChannelPipelineInstrumentation.java b/...entelemetry/javaagent/instrumentation/netty/v4_1/NettyChannelPipelineInstrumentation.java
@@ -114,7 +114,7 @@ public static void addHandler(
           pipeline.addAfter(name, ourHandler.getClass().getName(), ourHandler);
           // associate our handle with original handler so they could be removed together
           VirtualField.find(ChannelHandler.class, ChannelHandler.class)
-              .setIfAbsentAndGet(handler, ourHandler);
+              .setIfNullAndGet(handler, ourHandler);
         } catch (IllegalArgumentException e) {
           // Prevented adding duplicate handlers.
         }

diff --git a/...ava/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3FilterInitAdvice.java b/...ava/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3FilterInitAdvice.java
@@ -21,6 +21,6 @@ public static void filterInit(
       return;
     }
     VirtualField.find(Filter.class, MappingResolver.Factory.class)
-        .setIfAbsentAndGet(filter, new Servlet3FilterMappingResolverFactory(filterConfig));
+        .setIfNullAndGet(filter, new Servlet3FilterMappingResolverFactory(filterConfig));
   }
 }
diff --git a/...main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3InitAdvice.java b/...main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3InitAdvice.java
@@ -21,6 +21,6 @@ public static void servletInit(
       return;
     }
     VirtualField.find(Servlet.class, MappingResolver.Factory.class)
-        .setIfAbsentAndGet(servlet, new Servlet3MappingResolverFactory(servletConfig));
+        .setIfNullAndGet(servlet, new Servlet3MappingResolverFactory(servletConfig));
   }
 }
diff --git a/...emetry/javaagent/instrumentation/servlet/v5_0/service/JakartaServletFilterInitAdvice.java b/...emetry/javaagent/instrumentation/servlet/v5_0/service/JakartaServletFilterInitAdvice.java
@@ -21,6 +21,6 @@ public static void filterInit(
       return;
     }
     VirtualField.find(Filter.class, MappingResolver.Factory.class)
-        .setIfAbsentAndGet(filter, new JakartaServletFilterMappingResolverFactory(filterConfig));
+        .setIfNullAndGet(filter, new JakartaServletFilterMappingResolverFactory(filterConfig));
   }
 }
diff --git a/...pentelemetry/javaagent/instrumentation/servlet/v5_0/service/JakartaServletInitAdvice.java b/...pentelemetry/javaagent/instrumentation/servlet/v5_0/service/JakartaServletInitAdvice.java
@@ -21,6 +21,6 @@ public static void servletInit(
       return;
     }
     VirtualField.find(Servlet.class, MappingResolver.Factory.class)
-        .setIfAbsentAndGet(servlet, new JakartaServletMappingResolverFactory(servletConfig));
+        .setIfNullAndGet(servlet, new JakartaServletMappingResolverFactory(servletConfig));
   }
 }
diff --git a/.../java/io/opentelemetry/javaagent/instrumentation/api/concurrent/ExecutorAdviceHelper.java b/.../java/io/opentelemetry/javaagent/instrumentation/api/concurrent/ExecutorAdviceHelper.java
@@ -44,7 +44,7 @@ public static boolean shouldPropagateContext(Context context, @Nullable Object t
   public static <T> PropagatedContext attachContextToTask(
       Context context, VirtualField<T, PropagatedContext> 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/...tooling/src/main/java/io/opentelemetry/javaagent/tooling/context/FieldBackedProvider.java b/...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<Object> fieldValueSupplier) {
+    public Object setIfNullAndGet(Object object, Supplier<Object> fieldValueSupplier) {
       Object existingContext = realGet(object);
       if (null != existingContext) {
         return existingContext;

diff --git a/...groovy/io/opentelemetry/javaagent/tooling/muzzle/InstrumentationClassPredicateTest.groovy b/...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
@@ -52,7 +52,7 @@ public static void methodExit(
         @Advice.This KeyClass thiz, @Advice.Return(readOnly = false) int contextCount) {
       VirtualField<KeyClass, Context> 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<KeyClass, Context> virtualField =
           VirtualField.find(KeyClass.class, Context.class);
-      Context context = virtualField.setIfAbsentAndGet(thiz, Context.FACTORY);
+      Context context = virtualField.setIfNullAndGet(thiz, Context.FACTORY);
       contextCount = ++context.count;
     }
   }
-Original file line number
+Diff line change
@@ Expand Up / @@ -90,7 +90,7 @@ public static <S, T> void attachSpanFromStore( @@
           return;
         }
-        targetVirtualField.setIfAbsentAndGet(target, sessionContext);
+        targetVirtualField.setIfNullAndGet(target, sessionContext);
       }
       public static String getSessionMethodSpanName(String methodName) {
@@ Expand Down @@