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/01/08 06:54:27 UTC
[GitHub] [skywalking] JaredTan95 opened a new pull request #4197: add token
authentication between agent and oap receiver.
JaredTan95 opened a new pull request #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197
Please answer these questions before submitting pull request
- Why submit this pull request?
- [ ] Bug fix
- [ ] New feature provided
- [ ] Improve performance
- Related issues
https://github.com/apache/skywalking/issues/4182
___
### New feature or improvement
- Describe the details and related test reports.
Agent auth provided in Skywalking 5.x version, and I take it to 7.x version.
----------------------------------------------------------------
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
#4197: add token authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#discussion_r364103481
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/server/auth/AuthenticationHandler.java
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.oap.server.core.server.auth;
+
+import io.grpc.BindableService;
+import io.grpc.Metadata;
+import io.grpc.ServerCall;
+import io.grpc.ServerCallHandler;
+import io.grpc.ServerInterceptor;
+import io.grpc.ServerInterceptors;
+import io.grpc.Status;
+import org.apache.skywalking.apm.util.StringUtil;
+import org.apache.skywalking.oap.server.core.server.GRPCHandlerRegister;
+import org.apache.skywalking.oap.server.library.server.grpc.GRPCServer;
+
+/**
+ * Active the authentication(agent <---> oap) token checker if expected token exists in application.yml
+ *
+ * @author wusheng, jian.tan
+ */
+public enum AuthenticationHandler {
+ INSTANCE;
+
+ private static final Metadata.Key<String> AUTH_HEAD_HEADER_NAME = Metadata.Key.of("Authentication", Metadata.ASCII_STRING_MARSHALLER);
+
+ private String expectedToken = "";
+
+ public void build(GRPCServer grpcHandler, BindableService targetService) {
+ if (StringUtil.isNotEmpty(expectedToken)) {
+ grpcHandler.addHandler(ServerInterceptors.intercept(targetService, new ServerInterceptor() {
+ @Override
+ public <REQ, RESP> ServerCall.Listener<REQ> interceptCall(ServerCall<REQ, RESP> serverCall,
+ Metadata metadata,
+ ServerCallHandler<REQ, RESP> next) {
+ String token = metadata.get(AUTH_HEAD_HEADER_NAME);
+ if (expectedToken.equals(token)) {
+ return next.startCall(serverCall, metadata);
+ } else {
+ serverCall.close(Status.PERMISSION_DENIED, new Metadata());
+ return new ServerCall.Listener() {
+ };
Review comment:
Extract this listener to a field member instead of creating one for every call
----------------------------------------------------------------
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 a change in pull request #4197:
add token authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#discussion_r364218437
##########
File path: oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/server/GRPCHandlerRegisterImpl.java
##########
@@ -33,7 +35,7 @@ public GRPCHandlerRegisterImpl(GRPCServer server) {
}
@Override public void addHandler(BindableService handler) {
- server.addHandler(handler);
+ AuthenticationFilter.INSTANCE.build(server, handler);
Review comment:
I think you should add a method called
> @Override public void addFilter()
Then in provider, you should add a filter, add(AuthenticationFilter), first. Then calling addHanlder will make those filters active.
The current implementation is working, but the codes look strange.
----------------------------------------------------------------
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] JaredTan95 commented on a change in pull request
#4197: add token authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
JaredTan95 commented on a change in pull request #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#discussion_r365579674
##########
File path: oap-server/server-receiver-plugin/skywalking-sharing-server-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/sharing/server/ReceiverGRPCHandlerRegister.java
##########
@@ -18,22 +18,39 @@
package org.apache.skywalking.oap.server.receiver.sharing.server;
-import io.grpc.*;
+import io.grpc.BindableService;
+import io.grpc.ServerInterceptor;
+import io.grpc.ServerInterceptors;
+import io.grpc.ServerServiceDefinition;
+import java.util.Objects;
import lombok.Setter;
import org.apache.skywalking.oap.server.core.server.GRPCHandlerRegister;
/**
- * @author peng-yongsheng
+ * @author peng-yongsheng, jian.tan
*/
public class ReceiverGRPCHandlerRegister implements GRPCHandlerRegister {
@Setter private GRPCHandlerRegister grpcHandlerRegister;
+ private ServerInterceptor interceptor;
@Override public void addHandler(BindableService handler) {
- grpcHandlerRegister.addHandler(handler);
+ if (Objects.isNull(interceptor)) {
+ grpcHandlerRegister.addHandler(handler);
+ } else {
+ grpcHandlerRegister.addHandler(handlerInterceptorBind(handler, interceptor));
+ }
Review comment:
Token authentication filter should only be activated in sharding server module. Moving to `GRPCServer` may not very suitable, in my thought.
----------------------------------------------------------------
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 #4197: add token
authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#issuecomment-571951483
> AuthenticationHandler is actived through the sharing server, but after that, it could be working in core config. But if there is ip:port configuration in sharing server module, you will active the auth in core level, then server-internal communication will be blocked, the cluster will fail. Please fix this.
@kezhenxu94 Do you recheck 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] kezhenxu94 commented on issue #4197: add token
authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on issue #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#issuecomment-571964183
> > AuthenticationHandler is actived through the sharing server, but after that, it could be working in core config. But if there is ip:port configuration in sharing server module, you will active the auth in core level, then server-internal communication will be blocked, the cluster will fail. Please fix this.
>
> @kezhenxu94 Do you recheck this?
will check later
----------------------------------------------------------------
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 #4197: add token
authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#issuecomment-571957379
> I tested in my local, and i did not found org.apache.skywalking.oap.server.core.remote.RemoteServiceHandler instance step into org.apache.skywalking.oap.server.core.server.auth.AuthenticationHandler, do i miss something?
@JaredTan95 Looking at the codes, it should be. Another thing is, the auth should be a filter, rather than a handler. Otherwise, how are you sure the auth handler would execute before the other receiver handler?
----------------------------------------------------------------
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 #4197: add token
authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#issuecomment-573333063
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=h1) Report
> Merging [#4197](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/5625d4004c36329875f443e9f1fe08a434c0c928?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4197 +/- ##
=======================================
Coverage 26.45% 26.45%
=======================================
Files 1179 1179
Lines 25794 25794
Branches 3751 3751
=======================================
Hits 6823 6823
Misses 18359 18359
Partials 612 612
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4197?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/4197?src=pr&el=footer). Last update [5625d40...9480420](https://codecov.io/gh/apache/skywalking/pull/4197?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
#4197: add token authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#discussion_r364104603
##########
File path: docs/en/setup/backend/backend-token-auth.md
##########
@@ -0,0 +1,44 @@
+# Token Authentication
+## Supported version
+7.0.0+
+
+## How need token authentication after we have TLS?
+TLS is about transport security, which makes sure the network can be trusted.
+The token authentication is about monitoring application data **can be trusted**.
+
+## Token
+In current version, Token is considered as a simple string.
+
+### Set Token
+1. Set token in agent.config file
+```properties
+# Authentication active is based on backend setting, see application.yml for more details.
+agent.authentication = ${SW_AGENT_AUTHENTICATION:xxxx}
+```
+
+2. Set token in `application.yml` file
+```yaml
+······
+receiver-sharing-server:
+ default:
+ authentication: ${SW_AUTHENTICATION:""}
+······
+```
+
+## Authentication fails
+The Skywalking OAP verifies every request from agent, allowed only the token match.
Review comment:
```suggestion
The Skywalking OAP verifies every request from agent, only allows requests whose token matches the one configured in `application.yml`.
```
----------------------------------------------------------------
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] JaredTan95 removed a comment on issue #4197: add token
authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
JaredTan95 removed a comment on issue #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#issuecomment-571954373
> 1. server-internal communication will be blocked, the cluster will fail. Please fix this.
I'm not sure this can avoid it:
```
if (!(targetService instanceof RemoteServiceHandler) && StringUtil.isNotEmpty(expectedToken)) {}
```
----------------------------------------------------------------
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 #4197: add token
authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#issuecomment-573333063
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@50aea09`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `36.42%`.
[](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4197 +/- ##
=========================================
Coverage ? 26.42%
=========================================
Files ? 1180
Lines ? 25859
Branches ? 3763
=========================================
Hits ? 6832
Misses ? 18422
Partials ? 605
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [.../plugin/elasticsearch/query/MetricsQueryEsDAO.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoLXBsdWdpbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL3N0b3JhZ2UvcGx1Z2luL2VsYXN0aWNzZWFyY2gvcXVlcnkvTWV0cmljc1F1ZXJ5RXNEQU8uamF2YQ==) | `0% <0%> (ø)` | |
| [...ap/server/exporter/provider/grpc/GRPCExporter.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9leHBvcnRlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2V4cG9ydGVyL3Byb3ZpZGVyL2dycGMvR1JQQ0V4cG9ydGVyLmphdmE=) | `77.77% <0%> (ø)` | |
| [...server/core/storage/annotation/ValueColumnIds.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvc3RvcmFnZS9hbm5vdGF0aW9uL1ZhbHVlQ29sdW1uSWRzLmphdmE=) | `0% <0%> (ø)` | |
| [.../storage/plugin/jdbc/h2/dao/H2MetricsQueryDAO.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1qZGJjLWhpa2FyaWNwLXBsdWdpbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL3N0b3JhZ2UvcGx1Z2luL2pkYmMvaDIvZGFvL0gyTWV0cmljc1F1ZXJ5REFPLmphdmE=) | `0% <0%> (ø)` | |
| [...alking/oap/server/core/query/entity/IntValues.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvcXVlcnkvZW50aXR5L0ludFZhbHVlcy5qYXZh) | `0% <0%> (ø)` | |
| [...king/oap/server/core/query/MetricQueryService.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvcXVlcnkvTWV0cmljUXVlcnlTZXJ2aWNlLmphdmE=) | `0% <0%> (ø)` | |
| [...alking/oap/query/graphql/resolver/MetricQuery.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcXVlcnktcGx1Z2luL3F1ZXJ5LWdyYXBocWwtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9xdWVyeS9ncmFwaHFsL3Jlc29sdmVyL01ldHJpY1F1ZXJ5LmphdmE=) | `0% <0%> (ø)` | |
| [...king/oap/server/core/alarm/provider/Threshold.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9UaHJlc2hvbGQuamF2YQ==) | `100% <100%> (ø)` | |
| [...p/server/core/alarm/provider/MetricsValueType.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9NZXRyaWNzVmFsdWVUeXBlLmphdmE=) | `100% <100%> (ø)` | |
| [...ng/oap/server/core/alarm/provider/RunningRule.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9SdW5uaW5nUnVsZS5qYXZh) | `65.9% <44%> (ø)` | |
| ... and [1 more](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4197?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/4197?src=pr&el=footer). Last update [50aea09...d2916a8](https://codecov.io/gh/apache/skywalking/pull/4197?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 #4197: add token
authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#issuecomment-571954937
You should support add extra handler out of the core, then sharing server module could add the auth by itself.
----------------------------------------------------------------
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 a change in pull request #4197:
add token authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#discussion_r364215446
##########
File path: oap-server/server-bootstrap/src/main/resources/application.yml
##########
@@ -98,33 +98,33 @@ storage:
# segmentQueryMaxSize: ${SW_STORAGE_ES_QUERY_SEGMENT_SIZE:200}
# profileTaskQueryMaxSize: ${SW_STORAGE_ES_QUERY_PROFILE_TASK_SIZE:200}
# advanced: ${SW_STORAGE_ES_ADVANCED:""}
- elasticsearch7:
- nameSpace: ${SW_NAMESPACE:""}
- clusterNodes: ${SW_STORAGE_ES_CLUSTER_NODES:localhost:9200}
- protocol: ${SW_STORAGE_ES_HTTP_PROTOCOL:"http"}
- #trustStorePath: ${SW_SW_STORAGE_ES_SSL_JKS_PATH:"../es_keystore.jks"}
- #trustStorePass: ${SW_SW_STORAGE_ES_SSL_JKS_PASS:""}
- user: ${SW_ES_USER:""}
- password: ${SW_ES_PASSWORD:""}
- indexShardsNumber: ${SW_STORAGE_ES_INDEX_SHARDS_NUMBER:2}
- indexReplicasNumber: ${SW_STORAGE_ES_INDEX_REPLICAS_NUMBER:0}
- # Those data TTL settings will override the same settings in core module.
- recordDataTTL: ${SW_STORAGE_ES_RECORD_DATA_TTL:7} # Unit is day
- otherMetricsDataTTL: ${SW_STORAGE_ES_OTHER_METRIC_DATA_TTL:45} # Unit is day
- monthMetricsDataTTL: ${SW_STORAGE_ES_MONTH_METRIC_DATA_TTL:18} # Unit is month
- # Batch process setting, refer to https://www.elastic.co/guide/en/elasticsearch/client/java-api/5.5/java-docs-bulk-processor.html
- bulkActions: ${SW_STORAGE_ES_BULK_ACTIONS:1000} # Execute the bulk every 1000 requests
- flushInterval: ${SW_STORAGE_ES_FLUSH_INTERVAL:10} # flush the bulk every 10 seconds whatever the number of requests
- concurrentRequests: ${SW_STORAGE_ES_CONCURRENT_REQUESTS:2} # the number of concurrent requests
- resultWindowMaxSize: ${SW_STORAGE_ES_QUERY_MAX_WINDOW_SIZE:10000}
- metadataQueryMaxSize: ${SW_STORAGE_ES_QUERY_MAX_SIZE:5000}
- segmentQueryMaxSize: ${SW_STORAGE_ES_QUERY_SEGMENT_SIZE:200}
- advanced: ${SW_STORAGE_ES_ADVANCED:""}
-# h2:
-# driver: ${SW_STORAGE_H2_DRIVER:org.h2.jdbcx.JdbcDataSource}
-# url: ${SW_STORAGE_H2_URL:jdbc:h2:mem:skywalking-oap-db}
-# user: ${SW_STORAGE_H2_USER:sa}
-# metadataQueryMaxSize: ${SW_STORAGE_H2_QUERY_MAX_SIZE:5000}
+# elasticsearch7:
+# nameSpace: ${SW_NAMESPACE:""}
+# clusterNodes: ${SW_STORAGE_ES_CLUSTER_NODES:localhost:9200}
+# protocol: ${SW_STORAGE_ES_HTTP_PROTOCOL:"http"}
+# #trustStorePath: ${SW_SW_STORAGE_ES_SSL_JKS_PATH:"../es_keystore.jks"}
+# #trustStorePass: ${SW_SW_STORAGE_ES_SSL_JKS_PASS:""}
+# user: ${SW_ES_USER:""}
+# password: ${SW_ES_PASSWORD:""}
+# indexShardsNumber: ${SW_STORAGE_ES_INDEX_SHARDS_NUMBER:2}
+# indexReplicasNumber: ${SW_STORAGE_ES_INDEX_REPLICAS_NUMBER:0}
+# # Those data TTL settings will override the same settings in core module.
+# recordDataTTL: ${SW_STORAGE_ES_RECORD_DATA_TTL:7} # Unit is day
+# otherMetricsDataTTL: ${SW_STORAGE_ES_OTHER_METRIC_DATA_TTL:45} # Unit is day
+# monthMetricsDataTTL: ${SW_STORAGE_ES_MONTH_METRIC_DATA_TTL:18} # Unit is month
+# # Batch process setting, refer to https://www.elastic.co/guide/en/elasticsearch/client/java-api/5.5/java-docs-bulk-processor.html
+# bulkActions: ${SW_STORAGE_ES_BULK_ACTIONS:1000} # Execute the bulk every 1000 requests
+# flushInterval: ${SW_STORAGE_ES_FLUSH_INTERVAL:10} # flush the bulk every 10 seconds whatever the number of requests
+# concurrentRequests: ${SW_STORAGE_ES_CONCURRENT_REQUESTS:2} # the number of concurrent requests
+# resultWindowMaxSize: ${SW_STORAGE_ES_QUERY_MAX_WINDOW_SIZE:10000}
+# metadataQueryMaxSize: ${SW_STORAGE_ES_QUERY_MAX_SIZE:5000}
+# segmentQueryMaxSize: ${SW_STORAGE_ES_QUERY_SEGMENT_SIZE:200}
+# advanced: ${SW_STORAGE_ES_ADVANCED:""}
+ h2:
+ driver: ${SW_STORAGE_H2_DRIVER:org.h2.jdbcx.JdbcDataSource}
+ url: ${SW_STORAGE_H2_URL:jdbc:h2:mem:skywalking-oap-db}
+ user: ${SW_STORAGE_H2_USER:sa}
+ metadataQueryMaxSize: ${SW_STORAGE_H2_QUERY_MAX_SIZE:5000}
Review comment:
You submit it by mistake.
----------------------------------------------------------------
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
#4197: add token authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#discussion_r365575788
##########
File path: oap-server/server-receiver-plugin/skywalking-sharing-server-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/sharing/server/ReceiverGRPCHandlerRegister.java
##########
@@ -18,22 +18,39 @@
package org.apache.skywalking.oap.server.receiver.sharing.server;
-import io.grpc.*;
+import io.grpc.BindableService;
+import io.grpc.ServerInterceptor;
+import io.grpc.ServerInterceptors;
+import io.grpc.ServerServiceDefinition;
+import java.util.Objects;
import lombok.Setter;
import org.apache.skywalking.oap.server.core.server.GRPCHandlerRegister;
/**
- * @author peng-yongsheng
+ * @author peng-yongsheng, jian.tan
*/
public class ReceiverGRPCHandlerRegister implements GRPCHandlerRegister {
@Setter private GRPCHandlerRegister grpcHandlerRegister;
+ private ServerInterceptor interceptor;
@Override public void addHandler(BindableService handler) {
- grpcHandlerRegister.addHandler(handler);
+ if (Objects.isNull(interceptor)) {
+ grpcHandlerRegister.addHandler(handler);
+ } else {
+ grpcHandlerRegister.addHandler(handlerInterceptorBind(handler, interceptor));
+ }
Review comment:
So you have an implicit prerequisite here: the `addFilter` method must be called before adding all handlers (`addHandler`), otherwise the filter doesn't fully take effect, right? If so, this prerequisite must be pointed out in the `interface`, but I don't think this is a good design, I'd suggest moving the interceptor to:
https://github.com/apache/skywalking/blob/6f90545a537d3830d0fcc1b82f51b444f848b5d4/oap-server/server-library/library-server/src/main/java/org/apache/skywalking/oap/server/library/server/grpc/GRPCServer.java#L109
what do you think @wu-sheng
----------------------------------------------------------------
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 #4197: add token
authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#issuecomment-573333063
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@972d245`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `44.44%`.
[](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4197 +/- ##
=========================================
Coverage ? 26.45%
=========================================
Files ? 1179
Lines ? 25794
Branches ? 3751
=========================================
Hits ? 6823
Misses ? 18359
Partials ? 612
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ing/oap/server/library/server/grpc/GRPCServer.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItbGlicmFyeS9saWJyYXJ5LXNlcnZlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2xpYnJhcnkvc2VydmVyL2dycGMvR1JQQ1NlcnZlci5qYXZh) | `0% <ø> (ø)` | |
| [...er/core/server/auth/AuthenticationInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvc2VydmVyL2F1dGgvQXV0aGVudGljYXRpb25JbnRlcmNlcHRvci5qYXZh) | `0% <ø> (ø)` | |
| [...pm/agent/core/profile/ProfileTaskQueryService.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza1F1ZXJ5U2VydmljZS5qYXZh) | `46.15% <0%> (ø)` | |
| [.../skywalking/apm/agent/core/remote/GRPCChannel.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0dSUENDaGFubmVsLmphdmE=) | `81.25% <100%> (ø)` | |
| [...king/apm/agent/core/remote/GRPCChannelManager.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0dSUENDaGFubmVsTWFuYWdlci5qYXZh) | `67.94% <75%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4197?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/4197?src=pr&el=footer). Last update [972d245...5625d40](https://codecov.io/gh/apache/skywalking/pull/4197?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 a change in pull request #4197:
add token authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#discussion_r365561670
##########
File path: oap-server/server-receiver-plugin/skywalking-sharing-server-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/sharing/server/ReceiverGRPCHandlerRegister.java
##########
@@ -18,22 +18,39 @@
package org.apache.skywalking.oap.server.receiver.sharing.server;
-import io.grpc.*;
+import io.grpc.BindableService;
+import io.grpc.ServerInterceptor;
+import io.grpc.ServerInterceptors;
+import io.grpc.ServerServiceDefinition;
+import java.util.Objects;
import lombok.Setter;
import org.apache.skywalking.oap.server.core.server.GRPCHandlerRegister;
/**
- * @author peng-yongsheng
+ * @author peng-yongsheng, jian.tan
*/
public class ReceiverGRPCHandlerRegister implements GRPCHandlerRegister {
@Setter private GRPCHandlerRegister grpcHandlerRegister;
+ private ServerInterceptor interceptor;
@Override public void addHandler(BindableService handler) {
- grpcHandlerRegister.addHandler(handler);
+ if (Objects.isNull(interceptor)) {
+ grpcHandlerRegister.addHandler(handler);
+ } else {
+ grpcHandlerRegister.addHandler(handlerInterceptorBind(handler, interceptor));
+ }
}
@Override public void addHandler(ServerServiceDefinition definition) {
grpcHandlerRegister.addHandler(definition);
}
+
+ @Override public void addFilter(ServerInterceptor interceptor) {
+ this.interceptor = interceptor;
+ }
+
+ public ServerServiceDefinition handlerInterceptorBind(BindableService handler, ServerInterceptor interceptor) {
Review comment:
This should be private.
----------------------------------------------------------------
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 a change in pull request #4197:
add token authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#discussion_r364092998
##########
File path: docs/en/setup/backend/backend-token-auth.md
##########
@@ -0,0 +1,44 @@
+# Token Authentication
Review comment:
I think this doc should be linked from main doc. At least, from following places
1. https://github.com/apache/skywalking/blob/master/docs/en/setup/backend/backend-setup.md#advanced-feature-document-link-list
1. Quick link from https://github.com/apache/skywalking/blob/master/docs/README.md
----------------------------------------------------------------
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
#4197: add token authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#discussion_r364104022
##########
File path: docs/en/setup/backend/backend-token-auth.md
##########
@@ -0,0 +1,44 @@
+# Token Authentication
+## Supported version
+7.0.0+
+
+## How need token authentication after we have TLS?
Review comment:
```suggestion
## Why need token authentication after we have TLS?
```
----------------------------------------------------------------
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 #4197: add token
authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#issuecomment-573333063
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=h1) Report
> Merging [#4197](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/5625d4004c36329875f443e9f1fe08a434c0c928?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4197 +/- ##
=======================================
Coverage 26.45% 26.45%
=======================================
Files 1179 1179
Lines 25794 25794
Branches 3751 3751
=======================================
Hits 6823 6823
Misses 18359 18359
Partials 612 612
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4197?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/4197?src=pr&el=footer). Last update [5625d40...9480420](https://codecov.io/gh/apache/skywalking/pull/4197?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 #4197: add token
authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#issuecomment-573333063
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=h1) Report
> Merging [#4197](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/5625d4004c36329875f443e9f1fe08a434c0c928?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `93.61%`.
[](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4197 +/- ##
==========================================
+ Coverage 26.45% 26.45% +<.01%
==========================================
Files 1179 1179
Lines 25794 25714 -80
Branches 3751 3728 -23
==========================================
- Hits 6823 6803 -20
+ Misses 18359 18315 -44
+ Partials 612 596 -16
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [.../plugin/trace/ignore/TraceIgnoreExtendService.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvb3B0aW9uYWwtcGx1Z2lucy90cmFjZS1pZ25vcmUtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vdHJhY2UvaWdub3JlL1RyYWNlSWdub3JlRXh0ZW5kU2VydmljZS5qYXZh) | `82.35% <75%> (+17.35%)` | :arrow_up: |
| [...m/plugin/trace/ignore/matcher/FastPathMatcher.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvb3B0aW9uYWwtcGx1Z2lucy90cmFjZS1pZ25vcmUtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vdHJhY2UvaWdub3JlL21hdGNoZXIvRmFzdFBhdGhNYXRjaGVyLmphdmE=) | `97.43% <97.43%> (ø)` | |
| [...m/agent/core/remote/TraceSegmentServiceClient.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL1RyYWNlU2VnbWVudFNlcnZpY2VDbGllbnQuamF2YQ==) | `82.35% <0%> (+1.47%)` | :arrow_up: |
| [.../core/remote/ServiceAndEndpointRegisterClient.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL1NlcnZpY2VBbmRFbmRwb2ludFJlZ2lzdGVyQ2xpZW50LmphdmE=) | `31.46% <0%> (+3.37%)` | :arrow_up: |
| [...pm/agent/core/profile/ProfileTaskQueryService.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza1F1ZXJ5U2VydmljZS5qYXZh) | `51.28% <0%> (+5.12%)` | :arrow_up: |
| [...alking/apm/agent/core/remote/AgentIDDecorator.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0FnZW50SUREZWNvcmF0b3IuamF2YQ==) | `85.71% <0%> (+17.85%)` | :arrow_up: |
| [...ache/skywalking/apm/agent/core/jvm/JVMService.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvanZtL0pWTVNlcnZpY2UuamF2YQ==) | `77.04% <0%> (+18.03%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4197?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/4197?src=pr&el=footer). Last update [5625d40...9480420](https://codecov.io/gh/apache/skywalking/pull/4197?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 #4197: add token
authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#issuecomment-572554717
Look like CI still failing.
----------------------------------------------------------------
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] JaredTan95 commented on issue #4197: add token
authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
JaredTan95 commented on issue #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#issuecomment-571954373
> 1. server-internal communication will be blocked, the cluster will fail. Please fix this.
I'm not sure this can avoid it:
```
if (!(targetService instanceof RemoteServiceHandler) && StringUtil.isNotEmpty(expectedToken)) {}
```
----------------------------------------------------------------
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 a change in pull request #4197:
add token authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#discussion_r365561755
##########
File path: oap-server/server-receiver-plugin/skywalking-sharing-server-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/sharing/server/SharingServerModuleProvider.java
##########
@@ -81,11 +89,17 @@ public SharingServerModuleProvider() {
if (config.getGRPCThreadPoolSize() > 0) {
grpcServer.setThreadPoolSize(config.getGRPCThreadPoolSize());
}
+
grpcServer.initialize();
- this.registerServiceImplementation(GRPCHandlerRegister.class, new GRPCHandlerRegisterImpl(grpcServer));
+ GRPCHandlerRegister grpcHandlerRegister = new GRPCHandlerRegisterImpl(grpcServer);
+
+ this.registerServiceImplementation(GRPCHandlerRegister.class, grpcHandlerRegister);
Review comment:
Why change this? It seems the same.
----------------------------------------------------------------
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 #4197: add token
authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#issuecomment-573333063
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@50aea09`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `36.42%`.
[](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4197 +/- ##
=========================================
Coverage ? 26.42%
=========================================
Files ? 1180
Lines ? 25859
Branches ? 3763
=========================================
Hits ? 6832
Misses ? 18422
Partials ? 605
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [.../plugin/elasticsearch/query/MetricsQueryEsDAO.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoLXBsdWdpbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL3N0b3JhZ2UvcGx1Z2luL2VsYXN0aWNzZWFyY2gvcXVlcnkvTWV0cmljc1F1ZXJ5RXNEQU8uamF2YQ==) | `0% <0%> (ø)` | |
| [...ap/server/exporter/provider/grpc/GRPCExporter.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9leHBvcnRlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2V4cG9ydGVyL3Byb3ZpZGVyL2dycGMvR1JQQ0V4cG9ydGVyLmphdmE=) | `77.77% <0%> (ø)` | |
| [...server/core/storage/annotation/ValueColumnIds.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvc3RvcmFnZS9hbm5vdGF0aW9uL1ZhbHVlQ29sdW1uSWRzLmphdmE=) | `0% <0%> (ø)` | |
| [.../storage/plugin/jdbc/h2/dao/H2MetricsQueryDAO.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1qZGJjLWhpa2FyaWNwLXBsdWdpbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL3N0b3JhZ2UvcGx1Z2luL2pkYmMvaDIvZGFvL0gyTWV0cmljc1F1ZXJ5REFPLmphdmE=) | `0% <0%> (ø)` | |
| [...alking/oap/server/core/query/entity/IntValues.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvcXVlcnkvZW50aXR5L0ludFZhbHVlcy5qYXZh) | `0% <0%> (ø)` | |
| [...king/oap/server/core/query/MetricQueryService.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvcXVlcnkvTWV0cmljUXVlcnlTZXJ2aWNlLmphdmE=) | `0% <0%> (ø)` | |
| [...alking/oap/query/graphql/resolver/MetricQuery.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItcXVlcnktcGx1Z2luL3F1ZXJ5LWdyYXBocWwtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9xdWVyeS9ncmFwaHFsL3Jlc29sdmVyL01ldHJpY1F1ZXJ5LmphdmE=) | `0% <0%> (ø)` | |
| [...king/oap/server/core/alarm/provider/Threshold.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9UaHJlc2hvbGQuamF2YQ==) | `100% <100%> (ø)` | |
| [...p/server/core/alarm/provider/MetricsValueType.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9NZXRyaWNzVmFsdWVUeXBlLmphdmE=) | `100% <100%> (ø)` | |
| [...ng/oap/server/core/alarm/provider/RunningRule.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItYWxhcm0tcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY29yZS9hbGFybS9wcm92aWRlci9SdW5uaW5nUnVsZS5qYXZh) | `65.9% <44%> (ø)` | |
| ... and [1 more](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4197?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/4197?src=pr&el=footer). Last update [50aea09...d2916a8](https://codecov.io/gh/apache/skywalking/pull/4197?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] JaredTan95 commented on issue #4197: add token
authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
JaredTan95 commented on issue #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#issuecomment-571955168
I tested in my local, and i did not found `org.apache.skywalking.oap.server.core.remote.RemoteServiceHandler` instance step into `org.apache.skywalking.oap.server.core.server.auth.AuthenticationHandler`, do i miss something?
----------------------------------------------------------------
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 #4197: add token
authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#issuecomment-573333063
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@972d245`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `44.44%`.
[](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4197 +/- ##
=========================================
Coverage ? 26.45%
=========================================
Files ? 1179
Lines ? 25794
Branches ? 3751
=========================================
Hits ? 6823
Misses ? 18359
Partials ? 612
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ing/oap/server/library/server/grpc/GRPCServer.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItbGlicmFyeS9saWJyYXJ5LXNlcnZlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2xpYnJhcnkvc2VydmVyL2dycGMvR1JQQ1NlcnZlci5qYXZh) | `0% <ø> (ø)` | |
| [...er/core/server/auth/AuthenticationInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvc2VydmVyL2F1dGgvQXV0aGVudGljYXRpb25JbnRlcmNlcHRvci5qYXZh) | `0% <ø> (ø)` | |
| [...pm/agent/core/profile/ProfileTaskQueryService.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza1F1ZXJ5U2VydmljZS5qYXZh) | `46.15% <0%> (ø)` | |
| [.../skywalking/apm/agent/core/remote/GRPCChannel.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0dSUENDaGFubmVsLmphdmE=) | `81.25% <100%> (ø)` | |
| [...king/apm/agent/core/remote/GRPCChannelManager.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0dSUENDaGFubmVsTWFuYWdlci5qYXZh) | `67.94% <75%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4197?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/4197?src=pr&el=footer). Last update [972d245...5625d40](https://codecov.io/gh/apache/skywalking/pull/4197?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
#4197: add token authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#discussion_r365580404
##########
File path: oap-server/server-receiver-plugin/skywalking-sharing-server-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/sharing/server/ReceiverGRPCHandlerRegister.java
##########
@@ -18,22 +18,39 @@
package org.apache.skywalking.oap.server.receiver.sharing.server;
-import io.grpc.*;
+import io.grpc.BindableService;
+import io.grpc.ServerInterceptor;
+import io.grpc.ServerInterceptors;
+import io.grpc.ServerServiceDefinition;
+import java.util.Objects;
import lombok.Setter;
import org.apache.skywalking.oap.server.core.server.GRPCHandlerRegister;
/**
- * @author peng-yongsheng
+ * @author peng-yongsheng, jian.tan
*/
public class ReceiverGRPCHandlerRegister implements GRPCHandlerRegister {
@Setter private GRPCHandlerRegister grpcHandlerRegister;
+ private ServerInterceptor interceptor;
@Override public void addHandler(BindableService handler) {
- grpcHandlerRegister.addHandler(handler);
+ if (Objects.isNull(interceptor)) {
+ grpcHandlerRegister.addHandler(handler);
+ } else {
+ grpcHandlerRegister.addHandler(handlerInterceptorBind(handler, interceptor));
+ }
Review comment:
> Token authentication filter should only be activated in sharding server module. Moving to `GRPCServer` may not very suitable, in my thought.
Well I missed 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] wu-sheng commented on a change in pull request #4197:
add token authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#discussion_r364612577
##########
File path: apm-webapp/pom.xml
##########
@@ -133,7 +133,7 @@
<goal>npm</goal>
</goals>
<configuration>
- <arguments>install --registry=https://registry.npmjs.org/</arguments>
+ <arguments>install --registry=https://registry.npm.taobao.org</arguments>
Review comment:
This should not be submitted.
----------------------------------------------------------------
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] JaredTan95 merged pull request #4197: add token
authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
JaredTan95 merged pull request #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197
----------------------------------------------------------------
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 #4197: add token
authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#issuecomment-573333063
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@972d245`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `44.44%`.
[](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4197 +/- ##
=========================================
Coverage ? 26.45%
=========================================
Files ? 1179
Lines ? 25794
Branches ? 3751
=========================================
Hits ? 6823
Misses ? 18359
Partials ? 612
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ing/oap/server/library/server/grpc/GRPCServer.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItbGlicmFyeS9saWJyYXJ5LXNlcnZlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2xpYnJhcnkvc2VydmVyL2dycGMvR1JQQ1NlcnZlci5qYXZh) | `0% <ø> (ø)` | |
| [...er/core/server/auth/AuthenticationInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvc2VydmVyL2F1dGgvQXV0aGVudGljYXRpb25JbnRlcmNlcHRvci5qYXZh) | `0% <ø> (ø)` | |
| [...pm/agent/core/profile/ProfileTaskQueryService.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza1F1ZXJ5U2VydmljZS5qYXZh) | `46.15% <0%> (ø)` | |
| [.../skywalking/apm/agent/core/remote/GRPCChannel.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0dSUENDaGFubmVsLmphdmE=) | `81.25% <100%> (ø)` | |
| [...king/apm/agent/core/remote/GRPCChannelManager.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0dSUENDaGFubmVsTWFuYWdlci5qYXZh) | `67.94% <75%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4197?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/4197?src=pr&el=footer). Last update [972d245...5625d40](https://codecov.io/gh/apache/skywalking/pull/4197?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 #4197: add token
authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#issuecomment-573333063
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=h1) Report
> Merging [#4197](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/5625d4004c36329875f443e9f1fe08a434c0c928?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `93.61%`.
[](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4197 +/- ##
==========================================
+ Coverage 26.45% 26.45% +<.01%
==========================================
Files 1179 1179
Lines 25794 25714 -80
Branches 3751 3728 -23
==========================================
- Hits 6823 6803 -20
+ Misses 18359 18315 -44
+ Partials 612 596 -16
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [.../plugin/trace/ignore/TraceIgnoreExtendService.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvb3B0aW9uYWwtcGx1Z2lucy90cmFjZS1pZ25vcmUtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vdHJhY2UvaWdub3JlL1RyYWNlSWdub3JlRXh0ZW5kU2VydmljZS5qYXZh) | `82.35% <75%> (+17.35%)` | :arrow_up: |
| [...m/plugin/trace/ignore/matcher/FastPathMatcher.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvb3B0aW9uYWwtcGx1Z2lucy90cmFjZS1pZ25vcmUtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL2FwbS9wbHVnaW4vdHJhY2UvaWdub3JlL21hdGNoZXIvRmFzdFBhdGhNYXRjaGVyLmphdmE=) | `97.43% <97.43%> (ø)` | |
| [...m/agent/core/remote/TraceSegmentServiceClient.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL1RyYWNlU2VnbWVudFNlcnZpY2VDbGllbnQuamF2YQ==) | `82.35% <0%> (+1.47%)` | :arrow_up: |
| [.../core/remote/ServiceAndEndpointRegisterClient.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL1NlcnZpY2VBbmRFbmRwb2ludFJlZ2lzdGVyQ2xpZW50LmphdmE=) | `31.46% <0%> (+3.37%)` | :arrow_up: |
| [...pm/agent/core/profile/ProfileTaskQueryService.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza1F1ZXJ5U2VydmljZS5qYXZh) | `51.28% <0%> (+5.12%)` | :arrow_up: |
| [...alking/apm/agent/core/remote/AgentIDDecorator.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0FnZW50SUREZWNvcmF0b3IuamF2YQ==) | `85.71% <0%> (+17.85%)` | :arrow_up: |
| [...ache/skywalking/apm/agent/core/jvm/JVMService.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvanZtL0pWTVNlcnZpY2UuamF2YQ==) | `77.04% <0%> (+18.03%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4197?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/4197?src=pr&el=footer). Last update [5625d40...9480420](https://codecov.io/gh/apache/skywalking/pull/4197?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 a change in pull request #4197:
add token authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#discussion_r365561741
##########
File path: oap-server/server-receiver-plugin/skywalking-sharing-server-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/sharing/server/ReceiverGRPCHandlerRegister.java
##########
@@ -18,22 +18,39 @@
package org.apache.skywalking.oap.server.receiver.sharing.server;
-import io.grpc.*;
+import io.grpc.BindableService;
+import io.grpc.ServerInterceptor;
+import io.grpc.ServerInterceptors;
+import io.grpc.ServerServiceDefinition;
+import java.util.Objects;
import lombok.Setter;
import org.apache.skywalking.oap.server.core.server.GRPCHandlerRegister;
/**
- * @author peng-yongsheng
+ * @author peng-yongsheng, jian.tan
*/
public class ReceiverGRPCHandlerRegister implements GRPCHandlerRegister {
@Setter private GRPCHandlerRegister grpcHandlerRegister;
+ private ServerInterceptor interceptor;
@Override public void addHandler(BindableService handler) {
- grpcHandlerRegister.addHandler(handler);
+ if (Objects.isNull(interceptor)) {
+ grpcHandlerRegister.addHandler(handler);
+ } else {
+ grpcHandlerRegister.addHandler(handlerInterceptorBind(handler, interceptor));
+ }
}
@Override public void addHandler(ServerServiceDefinition definition) {
grpcHandlerRegister.addHandler(definition);
}
+
+ @Override public void addFilter(ServerInterceptor interceptor) {
Review comment:
Based on your codes, the interceptor could only be one, so this method should be set, rather than add.
And, this method must be called before `addHandler`, should add this to the comment.
----------------------------------------------------------------
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 #4197: add token
authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#issuecomment-573333063
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@972d245`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `44.44%`.
[](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4197 +/- ##
=========================================
Coverage ? 26.45%
=========================================
Files ? 1179
Lines ? 25794
Branches ? 3751
=========================================
Hits ? 6823
Misses ? 18359
Partials ? 612
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4197?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ing/oap/server/library/server/grpc/GRPCServer.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItbGlicmFyeS9saWJyYXJ5LXNlcnZlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2xpYnJhcnkvc2VydmVyL2dycGMvR1JQQ1NlcnZlci5qYXZh) | `0% <ø> (ø)` | |
| [...er/core/server/auth/AuthenticationInterceptor.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvc2VydmVyL2F1dGgvQXV0aGVudGljYXRpb25JbnRlcmNlcHRvci5qYXZh) | `0% <ø> (ø)` | |
| [...pm/agent/core/profile/ProfileTaskQueryService.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcHJvZmlsZS9Qcm9maWxlVGFza1F1ZXJ5U2VydmljZS5qYXZh) | `46.15% <0%> (ø)` | |
| [.../skywalking/apm/agent/core/remote/GRPCChannel.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0dSUENDaGFubmVsLmphdmE=) | `81.25% <100%> (ø)` | |
| [...king/apm/agent/core/remote/GRPCChannelManager.java](https://codecov.io/gh/apache/skywalking/pull/4197/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0dSUENDaGFubmVsTWFuYWdlci5qYXZh) | `67.94% <75%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4197?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/4197?src=pr&el=footer). Last update [972d245...5625d40](https://codecov.io/gh/apache/skywalking/pull/4197?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 #4197: add token
authentication between agent and oap receiver.
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #4197: add token authentication between agent and oap receiver.
URL: https://github.com/apache/skywalking/pull/4197#issuecomment-572120658
Please fix the CI, and add auth to one e2e case.
----------------------------------------------------------------
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