You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2021/04/12 03:02:05 UTC

[GitHub] [skywalking] wallezhang opened a new issue #6730: pulsar plugin breaks trace on the consumer size

wallezhang opened a new issue #6730:
URL: https://github.com/apache/skywalking/issues/6730


   Please answer these questions before submitting your issue.
   
   - Why do you submit this issue?
   - [ ] Question or discussion
   - [ ✅ ] Bug
   - [ ] Requirement
   - [ ] Feature or performance improvement
   
   ___
   ### Question
   - What do you want to know?
   
   ___
   ### Bug
   - Which version of SkyWalking, OS, and JRE?
   SkyWalking:8.4.0
   OS:MacOS
   JRE:11
   - Which company or project?
   
   - What happened?
   If possible, provide a way to reproduce the error. e.g. demo application, component version.
   Pulsar plugin is only enhance `org.apache.pulsar.client.impl.ConsumerImpl` class, however pulsar will use `org.apache.pulsar.client.impl.MultiTopicsConsumerImpl` class to consume when topic's partition size greater than 0. So the trace will break down in consumer thread.
   
   The entry method is `org.apache.pulsar.client.impl.PulsarClientImpl#doSingleTopicSubscribeAsync`.
   ___
   ### Requirement or improvement
   - Please describe your requirements or improvement suggestions.
   


-- 
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.

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



[GitHub] [skywalking] wallezhang commented on issue #6730: pulsar plugin breaks trace on the consumer size

Posted by GitBox <gi...@apache.org>.
wallezhang commented on issue #6730:
URL: https://github.com/apache/skywalking/issues/6730#issuecomment-817740462


   > @wallezhang Could you please share the way that you get the message trace?
   
   ```
   ConsumerBuilder<?> builder = pulsarClient.newConsumer(getSchema(holder));
               builder.consumerName("consumer-" + name)
                       .subscriptionName(subscriptionName)
                       .subscriptionType(holder.getAnnotation().subscriptionType())
                       .messageListener((consumer, msg) -> {
                           try {
                               final Method method = holder.getHandler();
                               log.info("on message. data:{}", msg.getValue());
                               method.setAccessible(true);
                               method.invoke(holder.getBean(), msg.getValue());
   
                               consumer.acknowledge(msg);
                           } catch (Exception e) {
                               e.printStackTrace();
                               consumer.negativeAcknowledge(msg);
                               exceptionEmitter.onNext(new FailedMessage(e, consumer, msg));
                           }
                       });
               return builder.subscribe();
   ```
   
   Log In message listener can not get message trace, log output: *****TID: N/A*****


-- 
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.

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



[GitHub] [skywalking] wu-sheng commented on issue #6730: pulsar plugin breaks trace on the consumer size

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #6730:
URL: https://github.com/apache/skywalking/issues/6730#issuecomment-817667109


   @codelipenghui Could you take a look? I don't know what it 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.

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



[GitHub] [skywalking] codelipenghui commented on issue #6730: pulsar plugin breaks trace on the consumer size

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on issue #6730:
URL: https://github.com/apache/skywalking/issues/6730#issuecomment-817738528


   @wallezhang Could you please share the way that you get the message trace? 


-- 
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.

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



[GitHub] [skywalking] codelipenghui commented on issue #6730: pulsar plugin breaks trace on the consumer size

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on issue #6730:
URL: https://github.com/apache/skywalking/issues/6730#issuecomment-817692999


   @wu-sheng sure. will take a look ASAP.


-- 
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.

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



[GitHub] [skywalking] wallezhang commented on issue #6730: pulsar plugin breaks trace on the consumer size

Posted by GitBox <gi...@apache.org>.
wallezhang commented on issue #6730:
URL: https://github.com/apache/skywalking/issues/6730#issuecomment-818381991


   > @wallezhang Seems the problem is related to passing the span across threads. Have you tried to get the `ContextCarrier` from the properties of the message?
   > 
   > ```
   > ContextCarrier carrier = new ContextCarrier();
   >             CarrierItem next = carrier.items();
   >             while (next.hasNext()) {
   >                 next = next.next();
   >                 next.setHeadValue(msg.getProperty(next.getHeadKey()));
   >             }
   > ```
   > 
   > Then you can use the carrier to create a span or other operations
   > 
   > ```
   > ContextManager.createEntrySpan(<operationName>, carrier);
   > ```
   
   Why not pass the span in the message across the thread?


