diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/AppServerBridge.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/AppServerBridge.java index 34af542379a1..a5725390e146 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/AppServerBridge.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/AppServerBridge.java @@ -7,7 +7,7 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.ContextKey; -import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; /** * Helper container for Context attributes for transferring certain information between servlet @@ -18,6 +18,14 @@ public class AppServerBridge { private static final ContextKey CONTEXT_KEY = ContextKey.named("opentelemetry-servlet-app-server-bridge"); + private static final AtomicIntegerFieldUpdater + servletUpdatedServerSpanNameUpdater = + AtomicIntegerFieldUpdater.newUpdater( + AppServerBridge.class, "servletUpdatedServerSpanName"); + + private static final int FALSE = 0; + private static final int TRUE = 1; + /** * Attach AppServerBridge to context. * @@ -42,42 +50,30 @@ public static Context init(Context ctx, boolean shouldRecordException) { return ctx.with(AppServerBridge.CONTEXT_KEY, new AppServerBridge(shouldRecordException)); } - private final AtomicBoolean servletUpdatedServerSpanName = new AtomicBoolean(false); - private final AtomicBoolean servletShouldRecordException; + private final boolean servletShouldRecordException; + + private volatile int servletUpdatedServerSpanName = FALSE; private AppServerBridge(boolean shouldRecordException) { - servletShouldRecordException = new AtomicBoolean(shouldRecordException); + servletShouldRecordException = shouldRecordException; } /** - * Returns true, if servlet integration should update server span name. After server span name has - * been updated with setServletUpdatedServerSpanName this method will return - * false. + * Returns true, if servlet integration has not already updated the server span name. Subsequent + * invocations will return {@code false}. This is meant to be used in a compare-and-set fashion. * * @param ctx server context - * @return true, if the server span name should be updated by servlet integration, or - * false otherwise. + * @return true, if the server span name had not already been updated by servlet + * integration, or false otherwise. */ - public static boolean shouldUpdateServerSpanName(Context ctx) { + public static boolean setUpdatedServerSpanName(Context ctx) { AppServerBridge appServerBridge = ctx.get(AppServerBridge.CONTEXT_KEY); if (appServerBridge != null) { - return !appServerBridge.servletUpdatedServerSpanName.get(); + return servletUpdatedServerSpanNameUpdater.compareAndSet(appServerBridge, FALSE, TRUE); } return false; } - /** - * Indicate that the servlet integration has updated the name for the server span. - * - * @param ctx server context - */ - public static void setServletUpdatedServerSpanName(Context ctx, boolean value) { - AppServerBridge appServerBridge = ctx.get(AppServerBridge.CONTEXT_KEY); - if (appServerBridge != null) { - appServerBridge.servletUpdatedServerSpanName.set(value); - } - } - /** * Returns true, if servlet integration should record exception thrown during servlet invocation * in server span. This method should return false on servers where exceptions thrown @@ -91,7 +87,7 @@ public static void setServletUpdatedServerSpanName(Context ctx, boolean value) { public static boolean shouldRecordException(Context ctx) { AppServerBridge appServerBridge = ctx.get(AppServerBridge.CONTEXT_KEY); if (appServerBridge != null) { - return appServerBridge.servletShouldRecordException.get(); + return appServerBridge.servletShouldRecordException; } return true; } diff --git a/instrumentation-core/servlet-2.2/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java b/instrumentation-core/servlet-2.2/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java index 3f4ed5db2c03..59bcaa1b2558 100644 --- a/instrumentation-core/servlet-2.2/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java +++ b/instrumentation-core/servlet-2.2/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java @@ -164,9 +164,8 @@ public static String getSpanName(HttpServletRequest request) { * reflected in the span name. */ public Context runOnceUnderAppServer(Context context, HttpServletRequest request) { - if (AppServerBridge.shouldUpdateServerSpanName(context)) { + if (AppServerBridge.setUpdatedServerSpanName(context)) { updateSpanName(Span.fromContext(context), request); - AppServerBridge.setServletUpdatedServerSpanName(context, true); return addServletContextPath(context, request); } return context; diff --git a/javaagent-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/concurrent/State.java b/javaagent-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/concurrent/State.java index eb01d2e4c5bc..122104d55530 100644 --- a/javaagent-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/concurrent/State.java +++ b/javaagent-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/concurrent/State.java @@ -7,7 +7,7 @@ import io.opentelemetry.context.Context; import io.opentelemetry.javaagent.instrumentation.api.ContextStore; -import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -15,30 +15,37 @@ public class State { private static final Logger log = LoggerFactory.getLogger(State.class); + private static final AtomicReferenceFieldUpdater parentContextUpdater = + AtomicReferenceFieldUpdater.newUpdater(State.class, Context.class, "parentContext"); + public static final ContextStore.Factory FACTORY = State::new; - private final AtomicReference parentContextRef = new AtomicReference<>(null); + private volatile Context parentContext; private State() {} public void setParentContext(Context parentContext) { - boolean result = parentContextRef.compareAndSet(null, parentContext); - if (!result && parentContextRef.get() != parentContext) { - if (log.isDebugEnabled()) { - log.debug( - "Failed to set parent context because another parent context is already set {}: new: {}, old: {}", - this, - parentContext, - parentContextRef.get()); + boolean result = parentContextUpdater.compareAndSet(this, null, parentContext); + if (!result) { + Context currentParent = parentContextUpdater.get(this); + if (currentParent != parentContext) { + if (log.isDebugEnabled()) { + log.debug( + "Failed to set parent context because another parent context is " + + "already set {}: new: {}, old: {}", + this, + parentContext, + currentParent); + } } } } public void clearParentContext() { - parentContextRef.set(null); + parentContextUpdater.set(this, null); } public Context getAndResetParentContext() { - return parentContextRef.getAndSet(null); + return parentContextUpdater.getAndSet(this, null); } }