Hi @xuanyu66, as https://github.com/apache/skywalking/issues/6105#issuecomment-753248303 mentioned, we start the discussion about the dynamic configuration for the agent. FYI all @apache/skywalking-committers , if any of you wants to work on this, please reply.
We have command system at the agent side, we just need to extend a new command for dynamic configuration. Like we did in the backend side, the configurations should be separated from the the initial agent.config definition. The implementation decides whether they support the configuration dynamically changes.
From my preference, we may need a new gRPC service for the agent for dynamic configuration. Note, we need a ID(UUID or timestamp) to represent the configuration to avoid responding the configuration to the agent repeatedly. Also, the configurations of different services are different.
I'm interested in this.
In terms of gRPC implementation, what's the idea of modifying the configurations? If I understand correctly, this is going to be similar with the backend dynamic configurations, one should expose a gRPC service providing the latest configs to the OAP, and the OAP responds these to the agent, right?
@kezhenxu94 The agent configuration isn't the agent dynamic configuration. They may logically be supported in the same features, but they are plugins' choices.
If that's the case, I would suggest reversing the roles (server / client) of OAP and "user-defined gRPC service", which makes it possible for the CLI to update the configs without a long-running process.
The agent wouldn't accept any remote request, because of 2 reasons
@kezhenxu94 The agent configuration isn't the agent dynamic configuration. They may logically be supported in the same features, but they are plugins' choices.
Totally confused by "The agent configuration isn't the agent dynamic configuration" and the issue title "Dynamic Configuration for the agent".
The agent wouldn't accept any remote request, because of 2 reasons
That's completely not what I was saying if you read carefully.
I was proposing to let the OAP accept configs updates from CLI (or user-defined grpc service), but finally realize that doesn't belong this the scope of this issue.
config server(nacos,zk) <--update Event--> OAP --command--> agent
did I get that right?
Totally confused by "The agent configuration isn't the agent dynamic configuration" and the issue title "Dynamic Configuration for the agent".
I think configuration of agent core and plugins defined through Java object, right? The dynamic configuration isn't going to change the value of the Java object. It should like the backend, the core and plugins need to declare the changeable configs explicitly.
config server(nacos,zk) <--update Event--> OAP --command--> agent
did I get that right?
Yes, so I was proposing to add a grpc service in OAP and make CLI possible to update
config server(nacos,zk) <--update Event--> OAP --command--> agent
^
| <== this is what I proposed to added
CLI
I think the sync between OAP nodes is a problem so this is not as easy as I thought
Totally confused by "The agent configuration isn't the agent dynamic configuration" and the issue title "Dynamic Configuration for the agent".
I think configuration of agent core and plugins defined through Java object, right? The dynamic configuration isn't going to change the value of the Java object. It should like the backend, the core and plugins need to declare the changeable configs explicitly.
Yes totally agree. That's not a doubt for me.
config server(nacos,zk) <--update Event--> OAP --command--> agent
did I get that right?
Yes, but we need more details about how to make this happens. Such as
config server(nacos,zk) <--update Event--> OAP --command--> agent
did I get that right?Yes, so I was proposing to add a grpc service in OAP and make CLI possible to update
config server(nacos,zk) <--update Event--> OAP --command--> agent ^ | <== this is what I proposed to added CLII think the sync between OAP nodes is a problem so this is not as easy as I thought
This is definitely not easy, and also I think this is not a problem we should resolve. If CLI could trigger this, you actually build a CLI implementation to change every config server implementation. :) Such as ZooKeeper has its client CLI, which could do update natively. If the config server doesn't support it, it is the issue of their community.
config server(nacos,zk) <--update Event--> OAP --command--> agent
did I get that right?Yes, so I was proposing to add a grpc service in OAP and make CLI possible to update
config server(nacos,zk) <--update Event--> OAP --command--> agent ^ | <== this is what I proposed to added CLII think the sync between OAP nodes is a problem so this is not as easy as I thought
I think it will bring some unnecessary complexity.
As far as I known, it is not complicated to update config on zk or nacos.
config server(nacos,zk) <--update Event--> OAP --command--> agent
did I get that right?Yes, but we need more details about how to make this happens. Such as
- The gRPC service between agent and OAP
- How to define the configuration of every service in the configuration center. Such as, do you provide one key with a YAML as value for all services?
- How the agent core and plugin declare the config is changeable, and listening to the change event from the command.
for 2, How about we define an agent-dynamic-config.yml, in which the relationship between service and dynamic configuration is stored. The content of the file can be modified in the configuration center. every config key same as agent.config item, like this,
dynamic-configs:
order-service:
trace.sample_rate: 1000
trace.ignore_path: /api/seller/seller/*
otherConfig: ...
pay-service:
trace.sample_rate: 0
trace.ignore_path: /eureka/**
for 3, I want to add a ConfigWatcherRegister similar to OAP for the agent, but need to specify the name of the configuration item in the agent.config.
hope some suggestions, and looking forward to a better way.
for 2, How about we define an agent-dynamic-config.yml, in which the relationship between service and dynamic configuration is stored. The content of the file can be modified in the configuration center. every config key same as agent.config item, like this,
dynamic-configs: should be renamed as agent-config.
for 3, I want to add a ConfigWatcherRegister similar to OAP for the agent, but need to specify the name of the configuration item in the agent.config.
I think we could, but due to the event change driven by command, we need to be carefully.
__
And we need the gRPC service definition.
And, you should know YAML is type sensitive, such as int VS string. You need to be careful when they are pushed through gRPC.
And, you should know YAML is type sensitive, such as int VS string. You need to be careful when they are pushed through gRPC.
Get it, thanks for your suggestion.
This feature could be submitted through 3 PRs, 1 for data-collect-protocol repo, and 2 for OAP server and agent implementations.
I have a little question about how to identify the configuration is not changed. Should we need to checksum the data and use md5 as the data identity? It could avoid responding to them repeatedly.
Randomly ID is not safe at the distribute backend, because we don't know which one created the ID or need to store them.
I have a little question about how to identify the configuration is not changed. Should we need to checksum the data and use
md5as the data identity? It could avoid responding to them repeatedly.
Randomly ID is not safe at the distribute backend, because we don't know which one created the ID or need to store them.
There is just a bug fix about this https://github.com/apache/skywalking/pull/6115 :) Take a look. UUID is required for the server side implementation logically. Technically, md5 for each service's configs makes sense or something similar.
I have a little question about how to identify the configuration is not changed. Should we need to checksum the data and use
md5as the data identity? It could avoid responding to them repeatedly.
Randomly ID is not safe at the distribute backend, because we don't know which one created the ID or need to store them.There is just a bug fix about this #6115 :) Take a look. UUID is required for the server side implementation logically. Technically,
md5for each service's configs makes sense or something similar.
If we are using UUID to store config ID on memory, and the agent side changes the connection between agent and backend, It will be repeated to change the config. Using md5 will avoid this question because the value is base on config content.
I think there is not a conflict between these two. As I said, md5 is an UUID implementation. md5 is an one way encrytion Algorithm, sha1 and sha512 work too.
Note all this kind of algorithm is only making sure same text has certain encrypted text, but different texts could have same encrypted text too, just little chance.
I am not saying we should not use it. Instead, we should choose one, but we just should know the risk in theory.
Note all this kind of algorithm is only making sure same text has certain encrypted text, but different texts could have same encrypted text too, just little chance.
I am not saying we should not use it. Instead, we should choose one, but we just should know the risk in theory.
That's true. I just want to declare, there will have an agent config repeat change issue.
The text-based uuid generation mechanism will be included. So, wouldn't face the issue you mentioned.
config server(nacos,zk) <--update Event--> OAP --command--> agent
did I get that right?Yes, but we need more details about how to make this happens. Such as
- The gRPC service between agent and OAP
- How to define the configuration of every servic in the configuration center. Such as, do you provide one key with a YAML as value for all services?
- How the agent core and plugin declare the config is changeable, and listening to the change event from the command.
import "common/Common.proto";
service AgentDynamicConfigTask {
// query all sniffer need to execute dynamic config task commands
rpc getDynamicConfigCommands (DynamicConfigTaskCommandQuery) returns (Commands) {
}
message DynamicConfigTaskCommandQuery {
// last command timestamp
int64 lastCommandTime = 1;
}
hope some suggestions.
config server(nacos,zk) <--update Event--> OAP --command--> agent
did I get that right?Yes, but we need more details about how to make this happens. Such as
- The gRPC service between agent and OAP
- How to define the configuration of every servic in the configuration center. Such as, do you provide one key with a YAML as value for all services?
- How the agent core and plugin declare the config is changeable, and listening to the change event from the command.
- define a gRPC service
import "common/Common.proto"; service AgentDynamicConfigTask { // query all sniffer need to execute dynamic config task commands rpc getDynamicConfigCommands (DynamicConfigTaskCommandQuery) returns (Commands) { } message DynamicConfigTaskCommandQuery { // last command timestamp int64 lastCommandTime = 1; }
- implement ConfigChangeWatcher called AgentConfigWatcher, if notified, save task to AgentDynamicConfigTaskCache
- On agent side implement DynamicConfigTaskChannelService, schedule a thread to poll grpc service. same as _org.apache.skywalking.apm.agent.core.profile.ProfileTaskChannelService_
- I don‘t know whether yaml is proper way to save agent config. Do we need to validate yaml?
hope some suggestions.
So sorry, I have completed this feature, and plan to submit it in the next two days.
To all in this thread, the proto review happens here, https://github.com/apache/skywalking-data-collect-protocol/pull/43
FYI @apache/skywalking-committers
config server(nacos,zk) <--update Event--> OAP --command--> agent
did I get that right?Yes, but we need more details about how to make this happens. Such as
- The gRPC service between agent and OAP
- How to define the configuration of every servic in the configuration center. Such as, do you provide one key with a YAML as value for all services?
- How the agent core and plugin declare the config is changeable, and listening to the change event from the command.
- define a gRPC service
import "common/Common.proto"; service AgentDynamicConfigTask { // query all sniffer need to execute dynamic config task commands rpc getDynamicConfigCommands (DynamicConfigTaskCommandQuery) returns (Commands) { } message DynamicConfigTaskCommandQuery { // last command timestamp int64 lastCommandTime = 1; }
- implement ConfigChangeWatcher called AgentConfigWatcher, if notified, save task to AgentDynamicConfigTaskCache
- On agent side implement DynamicConfigTaskChannelService, schedule a thread to poll grpc service. same as _org.apache.skywalking.apm.agent.core.profile.ProfileTaskChannelService_
- I don‘t know whether yaml is proper way to save agent config. Do we need to validate yaml?
hope some suggestions.
So sorry, I have completed this feature, and plan to submit it in the next two days.
It doesn't matter, I will focus on this feather
@xuanyu66 I think @zifeihan is only working on the gRPC service and basic implementation for sampling config as the step one. We will have many configurations should support this one by one. Look forward to your contributions.
config server(nacos,zk) <--update Event--> OAP --command--> agent
did I get that right?Yes, but we need more details about how to make this happens. Such as
- The gRPC service between agent and OAP
- How to define the configuration of every servic in the configuration center. Such as, do you provide one key with a YAML as value for all services?
- How the agent core and plugin declare the config is changeable, and listening to the change event from the command.
- define a gRPC service
import "common/Common.proto"; service AgentDynamicConfigTask { // query all sniffer need to execute dynamic config task commands rpc getDynamicConfigCommands (DynamicConfigTaskCommandQuery) returns (Commands) { } message DynamicConfigTaskCommandQuery { // last command timestamp int64 lastCommandTime = 1; }
- implement ConfigChangeWatcher called AgentConfigWatcher, if notified, save task to AgentDynamicConfigTaskCache
- On agent side implement DynamicConfigTaskChannelService, schedule a thread to poll grpc service. same as _org.apache.skywalking.apm.agent.core.profile.ProfileTaskChannelService_
- I don‘t know whether yaml is proper way to save agent config. Do we need to validate yaml?
hope some suggestions.
So sorry, I have completed this feature, and plan to submit it in the next two days.
It doesn't matter, I will focus on this feather
Thank you, maybe you also started this work, let us improve it together, and I will invite you to review.
@zifeihan OK, I've already read ConfigurationDiscoveryService.proto.
Hi @mrproliu
Could we deliver ProfileTask by using DC?
If so, MQ reporter will get more simple. Because it does not need to depend on GRPC anymore.
Hi @mrproliu
Could we deliver ProfileTask by using DC?
If so, MQ reporter will get more simple. Because it does not need to depend on GRPC anymore.
I think we don't need to change that. Keeping grpc working should be more simplier. The advantage of MQ transporter is for traces and profile snapshots, rather than command itself. Right?
We don't need to break the protocol from my understanding. Command is just an abstract concept. It is common to share among services.
Any issue crosses your mind?
I think we don't need to change that. Keeping grpc working should be more simplier. The advantage of MQ transporter is for traces and profile snapshots, rather than command itself. Right?
We don't need to break the protocol from my understanding. Command is just an abstract concept. It is common to share among services.
Any issue crosses your mind?
Got it. I was thoughtless!
Is there ant progress?
@xuanyu66 The proto has been merged. Others should take some time. @zifeihan is working on that.
Is there ant progress?
Sorry, something was delayed today, I will submit it tomorrow.
Don't hurry. The quality will have higher priority. As a new feature, it takes time always. We don't have this for 5 years, things don't go wrong. 😀
In the open source, it is common to have delay, rechedule, reimplement, but the key is, things are moving forward to the right direction.
This is not a job assignment, please don't take too much pressure.
Don't hurry. The quality will have higher priority. As a new feature, it takes time always. We don't have this for 5 years, things don't go wrong. 😀
In the open source, it is common to have delay, rechedule, reimplement, but the key is, things are moving forward to he right direction.
This is not a job assignment, please don't take too much pressure.
Thanks, I will double check and submit pr tomorrow.
Most helpful comment
There is just a bug fix about this https://github.com/apache/skywalking/pull/6115 :) Take a look. UUID is required for the server side implementation logically. Technically,
md5for each service's configs makes sense or something similar.