You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2021/12/13 18:12:20 UTC

[GitHub] [camel] javaduke opened a new pull request #6535: BacklogDebugger API enhancements

javaduke opened a new pull request #6535:
URL: https://github.com/apache/camel/pull/6535


   This PR adds the following functionality to the BacklogDebugger MBean:
   - Exchange Properties are now exposed via JMX as XML;
   - An ability to evaluate an expression and return result.
   
    


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel] javaduke commented on a change in pull request #6535: BacklogDebugger API enhancements

Posted by GitBox <gi...@apache.org>.
javaduke commented on a change in pull request #6535:
URL: https://github.com/apache/camel/pull/6535#discussion_r768720481



##########
File path: core/camel-management/pom.xml
##########
@@ -85,6 +85,13 @@
             <artifactId>log4j-slf4j-impl</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>

Review comment:
       I need this dependency to test if the result is Serializable. I understand your point about debugger being used by many different tools, perhaps we need another method which evaluates an expression and converts it to String?
   




-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel] davsclaus commented on pull request #6535: [CAMEL-17341, CAMEL-17342] BacklogDebugger API enhancements

Posted by GitBox <gi...@apache.org>.
davsclaus commented on pull request #6535:
URL: https://github.com/apache/camel/pull/6535#issuecomment-994925541


   > ented.
   
   Yeah I noticed that too, but I think we can improve both at the same time: tracer and debugger by allows to capture exchange properties too. So you can say (true|false) to include exchange properties. If doing so, then yes. the root tag is different to be able to embed the exchange.
   
   


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel] javaduke edited a comment on pull request #6535: [CAMEL-17341, CAMEL-17342] BacklogDebugger API enhancements

Posted by GitBox <gi...@apache.org>.
javaduke edited a comment on pull request #6535:
URL: https://github.com/apache/camel/pull/6535#issuecomment-994929655


   I agree the tracer needs to be improved, but for now I just do some strings manipulation. I'll review the tracer later and see if I can pass the exchange and thus simplify the code.
   And yes, my goal was not to break the backwards compatibility as much as possible, because the debugger should be able to work with the older versions of Camel (with limitations)


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel] javaduke commented on pull request #6535: [CAMEL-17341, CAMEL-17342] BacklogDebugger API enhancements

Posted by GitBox <gi...@apache.org>.
javaduke commented on pull request #6535:
URL: https://github.com/apache/camel/pull/6535#issuecomment-994929655


   I agree the tracer needs to be improved, but for now I just do some strings manipulation. I'll review the tracer later and see if I can pass the exchange and thus simplify the code.


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel] davsclaus commented on a change in pull request #6535: BacklogDebugger API enhancements

Posted by GitBox <gi...@apache.org>.
davsclaus commented on a change in pull request #6535:
URL: https://github.com/apache/camel/pull/6535#discussion_r768939716



##########
File path: core/camel-management/pom.xml
##########
@@ -85,6 +85,13 @@
             <artifactId>log4j-slf4j-impl</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>

Review comment:
       We cannot use 3rd party dependency in the core, you need to remove it.
   
   I sugget you make this into more PRs where you first work on this that can be accepted, and then later the seriization stuff.
   
   




-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel] javaduke removed a comment on pull request #6535: [CAMEL-17341, CAMEL-17342] BacklogDebugger API enhancements

Posted by GitBox <gi...@apache.org>.
javaduke removed a comment on pull request #6535:
URL: https://github.com/apache/camel/pull/6535#issuecomment-994822171


   > We need a JIRA ticket about this, and the commit messages should link to the JIRA ticket, eg [CAMEL-12345](https://issues.apache.org/jira/browse/CAMEL-12345): bla bla
   
   I don't think I have access to the Jira, can you please create one for me?


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel] javaduke commented on a change in pull request #6535: BacklogDebugger API enhancements

Posted by GitBox <gi...@apache.org>.
javaduke commented on a change in pull request #6535:
URL: https://github.com/apache/camel/pull/6535#discussion_r768964974



##########
File path: core/camel-management/src/main/java/org/apache/camel/management/mbean/ManagedBacklogDebugger.java
##########
@@ -274,4 +283,77 @@ public long getFallbackTimeout() {
     public void setFallbackTimeout(long fallbackTimeout) {
         backlogDebugger.setFallbackTimeout(fallbackTimeout);
     }
+
+    @Override
+    public String dumpExchangePropertiesAsXml(String id) {
+        StringBuilder sb = new StringBuilder();
+        sb.append("<properties>");
+        Exchange suspendedExchange = backlogDebugger.getSuspendedExchange(id);
+        if (suspendedExchange != null) {
+            Map<String, Object> properties = suspendedExchange.getAllProperties();
+            properties.forEach((propertyName, propertyValue) -> {
+                String type = ObjectHelper.classCanonicalName(propertyValue);
+                sb.append("<property name=\"").append(propertyName).append("\"");
+                if (type != null) {
+                    sb.append(" type=\"").append(type).append("\"");
+                }
+                sb.append(">");
+                // dump property value as XML, use Camel type converter to convert
+                // to String
+                if (propertyValue != null) {
+                    try {
+                        String xml = suspendedExchange.getContext().getTypeConverter().tryConvertTo(String.class,
+                                suspendedExchange, propertyValue);
+                        if (xml != null) {
+                            // must always xml encode
+                            sb.append(StringHelper.xmlEncode(xml));
+                        }
+                    } catch (Throwable e) {
+                        // ignore as the body is for logging purpose
+                    }
+                }
+                sb.append("</property>\n");
+            });
+        }
+        sb.append("</properties>");
+        return sb.toString();
+    }
+
+    @Override
+    public String evaluateExpressionAtBreakpoint(String id, String language, String expression) {
+        return evaluateExpressionAtBreakpoint(id, language, expression, "java.lang.String").toString();
+    }
+
+    @Override
+    public Object evaluateExpressionAtBreakpoint(String id, String language, String expression, String resultType) {
+        Exchange suspendedExchange = null;
+        try {
+            Language lan = camelContext.resolveLanguage(language);
+            suspendedExchange = backlogDebugger.getSuspendedExchange(id);
+            if (suspendedExchange != null) {
+                Object result = null;
+                Class resultClass = Class.forName(resultType);

Review comment:
       thanks, fixed!




-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel] javaduke commented on a change in pull request #6535: BacklogDebugger API enhancements

Posted by GitBox <gi...@apache.org>.
javaduke commented on a change in pull request #6535:
URL: https://github.com/apache/camel/pull/6535#discussion_r768718514



##########
File path: core/camel-management-api/pom.xml
##########
@@ -43,7 +43,10 @@
             <groupId>org.slf4j</groupId>
             <artifactId>slf4j-api</artifactId>
         </dependency>
-
+        <dependency>

Review comment:
       Sorry, looks like it was added automatically by IntelliJ. I removed it.




-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel] davsclaus merged pull request #6535: [CAMEL-17341, CAMEL-17342] BacklogDebugger API enhancements

Posted by GitBox <gi...@apache.org>.
davsclaus merged pull request #6535:
URL: https://github.com/apache/camel/pull/6535


   


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel] davsclaus commented on a change in pull request #6535: BacklogDebugger API enhancements

Posted by GitBox <gi...@apache.org>.
davsclaus commented on a change in pull request #6535:
URL: https://github.com/apache/camel/pull/6535#discussion_r768335264



##########
File path: core/camel-management/pom.xml
##########
@@ -85,6 +85,13 @@
             <artifactId>log4j-slf4j-impl</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>

Review comment:
       We do NOT want this dependency

##########
File path: core/camel-management-api/pom.xml
##########
@@ -43,7 +43,10 @@
             <groupId>org.slf4j</groupId>
             <artifactId>slf4j-api</artifactId>
         </dependency>
-
+        <dependency>

Review comment:
       Remove this added dependency




-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel] javaduke commented on pull request #6535: BacklogDebugger API enhancements

Posted by GitBox <gi...@apache.org>.
javaduke commented on pull request #6535:
URL: https://github.com/apache/camel/pull/6535#issuecomment-994822171


   > We need a JIRA ticket about this, and the commit messages should link to the JIRA ticket, eg [CAMEL-12345](https://issues.apache.org/jira/browse/CAMEL-12345): bla bla
   
   I don't think I have access to the Jira, can you please create one for me?


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel] davsclaus commented on pull request #6535: BacklogDebugger API enhancements

Posted by GitBox <gi...@apache.org>.
davsclaus commented on pull request #6535:
URL: https://github.com/apache/camel/pull/6535#issuecomment-994713172


   We should not introduce a new operation for dumping the exchange properties - I would rather add a boolean option to the existing dump, where you can specify if the dump should be with exchange properties, eg
   
   ```
       @ManagedOperation(description = "Dumps the messages in xml format from the suspended breakpoint at the given node")
       String dumpTracedMessagesAsXml(String nodeId, boolean includeExchangeProperties);
   ```
   
   


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel] davsclaus commented on pull request #6535: BacklogDebugger API enhancements

