Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initializing to use SkyWalking to replace Zipkin Server in v3 #3564

Merged
merged 2 commits into from
Sep 7, 2023

Conversation

mrproliu
Copy link
Contributor

@mrproliu mrproliu commented Sep 6, 2023

In the upcoming release of Zipkin v3, we are attempting to replace Zipkin Server with SkyWalking OAP. In this PR, I mainly removed the Storage and Collector involved in Zipkin and replaced the Zipkin Server with SkyWalking OAP for data collection and querying. This is a primary functionality replacement, and later on, I will submit more PRs to reintroduce all the functionalities of Zipkin.

@mrproliu mrproliu marked this pull request as draft September 6, 2023 12:16
@mrproliu mrproliu changed the title [WIP] Initializing to use SkyWalking to replace Zipkin Server in v3 Initializing to use SkyWalking to replace Zipkin Server in v3 Sep 6, 2023
@mrproliu mrproliu marked this pull request as ready for review September 6, 2023 12:22
@mrproliu mrproliu added the server label Sep 6, 2023
@jcchavezs
Copy link
Contributor

Instead of removing .github/workflows/test.yml I'd rather comment it out, we need guarantees that the new server will work as good as the old and adapt the tests to guarantee that.

@jcchavezs
Copy link
Contributor

Also at some point we should make sure the quickstart.sh script works so we can integrate it in https://github.com/openzipkin/zipkin-php-example/blob/master/.circleci/config.yml which runs a nightly e2e test.

@mrproliu
Copy link
Contributor Author

mrproliu commented Sep 6, 2023

Instead of removing .github/workflows/test.yml I'd rather comment it out, we need guarantees that the new server will work as good as the old and adapt the tests to guarantee that.

Go it! However, since storage and receiver are both in the skywalking repo, I will write some IT specifically to complete the testing in another PR.

@mrproliu
Copy link
Contributor Author

mrproliu commented Sep 6, 2023

Also at some point we should make sure the quickstart.sh script works so we can integrate it in https://github.com/openzipkin/zipkin-php-example/blob/master/.circleci/config.yml which runs a nightly e2e test.

Of course, by reading quickstart.sh, I found that it will download the latest exec.jar from the Maven repository and run it. According to the current logic, it should be the same. It's just that it hasn't been released to the Maven repository yet (in v3 branch), so merging PRs won't trigger the behavior of uploading to the Maven repository.

@wu-sheng
Copy link
Member

wu-sheng commented Sep 6, 2023

I want to confirm, as we are using a submodule that locks to skywaling upstream version. Actually, are we using the skywalking dev version mostly as the codebase?
I support this as it provides Zipkin side release more flexibly.

@mrproliu
Copy link
Contributor Author

mrproliu commented Sep 6, 2023

Yes, currently it is introducing the latest SHA of Skywalking as a submodule.

@marcingrzejszczak
Copy link

Hey, out of curiosity is there any backlog of why this is happening or other steps for Zipkin v3? Also, maybe this could be added to v3 #3561 in a better way?

@basvanbeek
Copy link
Member

There have been discussions within the Zipkin admin group about future support for the existing server and the bandwidth that we have to keep v2 up and running. With SkyWalking fully supporting the Zipkin datamodel and API's (both ingestion and query) and working well with the Zipkin Lens UI, it is suitable as a drop-in replacement. It also is actively maintaining a multitude of storage solutions as well as having OTLP tracing data support since version 9.6.0.

Since we've collaborated closely with the SkyWalking community over the last few years always in good faith and lots of good energy, this seemed to us the most logical step forward. This is not at risk to be a PR and abandon effort. We actually have site owners and companies backing this v3 effort to provide Zipkin with a modern server component which will be actively maintained for the coming years.

The upstreaming effort is happening in a separate branch until ready for prime time. We encourage all Zipkin stakeholders to provide input on encountered issues, what needs to be supported, what future enhancements might be on the horizon. We will provide a timeline and call to action on the main site as well as the server's Github repo so it will not go unnoticed.

@marcingrzejszczak
Copy link

That sounds interesting! So that way my PR for OTLP ingress can be dropped unless we want it to be there for Zipkin 2.

So what will remain there in Zipkin 3 codebase if it gets replaced with SkyWalking?

Copy link
Member

@basvanbeek basvanbeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excited to see this become ready for stable / production usage.

@basvanbeek
Copy link
Member

That sounds interesting! So that way my PR for OTLP ingress can be dropped unless we want it to be there for Zipkin 2.

I think since we will not want to switch too quickly from marking v2 deprecated and v3 stable, it still makes sense to add this PR to v2. Since I lack bandwidth the next few weeks I do hope that one of the other maintainers can validate and review. FYA @openzipkin/core

So what will remain there in Zipkin 3 codebase if it gets replaced with SkyWalking?

The v3 distribution will combine core SkyWalking around tracing, the Zipkin v2 API's (both query and ingestion) and bundle the latest Zipkin Lens UI which will remain under active development. Internals will be quite new but user facing API's will remain the same.

The key thing to understand is that when upgrading from v2 to v3 you will not be able to automatically migrate your tracing data as the storage data schemas are not the same. Unless someone volunteers we have no plans to do schema migrations for any of the storage modules. Since Zipkin data typically has a short retention span we expect site owners to keep the old Zipkin deployment active until all data is purged in case they must keep the data during the retention period. It also means that any custom cold storage pipelines need adjustment on their inputs to the new schemas.

@wu-sheng we should publish the storage schemas for the various storage options we'll have with v3.

@wu-sheng
Copy link
Member

wu-sheng commented Sep 7, 2023

we should publish the storage schemas for the various storage options we'll have with v3.

Make sense. Do you have previous doc about this for v2? We could follow that path. If there isn't, where do you suggest to put?

@wu-sheng
Copy link
Member

wu-sheng commented Sep 7, 2023

Merging the PR to continue the further steps. We could continue the docs relative discussion here :)

@wu-sheng wu-sheng merged commit 47e6428 into zipkin-v3 Sep 7, 2023
@wu-sheng wu-sheng deleted the sw-core branch September 7, 2023 00:40
@wu-sheng wu-sheng added this to the 3.0.0 milestone Sep 7, 2023
@shakuzen
Copy link
Member

shakuzen commented Sep 8, 2023

Maybe I don't understand all the context and implications here. What is the difference between publishing a Zipkin Server v3 that embeds Skywalking server versus we stop publishing new versions of Zipkin Server and tell users to use Skywalking server (or whatever else they want) instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants