From d3377c5dfc35ad869349a3e414561334d11c7f43 Mon Sep 17 00:00:00 2001 From: Takayuki Maruyama Date: Sat, 8 Jun 2024 15:54:48 +0900 Subject: [PATCH] fix: Tracer instance memory leak (#9) (#11) * fix: Tracer instance memory leak (#9) * fix(interceptor): tracer instance memory leak * fix(phase listener): tracer instance memory leak * chore: bump version * deps(dev): bump opentraing-interceptors --- pom.xml | 10 +- .../SerializableOpenTracingInterceptor.java | 130 +----------------- .../faces/phase/TracingPhaseListener.java | 18 ++- 3 files changed, 22 insertions(+), 136 deletions(-) diff --git a/pom.xml b/pom.xml index 7d0972e..e612a35 100644 --- a/pom.xml +++ b/pom.xml @@ -2,7 +2,7 @@ 4.0.0 net.bis5.opentracing opentracing-faces - 1.2.1-SNAPSHOT + 1.3.0-SNAPSHOT MicroProfile OpenTracing for Jakarta Server Faces Integrate Jakarta Server Faces with MicroProfile OpenTracing @@ -76,14 +76,14 @@ io.opentracing.contrib - opentracing-interceptors - 0.0.3 + opentracing-tracerresolver + 0.1.8 provided io.opentracing.contrib - opentracing-tracerresolver - 0.1.8 + opentracing-interceptors + 0.1.0 provided diff --git a/src/main/java/net/bis5/opentracing/faces/interceptor/SerializableOpenTracingInterceptor.java b/src/main/java/net/bis5/opentracing/faces/interceptor/SerializableOpenTracingInterceptor.java index c7f9100..a832e8f 100644 --- a/src/main/java/net/bis5/opentracing/faces/interceptor/SerializableOpenTracingInterceptor.java +++ b/src/main/java/net/bis5/opentracing/faces/interceptor/SerializableOpenTracingInterceptor.java @@ -1,25 +1,13 @@ package net.bis5.opentracing.faces.interceptor; import java.io.Serializable; -import java.lang.reflect.Method; -import java.util.HashMap; -import java.util.Map; -import java.util.logging.Logger; import javax.annotation.Priority; -import javax.enterprise.inject.Instance; -import javax.inject.Inject; import javax.interceptor.AroundInvoke; import javax.interceptor.Interceptor; import javax.interceptor.InvocationContext; -import javax.ws.rs.Path; -import io.opentracing.Scope; -import io.opentracing.Span; -import io.opentracing.SpanContext; -import io.opentracing.Tracer; -import io.opentracing.contrib.tracerresolver.TracerResolver; -import io.opentracing.tag.Tags; +import io.opentracing.contrib.interceptors.OpenTracingInterceptor; /** * @see io.opentracing.contrib.interceptors.OpenTracingInterceptor @@ -27,121 +15,11 @@ @TracedSerializable @Interceptor @Priority(value = Interceptor.Priority.LIBRARY_BEFORE + 1) -public class SerializableOpenTracingInterceptor implements Serializable { - private static final long serialVersionUID = 1L; - public static final String SPAN_CONTEXT = "__opentracing_span_context"; - private static final Logger log = Logger.getLogger(SerializableOpenTracingInterceptor.class.getName()); - - @Inject - Instance tracerInstance; +public class SerializableOpenTracingInterceptor extends OpenTracingInterceptor implements Serializable { @AroundInvoke + @Override public Object wrap(InvocationContext ctx) throws Exception { - if (skipJaxRs(ctx.getMethod())) { - return ctx.proceed(); - } - - if (!traced(ctx.getMethod())) { - return ctx.proceed(); - } - - Tracer tracer = getTracer(); - Tracer.SpanBuilder spanBuilder = tracer.buildSpan(getOperationName(ctx.getMethod())); - - int contextParameterIndex = -1; - for (int i = 0 ; i < ctx.getParameters().length ; i++) { - Object parameter = ctx.getParameters()[i]; - if (parameter instanceof SpanContext) { - log.fine("Found parameter as span context. Using it as the parent of this new span"); - spanBuilder.asChildOf((SpanContext) parameter); - contextParameterIndex = i; - break; - } - - if (parameter instanceof Span) { - log.fine("Found parameter as span. Using it as the parent of this new span"); - spanBuilder.asChildOf((Span) parameter); - contextParameterIndex = i; - break; - } - } - - if (contextParameterIndex < 0) { - log.fine("No parent found. Trying to get span context from context data"); - Object ctxParentSpan = ctx.getContextData().get(SPAN_CONTEXT); - if (ctxParentSpan instanceof SpanContext) { - log.fine("Found span context from context data."); - SpanContext parentSpan = (SpanContext) ctxParentSpan; - spanBuilder.asChildOf(parentSpan); - } - } - - Span span = spanBuilder.start(); - try (Scope scope = tracer.scopeManager().activate(span)) { - log.fine("Adding span context into the invocation context."); - ctx.getContextData().put(SPAN_CONTEXT, span.context()); - - if (contextParameterIndex >= 0) { - log.fine("Overriding the original span context with our new context."); - for (int i = 0 ; i < ctx.getParameters().length ; i++) { - if (ctx.getParameters()[contextParameterIndex] instanceof Span) { - ctx.getParameters()[contextParameterIndex] = span; - } - - if (ctx.getParameters()[contextParameterIndex] instanceof SpanContext) { - ctx.getParameters()[contextParameterIndex] = span.context(); - } - } - } - - return ctx.proceed(); - } catch (Exception e) { - logException(span, e); - throw e; - } finally { - span.finish(); - } - } - - public Tracer getTracer() { - if (null != tracerInstance && !tracerInstance.isUnsatisfied()) { - return this.tracerInstance.get(); - } - - return TracerResolver.resolveTracer(); - } - - private boolean traced(Method method) { - TracedSerializable classTraced = method.getDeclaringClass().getAnnotation(TracedSerializable.class); - TracedSerializable methodTraced = method.getAnnotation(TracedSerializable.class); - if (methodTraced != null) { - return methodTraced.value(); - } - return classTraced != null && classTraced.value(); - } - - private boolean skipJaxRs(Method method) { - return method.getAnnotation(Path.class) != null || - method.getDeclaringClass().getAnnotation(Path.class) != null; + return super.wrap(ctx); } - - private String getOperationName(Method method) { - TracedSerializable classTraced = method.getDeclaringClass().getAnnotation(TracedSerializable.class); - TracedSerializable methodTraced = method.getAnnotation(TracedSerializable.class); - if (methodTraced != null && methodTraced.operationName().length() > 0) { - return methodTraced.operationName(); - } else if (classTraced != null && classTraced.operationName().length() > 0) { - return classTraced.operationName(); - } - return String.format("%s.%s", method.getDeclaringClass().getName(), method.getName()); - } - - private void logException(Span span, Exception e) { - Map log = new HashMap<>(); - log.put("event", Tags.ERROR.getKey()); - log.put("error.object", e); - span.log(log); - Tags.ERROR.set(span, true); - } - } \ No newline at end of file diff --git a/src/main/java/net/bis5/opentracing/faces/phase/TracingPhaseListener.java b/src/main/java/net/bis5/opentracing/faces/phase/TracingPhaseListener.java index cc64e3a..0183ea8 100644 --- a/src/main/java/net/bis5/opentracing/faces/phase/TracingPhaseListener.java +++ b/src/main/java/net/bis5/opentracing/faces/phase/TracingPhaseListener.java @@ -1,10 +1,13 @@ package net.bis5.opentracing.faces.phase; +import java.util.Map; + import javax.enterprise.inject.Instance; +import javax.enterprise.inject.spi.CDI; +import javax.faces.context.FacesContext; import javax.faces.event.PhaseEvent; import javax.faces.event.PhaseId; import javax.faces.event.PhaseListener; -import javax.inject.Inject; import io.opentracing.Scope; import io.opentracing.Span; @@ -15,9 +18,6 @@ public class TracingPhaseListener implements PhaseListener { private static final long serialVersionUID = 1L; - @Inject - Instance tracerInstance; - private final transient ThreadLocal rootSpan = new ThreadLocal<>(); private final transient ThreadLocal rootScope = new ThreadLocal<>(); private final transient ThreadLocal currentSpan = new ThreadLocal<>(); @@ -51,9 +51,17 @@ public void afterPhase(PhaseEvent event) { } } + private static final String TRACER_KEY = TracingPhaseListener.class.getName() + ".Tracer"; + private Tracer getTracer() { + Map request = FacesContext.getCurrentInstance().getExternalContext().getRequestMap(); + return (Tracer) request.computeIfAbsent(TRACER_KEY, ignore -> resolveTracer()); + } + + private Tracer resolveTracer() { + Instance tracerInstance = CDI.current().getBeanManager().createInstance().select(Tracer.class); if (!tracerInstance.isUnsatisfied()) { - return this.tracerInstance.get(); + return tracerInstance.get(); } return TracerResolver.resolveTracer(); }