Zipkin receiver in SkyWalking collector #1273

wu-sheng · 2018-05-28T07:57:38Z

Dear @peng-yongsheng @ascrutae @candyleer @liuhaoyang @adriancole @basvanbeek @jcchavezs,

I am working on this pull request, which is about receiving and analysis Zipkin trace. I paste the REAMDE doc of this module at here, to give everyone a brief:

Zipkin receiver

Zipkin receiver provides the feature to receive span data from Zipkin instrumented applications. SkyWalking backend provides
analysis, aggregation and visualization. So the user will not need to learn how SkyWalking auto instrumentation
agents(Java, .NET, node.js) work, or they don't want to change for some reasons, such as Zipkin integration has been completed.

Zipkin receiver is only an optional features in SkyWalking, even now it is an incubating feature.

Limits

As an incubating feature, it is a prototype. So it has following limits:

Don't try to use SkyWalking native agents and Zipkin's libs in the same distributed system. Considering HEADERs of Zipkin and SkyWalking aren't shared/interoperable, their two will not propagate context for each other. Trace will not continue.
Don't support cluster mode.
Analysis based on trace will be finished in the certain and given duration. The default assumption is 2 min most. SkyWalking used more complex header and context to avoid this in analysis stage.

Right now(28th, May.), I just finished my codes with following features:

Open and listen /api/v2/spans service
Receive and deserialize spans from existed openzipkin/sleuth-webmvc-example.
Use caffeine cache implementor to organize all spans into a trace.
Use cache expired mechanism, assume trace can be analysis x(setting) mins after last span of the certain traceid reported.

I want to ask any one who has time, interest and is familiar with Zipkin format, especially @adriancole @basvanbeek @jcchavezs , to check whether I miss anything for a zipkin v2 json format.

The next I am going to do is:

After trace finished, analysis the whole trace(spans), transfer them to TraceSegment based on its tree structure and localendpoint/serviceName as application code.

For milestone, in beta2, I will definitely consider this as an incubating feature only. Wait for me or someone else to change the local cache implementor to Redis(cluster) based in 5.1.x series, maybe.

…put some doc for it.

coveralls · 2018-05-28T08:26:59Z

Coverage increased (+0.5%) to 24.463% when pulling 8798e2e on zipkin/receiver-v1 into da78420 on master.

peng-yongsheng

Where is the dequeue coding for the span cache?

peng-yongsheng · 2018-05-28T14:11:49Z

...ava/org/apache/skywalking/apm/collector/receiver/zipkin/provider/ZipkinReceiverProvider.java

