From 64cfaa50a12cfe06136e0f57535c7e2990d59675 Mon Sep 17 00:00:00 2001 From: emeroad Date: Tue, 21 Feb 2023 15:29:37 +0900 Subject: [PATCH] [#9595] Fix an issue where metric values in non sampling state were incorrect --- .../RequestStartAsyncInterceptor.java | 4 +-- .../RequestStartAsyncInterceptor.java | 4 +-- .../context/DefaultBaseTraceFactory.java | 10 ++++---- .../context/active/ActiveTraceRepository.java | 3 --- .../active/DefaultActiveTraceRepository.java | 25 +++++-------------- .../active/EmptyActiveTraceRepository.java | 6 ----- 6 files changed, 15 insertions(+), 37 deletions(-) diff --git a/plugins/tomcat/src/main/java/com/navercorp/pinpoint/plugin/tomcat/jakarta/interceptor/RequestStartAsyncInterceptor.java b/plugins/tomcat/src/main/java/com/navercorp/pinpoint/plugin/tomcat/jakarta/interceptor/RequestStartAsyncInterceptor.java index d393e946834f..a235a161c6fe 100644 --- a/plugins/tomcat/src/main/java/com/navercorp/pinpoint/plugin/tomcat/jakarta/interceptor/RequestStartAsyncInterceptor.java +++ b/plugins/tomcat/src/main/java/com/navercorp/pinpoint/plugin/tomcat/jakarta/interceptor/RequestStartAsyncInterceptor.java @@ -48,7 +48,7 @@ public void before(Object target, Object[] args) { logger.beforeInterceptor(target, args); } - final Trace trace = traceContext.currentTraceObject(); + final Trace trace = traceContext.currentRawTraceObject(); if (trace == null) { return; } @@ -61,7 +61,7 @@ public void after(Object target, Object[] args, Object result, Throwable throwab logger.afterInterceptor(target, args, result, throwable); } - final Trace trace = traceContext.currentTraceObject(); + final Trace trace = traceContext.currentRawTraceObject(); if (trace == null) { return; } diff --git a/plugins/tomcat/src/main/java/com/navercorp/pinpoint/plugin/tomcat/javax/interceptor/RequestStartAsyncInterceptor.java b/plugins/tomcat/src/main/java/com/navercorp/pinpoint/plugin/tomcat/javax/interceptor/RequestStartAsyncInterceptor.java index 79de3d7b2ed6..dba57872ccd0 100644 --- a/plugins/tomcat/src/main/java/com/navercorp/pinpoint/plugin/tomcat/javax/interceptor/RequestStartAsyncInterceptor.java +++ b/plugins/tomcat/src/main/java/com/navercorp/pinpoint/plugin/tomcat/javax/interceptor/RequestStartAsyncInterceptor.java @@ -49,7 +49,7 @@ public void before(Object target, Object[] args) { logger.beforeInterceptor(target, args); } - final Trace trace = traceContext.currentTraceObject(); + final Trace trace = traceContext.currentRawTraceObject(); if (trace == null) { return; } @@ -62,7 +62,7 @@ public void after(Object target, Object[] args, Object result, Throwable throwab logger.afterInterceptor(target, args, result, throwable); } - final Trace trace = traceContext.currentTraceObject(); + final Trace trace = traceContext.currentRawTraceObject(); if (trace == null) { return; } diff --git a/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/DefaultBaseTraceFactory.java b/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/DefaultBaseTraceFactory.java index 1997188c86a5..d086e88119cf 100644 --- a/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/DefaultBaseTraceFactory.java +++ b/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/DefaultBaseTraceFactory.java @@ -133,7 +133,11 @@ public Trace continueAsyncContextTraceObject(TraceRoot traceRoot, LocalAsyncId l @Override public Trace continueDisableAsyncContextTraceObject(LocalTraceRoot traceRoot) { - final AsyncState asyncState = new DisableAsyncState(); + final ActiveTraceHandle handle = registerActiveTrace(traceRoot); + final AsyncState asyncState = new ListenableAsyncState(traceRoot, + ListenableAsyncState.AsyncStateListener.EMPTY, + handle, uriStatStorage); + SpanRecorder spanRecorder = recorderFactory.newDisableSpanRecorder(traceRoot); SpanEventRecorder spanEventRecorder = recorderFactory.newDisableSpanEventRecorder(traceRoot, asyncState); return new DisableAsyncChildTrace(traceRoot, spanRecorder, spanEventRecorder); @@ -238,10 +242,6 @@ private Trace newAsyncLocalTrace(long nextDisabledId) { return new AsyncDisableTrace(traceRoot, spanRecorder, spanEventRecorder, asyncState); } - private ActiveTraceHandle registerActiveTrace(TraceRoot traceRoot) { - return activeTraceRepository.register(traceRoot); - } - private ActiveTraceHandle registerActiveTrace(LocalTraceRoot localTraceRoot) { return activeTraceRepository.register(localTraceRoot); } diff --git a/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/active/ActiveTraceRepository.java b/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/active/ActiveTraceRepository.java index e2248fd779f5..1238f9cac00f 100644 --- a/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/active/ActiveTraceRepository.java +++ b/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/active/ActiveTraceRepository.java @@ -17,7 +17,6 @@ package com.navercorp.pinpoint.profiler.context.active; import com.navercorp.pinpoint.profiler.context.id.LocalTraceRoot; -import com.navercorp.pinpoint.profiler.context.id.TraceRoot; import java.util.List; @@ -32,8 +31,6 @@ public interface ActiveTraceRepository { List getThreadIdList(); - ActiveTraceHandle register(TraceRoot traceRoot); - ActiveTraceHandle register(LocalTraceRoot traceRoot); } diff --git a/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/active/DefaultActiveTraceRepository.java b/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/active/DefaultActiveTraceRepository.java index 9d75bef9275f..492eec6f0714 100644 --- a/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/active/DefaultActiveTraceRepository.java +++ b/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/active/DefaultActiveTraceRepository.java @@ -22,7 +22,6 @@ import com.navercorp.pinpoint.common.trace.HistogramSchema; import com.navercorp.pinpoint.common.trace.HistogramSlot; import com.navercorp.pinpoint.profiler.context.id.LocalTraceRoot; -import com.navercorp.pinpoint.profiler.context.id.TraceRoot; import com.navercorp.pinpoint.profiler.monitor.metric.response.ResponseTimeCollector; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -83,32 +82,20 @@ private void remove(ActiveTraceHandle key, long purgeTime) { } } - @Override - public ActiveTraceHandle register(TraceRoot traceRoot) { - final ActiveTrace activeTrace = new DefaultActiveTrace(traceRoot); - return register0(activeTrace); - } @Override public ActiveTraceHandle register(LocalTraceRoot localTraceRoot) { - final ActiveTrace activeTrace = new DefaultActiveTrace(localTraceRoot); - return register0(activeTrace); - } - - private ActiveTraceHandle register0(ActiveTrace activeTrace) { if (isDebug) { - logger.debug("register ActiveTrace key:{}", activeTrace); + logger.debug("register ActiveTrace key:{}", localTraceRoot); } - final long id = activeTrace.getId(); + final long id = localTraceRoot.getLocalTransactionId(); + final ActiveTraceHandle handle = new DefaultActiveTraceHandle(id); - final ActiveTrace old = this.activeTraceInfoMap.put(handle, activeTrace); - if (old != null) { - if (logger.isWarnEnabled()) { - logger.warn("old activeTrace exist:{}", old); - } - } + + this.activeTraceInfoMap.computeIfAbsent(handle, activeTraceHandle -> new DefaultActiveTrace(localTraceRoot)); + return handle; } diff --git a/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/active/EmptyActiveTraceRepository.java b/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/active/EmptyActiveTraceRepository.java index 5de3491afb77..36f5d126be82 100644 --- a/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/active/EmptyActiveTraceRepository.java +++ b/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/active/EmptyActiveTraceRepository.java @@ -18,7 +18,6 @@ import com.navercorp.pinpoint.common.trace.BaseHistogramSchema; import com.navercorp.pinpoint.profiler.context.id.LocalTraceRoot; -import com.navercorp.pinpoint.profiler.context.id.TraceRoot; import com.navercorp.pinpoint.profiler.monitor.metric.response.ResponseTimeCollector; import java.util.Collections; @@ -53,11 +52,6 @@ public List snapshot() { return Collections.emptyList(); } - @Override - public ActiveTraceHandle register(TraceRoot traceRoot) { - Objects.requireNonNull(traceRoot, "traceRoot"); - return new EmptyActiveTraceHandle(traceRoot.getTraceStartTime()); - } @Override public ActiveTraceHandle register(LocalTraceRoot traceRoot) {