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/07/23 07:04:14 UTC
[GitHub] [skywalking] qsliu2017 opened a new pull request #7363: Add an agent plugin to support `spring-rabbit-2.3.x`
qsliu2017 opened a new pull request #7363:
URL: https://github.com/apache/skywalking/pull/7363
<!--
⚠️ Please make sure to read this template first, pull requests that don't accord with this template
maybe closed without notice.
Texts surrounded by `<` and `>` are meant to be replaced by you, e.g. <framework name>, <issue number>.
Put an `x` in the `[ ]` to mark the item as CHECKED. `[x]`
-->
<!-- ==== 🐛 Remove this line WHEN AND ONLY WHEN you're fixing a bug, follow the checklist 👇 ====
### Fix <bug description or the bug issue number or bug issue link>
- [ ] Add a unit test to verify that the fix works.
- [ ] Explain briefly why the bug exists and how to fix it.
==== 🐛 Remove this line WHEN AND ONLY WHEN you're fixing a bug, follow the checklist 👆 ==== -->
### Add an agent plugin to support `spring-rabbit-2.3.x`
- [ ] Add a test case for the new plugin, refer to [the doc](https://github.com/apache/skywalking/blob/master/docs/en/guides/Plugin-test.md)
- [ ] Add a component id in [the component-libraries.yml](https://github.com/apache/skywalking/blob/master/oap-server/server-bootstrap/src/main/resources/component-libraries.yml)
- [ ] Add a logo in [the UI repo](https://github.com/apache/skywalking-rocketbot-ui/tree/master/src/views/components/topology/assets)
<!-- ==== 📈 Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist 👇 ====
### Improve the performance of <class or module or ...>
- [ ] Add a benchmark for the improvement, refer to [the existing ones](https://github.com/apache/skywalking/blob/master/apm-commons/apm-datacarrier/src/test/java/org/apache/skywalking/apm/commons/datacarrier/LinkedArrayBenchmark.java)
- [ ] The benchmark result.
```text
<Paste the benchmark results here>
```
- [ ] Links/URLs to the theory proof or discussion articles/blogs. <links/URLs here>
==== 📈 Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist 👆 ==== -->
<!-- ==== 🆕 Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist 👇 ====
### <Feature description>
- [ ] If this is non-trivial feature, paste the links/URLs to the design doc.
- [ ] Update the documentation to include this new feature.
- [ ] Tests(including UT, IT, E2E) are added to verify the new feature.
- [ ] If it's UI related, attach the screenshots below.
==== 🆕 Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist 👆 ==== -->
- [x] If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #2719 .
- [ ] Update the [`CHANGES` log](https://github.com/apache/skywalking/blob/master/CHANGES.md).
As mentioned in #2719 , this pr intends to add a plugin to fix broken traces in rabbitmq. When using with the original `rabbitmq-plugin` it causes redundant traces. So I put it in optional plugins.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] qsliu2017 commented on pull request #7363: Add an agent plugin to support `spring-rabbit-2.3.x`
Posted by GitBox <gi...@apache.org>.
qsliu2017 commented on pull request #7363:
URL: https://github.com/apache/skywalking/pull/7363#issuecomment-885634819
> > The existing plugin enhances rabbitmq java client interface so it works for all rabbitmq client implementations. But this plugin only enhances the implementation of rabbitmq client in spring. When a spring application calling rabbitmq, both plugins can be active. But for clients other than spring, this one will not be active. That's why we can't replace the existing one.
> > I hope I've made myself clear.😅
>
> This explained why you need one, but doesn't explain why you need a new one. You said, the existing plugin works for all case, including through Spring, then why need this?
>
> The key point I keep asking is, why this has to be optional? Why can't optimize it to work in default and only necessary when existing plugin doesn't work.
As said in #2719, the existing rabbitmq plugin divides a total trace into two parts because of the internals of rabbitmq. I need a new one which can make a trace complete.
This plugin can fix it but **only** in spring. I haven't found a way to disable the existing one automatically when this plugin is active.😅 So I put it in optional plugins, to not make duplicate spans but give users an option to replace the existing plugin manually.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #7363: Add an agent plugin to support `spring-rabbit-2.3.x`
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #7363:
URL: https://github.com/apache/skywalking/pull/7363#issuecomment-886005696
I don't say we don't take care of Spring ecosystem. I mean you still don't explain the real reason why trace breaks.
Plugin dev doc explains the snapshot, and capture/continue operation.
When you add a thing, you should make sure it is compatible with other cases, rather than just give users to choose. SkyWalking never asked you to choose plugin, do we.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #7363: Add an agent plugin to support `spring-rabbit-2.3.x`
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #7363:
URL: https://github.com/apache/skywalking/pull/7363#issuecomment-885662803
You haven't gotten my point. If spans could be duplicate, then both plugins work, then the only possibility a trace breaks, because the context doesn't propagate, you just need a snapshot to propagate a context. Right now, you are creating another span, which makes you facing the duplicate span issue. We have plugins for Spring scope and out of Spring scope, they should not influence each other.
The plugin system is complex, we should never make users to determine how to choose in or out Spring use case, they will be confused about the difference.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] sonatype-lift[bot] commented on a change in pull request #7363: Add an agent plugin to support `spring-rabbit-2.3.x`
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on a change in pull request #7363:
URL: https://github.com/apache/skywalking/pull/7363#discussion_r675389509
##########
File path: apm-sniffer/optional-plugins/optional-spring-plugins/spring-rabbit-2.3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/rabbitmq/RabbitMQConsumerInterceptor.java
##########
@@ -0,0 +1,89 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.apm.plugin.spring.rabbitmq;
+
+import com.rabbitmq.client.Channel;
+import com.rabbitmq.client.Connection;
+import org.apache.skywalking.apm.agent.core.context.CarrierItem;
+import org.apache.skywalking.apm.agent.core.context.ContextCarrier;
+import org.apache.skywalking.apm.agent.core.context.ContextManager;
+import org.apache.skywalking.apm.agent.core.context.tag.Tags;
+import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan;
+import org.apache.skywalking.apm.agent.core.context.trace.SpanLayer;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult;
+import org.apache.skywalking.apm.network.trace.component.ComponentsDefine;
+import org.springframework.amqp.core.Message;
+import org.springframework.amqp.core.MessageProperties;
+
+import java.lang.reflect.Method;
+import java.util.List;
+
+public class RabbitMQConsumerInterceptor implements InstanceMethodsAroundInterceptor {
+ public static final String OPERATE_NAME_PREFIX = "RabbitMQ/";
+ public static final String CONSUMER_OPERATE_NAME_SUFFIX = "/Consumer";
+
+ @Override
+ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
+ MethodInterceptResult result) throws Throwable {
+ ContextCarrier contextCarrier = new ContextCarrier();
+
+ Connection connection = ((Channel) allArguments[0]).getConnection();
+ String url = connection.getAddress().toString().replace("/", "") + ":" + connection.getPort();
+ Object data = allArguments[1];
+ Message message = null;
+ if (data instanceof List) {
+ message = ((List<Message>) data).get(0);
+ } else if (data instanceof Message) {
+ message = (Message) data;
+ }
+ MessageProperties msgProperties = message.getMessageProperties();
Review comment:
*NULL_DEREFERENCE:* object `message` last assigned on line 51 could be null and is dereferenced at line 57.
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
##########
File path: apm-sniffer/optional-plugins/optional-spring-plugins/spring-rabbit-2.3.x-plugin/pom.xml
##########
@@ -0,0 +1,44 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one or more
+ ~ contributor license agreements. See the NOTICE file distributed with
+ ~ this work for additional information regarding copyright ownership.
+ ~ The ASF licenses this file to You under the Apache License, Version 2.0
+ ~ (the "License"); you may not use this file except in compliance with
+ ~ the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing, software
+ ~ distributed under the License is distributed on an "AS IS" BASIS,
+ ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ ~ See the License for the specific language governing permissions and
+ ~ limitations under the License.
+ ~
+ -->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+
+ <parent>
+ <groupId>org.apache.skywalking</groupId>
+ <artifactId>optional-spring-plugins</artifactId>
+ <version>8.7.0-SNAPSHOT</version>
+ </parent>
+
+ <artifactId>apm-spring-rabbit-2.3.x-plugin</artifactId>
+
+ <name>spring-rabbit-2.3.x-plugin</name>
+
+ <properties>
+ <spring-rabbit.version>2.3.9</spring-rabbit.version>
+ </properties>
+
+ <dependencies>
+ <dependency>
Review comment:
*Critical OSS Vulnerability:*
### pkg:maven/net.minidev/json-smart@2.3
1 Critical, 0 Severe, 0 Moderate and 0 Unknown vulnerabilities have been found in a transitive dependency of pkg:maven/org.springframework.amqp/spring-rabbit@2.3.9
<!-- Lift_Details -->
<details>
<summary><b>CRITICAL Vulnerabilities (1)</b></summary>
<ul>
***
> #### [CVE-2021-27568] An issue was discovered in netplex json-smart-v1 through 2015-10-23 and json-sma...
> An issue was discovered in netplex json-smart-v1 through 2015-10-23 and json-smart-v2 through 2.4. An exception is thrown from a function, but it is not caught, as demonstrated by NumberFormatException. When it is not caught, it may cause programs using the library to crash or expose sensitive information.
>
> **CVSS Score:** 9.1
>
> **CVSS Vector:** CVSS:3.0/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:N/A:H
***
</ul>
</details>
(at-me [in a reply](https://help.sonatype.com/lift) with `help` or `ignore`)
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #7363: Add an agent plugin to support `spring-rabbit-2.3.x`
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #7363:
URL: https://github.com/apache/skywalking/pull/7363#issuecomment-887641311
Thanks for checking and clarification. Are you going to rework on this PR?
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] qsliu2017 commented on pull request #7363: Add an agent plugin to support `spring-rabbit-2.3.x`
Posted by GitBox <gi...@apache.org>.
qsliu2017 commented on pull request #7363:
URL: https://github.com/apache/skywalking/pull/7363#issuecomment-886004145
> You haven't gotten my point. If spans could be duplicate, then both plugins work, then the only possibility a trace breaks, because the context doesn't propagate(across threads mostly), you just need a snapshot to propagate a context. Right now, you are creating another span, which makes you facing the duplicate span issue. We have plugins for Spring scope and out of Spring scope, they should not influence each other.
>
> The plugin system is complex, we should never make users to determine how to choose in or out Spring use case, they will be confused about the difference.
well, it looks like I misunderstand *optional* plugins.😅 does that mean we need to focus on rabbitmq itself, rather than spring rabbit? And what do you mean by **a snapshot to propagate a context**?
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #7363: Add an agent plugin to support `spring-rabbit-2.3.x`
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #7363:
URL: https://github.com/apache/skywalking/pull/7363#issuecomment-886212776
If there is only a case of across thread, please capture a snapshot in `ConsumerDispatcher#handleDelivery` method, then propagate the context into `executeUnlessShuttingDown` method with a wrapper runnable.
This is the recommended way to process an across thread case.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] qsliu2017 commented on pull request #7363: Add an agent plugin to support `spring-rabbit-2.3.x`
Posted by GitBox <gi...@apache.org>.
qsliu2017 commented on pull request #7363:
URL: https://github.com/apache/skywalking/pull/7363#issuecomment-886211975
In rabbitmq client, dispatcher handles a message delivery and calls the consumer in a new thread.
```java
package com.rabbitmq.client.impl;
final class ConsumerDispatcher {
public void handleDelivery(final Consumer delegate,
final String consumerTag,
final Envelope envelope,
final AMQP.BasicProperties properties,
final byte[] body) throws IOException {
executeUnlessShuttingDown(
new Runnable() {
@Override
public void run() {
try {
delegate.handleDelivery(consumerTag,
envelope,
properties,
body);
} catch (Throwable ex) {
connection.getExceptionHandler().handleConsumerException(
channel,
ex,
delegate,
consumerTag,
"handleDelivery");
}
}
});
}
}
```
```java
package com.rabbitmq.client;
public interface Consumer {
void handleDelivery(String consumerTag,
Envelope envelope,
AMQP.BasicProperties properties,
byte[] body)
throws IOException;
}
```
The existing plugin enhances `Consumer#handleDelivery`, so the consumer span is cross-thread, which causes broken traces. @wu-sheng Does this make sense?
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #7363: Add an agent plugin to support `spring-rabbit-2.3.x`
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #7363:
URL: https://github.com/apache/skywalking/pull/7363#issuecomment-885597780
This doesn't make sense. You said there is duplicate spans, then these is at least one case both plugins 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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #7363: Add an agent plugin to support `spring-rabbit-2.3.x`
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #7363:
URL: https://github.com/apache/skywalking/pull/7363#issuecomment-885459642
Your commit logs seem not clean. Could you make sure it includes your changes only? You should learn how to sync witg upstream.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] qsliu2017 commented on pull request #7363: Add an agent plugin to support `spring-rabbit-2.3.x`
Posted by GitBox <gi...@apache.org>.
qsliu2017 commented on pull request #7363:
URL: https://github.com/apache/skywalking/pull/7363#issuecomment-885506056
> 2 things first.
>
> 1. changes.md in the root should be updated.
> 2. As you said duplicate case in pure rabbitmq case, it is possible to use this replace the existing plugin rather than adding a new one?
1. I have updated it
2. The existing plugin works for `rabbitmq java client`, but this one only works for its implementation in spring.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng closed pull request #7363: Add an agent plugin to support `spring-rabbit-2.3.x`
Posted by GitBox <gi...@apache.org>.
wu-sheng closed pull request #7363:
URL: https://github.com/apache/skywalking/pull/7363
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #7363: Add an agent plugin to support `spring-rabbit-2.3.x`
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #7363:
URL: https://github.com/apache/skywalking/pull/7363#issuecomment-885618138
> The existing plugin enhances rabbitmq java client interface so it works for all rabbitmq client implementations. But this plugin only enhances the implementation of rabbitmq client in spring. When a spring application calling rabbitmq, both plugins can be active. But for clients other than spring, this one will not be active. That's why we can't replace the existing one.
>
>
>
> I hope I've made myself clear.:sweat_smile:
This explained why you need one, but doesn't explain why you need a new one. You said, the existing plugin works for all case, including through Spring, then why need this?
The key point I keep asking is, why this has to be optional? Why can't optimize it to work in default and only necessary when existing plugin doesn't 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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #7363: Add an agent plugin to support `spring-rabbit-2.3.x`
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #7363:
URL: https://github.com/apache/skywalking/pull/7363#issuecomment-891632533
@qsliu2017 Do you still work on 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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #7363: Add an agent plugin to support `spring-rabbit-2.3.x`
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #7363:
URL: https://github.com/apache/skywalking/pull/7363#issuecomment-885459201
2 things first.
1. changes.md in the root should be updated.
2. As you said duplicate case in pure rabbitmq case, it is possible to use this replace the existing plugin rather than adding a new one?
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] qsliu2017 commented on pull request #7363: Add an agent plugin to support `spring-rabbit-2.3.x`
Posted by GitBox <gi...@apache.org>.
qsliu2017 commented on pull request #7363:
URL: https://github.com/apache/skywalking/pull/7363#issuecomment-885615117
The existing plugin enhances rabbitmq java client interface so it works for all rabbitmq client implementations. But this plugin only enhances the implementation of rabbitmq client in spring. When a spring application calling rabbitmq, both plugins can be active. But for clients other than spring, this one will not be active. That's why we can't replace the existing one.
I hope I've made myself clear.:sweat_smile:
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng edited a comment on pull request #7363: Add an agent plugin to support `spring-rabbit-2.3.x`
Posted by GitBox <gi...@apache.org>.
wu-sheng edited a comment on pull request #7363:
URL: https://github.com/apache/skywalking/pull/7363#issuecomment-885662803
You haven't gotten my point. If spans could be duplicate, then both plugins work, then the only possibility a trace breaks, because the context doesn't propagate(across threads mostly), you just need a snapshot to propagate a context. Right now, you are creating another span, which makes you facing the duplicate span issue. We have plugins for Spring scope and out of Spring scope, they should not influence each other.
The plugin system is complex, we should never make users to determine how to choose in or out Spring use case, they will be confused about the difference.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] qsliu2017 commented on pull request #7363: Add an agent plugin to support `spring-rabbit-2.3.x`
Posted by GitBox <gi...@apache.org>.
qsliu2017 commented on pull request #7363:
URL: https://github.com/apache/skywalking/pull/7363#issuecomment-888028799
> Thanks for checking and clarification. Are you going to rework on this PR?
Yes, I'll try to modify this plugin
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] qsliu2017 commented on pull request #7363: Add an agent plugin to support `spring-rabbit-2.3.x`
Posted by GitBox <gi...@apache.org>.
qsliu2017 commented on pull request #7363:
URL: https://github.com/apache/skywalking/pull/7363#issuecomment-887627284
After trying to propagate a snapshot from `ConsumerDispatcher` to `Consumer`, I've found it doesn't work.😅
Because rabbit java client **does** work and does **not** cause broken traces. In another word, the existing plugin works well with java clients except `spring-rabbit`.(sry I didn't try other java clients before... 😅)
The real reason for broken traces is that `spring-rabbit` using a container to dispatch listeners (in different threads) to consume rabbit messages. So adding a plugin to propagate snapshots from container to listeners should fix this issue and not conflict with the existing one.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org