You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "jmjoy (via GitHub)" <gi...@apache.org> on 2023/04/11 10:37:45 UTC

[GitHub] [skywalking-php] jmjoy opened a new pull request, #64: Add amqplib plugin.

jmjoy opened a new pull request, #64:
URL: https://github.com/apache/skywalking-php/pull/64

   - [x] Add amqplib plugin.
   - [x] Refactor `try_get_sw_header` method.
   - [ ] Add integration tests.
   - [ ] Update documents.


-- 
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-php] jmjoy commented on a diff in pull request #64: Add amqplib plugin for producer.

Posted by "jmjoy (via GitHub)" <gi...@apache.org>.
jmjoy commented on code in PR #64:
URL: https://github.com/apache/skywalking-php/pull/64#discussion_r1164873592


##########
README.md:
##########
@@ -33,6 +33,7 @@ SkyWalking PHP Agent requires SkyWalking 8.4+ and PHP 7.2+
   * [ ] [php-amqp](https://github.com/php-amqp/php-amqp)
   * [ ] [php-rdkafka](https://github.com/arnaud-lb/php-rdkafka)
   * [x] [predis](https://github.com/predis/predis)
+  * [x] [php-amqplib](https://github.com/php-amqplib/php-amqplib)

Review Comment:
   Not only RabbitMQ supported, but I don't found the component ID about amqp protocol, the library `php-amqplib` is commonly used in message queues services which supports the amqp protocol.



-- 
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-php] wu-sheng merged pull request #64: Add amqplib plugin for producer.

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng merged PR #64:
URL: https://github.com/apache/skywalking-php/pull/64


-- 
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-php] wu-sheng commented on a diff in pull request #64: Add amqplib plugin.

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #64:
URL: https://github.com/apache/skywalking-php/pull/64#discussion_r1164844301


##########
src/component.rs:
##########
@@ -24,3 +24,4 @@ pub const COMPONENT_PHP_MYSQLI_ID: i32 = 8004;
 pub const COMPONENT_PHP_PREDIS_ID: i32 = 8006;
 pub const COMPONENT_PHP_MEMCACHED_ID: i32 = 20;
 pub const COMPONENT_PHP_REDIS_ID: i32 = 7;
+pub const COMPONENT_RABBITMQ_PRODUCER_ID: i32 = 52;

Review Comment:
   So, only the producer side? No consumer?



-- 
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-php] wu-sheng commented on a diff in pull request #64: Add amqplib plugin.

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #64:
URL: https://github.com/apache/skywalking-php/pull/64#discussion_r1164859888


##########
README.md:
##########
@@ -33,6 +33,7 @@ SkyWalking PHP Agent requires SkyWalking 8.4+ and PHP 7.2+
   * [ ] [php-amqp](https://github.com/php-amqp/php-amqp)
   * [ ] [php-rdkafka](https://github.com/arnaud-lb/php-rdkafka)
   * [x] [predis](https://github.com/predis/predis)
+  * [x] [php-amqplib](https://github.com/php-amqplib/php-amqplib)

Review Comment:
   Also, you mentioned amqp here, but the component ID is `rabbitmq-producer`. Is only Rabbit MQ supported?



-- 
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-php] huangdijia commented on pull request #64: Add amqplib plugin.

Posted by "huangdijia (via GitHub)" <gi...@apache.org>.
huangdijia commented on PR #64:
URL: https://github.com/apache/skywalking-php/pull/64#issuecomment-1504565988

   Looking forward to supporting the swoole coroutine component as soon as possible.


-- 
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-php] jmjoy commented on pull request #64: Add amqplib plugin for producer.

Posted by "jmjoy (via GitHub)" <gi...@apache.org>.
jmjoy commented on PR #64:
URL: https://github.com/apache/skywalking-php/pull/64#issuecomment-1518609166

   > Looking forward to supporting the swoole coroutine component as soon as possible.
   
   I don't use swoole frequently, but I found that swoole supports [SWOOLE_HOOK_ALL](https://wiki.swoole.com/#/runtime?id=swoole_hook_all), so can we reuse FPM ecological components?
   
   


-- 
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-php] jmjoy commented on a diff in pull request #64: Add amqplib plugin.

Posted by "jmjoy (via GitHub)" <gi...@apache.org>.
jmjoy commented on code in PR #64:
URL: https://github.com/apache/skywalking-php/pull/64#discussion_r1164853624


##########
src/component.rs:
##########
@@ -24,3 +24,4 @@ pub const COMPONENT_PHP_MYSQLI_ID: i32 = 8004;
 pub const COMPONENT_PHP_PREDIS_ID: i32 = 8006;
 pub const COMPONENT_PHP_MEMCACHED_ID: i32 = 20;
 pub const COMPONENT_PHP_REDIS_ID: i32 = 7;
+pub const COMPONENT_RABBITMQ_PRODUCER_ID: i32 = 52;

Review Comment:
   The consumer plan will be added later (PHP is not very suitable as a consumer).



-- 
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-php] wu-sheng commented on a diff in pull request #64: Add amqplib plugin.

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #64:
URL: https://github.com/apache/skywalking-php/pull/64#discussion_r1164859151


##########
README.md:
##########
@@ -33,6 +33,7 @@ SkyWalking PHP Agent requires SkyWalking 8.4+ and PHP 7.2+
   * [ ] [php-amqp](https://github.com/php-amqp/php-amqp)
   * [ ] [php-rdkafka](https://github.com/arnaud-lb/php-rdkafka)
   * [x] [predis](https://github.com/predis/predis)
+  * [x] [php-amqplib](https://github.com/php-amqplib/php-amqplib)

Review Comment:
   Do you think we need to put `producer` here?
   ```suggestion
     * [x] [php-amqplib](https://github.com/php-amqplib/php-amqplib) for Message Queuing Producer.
   ```



-- 
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-php] jmjoy commented on a diff in pull request #64: Add amqplib plugin.

Posted by "jmjoy (via GitHub)" <gi...@apache.org>.
jmjoy commented on code in PR #64:
URL: https://github.com/apache/skywalking-php/pull/64#discussion_r1164869329


##########
README.md:
##########
@@ -33,6 +33,7 @@ SkyWalking PHP Agent requires SkyWalking 8.4+ and PHP 7.2+
   * [ ] [php-amqp](https://github.com/php-amqp/php-amqp)
   * [ ] [php-rdkafka](https://github.com/arnaud-lb/php-rdkafka)
   * [x] [predis](https://github.com/predis/predis)
+  * [x] [php-amqplib](https://github.com/php-amqplib/php-amqplib)

Review Comment:
   I will update the README and doc 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.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

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

[GitHub] [skywalking-php] wu-sheng commented on a diff in pull request #64: Add amqplib plugin for producer.

Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #64:
URL: https://github.com/apache/skywalking-php/pull/64#discussion_r1164884989


##########
README.md:
##########
@@ -33,6 +33,7 @@ SkyWalking PHP Agent requires SkyWalking 8.4+ and PHP 7.2+
   * [ ] [php-amqp](https://github.com/php-amqp/php-amqp)
   * [ ] [php-rdkafka](https://github.com/arnaud-lb/php-rdkafka)
   * [x] [predis](https://github.com/predis/predis)
+  * [x] [php-amqplib](https://github.com/php-amqplib/php-amqplib)

Review Comment:
   If you need a general type for `amqp`, you have to register it here, https://github.com/apache/skywalking/blob/master/oap-server/server-starter/src/main/resources/component-libraries.yml
   
   Also, you may need mapping to indicate it is a queue server(without a specific MQ type), or an icon for topology.



-- 
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-php] jmjoy commented on a diff in pull request #64: Add amqplib plugin.

Posted by "jmjoy (via GitHub)" <gi...@apache.org>.
jmjoy commented on code in PR #64:
URL: https://github.com/apache/skywalking-php/pull/64#discussion_r1164873592


##########
README.md:
##########
@@ -33,6 +33,7 @@ SkyWalking PHP Agent requires SkyWalking 8.4+ and PHP 7.2+
   * [ ] [php-amqp](https://github.com/php-amqp/php-amqp)
   * [ ] [php-rdkafka](https://github.com/arnaud-lb/php-rdkafka)
   * [x] [predis](https://github.com/predis/predis)
+  * [x] [php-amqplib](https://github.com/php-amqplib/php-amqplib)

Review Comment:
   Not only RabbitMQ supported, but I don't found the component ID about amqp protocol, the library `php-amqplib` is commonly used in message queues services which supports the ampq protocol.



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