+ *
+ */
+
+/*


Duplicate apache license header.

peng-yongsheng · 2018-05-28T14:42:18Z

...a/org/apache/skywalking/apm/collector/receiver/zipkin/provider/handler/SpanJettyHandler.java

+
+            List<ZipkinSpan> spans = gson.fromJson(br, spanListType);
+            spans.forEach(span ->
+                CacheFactory.INSTANCE.get(config).addSpan(span)


INSTANCE variable was synchronized -> addSpan method is single threaded -> SpanJettyHandler is single threaded.

Is my understanding correct?

wu-sheng · 2018-05-28T14:55:40Z

@peng-yongsheng This pr is Work In Progress. Open this to keep you posted. Transform codes are still on going.

When you process data in stack, you didn't bother concurrency issue when you use local variables. For cached trace and spans, #AddSpan method has a reentrancelock to protect.

codefromthecrypt

thanks for scoping this out. main comment is maybe re-use the zipkin main jar as it is less than 200KiB and has no dependencies. This can provide the model and codec operations. Maybe this can help? https://github.com/openzipkin/zipkin/blob/master/zipkin-junit/src/main/java/zipkin/junit/ZipkinDispatcher.java#L88

codefromthecrypt · 2018-05-28T23:16:48Z

...a/org/apache/skywalking/apm/collector/receiver/zipkin/provider/handler/SpanJettyHandler.java

+        try {
+            BufferedReader br = req.getReader();
+
+            List<ZipkinSpan> spans = gson.fromJson(br, spanListType);


Curious.. instead of defining the same model here and also gson decoding.. why not depend on io.zipkin.zipkin2:zipkin and use SpanBytesDecoder? you will get proto3 format for free this way

@adriancole I will try to switch to it today. I just don't know, haha. Sorry.

…ancole

…ancole

@peng-yongsheng

… and change the hierarchy tree. @peng-yongsheng

wu-sheng · 2018-05-29T02:04:41Z

thanks for scoping this out. main comment is maybe re-use the zipkin main jar as it is less than 200KiB and has no dependencies. This can provide the model and codec operations. Maybe this can help? https://github.com/openzipkin/zipkin/blob/master/zipkin-junit/src/main/java/zipkin/junit/ZipkinDispatcher.java#L88

@adriancole Done. I am moving on.

wu-sheng · 2018-05-29T02:07:02Z

Where is the dequeue coding for the span cache?

I am not using any explicit or manual dequeue. I am using the expired mechanism. See CaffeineSpanCache#onRemoval. When we move the in memory buffer to Redis. this mechanism could stay, but only active the in-cache/remove-from-cache by root span, which will make the cluster mode works.

…nse text. All Apache 2.0 LICENSE files will be removed before beta2 release.

codefromthecrypt

so far so good

…OsInfo instead of String. 2. Try to regester application code and instance id in receiving spans.

jcchavezs · 2018-05-29T10:53:39Z

apm-collector/apm-collector-boot/src/main/resources/application.yml

+    thermodynamicCountOfResponseTimeSteps: 40
+# TODO: receiver_zipkin need to remove before merge, and only provide this in document.
+receiver_zipkin:


Is it necessary to pass this information being structured? Can't you just pass the endpoint (e.g. endpoint: http://localhost:9411/api/v2/spans as we usually do for zipkin? cc @adriancole @wu-sheng

Which parts do you refer? receiver_zipkin?

@jcchavezs I guess you mean, ip, port, context path, servlet mapping path, etc. That is because I don't use an URL util to analysis the endpoint string. Does Zipkin do this by providing an URL endpoint only? If this is a tradition, I can follow that.

codefromthecrypt · 2018-05-29T21:39:53Z

we pass the full url when specifying config for clients (in case it is remapped by a proxy). since this is a server suppose it doesn't matter. for example zipkin's server doesn't use a url style to indicate the listen info, rather it allows you only to change the port. how you handle things here is up to you I think.

… analysis module. Wait for further tests.

…improve in another PR.

wu-sheng · 2018-06-15T09:08:13Z

I finish the prototype of this feature. Here is how people to test this:

Download and build the project.
Find application.yml in apm-collector-boot module, and open receiver_zipkin section in the yaml.
Install ElasticSearch 5.x(5.5 or 5.6), change the elasticsearch.yml like this: https://gist.github.com/wu-sheng/4f869fb465bbd15f0852fa923357b4a6
Find CollectorBootStartUp#main also in apm-collector-boot module, and startup.
Follow build document to package the source codes in this branch.
Find release in dist folder and enter bin folder to start up webapp by webappService.sh.
Access the Sleuth example.

Right now, I use Caffeine as an expire checker, and it doesn't work well. So just ignore the following errors. Just access Spring Sleuth more than once. I usually set a timer in browser. I intend to replace Caffeine.

java.lang.NullPointerException: null
	at org.apache.skywalking.apm.collector.receiver.zipkin.provider.transform.SegmentBuilder.buildRef(SegmentBuilder.java:235) ~[classes/:?]
	at org.apache.skywalking.apm.collector.receiver.zipkin.provider.transform.SegmentBuilder.initSpan(SegmentBuilder.java:166) ~[classes/:?]
	at org.apache.skywalking.apm.collector.receiver.zipkin.provider.transform.SegmentBuilder.scanSpansFromRoot(SegmentBuilder.java:126) ~[classes/:?]
	at org.apache.skywalking.apm.collector.receiver.zipkin.provider.transform.SegmentBuilder.build(SegmentBuilder.java:89) ~[classes/:?]
	at org.apache.skywalking.apm.collector.receiver.zipkin.provider.transform.Zipkin2SkyWalkingTransfer.transfer(Zipkin2SkyWalkingTransfer.java:59) ~[classes/:?]
	at org.apache.skywalking.apm.collector.receiver.zipkin.provider.cache.caffeine.CaffeineSpanCache.onRemoval(CaffeineSpanCache.java:77) ~[classes/:?]
	at org.apache.skywalking.apm.collector.receiver.zipkin.provider.cache.caffeine.CaffeineSpanCache.onRemoval(CaffeineSpanCache.java:45) ~[classes/:?]
	at com.github.benmanes.caffeine.cache.BoundedLocalCache.lambda$notifyRemoval$1(BoundedLocalCache.java:286) ~[caffeine-2.6.2.jar:?]
	at java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(ForkJoinTask.java:1402) [?:1.8.0_91]
	at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:289) [?:1.8.0_91]
	at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1056) [?:1.8.0_91]
	at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1692) [?:1.8.0_91]
	at java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:157) [?:1.8.0_91]

wu-sheng · 2018-06-15T09:28:16Z

I tested and only tested by basic sleuth example: https://github.com/openzipkin/sleuth-webmvc-example

So, I intend to merge this if this is passed. Here are my screenshots.

Then I will work on more about new expire mechanism, cluster mode and more zipkin examples.

…tor-skywalking into zipkin/receiver-v1

codefromthecrypt · 2018-06-15T14:06:02Z

first cut works.. well done! I'll review closely more easily later as my workspace is all sorted etc

wu-sheng · 2018-06-15T14:19:26Z

first cut works.. well done! I'll review closely more easily later as my workspace is all sorted etc

Yes. Do a lot of assumptions in the transfer. Need Zipkin community to confirm. Of course, we can improve them in the further test. I will try more cases :)

peng-yongsheng · 2018-06-15T15:08:46Z

...java/org/apache/skywalking/apm/collector/receiver/zipkin/provider/handler/SpanProcessor.java

+            if (applicationCode != null) {
+                int applicationId = registerServices.getApplicationIDService().getOrCreateForApplicationCode(applicationCode);
+                if (applicationId != 0) {
+                    registerServices.getOrCreateApplicationInstanceId(applicationId, applicationCode);


Use applicationCode instead of UUID will let all the instance in same application to be a single one.

Yes, but do you have new idea? Zipkin has no specific tag for instance. No such concept.

codefromthecrypt · 2018-06-19T02:41:39Z

in general, things not defined (which many things are not) simply go into the relevant namespace. ex skywalking.application-id or similar

wu-sheng added 6 commits May 26, 2018 10:19

Add zipkin receiver module

9840be5

Open HTTP service at zipkin tranditional port, and context path. And …

803d91a

…put some doc for it.

Update description.

ca05693

Add the link to zipkin.io

5045458

Make zipkin sdk connected.

ef087e4

Finish zipkin trace cache and finish mechanism(based on timeout)

b6e2189

wu-sheng added backend OAP backend related. feature New feature labels May 28, 2018

wu-sheng added this to the 5.0.0-beta2 milestone May 28, 2018

wu-sheng requested review from liuhaoyang, ascrutae and peng-yongsheng May 28, 2018 07:57

Add a TODO reminder in yml. Max cache size setting.

50ac8df

peng-yongsheng reviewed May 28, 2018

View reviewed changes

codefromthecrypt reviewed May 28, 2018

View reviewed changes

wu-sheng added 2 commits May 29, 2018 09:44

Replace my span data structure by zipkin native lib (zipkin2) , @adri…

84e100d

…ancole

I refactor the JettyHandler, move the base class to JettyJsonHandler,…

80fc153

… and change the hierarchy tree. @peng-yongsheng

Add zipkin LICENSE in distribution LICENSE file. But no need the lice…

9e6044c

…nse text. All Apache 2.0 LICENSE files will be removed before beta2 release.

codefromthecrypt reviewed May 29, 2018

View reviewed changes

wu-sheng added 4 commits May 29, 2018 14:54

1. Refactor IInstanceIDService#getOrCreateByAgentUUID to accept Agent…

2768dd8

…OsInfo instead of String. 2. Try to regester application code and instance id in receiving spans.

Finish the stack core context for segment build.

f572f65

Fix ci.

482bfd4

Reformat.

72114c2

jcchavezs reviewed May 29, 2018

View reviewed changes

Make Caffeine works like an auto expire queue. Not elegance, but work.

4792e56

wu-sheng added 4 commits June 15, 2018 10:42

Refactor trace transfer listener, and send the skywalking segments to…

7394f21

… analysis module. Wait for further tests.

Save, analysis, aggregate segment without error, now.

9f6956d

Finish baseline codes of zipkin receiver. Prepare for documents. And …

7a4962b

…improve in another PR.

Update document and remove TODO

8c19c1a

wu-sheng added 2 commits June 15, 2018 17:08

Merge branch 'master' into zipkin/receiver-v1

45f2b8b

Fix test case.

fdf2130

wu-sheng added 2 commits June 15, 2018 17:32

Remove an assert which wouldn't work in CI or other env.

6facd18

Merge branch 'zipkin/receiver-v1' of https://github.com/apache/incuba…

76f75f7

…tor-skywalking into zipkin/receiver-v1

wu-sheng changed the title ~~[WIP]Zipkin receiver in SkyWalking collector~~ Zipkin receiver in SkyWalking collector Jun 15, 2018

wu-sheng mentioned this pull request Jun 15, 2018

[Tracer Interop] Google OpenCensus / OpenZipkin #665

Closed

peng-yongsheng reviewed Jun 15, 2018

View reviewed changes

Merge branch 'master' into zipkin/receiver-v1

8d08b97

ascrutae previously approved these changes Jun 16, 2018

View reviewed changes

This was referenced Jun 16, 2018

Replace Caffeine with new expired mechanism in zipkin receiver #1358

Closed

Run more tests in Zipkin examples #1359

Closed

Update README.md

5da5efb

wu-sheng dismissed ascrutae’s stale review via 5da5efb June 16, 2018 13:01

wu-sheng added 2 commits June 16, 2018 21:03

Update README_ZH.md

d8c76d5

Update README.md

7d47def

ascrutae approved these changes Jun 17, 2018

View reviewed changes

peng-yongsheng approved these changes Jun 18, 2018

View reviewed changes

Merge branch 'master' into zipkin/receiver-v1

8798e2e

wu-sheng merged commit ecc0f94 into master Jun 18, 2018

wu-sheng deleted the zipkin/receiver-v1 branch June 18, 2018 15:36

wu-sheng mentioned this pull request Jul 2, 2018

Zipkin and Jaeger are supported in Zipkin formats openapm/landscape-model#36

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Zipkin receiver in SkyWalking collector #1273

Zipkin receiver in SkyWalking collector #1273

wu-sheng commented May 28, 2018

coveralls commented May 28, 2018 •

edited

peng-yongsheng left a comment

peng-yongsheng May 28, 2018

wu-sheng May 28, 2018

peng-yongsheng May 28, 2018

wu-sheng commented May 28, 2018

codefromthecrypt left a comment

codefromthecrypt May 28, 2018

wu-sheng May 28, 2018

wu-sheng commented May 29, 2018

wu-sheng commented May 29, 2018

codefromthecrypt left a comment

jcchavezs May 29, 2018

wu-sheng May 29, 2018

wu-sheng May 29, 2018

codefromthecrypt commented May 29, 2018 via email

wu-sheng commented Jun 15, 2018 •

edited

wu-sheng commented Jun 15, 2018

codefromthecrypt commented Jun 15, 2018

wu-sheng commented Jun 15, 2018

peng-yongsheng Jun 15, 2018

wu-sheng Jun 15, 2018

codefromthecrypt commented Jun 19, 2018 via email

+               *
+               */
+              /*

Zipkin receiver in SkyWalking collector #1273

Zipkin receiver in SkyWalking collector #1273

Conversation

wu-sheng commented May 28, 2018

Zipkin receiver

Limits

coveralls commented May 28, 2018 • edited

peng-yongsheng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wu-sheng commented May 28, 2018

codefromthecrypt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wu-sheng commented May 29, 2018

wu-sheng commented May 29, 2018

codefromthecrypt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codefromthecrypt commented May 29, 2018 via email

wu-sheng commented Jun 15, 2018 • edited

wu-sheng commented Jun 15, 2018

codefromthecrypt commented Jun 15, 2018

wu-sheng commented Jun 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codefromthecrypt commented Jun 19, 2018 via email

coveralls commented May 28, 2018 •

edited

wu-sheng commented Jun 15, 2018 •

edited