Posted by GitBox <gi...@apache.org>.
davsclaus commented on pull request #6535:
URL: https://github.com/apache/camel/pull/6535#issuecomment-994814461


   We need a JIRA ticket about this, and the commit messages should link to the JIRA ticket, eg CAMEL-12345: bla bla


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel] davsclaus commented on pull request #6535: [CAMEL-17341, CAMEL-17342] BacklogDebugger API enhancements

Posted by GitBox <gi...@apache.org>.
davsclaus commented on pull request #6535:
URL: https://github.com/apache/camel/pull/6535#issuecomment-994929045


   Oh we can also do as you did, just embed the `<exchangeProperties>` in the existing xml, as this only happens when you say (true) so its a new API and the old is as-is.
   
   


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel] javaduke commented on pull request #6535: BacklogDebugger API enhancements

Posted by GitBox <gi...@apache.org>.
javaduke commented on pull request #6535:
URL: https://github.com/apache/camel/pull/6535#issuecomment-994821620


   > We should not introduce a new operation for dumping the exchange properties - I would rather add a boolean option to the existing dump, where you can specify if the dump should be with exchange properties
   
   Yes, I thought about this, but two things stopped me - one is that exchange properties are technically not part of the message, they are part of the Exchange, and the second problem is that the XML generation logic is implemented in the DefaultBacklogTracerEventMessage and it doesn't have the Exchange instance. Please let me know if you have any good idea of how this should be implemented.
   
   
   


-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel] javaduke commented on a change in pull request #6535: BacklogDebugger API enhancements

Posted by GitBox <gi...@apache.org>.
javaduke commented on a change in pull request #6535:
URL: https://github.com/apache/camel/pull/6535#discussion_r768969485



##########
File path: core/camel-management/pom.xml
##########
@@ -85,6 +85,13 @@
             <artifactId>log4j-slf4j-impl</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>

Review comment:
       No problem, I actually figured out how to check if the object is serializable without using third party library, it's gone now.




-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel] davsclaus commented on a change in pull request #6535: BacklogDebugger API enhancements

Posted by GitBox <gi...@apache.org>.
davsclaus commented on a change in pull request #6535:
URL: https://github.com/apache/camel/pull/6535#discussion_r768941236



##########
File path: core/camel-management/src/main/java/org/apache/camel/management/mbean/ManagedBacklogDebugger.java
##########
@@ -274,4 +283,77 @@ public long getFallbackTimeout() {
     public void setFallbackTimeout(long fallbackTimeout) {
         backlogDebugger.setFallbackTimeout(fallbackTimeout);
     }
+
+    @Override
+    public String dumpExchangePropertiesAsXml(String id) {
+        StringBuilder sb = new StringBuilder();
+        sb.append("<properties>");
+        Exchange suspendedExchange = backlogDebugger.getSuspendedExchange(id);
+        if (suspendedExchange != null) {
+            Map<String, Object> properties = suspendedExchange.getAllProperties();
+            properties.forEach((propertyName, propertyValue) -> {
+                String type = ObjectHelper.classCanonicalName(propertyValue);
+                sb.append("<property name=\"").append(propertyName).append("\"");
+                if (type != null) {
+                    sb.append(" type=\"").append(type).append("\"");
+                }
+                sb.append(">");
+                // dump property value as XML, use Camel type converter to convert
+                // to String
+                if (propertyValue != null) {
+                    try {
+                        String xml = suspendedExchange.getContext().getTypeConverter().tryConvertTo(String.class,
+                                suspendedExchange, propertyValue);
+                        if (xml != null) {
+                            // must always xml encode
+                            sb.append(StringHelper.xmlEncode(xml));
+                        }
+                    } catch (Throwable e) {
+                        // ignore as the body is for logging purpose
+                    }
+                }
+                sb.append("</property>\n");
+            });
+        }
+        sb.append("</properties>");
+        return sb.toString();
+    }
+
+    @Override
+    public String evaluateExpressionAtBreakpoint(String id, String language, String expression) {
+        return evaluateExpressionAtBreakpoint(id, language, expression, "java.lang.String").toString();
+    }
+
+    @Override
+    public Object evaluateExpressionAtBreakpoint(String id, String language, String expression, String resultType) {
+        Exchange suspendedExchange = null;
+        try {
+            Language lan = camelContext.resolveLanguage(language);
+            suspendedExchange = backlogDebugger.getSuspendedExchange(id);
+            if (suspendedExchange != null) {
+                Object result = null;
+                Class resultClass = Class.forName(resultType);

Review comment:
       You cannot use this class loading, Camel has a ClassResolver API via CamelContext that must be used.
   https://github.com/apache/camel/blob/main/core/camel-api/src/main/java/org/apache/camel/spi/ClassResolver.java




-- 
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: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org