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 2020/02/22 13:04:53 UTC
[GitHub] [skywalking] wu-sheng opened a new pull request #4397: Add Nginx
component ID to main repo
wu-sheng opened a new pull request #4397: Add Nginx component ID to main repo
URL: https://github.com/apache/skywalking/pull/4397
Add 84 as Nginx component ID
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4397: Add Nginx component
ID to main repo
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4397: Add Nginx component ID to main repo
URL: https://github.com/apache/skywalking/pull/4397#issuecomment-589958323
@kezhenxu94 If you think we will have more span in Nginx level, I could change that.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking] codecov-io edited a comment on issue #4397: Add Nginx
component ID to main repo
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4397: Add Nginx component ID to main repo
URL: https://github.com/apache/skywalking/pull/4397#issuecomment-589966808
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4397?src=pr&el=h1) Report
> Merging [#4397](https://codecov.io/gh/apache/skywalking/pull/4397?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/67b7f22a4074a8d714b603cb27400fdcb9ec31f4?src=pr&el=desc) will **decrease** coverage by `0.09%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/skywalking/pull/4397?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4397 +/- ##
=========================================
- Coverage 25.65% 25.55% -0.1%
=========================================
Files 1209 1209
Lines 27914 27914
Branches 3851 3851
=========================================
- Hits 7160 7134 -26
- Misses 20103 20129 +26
Partials 651 651
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4397?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...apm/agent/core/remote/GRPCStreamServiceStatus.java](https://codecov.io/gh/apache/skywalking/pull/4397/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0dSUENTdHJlYW1TZXJ2aWNlU3RhdHVzLmphdmE=) | `41.17% <0%> (-41.18%)` | :arrow_down: |
| [...ache/skywalking/apm/agent/core/jvm/JVMService.java](https://codecov.io/gh/apache/skywalking/pull/4397/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvanZtL0pWTVNlcnZpY2UuamF2YQ==) | `60% <0%> (-20%)` | :arrow_down: |
| [...alking/apm/agent/core/remote/AgentIDDecorator.java](https://codecov.io/gh/apache/skywalking/pull/4397/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0FnZW50SUREZWNvcmF0b3IuamF2YQ==) | `66.66% <0%> (-18.52%)` | :arrow_down: |
| [...m/agent/core/remote/TraceSegmentServiceClient.java](https://codecov.io/gh/apache/skywalking/pull/4397/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL1RyYWNlU2VnbWVudFNlcnZpY2VDbGllbnQuamF2YQ==) | `78.08% <0%> (-1.37%)` | :arrow_down: |
| [...king/apm/agent/core/remote/GRPCChannelManager.java](https://codecov.io/gh/apache/skywalking/pull/4397/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0dSUENDaGFubmVsTWFuYWdlci5qYXZh) | `67.94% <0%> (-1.29%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4397?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4397?src=pr&el=footer). Last update [67b7f22...c5a4091](https://codecov.io/gh/apache/skywalking/pull/4397?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng merged pull request #4397: Add Nginx
component ID to main repo
Posted by GitBox <gi...@apache.org>.
wu-sheng merged pull request #4397: Add Nginx component ID to main repo
URL: https://github.com/apache/skywalking/pull/4397
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking] kezhenxu94 commented on issue #4397: Add Nginx
component ID to main repo
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on issue #4397: Add Nginx component ID to main repo
URL: https://github.com/apache/skywalking/pull/4397#issuecomment-589959945
I'm good to merge this anyway, it does no harm, feel free to merge 😄
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4397: Add Nginx component
ID to main repo
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4397: Add Nginx component ID to main repo
URL: https://github.com/apache/skywalking/pull/4397#issuecomment-589958038
> do we want to start from a larger number of id (instead of 84) for the NEW language Lua?
I don't know :P Currently, only one component for Nginx Lua 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking] codecov-io commented on issue #4397: Add Nginx
component ID to main repo
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #4397: Add Nginx component ID to main repo
URL: https://github.com/apache/skywalking/pull/4397#issuecomment-589966808
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4397?src=pr&el=h1) Report
> Merging [#4397](https://codecov.io/gh/apache/skywalking/pull/4397?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/67b7f22a4074a8d714b603cb27400fdcb9ec31f4?src=pr&el=desc) will **decrease** coverage by `0.09%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/skywalking/pull/4397?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4397 +/- ##
=========================================
- Coverage 25.65% 25.55% -0.1%
=========================================
Files 1209 1209
Lines 27914 27914
Branches 3851 3851
=========================================
- Hits 7160 7134 -26
- Misses 20103 20129 +26
Partials 651 651
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4397?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...apm/agent/core/remote/GRPCStreamServiceStatus.java](https://codecov.io/gh/apache/skywalking/pull/4397/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0dSUENTdHJlYW1TZXJ2aWNlU3RhdHVzLmphdmE=) | `41.17% <0%> (-41.18%)` | :arrow_down: |
| [...ache/skywalking/apm/agent/core/jvm/JVMService.java](https://codecov.io/gh/apache/skywalking/pull/4397/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvanZtL0pWTVNlcnZpY2UuamF2YQ==) | `60% <0%> (-20%)` | :arrow_down: |
| [...alking/apm/agent/core/remote/AgentIDDecorator.java](https://codecov.io/gh/apache/skywalking/pull/4397/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0FnZW50SUREZWNvcmF0b3IuamF2YQ==) | `66.66% <0%> (-18.52%)` | :arrow_down: |
| [...m/agent/core/remote/TraceSegmentServiceClient.java](https://codecov.io/gh/apache/skywalking/pull/4397/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL1RyYWNlU2VnbWVudFNlcnZpY2VDbGllbnQuamF2YQ==) | `78.08% <0%> (-1.37%)` | :arrow_down: |
| [...king/apm/agent/core/remote/GRPCChannelManager.java](https://codecov.io/gh/apache/skywalking/pull/4397/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0dSUENDaGFubmVsTWFuYWdlci5qYXZh) | `67.94% <0%> (-1.29%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4397?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4397?src=pr&el=footer). Last update [67b7f22...c5a4091](https://codecov.io/gh/apache/skywalking/pull/4397?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking] kezhenxu94 commented on issue #4397: Add Nginx
component ID to main repo
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on issue #4397: Add Nginx component ID to main repo
URL: https://github.com/apache/skywalking/pull/4397#issuecomment-589959411
> > do we want to start from a larger number of id (instead of 84) for the NEW language Lua?
>
> I don't know :P Currently, only one component for Nginx Lua now.
Seems the id ranges are divided according to different languages, instead of frameworks (i.e. Nginx), if there are any possibility that we may another components written by Lua(not necessarily in Nginx level) , I think it's better to start from another id number and give Lua a range; what's more, 84 makes the Java id range non-continuous (1-83 and 85 to xxx) 😅 odd, harmless though
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking] kezhenxu94 commented on issue #4397: Add Nginx
component ID to main repo
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on issue #4397: Add Nginx component ID to main repo
URL: https://github.com/apache/skywalking/pull/4397#issuecomment-589957928
@wu-sheng do we want to start from a larger number of id (instead of 84) for the NEW language Lua?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on issue #4397: Add Nginx component
ID to main repo
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4397: Add Nginx component ID to main repo
URL: https://github.com/apache/skywalking/pull/4397#issuecomment-589963201
@kezhenxu94 Updated ID to 6000 as a separated group.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking] aderm commented on issue #4397: Add Nginx component ID
to main repo
Posted by GitBox <gi...@apache.org>.
aderm commented on issue #4397: Add Nginx component ID to main repo
URL: https://github.com/apache/skywalking/pull/4397#issuecomment-589959238
LGTM
----------------------------------------------------------------
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
With regards,
Apache Git Services