Skip to content

Commit

Permalink
fix(oldapi): Tracer instance memory leak (#9) (#10)
Browse files Browse the repository at this point in the history
* fix(interceptor): tracer instance memory leak

* fix(phase listener): tracer instance memory leak

* chore: bump version
  • Loading branch information
maruTA-bis5 authored May 28, 2024
1 parent a884e81 commit 63403d7
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 136 deletions.
10 changes: 5 additions & 5 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>net.bis5.opentracing</groupId>
<artifactId>opentracing-faces</artifactId>
<version>1.2.1-oldapi.0-SNAPSHOT</version>
<version>1.3.0-oldapi.0-SNAPSHOT</version>

<name>MicroProfile OpenTracing for Jakarta Server Faces</name>
<description>Integrate Jakarta Server Faces with MicroProfile OpenTracing</description>
Expand Down Expand Up @@ -76,14 +76,14 @@
</dependency>
<dependency>
<groupId>io.opentracing.contrib</groupId>
<artifactId>opentracing-interceptors</artifactId>
<version>0.0.3</version>
<artifactId>opentracing-tracerresolver</artifactId>
<version>0.1.8</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>io.opentracing.contrib</groupId>
<artifactId>opentracing-tracerresolver</artifactId>
<version>0.1.8</version>
<artifactId>opentracing-interceptors</artifactId>
<version>0.0.4.1</version>
<scope>provided</scope>
</dependency>
<dependency>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,147 +1,25 @@
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
*/
@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<Tracer> 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, false)) {
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<String, Object> log = new HashMap<>();
log.put("event", Tags.ERROR.getKey());
log.put("error.object", e);
span.log(log);
Tags.ERROR.set(span, true);
}

}
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -15,9 +18,6 @@ public class TracingPhaseListener implements PhaseListener {

private static final long serialVersionUID = 1L;

@Inject
Instance<Tracer> tracerInstance;

private final transient ThreadLocal<Span> rootSpan = new ThreadLocal<>();
private final transient ThreadLocal<Scope> rootScope = new ThreadLocal<>();
private final transient ThreadLocal<Span> currentSpan = new ThreadLocal<>();
Expand Down Expand Up @@ -51,9 +51,17 @@ public void afterPhase(PhaseEvent event) {
}
}

private static final String TRACER_KEY = TracingPhaseListener.class.getName() + ".Tracer";

private Tracer getTracer() {
Map<String, Object> request = FacesContext.getCurrentInstance().getExternalContext().getRequestMap();
return (Tracer) request.computeIfAbsent(TRACER_KEY, ignore -> resolveTracer());
}

private Tracer resolveTracer() {
Instance<Tracer> tracerInstance = CDI.current().getBeanManager().createInstance().select(Tracer.class);
if (!tracerInstance.isUnsatisfied()) {
return this.tracerInstance.get();
return tracerInstance.get();
}
return TracerResolver.resolveTracer();
}
Expand Down

0 comments on commit 63403d7

Please sign in to comment.