-- 
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.

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



[GitHub] [skywalking] wallezhang commented on issue #6730: pulsar plugin breaks trace on the consumer size

Posted by GitBox <gi...@apache.org>.
wallezhang commented on issue #6730:
URL: https://github.com/apache/skywalking/issues/6730#issuecomment-835779779


   @wu-sheng @codelipenghui Please help to review the code. Thx


-- 
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.

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



[GitHub] [skywalking] codelipenghui commented on issue #6730: pulsar plugin breaks trace on the consumer size

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on issue #6730:
URL: https://github.com/apache/skywalking/issues/6730#issuecomment-817768403


   @wallezhang Seems the problem is related to passing the span across threads. Have you tried to get the `ContextCarrier` from the properties of the message?
   
   ```
   ContextCarrier carrier = new ContextCarrier();
               CarrierItem next = carrier.items();
               while (next.hasNext()) {
                   next = next.next();
                   next.setHeadValue(msg.getProperty(next.getHeadKey()));
               }
   ```
   
   Then you can use the carrier to create a span or other operations
   
   ```
   ContextManager.createEntrySpan(<operationName>, carrier);
   ```


-- 
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.

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



[GitHub] [skywalking] wallezhang commented on issue #6730: pulsar plugin breaks trace on the consumer size

Posted by GitBox <gi...@apache.org>.
wallezhang commented on issue #6730:
URL: https://github.com/apache/skywalking/issues/6730#issuecomment-821213987


   @codelipenghui Could you review the PR please?


-- 
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.

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



[GitHub] [skywalking] wallezhang commented on issue #6730: pulsar plugin breaks trace on the consumer size

Posted by GitBox <gi...@apache.org>.
wallezhang commented on issue #6730:
URL: https://github.com/apache/skywalking/issues/6730#issuecomment-820024130


   > @wallezhang Yes, agree. we can do a MessageListener enhancement for passing the span. Are you interested in pushing a PR for it? If you don't have a chance to deal with it, I can push a fix later.
   
   Yes, I'm working on 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.

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



[GitHub] [skywalking] wallezhang commented on issue #6730: pulsar plugin breaks trace on the consumer size

Posted by GitBox <gi...@apache.org>.
wallezhang commented on issue #6730:
URL: https://github.com/apache/skywalking/issues/6730#issuecomment-819263031


   I wonder if we can do a `MessageListener` enhancement, so that the business code can pay more attention to the business logic instead of the logic related to trace


-- 
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.

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



[GitHub] [skywalking] codelipenghui edited a comment on issue #6730: pulsar plugin breaks trace on the consumer size

Posted by GitBox <gi...@apache.org>.
codelipenghui edited a comment on issue #6730:
URL: https://github.com/apache/skywalking/issues/6730#issuecomment-819589454


   @wallezhang Yes, agree. we can do a MessageListener enhancement for passing the span. Are you interested in pushing a PR for it? If you don't have a chance to deal with it, I can push a fix later.


-- 
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.

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



[GitHub] [skywalking] wu-sheng closed issue #6730: pulsar plugin breaks trace on the consumer size

Posted by GitBox <gi...@apache.org>.
wu-sheng closed issue #6730:
URL: https://github.com/apache/skywalking/issues/6730


   


-- 
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.

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



[GitHub] [skywalking] wallezhang edited a comment on issue #6730: pulsar plugin breaks trace on the consumer size

Posted by GitBox <gi...@apache.org>.
wallezhang edited a comment on issue #6730:
URL: https://github.com/apache/skywalking/issues/6730#issuecomment-818381991


   > @wallezhang Seems the problem is related to passing the span across threads. Have you tried to get the `ContextCarrier` from the properties of the message?
   > 
   > ```
   > ContextCarrier carrier = new ContextCarrier();
   >             CarrierItem next = carrier.items();
   >             while (next.hasNext()) {
   >                 next = next.next();
   >                 next.setHeadValue(msg.getProperty(next.getHeadKey()));
   >             }
   > ```
   > 
   > Then you can use the carrier to create a span or other operations
   > 
   > ```
   > ContextManager.createEntrySpan(<operationName>, carrier);
   > ```
   
   Why not pass the span in the message across the thread? E.g. enhance the `callMessageListener` method, which is already in the listener thread, so that we can directly pass the span in the message


-- 
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.

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



[GitHub] [skywalking] wallezhang commented on issue #6730: pulsar plugin breaks trace on the consumer size

Posted by GitBox <gi...@apache.org>.
wallezhang commented on issue #6730:
URL: https://github.com/apache/skywalking/issues/6730#issuecomment-834488432


   @wu-sheng @codelipenghui I found a bad case when the number of subject partitions is greater than 1, it will lead to errors on the consumer side. I need to fix it on a new branch and submit another PR tomorrow. Of course I will write more test scenarios to cover the bad case. Sorry for this.


-- 
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.

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



[GitHub] [skywalking] wallezhang commented on issue #6730: pulsar plugin breaks trace on the consumer size

Posted by GitBox <gi...@apache.org>.
wallezhang commented on issue #6730:
URL: https://github.com/apache/skywalking/issues/6730#issuecomment-817733112


   > @wallezhang I want to double confirm this issue. If I understand correctly, you are using the message listener on a MultipleTopicConsumerImpl and the message trace does not work? Or use receive() method also does not work?
   
   yes, I can't get message trace in listener


-- 
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.

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



[GitHub] [skywalking] codelipenghui commented on issue #6730: pulsar plugin breaks trace on the consumer size

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on issue #6730:
URL: https://github.com/apache/skywalking/issues/6730#issuecomment-819589454


   @wallezhang Yes, agree. we can do a MessageListener enhancement for passing the span.


-- 
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.

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



[GitHub] [skywalking] wallezhang commented on issue #6730: pulsar plugin breaks trace on the consumer size

Posted by GitBox <gi...@apache.org>.
wallezhang commented on issue #6730:
URL: https://github.com/apache/skywalking/issues/6730#issuecomment-817731560


   @codelipenghui Hi, I found a solution that just modify the enhanced classes and methods.
   Enhance class: `org.apache.pulsar.client.impl.ConsumerBase` and method: `callMessageListener`
   However, this modification can only be applied to pulsar version 2.7+. There are no `callMessageListener` method in pulsar client versions earlier than 2.7, so I think we can enhance `internalReceive` method instead.


-- 
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.

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



[GitHub] [skywalking] wu-sheng closed issue #6730: pulsar plugin breaks trace on the consumer size

Posted by GitBox <gi...@apache.org>.
wu-sheng closed issue #6730:
URL: https://github.com/apache/skywalking/issues/6730


   


-- 
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.

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



[GitHub] [skywalking] wallezhang edited a comment on issue #6730: pulsar plugin breaks trace on the consumer size

Posted by GitBox <gi...@apache.org>.
wallezhang edited a comment on issue #6730:
URL: https://github.com/apache/skywalking/issues/6730#issuecomment-819263031


   I wonder if we can do a `MessageListener` enhancement, so that the business code can pay more attention to the business logic instead of the logic related to trace. What do you think about it? @codelipenghui 


-- 
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.

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



[GitHub] [skywalking] codelipenghui commented on issue #6730: pulsar plugin breaks trace on the consumer size

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on issue #6730:
URL: https://github.com/apache/skywalking/issues/6730#issuecomment-817730656


   @wallezhang I want to double confirm this issue. If I understand correctly, you are using the message listener on a MultipleTopicConsumerImpl and the message trace does not work? Or use receive() method also does not work?


-- 
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.

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