vp340 commented on code in PR #3343:
URL: https://github.com/apache/cxf/pull/3343#discussion_r3779940721
##########
rt/features/logging/src/main/java/org/apache/cxf/ext/logging/AbstractLoggingInterceptor.java:
##########
@@ -25,20 +25,21 @@
import java.util.regex.Pattern;
import org.apache.cxf.common.util.PropertyUtils;
-import org.apache.cxf.ext.logging.event.DefaultLogEventMapper;
-import org.apache.cxf.ext.logging.event.LogEvent;
-import org.apache.cxf.ext.logging.event.LogEventSender;
-import org.apache.cxf.ext.logging.event.PrettyLoggingFilter;
+import org.apache.cxf.ext.logging.event.*;
import org.apache.cxf.interceptor.Fault;
import org.apache.cxf.message.Exchange;
import org.apache.cxf.message.Message;
import org.apache.cxf.phase.AbstractPhaseInterceptor;
+import static
org.apache.cxf.ext.logging.event.DefaultLogEventMapper.normalizeFlow;
+
public abstract class AbstractLoggingInterceptor extends
AbstractPhaseInterceptor<Message> {
public static final int DEFAULT_LIMIT = 48 * 1024;
public static final int DEFAULT_THRESHOLD = -1;
public static final String CONTENT_SUPPRESSED = "--- Content suppressed
---";
protected static final String LIVE_LOGGING_PROP =
"org.apache.cxf.logging.enable";
+ protected static final String IDEMPOTENT_LOGGING_PROP =
"org.apache.cxf.idempotent.logging."; // the EventType (flow) and ExchangeId
will be concatenated
Review Comment:
Hi thanks.
I was updating the Jira ticket right now. (sorry I live in Italy and here
is almost 2 a.m right now and tomorrow I have work) .
Yeah the #3372 could be an easier and more understandable solution.
If I understand well the isRequestor() method is the same use to retrieve
the EventType for logging ..so isRequestor ? EventType.RESP_IN :
EventType.REQ_IN
It will set 'LIVE_LOGGING_PROP + true' if it's a client and a
'LIVE_LOGGING_PROP + false' if is a server.
The only doubt situation could be 2 backend call in a row...
So the flow would be:
REQ_IN (set LIVE_LOGGING_PROP + false but not propagated)
REQ_OUT (set LIVE_LOGGING_PROP + true but not propagated)
RESP_IN (set LIVE_LOGGING_PROP + true AND propagated!)
REQ_OUT (find LIVE_LOGGING_PROP + true ...so ghost REQ_OUT logging)
RESP_IN
RESP_OUT
(This is a situation that I thought right know... it should be tested)...
Another possible problem is that if we modify the property name itself,
someone who had already set that property on the Bus to completely disable the
logging would no longer be able to do so. This could therefore break backward
compatibility. (I read something related to this in
https://issues.apache.org/jira/browse/CXF-7000 )
If U are interested in reviewing another possible solution, going deeply in
debug in the sample project I believe I found the very place where the
LIVE_LOGGING_PROP is set and propagated.
I open a new https://github.com/apache/cxf/pull/3373
In the class org.apache.cxf.endpoint.ClientImpl ... processResult method
Message inMsg = exchange.getInMessage();
if (inMsg != null) {
if (null != resContext)
{
resContext.putAll(inMsg);
// remove the recursive reference if present
resContext.remove(Message.INVOCATION_CONTEXT);
// remove the logging disable property //ADDED
resContext.remove(Message.LIVE_LOGGING_PROP); //ADDED
setResponseContext(resContext);
}
Here someone already remove a Message.INVOCATION_CONTEXT property from the
Response Context ... so I think It could be a good place to prevent that the
logging properties is propagated in the ResponseContext at all!
To do so I had to transfer the String Constant from the
AbstractLoggingInterceptor to the Message.
This way we won't change the behaviour at all.
(I'm very confident that it works... at least in debug I removed it
manually and the RESP_OUT log reapperead... so the property was not propagated).
Let me know what U think about it.
And if you have any suggestions, especially regarding design patterns or the
overall architecture, please feel free to share them. If you think there’s a
better way to approach it, I’d be more than happy to hear it...I have a lot to
learn from you!
Have a great work!
Valentino Porta
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
