Adaptation enhancements for various features in v3 #3573
Conversation
zipkin-server/server-starter/src/main/java/zipkin/server/config/ApplicationConfigLoader.java
Show resolved
Hide resolved
...ver/health-query-plugin/src/main/java/zipkin/server/query/health/ZipkinHealthController.java
Outdated
Show resolved
Hide resolved
| private String restHost; | ||
| private int restPort; | ||
| private String restContextPath; | ||
| private int restMaxThreads = 200; | ||
| private long restIdleTimeOut = 30000; |
There was a problem hiding this comment.
What does this removing mean? Could you explain the diff between receiver-zipkin-http and http-query-plugin. They seem to be consufed.
There was a problem hiding this comment.
First, in the v2 version of Zipkin, they share the same HTTP port when receiver and query. So I moved the HTTP configuration in the HTTP Receiver and Querier into the core.
The receiver is used to collect spans from agents, and the Query is used to query the span for UI.
There was a problem hiding this comment.
I think that is how sharing server works. The receiver is still able to work on the separate port, especially query port is exposed for the internet, I assume a separate port is better.
Could you check the OAP sharing server and take a reference to optimize the receiver?
There was a problem hiding this comment.
I have separated the export ports for the receiver and querier modules (query span and health). Now, they can utilize the default HTTP server in the core module if the port is defined as -1. Otherwise, the module will start a new HTTP server and use it.
| @@ -0,0 +1,19 @@ | |||
| # | |||
| # Licensed to the Apache Software Foundation (ASF) under one or more | |||
There was a problem hiding this comment.
The header is somehow wrong, please fix it. And consider skywalking-eyes to verify headers. Notice, I am not sure year range is supported or not.
There was a problem hiding this comment.
The header is somehow wrong, please fix it. And consider skywalking-eyes to verify headers.
I will create another PR to fix all the files that I created.
Notice, I am not sure year range is supported or not.
I remember It's not supported, as JC says, we don't need to align the year, so, we don't need to check the year. Alternatively, maybe we can only check the files involved in the Pull Request. However, I'm not sure if this is possible.
There was a problem hiding this comment.
I remember It's not supported, as #3567 (comment), we don't need to align the year,
Not that. I mean I don't know whether skywalking-eyes support to check various start-end year range in all file headers.
There was a problem hiding this comment.
Got it, The eyes could verify the header by regex, Therefore, I believe it is capable of verifying the year range.
In this Pull Request, I have made several adaptation enhancements for the following features in the v3: