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/03/14 04:13:12 UTC
[GitHub] [skywalking] mrproliu opened a new pull request #4509: Provide the
data transmission protocol document
mrproliu opened a new pull request #4509: Provide the data transmission protocol document
URL: https://github.com/apache/skywalking/pull/4509
Please answer these questions before submitting pull request
- Why submit this pull request?
- [ ] Bug fix
- [x] New feature provided
- [ ] Improve performance
___
### New feature or improvement
- Provide the data transmission protocol document.
----------------------------------------------------------------
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 #4509: SkyWalking
Cross Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599012177
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=h1) Report
> Merging [#4509](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/d5efc97c803d3ea249a157751d6aa7af8b0fcc9b?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4509 +/- ##
=======================================
Coverage 25.49% 25.49%
=======================================
Files 1244 1244
Lines 28895 28895
Branches 3953 3953
=======================================
Hits 7367 7367
Misses 20849 20849
Partials 679 679
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?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/4509?src=pr&el=footer). Last update [d5efc97...3633c10](https://codecov.io/gh/apache/skywalking/pull/4509?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] dmsolr commented on issue #4509: Provide the data
transmission protocol document
Posted by GitBox <gi...@apache.org>.
dmsolr commented on issue #4509: Provide the data transmission protocol document
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599020337
> This is widely used in the binary format, because length could be represented in 2 bytes, but in string format, I am not sure whether we need this.
Yes. This format is more widely in binary format.
In fact, `:` and `,` are also represented in 2 bytes. So we spend 2 bytes more only.
----------------------------------------------------------------
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] mrproliu commented on issue #4509: SkyWalking Cross
Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
mrproliu commented on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599026788
> Even we are not really using W3C spec, but please take a refer from this, https://www.w3.org/TR/trace-context/#combined-header-value
>
> base64(key)=base64(value),base64(key)=base64(value)
I think that's just a character different, use this way is goods to 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.
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 #4509: SkyWalking
Cross Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599012177
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=h1) Report
> Merging [#4509](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/d5efc97c803d3ea249a157751d6aa7af8b0fcc9b&el=desc) will **not change** coverage by `%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4509 +/- ##
=======================================
Coverage 25.49% 25.49%
=======================================
Files 1244 1244
Lines 28895 28895
Branches 3953 3953
=======================================
Hits 7367 7367
Misses 20849 20849
Partials 679 679
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?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/4509?src=pr&el=footer). Last update [d5efc97...62f0504](https://codecov.io/gh/apache/skywalking/pull/4509?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 #4509: SkyWalking Cross
Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599021670
> > > I'm wondering whether it's worthwhile to make it JSON, I remember @mrproliu your first suggestion is to encode the key and value respectively, and concatenate them with :, which IMO is good enough and efficient, JSON is more human readable but cost more bandwidth
> >
> >
> > @kezhenxu94 , the JSON format is not written by me. I think @mrproliu changed this because your question about the `:` in the key or value.
>
> My first thought was to use internal encode as data encoding, but I later thought that there might be a problem. I just reread the base64 specification, but there is no risk of ':'. In fact, it can be used.
> Can we use 'Base64 (userkey1): Base64 (uservalue2), Base64 (userkey2): Base64 (uservalue2) `. But if we are thinking about coding constantly, will it affect the efficiency? @kezhenxu94 @wu-sheng Any Suggestion?
Why make it so complicated, I did say `:` could appear in the key or value, but `:` could never appear in **ENCODED** string, my thought is just encode the `key` and `value` respectively, and concatenate them with `:`, to put it simple, the final data would be something like: `<encoded_key>:<encoded_val>`, where `<encoded_key>` and `<encoded_val>` is Base64 encoded, and doesn't contain `:` at all, so the only `:` is our delimiter, did I miss anything @mrproliu ?
----------------------------------------------------------------
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 a change in pull request
#4509: Provide the data transmission protocol document
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4509: Provide the data transmission protocol document
URL: https://github.com/apache/skywalking/pull/4509#discussion_r392561133
##########
File path: docs/en/protocols/Skywalking-Cross-Process-Correlation-Headers-Protocol-v1.md
##########
@@ -0,0 +1,24 @@
+# SkyWalking Cross Process Correlation Headers Protocol
+* Version 1.0
+
+The Cross Process Correlation Headers Protocol is used to transport custom data by leveraging the capability of [Cross Process Propagation Headers Protocol](Skywalking-Cross-Process-Propagation-Headers-Protocol-v2.md).
+
+This is an optional and additional protocol for language tracer implementation. All tracer implementation could consider to implement this.
+Cross Process Correlation Header key is `sw7-correlation`. The value is (string key)-(string value) table in `Base64` encoded JSON format.
+
+## Value Example
+(string key)-(string value) table in `Base64` encoded JSON format.
+
+Original value raw data `{"userKey1","userValue1","userKey2":"userValue2"}` string, the `Base64` encoded value
+`eyJ1c2VyS2V5MSIsInVzZXJWYWx1ZTEiLCJ1c2VyS2V5MiI6InVzZXJWYWx1ZTIifQ==`.
+
+## Recommendations of language APIs
+Recommended implementation in different language API.
+
+1. `CorrelationContext#set` and `CorrelationContext#get` are recommended to write and read the correlation context, with key/value string.
+1. The key should be added if it is inexistence.
Review comment:
```suggestion
1. The key should be added if it is absent.
```
----------------------------------------------------------------
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 #4509: Provide
the data transmission protocol document
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4509: Provide the data transmission protocol document
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599012177
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=h1) Report
> Merging [#4509](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/d5efc97c803d3ea249a157751d6aa7af8b0fcc9b&el=desc) will **not change** coverage by `%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4509 +/- ##
=======================================
Coverage 25.49% 25.49%
=======================================
Files 1244 1244
Lines 28895 28895
Branches 3953 3953
=======================================
Hits 7367 7367
Misses 20849 20849
Partials 679 679
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?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/4509?src=pr&el=footer). Last update [d5efc97...62f0504](https://codecov.io/gh/apache/skywalking/pull/4509?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 commented on issue #4509: SkyWalking Cross
Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599027040
I think the key better point is, `:` has a similar Chinese character, but `=` doesn’t have this issue.
----------------------------------------------------------------
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 #4509: SkyWalking Cross
Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng merged pull request #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509
----------------------------------------------------------------
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] arugal commented on a change in pull request #4509:
SkyWalking Cross Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
arugal commented on a change in pull request #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#discussion_r392588153
##########
File path: docs/en/protocols/Skywalking-Cross-Process-Correlation-Headers-Protocol-v1.md
##########
@@ -0,0 +1,16 @@
+# SkyWalking Cross Process Correlation Headers Protocol
+* Version 1.0
+
+The Cross Process Correlation Headers Protocol is used to transport custom data by leveraging the capability of [Cross Process Propagation Headers Protocol](Skywalking-Cross-Process-Propagation-Headers-Protocol-v2.md).
+
+This is an optional and additional protocol for language tracer implementation. All tracer implementation could consider to implement this.
+Cross Process Correlation Header key is `sw7-correlation`. The value is the `encoded(key):encoded(value)` list with elements splitted by `,` such as `base64(string key):base64(string value),base64(string key2):base64(string value2)`.
+
+## Recommendations of language APIs
+Recommended implementation in different language API.
+
+1. `CorrelationContext#set` and `CorrelationContext#get` are recommended to write and read the correlation context, with key/value string.
+1. The key should be added if it is absent.
+1. The later writes should override the previous value.
+1. The total number of all keys should be less than 3, and the length of each value should be less than 128 bytes.
+1. The context should be propagated as well when tracing context is propagated across threads and processes..
Review comment:
```suggestion
1. The context should be propagated as well when tracing context is propagated across threads and processes.
```
----------------------------------------------------------------
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 #4509: SkyWalking Cross
Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599023289
Notice `tracestate` is not our thing, I just say refer the format only.
----------------------------------------------------------------
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 #4509: Provide the data
transmission protocol document
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4509: Provide the data transmission protocol document
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599019387
> I'm wondering whether it's worthwhile to make it JSON, I remember @mrproliu your first suggestion is to encode the key and value respectively, and concatenate them with :, which IMO is good enough and efficient, JSON is more human readable but cost more bandwidth
@kezhenxu94 , the JSON format is not written by me. I think @mrproliu changed this because your question about the `:` in the key or value.
----------------------------------------------------------------
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] mrproliu commented on issue #4509: Provide the data
transmission protocol document
Posted by GitBox <gi...@apache.org>.
mrproliu commented on issue #4509: Provide the data transmission protocol document
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599020469
> > I'm wondering whether it's worthwhile to make it JSON, I remember @mrproliu your first suggestion is to encode the key and value respectively, and concatenate them with :, which IMO is good enough and efficient, JSON is more human readable but cost more bandwidth
>
> @kezhenxu94 , the JSON format is not written by me. I think @mrproliu changed this because your question about the `:` in the key or value.
My first thought was to use internal encode as data encoding, but I later thought that there might be a problem. I just reread the base64 specification, but there is no risk of ':'. In fact, it can be used.
Can we use 'Base64 (userkey1): Base64 (uservalue2), Base64 (userkey2): Base64 (uservalue2) `. But if we are thinking about coding constantly, will it affect the efficiency? @kezhenxu94 @wu-sheng Any Suggestion?
----------------------------------------------------------------
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 #4509: SkyWalking Cross
Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599027835
> I have tried your scenarios. That's good to me, but we must make sure the key or value size must less than 32768, otherwise, the data will be lost or parsing error.
I think this is too complex.
----------------------------------------------------------------
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 #4509: SkyWalking Cross
Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599028855
@kezhenxu94 Don’t use that(not encode). That is what I mean we are not following W3C spec. That is not popular and not realistic. I just want to recommend `=`.
----------------------------------------------------------------
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 #4509: SkyWalking
Cross Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599012177
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=h1) Report
> Merging [#4509](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/d5efc97c803d3ea249a157751d6aa7af8b0fcc9b&el=desc) will **increase** coverage by `0.02%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4509 +/- ##
==========================================
+ Coverage 25.49% 25.51% +0.02%
==========================================
Files 1244 1244
Lines 28895 28895
Branches 3953 3953
==========================================
+ Hits 7367 7373 +6
+ Misses 20849 20842 -7
- Partials 679 680 +1
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [.../agent/core/profile/ProfileTaskChannelService.java](https://codecov.io/gh/apache/skywalking/pull/4509/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza0NoYW5uZWxTZXJ2aWNlLmphdmE=) | `24.73% <0.00%> (-1.08%)` | :arrow_down: |
| [...walking/oap/server/core/analysis/Downsampling.java](https://codecov.io/gh/apache/skywalking/pull/4509/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvYW5hbHlzaXMvRG93bnNhbXBsaW5nLmphdmE=) | `100.00% <0.00%> (+100.00%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?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/4509?src=pr&el=footer). Last update [d5efc97...3633c10](https://codecov.io/gh/apache/skywalking/pull/4509?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 commented on issue #4509: SkyWalking Cross
Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599021338
Add the original discuss thread, https://lists.apache.org/thread.html/r50ff0ee6cd927910cad610d258626a58b09982c9e4e8c55827d727e7%40%3Cdev.skywalking.apache.org%3E
----------------------------------------------------------------
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 #4509: SkyWalking Cross
Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599029720
@kezhenxu94 @mrproliu @dmsolr I changed the document by using `=` and `,`.
----------------------------------------------------------------
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 #4509: SkyWalking
Cross Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599012177
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=h1) Report
> Merging [#4509](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/d5efc97c803d3ea249a157751d6aa7af8b0fcc9b&el=desc) will **increase** coverage by `0.02%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4509 +/- ##
==========================================
+ Coverage 25.49% 25.51% +0.02%
==========================================
Files 1244 1244
Lines 28895 28895
Branches 3953 3953
==========================================
+ Hits 7367 7374 +7
+ Misses 20849 20842 -7
Partials 679 679
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...walking/oap/server/core/analysis/Downsampling.java](https://codecov.io/gh/apache/skywalking/pull/4509/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvYW5hbHlzaXMvRG93bnNhbXBsaW5nLmphdmE=) | `100.00% <0.00%> (+100.00%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?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/4509?src=pr&el=footer). Last update [d5efc97...5888ce2](https://codecov.io/gh/apache/skywalking/pull/4509?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] codecov-io edited a comment on issue #4509: SkyWalking
Cross Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599012177
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=h1) Report
> Merging [#4509](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/d5efc97c803d3ea249a157751d6aa7af8b0fcc9b&el=desc) will **not change** coverage by `%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4509 +/- ##
=======================================
Coverage 25.49% 25.49%
=======================================
Files 1244 1244
Lines 28895 28895
Branches 3953 3953
=======================================
Hits 7367 7367
Misses 20849 20849
Partials 679 679
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?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/4509?src=pr&el=footer). Last update [d5efc97...7cde361](https://codecov.io/gh/apache/skywalking/pull/4509?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] mrproliu commented on issue #4509: SkyWalking Cross
Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
mrproliu commented on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599027619
> > This is widely used in the binary format, because length could be represented in 2 bytes, but in string format, I am not sure whether we need this.
>
> Yes. This format is more widely in binary format.
> In fact, `:` and `,` are also represented in 2 bytes. So we spend 2 bytes more only.
I have tried your scenarios. That's good to me, but we must make sure the `key` or `value` size must less than `32768`, otherwise, the data will be lost or parsing error.
----------------------------------------------------------------
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 #4509: SkyWalking
Cross Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599012177
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=h1) Report
> Merging [#4509](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/61f98c81f036ca394ca879355c72735c1f95357e?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4509 +/- ##
=======================================
Coverage 25.56% 25.56%
=======================================
Files 1244 1244
Lines 28951 28951
Branches 3968 3968
=======================================
Hits 7400 7400
Misses 20867 20867
Partials 684 684
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?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/4509?src=pr&el=footer). Last update [61f98c8...e56b03a](https://codecov.io/gh/apache/skywalking/pull/4509?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 commented on issue #4509: SkyWalking Cross
Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599023096
Even we are not really using W3C spec, but please take a refer from this, https://www.w3.org/TR/trace-context/#combined-header-value
base64(key)=base64(value),base64(key)=base64(value)
----------------------------------------------------------------
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] mrproliu commented on issue #4509: SkyWalking Cross
Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
mrproliu commented on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599022885
> > > > I'm wondering whether it's worthwhile to make it JSON, I remember @mrproliu your first suggestion is to encode the key and value respectively, and concatenate them with :, which IMO is good enough and efficient, JSON is more human readable but cost more bandwidth
> > >
> > >
> > > @kezhenxu94 , the JSON format is not written by me. I think @mrproliu changed this because your question about the `:` in the key or value.
> >
> >
> > My first thought was to use internal encode as data encoding, but I later thought that there might be a problem. I just reread the base64 specification, but there is no risk of ':'. In fact, it can be used.
> > Can we use 'Base64 (userkey1): Base64 (uservalue2), Base64 (userkey2): Base64 (uservalue2) `. But if we are thinking about coding constantly, will it affect the efficiency? @kezhenxu94 @wu-sheng Any Suggestion?
>
> Why make it so complicated, I did say `:` could appear in the key or value, but `:` could never appear in **ENCODED** string, my thought is just encode the `key` and `value` respectively, and concatenate them with `:`, to put it simple, the final data would be something like: `<encoded_key>:<encoded_val>`, where `<encoded_key>` and `<encoded_val>` is Base64 encoded, and doesn't contain `:` at all, so the only `:` is our delimiter, did I miss anything @mrproliu ?
I think we're talking in the same way. I agree with this pattern:
`base64(key):base64(value),base64(key):base64(value)`
----------------------------------------------------------------
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 a change in pull request
#4509: Provide the data transmission protocol document
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4509: Provide the data transmission protocol document
URL: https://github.com/apache/skywalking/pull/4509#discussion_r392561293
##########
File path: docs/en/protocols/Skywalking-Cross-Process-Correlation-Headers-Protocol-v1.md
##########
@@ -0,0 +1,24 @@
+# SkyWalking Cross Process Correlation Headers Protocol
+* Version 1.0
+
+The Cross Process Correlation Headers Protocol is used to transport custom data by leveraging the capability of [Cross Process Propagation Headers Protocol](Skywalking-Cross-Process-Propagation-Headers-Protocol-v2.md).
+
+This is an optional and additional protocol for language tracer implementation. All tracer implementation could consider to implement this.
+Cross Process Correlation Header key is `sw7-correlation`. The value is (string key)-(string value) table in `Base64` encoded JSON format.
+
+## Value Example
+(string key)-(string value) table in `Base64` encoded JSON format.
+
+Original value raw data `{"userKey1","userValue1","userKey2":"userValue2"}` string, the `Base64` encoded value
+`eyJ1c2VyS2V5MSIsInVzZXJWYWx1ZTEiLCJ1c2VyS2V5MiI6InVzZXJWYWx1ZTIifQ==`.
+
+## Recommendations of language APIs
+Recommended implementation in different language API.
+
+1. `CorrelationContext#set` and `CorrelationContext#get` are recommended to write and read the correlation context, with key/value string.
+1. The key should be added if it is inexistence.
+1. The later the write should override the prev value.
+1. The number of all keys should less than 3, and the length of each value should be less than 128 bytes.
+1. The context should be propageted when across thread and across process like do tracing context propagation.
Review comment:
```suggestion
1. The context should be propagated as well when tracing context is propagated across threads and processes..
```
----------------------------------------------------------------
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 #4509: SkyWalking Cross
Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599028122
> I think the key better point is, `:` has a similar Chinese character, but `=` doesn’t have this issue.
@mrproliu please be advised that `=` is a valid character in encoded base64, and it often appears in encoded text as padding character, the documentations @wu-sheng you referred to is clear text, not encoded
----------------------------------------------------------------
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] dmsolr commented on issue #4509: Provide the data
transmission protocol document
Posted by GitBox <gi...@apache.org>.
dmsolr commented on issue #4509: Provide the data transmission protocol document
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599019398
> Some suggestions inline
>
> As for the formats, I forgot to reply to the mailing list thread,
>
> > Cross Process Correlation Header key is `sw7-correlation`. The value is (string key)-(string value) table in `Base64` encoded JSON format.
>
> I'm wondering whether it's worthwhile to make it JSON, I remember @mrproliu your first suggestion is to encode the `key` and `value` respectively, and concatenate them with `:`, which IMO is good enough and efficient, JSON is more human readable but cost more bandwidth
I have a raw idea. Format: [key.length][key][value.length][key]<[key.length][key][value.length][key]....>
We can use a character to represnet it's length.
----------------------------------------------------------------
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 #4509: SkyWalking
Cross Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599012177
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=h1) Report
> Merging [#4509](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/d5efc97c803d3ea249a157751d6aa7af8b0fcc9b&el=desc) will **increase** coverage by `0.02%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4509 +/- ##
==========================================
+ Coverage 25.49% 25.51% +0.02%
==========================================
Files 1244 1244
Lines 28895 28895
Branches 3953 3953
==========================================
+ Hits 7367 7374 +7
+ Misses 20849 20842 -7
Partials 679 679
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...walking/oap/server/core/analysis/Downsampling.java](https://codecov.io/gh/apache/skywalking/pull/4509/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvYW5hbHlzaXMvRG93bnNhbXBsaW5nLmphdmE=) | `100.00% <0.00%> (+100.00%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?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/4509?src=pr&el=footer). Last update [d5efc97...62f0504](https://codecov.io/gh/apache/skywalking/pull/4509?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] codecov-io edited a comment on issue #4509: SkyWalking
Cross Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599012177
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=h1) Report
> Merging [#4509](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/d5efc97c803d3ea249a157751d6aa7af8b0fcc9b&el=desc) will **not change** coverage by `%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4509 +/- ##
=======================================
Coverage 25.49% 25.49%
=======================================
Files 1244 1244
Lines 28895 28895
Branches 3953 3953
=======================================
Hits 7367 7367
Misses 20849 20849
Partials 679 679
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?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/4509?src=pr&el=footer). Last update [d5efc97...62f0504](https://codecov.io/gh/apache/skywalking/pull/4509?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 edited a comment on issue #4509: SkyWalking
Cross Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng edited a comment on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599029720
@kezhenxu94 @mrproliu @dmsolr I changed the document by using `:` and `,` due to `=` could be included in base64 encoded text.
----------------------------------------------------------------
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 #4509: Provide the data
transmission protocol document
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #4509: Provide the data transmission protocol document
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599012177
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=h1) Report
> Merging [#4509](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/d5efc97c803d3ea249a157751d6aa7af8b0fcc9b?src=pr&el=desc) will **increase** coverage by `0.02%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4509 +/- ##
==========================================
+ Coverage 25.49% 25.51% +0.02%
==========================================
Files 1244 1244
Lines 28895 28895
Branches 3953 3953
==========================================
+ Hits 7367 7374 +7
+ Misses 20849 20842 -7
Partials 679 679
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...walking/oap/server/core/analysis/Downsampling.java](https://codecov.io/gh/apache/skywalking/pull/4509/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvYW5hbHlzaXMvRG93bnNhbXBsaW5nLmphdmE=) | `100% <0%> (+100%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?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/4509?src=pr&el=footer). Last update [d5efc97...5d37f54](https://codecov.io/gh/apache/skywalking/pull/4509?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] codecov-io edited a comment on issue #4509: SkyWalking
Cross Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599012177
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=h1) Report
> Merging [#4509](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/d5efc97c803d3ea249a157751d6aa7af8b0fcc9b&el=desc) will **not change** coverage by `%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4509 +/- ##
=======================================
Coverage 25.49% 25.49%
=======================================
Files 1244 1244
Lines 28895 28895
Branches 3953 3953
=======================================
Hits 7367 7367
Misses 20849 20849
Partials 679 679
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?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/4509?src=pr&el=footer). Last update [d5efc97...3633c10](https://codecov.io/gh/apache/skywalking/pull/4509?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 commented on issue #4509: SkyWalking Cross
Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-600957760
I am going to merge this soon. @mrproliu You could feel free to work on this implementation.
----------------------------------------------------------------
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 #4509: Provide the data
transmission protocol document
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4509: Provide the data transmission protocol document
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599019487
> I have a raw idea. Format: [key.length][key][value.length][key]<[key.length][key][value.length][key]....>
This is widely used in the binary format, because length could be represented in 2 bytes, but in string format, I am not sure whether we need 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
With regards,
Apache Git Services
[GitHub] [skywalking] codecov-io edited a comment on issue #4509: SkyWalking
Cross Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599012177
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=h1) Report
> Merging [#4509](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/d5efc97c803d3ea249a157751d6aa7af8b0fcc9b&el=desc) will **increase** coverage by `0.02%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4509 +/- ##
==========================================
+ Coverage 25.49% 25.51% +0.02%
==========================================
Files 1244 1244
Lines 28895 28895
Branches 3953 3953
==========================================
+ Hits 7367 7374 +7
+ Misses 20849 20842 -7
Partials 679 679
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...walking/oap/server/core/analysis/Downsampling.java](https://codecov.io/gh/apache/skywalking/pull/4509/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvYW5hbHlzaXMvRG93bnNhbXBsaW5nLmphdmE=) | `100.00% <0.00%> (+100.00%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?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/4509?src=pr&el=footer). Last update [d5efc97...62f0504](https://codecov.io/gh/apache/skywalking/pull/4509?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 a change in pull request
#4509: Provide the data transmission protocol document
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4509: Provide the data transmission protocol document
URL: https://github.com/apache/skywalking/pull/4509#discussion_r392561143
##########
File path: docs/en/protocols/Skywalking-Cross-Process-Correlation-Headers-Protocol-v1.md
##########
@@ -0,0 +1,24 @@
+# SkyWalking Cross Process Correlation Headers Protocol
+* Version 1.0
+
+The Cross Process Correlation Headers Protocol is used to transport custom data by leveraging the capability of [Cross Process Propagation Headers Protocol](Skywalking-Cross-Process-Propagation-Headers-Protocol-v2.md).
+
+This is an optional and additional protocol for language tracer implementation. All tracer implementation could consider to implement this.
+Cross Process Correlation Header key is `sw7-correlation`. The value is (string key)-(string value) table in `Base64` encoded JSON format.
+
+## Value Example
+(string key)-(string value) table in `Base64` encoded JSON format.
+
+Original value raw data `{"userKey1","userValue1","userKey2":"userValue2"}` string, the `Base64` encoded value
+`eyJ1c2VyS2V5MSIsInVzZXJWYWx1ZTEiLCJ1c2VyS2V5MiI6InVzZXJWYWx1ZTIifQ==`.
+
+## Recommendations of language APIs
+Recommended implementation in different language API.
+
+1. `CorrelationContext#set` and `CorrelationContext#get` are recommended to write and read the correlation context, with key/value string.
+1. The key should be added if it is inexistence.
+1. The later the write should override the prev value.
Review comment:
```suggestion
1. The later writes should override the previous value.
```
----------------------------------------------------------------
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 #4509: SkyWalking
Cross Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599012177
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=h1) Report
> Merging [#4509](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/dcd66ee6e940bdc9a7ecf90415929bfd48a3a284?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4509 +/- ##
==========================================
- Coverage 25.56% 25.55% -0.01%
==========================================
Files 1244 1244
Lines 28951 28951
Branches 3968 3968
==========================================
- Hits 7400 7399 -1
Misses 20867 20867
- Partials 684 685 +1
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4509?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ache/skywalking/apm/agent/core/jvm/JVMService.java](https://codecov.io/gh/apache/skywalking/pull/4509/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvanZtL0pWTVNlcnZpY2UuamF2YQ==) | `55% <0%> (-1.67%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4509?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/4509?src=pr&el=footer). Last update [dcd66ee...62bd0a6](https://codecov.io/gh/apache/skywalking/pull/4509?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] mrproliu commented on issue #4509: SkyWalking Cross
Process Correlation Headers Protocol
Posted by GitBox <gi...@apache.org>.
mrproliu commented on issue #4509: SkyWalking Cross Process Correlation Headers Protocol
URL: https://github.com/apache/skywalking/pull/4509#issuecomment-599028876
> > I think the key better point is, `:` has a similar Chinese character, but `=` doesn’t have this issue.
>
> @mrproliu please be advised that `=` is a valid character in encoded base64, and it often appears in encoded text as padding character, the documentations @wu-sheng you referred to is clear text, not encoded
Ops, I forget this point. I think we need to keep the `:` to separate the `key/value`.
----------------------------------------------------------------
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 a change in pull request
#4509: Provide the data transmission protocol document
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4509: Provide the data transmission protocol document
URL: https://github.com/apache/skywalking/pull/4509#discussion_r392561184
##########
File path: docs/en/protocols/Skywalking-Cross-Process-Correlation-Headers-Protocol-v1.md
##########
@@ -0,0 +1,24 @@
+# SkyWalking Cross Process Correlation Headers Protocol
+* Version 1.0
+
+The Cross Process Correlation Headers Protocol is used to transport custom data by leveraging the capability of [Cross Process Propagation Headers Protocol](Skywalking-Cross-Process-Propagation-Headers-Protocol-v2.md).
+
+This is an optional and additional protocol for language tracer implementation. All tracer implementation could consider to implement this.
+Cross Process Correlation Header key is `sw7-correlation`. The value is (string key)-(string value) table in `Base64` encoded JSON format.
+
+## Value Example
+(string key)-(string value) table in `Base64` encoded JSON format.
+
+Original value raw data `{"userKey1","userValue1","userKey2":"userValue2"}` string, the `Base64` encoded value
+`eyJ1c2VyS2V5MSIsInVzZXJWYWx1ZTEiLCJ1c2VyS2V5MiI6InVzZXJWYWx1ZTIifQ==`.
+
+## Recommendations of language APIs
+Recommended implementation in different language API.
+
+1. `CorrelationContext#set` and `CorrelationContext#get` are recommended to write and read the correlation context, with key/value string.
+1. The key should be added if it is inexistence.
+1. The later the write should override the prev value.
+1. The number of all keys should less than 3, and the length of each value should be less than 128 bytes.
Review comment:
```suggestion
1. The total number of all keys should be less than 3, and the length of each value should be less than 128 bytes.
```
----------------------------------------------------------------
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