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/12/02 14:04:22 UTC

[GitHub] [skywalking-satellite] EvanLjp opened a new pull request #5: Add API && Plugin framework registry

EvanLjp opened a new pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5


   Add API && Plugin framework registry. Please review the reasonableness of these definitions


----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#issuecomment-737183228


   @surlymo Thx for review again. Both support pointer register and value register.
   ![image](https://user-images.githubusercontent.com/31562192/100869391-f6504200-34d7-11eb-9d58-b7a6c293b5e6.png)
   


----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] wu-sheng commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532207340



##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,94 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following graph illustrates the relationship between different plugin interface in api package.
+//
+//
+//                   Gatherer                                Processor
+//       -------------------------------      -------------------------------------------
+//      |  -----------       ---------   |   |  -----------                 -----------  |
+//      | | Collector | ==> |  Queue   | |==>| |  Filter   | ==>  ...  ==> |  Filter   | |
+//      | | (Parser)  |     | Mem/File | |   |  -----------                 -----------  |
+//      |  -----------       ----------  |   |      ||                          ||       |
+//       --------------------------------    |      \/	                        \/       |
+//                                           |  ---------------------------------------  |
+//                                           | |             OutputEventContext        | |
+//                                           |  ---------------------------------------  |
+//                                            -------------------------------------------
+//                             				   ||
+//                                             ||        --------------------------------
+//                                             ||     ->|         Sharing Client        |
+//                                             ||    |   --------------------------------
+//                                             ||    |
+//                                             \/    |            SegmentSender
+//                                            ---    |  ---------------------------------
+//                                           |   |   | |  -----------       -----------  |
+//                                           | D |   |-| |BatchBuffer| ==> | Forwarder | |
+//                                           | i |   | |  -----------       -----------  |
+//                                           | s |   |  ---------------------------------
+//                                           | p |   |
+//                                           | a |=> |              .......                 ===> Kafka/OAP
+//                                           | t |   |
+//                                           | c |   |             MeterSender
+//                                           | h |   | -----------------------------------
+//                                           | e |   -|  -------------       -----------  |
+//                                           | r |    | | BatchBuffer | ==> | Forwarder | |
+//                                           |   |    |  -------------       -----------  |
+//                                            ---      -----------------------------------
+//
+//
+// 1. The Collector plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to SerializationEvent that is supported
+//    to be stored in Queue.
+// 3. The Queue plugin stores the SerializationEvent. However, the serialization depends on
+//    the Queue implements. For example, the serialization is unnecessary when using a Memory
+//    Queue.
+// 4. The Filter plugin would pull an event from the Queue and process the event to create

Review comment:
       Do you set up the event process one by one? I think the `one by one` mode is on the `filter`'s perspective only.




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#issuecomment-734922926


   > 2\. Event in the doc should be the data of the processor. From my understanding, there should be no explicit boundary between input and output, because every output could be another filter's input. I am curious how you plan to use the InputEvent and OutputEvent.
   
   Event is a global context, only event is allowed to transfer in Satellite. InputEvent is a smaller concept for Gatherer. Because  Gatherer would put the data in Queue, InputEvent guarantees that Queue can be replaced in different plug-ins.  It's up to the plug-in to decide whether to take the original look or serialize to bytes.


----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532053497



##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,74 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following comments is to illustrate the relationship between different plugin interface in api package.
+//
+//
+//                                                 Processors
+//                                   -----------------------------------------
+//  ---------        ---------        -----------                 -----------
+// | Gatherer | ==> |  Queue   | ==> | Processor | ==>  ...  ==> | Processor |
+// | (Parser) |     | Mem/File |      -----------                 -----------
+//  ----------       ---------            ||                          ||
+//                                        \/	                      \/
+//                                    ---------------------------------------
+//                                   |             OutputEventContext        |
+//                                    ---------------------------------------
+//                                                                    ||
+//                                                                    \/
+//                                    -------------------       -------------
+//                                   | BatchOutputEvents | <== | BatchBuffer |
+//                                    -------------------       -------------
+//                                             ||
+//                                             \/
+//                                    -------------------
+//                                   |     Forwarder     | ==> Kakfa/OAP
+//                                    -------------------
+// 1. The Gatherer plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to InputEvent.
+//    If the event needs output, please tag it by the IsOutput
+//    method.
+// 3. The Queue plugin would store the InputEvent. But different

Review comment:
       Maybe SerializeEvent more fit the Queue?




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532039980



##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,74 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following comments is to illustrate the relationship between different plugin interface in api package.
+//
+//
+//                                                 Processors
+//                                   -----------------------------------------
+//  ---------        ---------        -----------                 -----------
+// | Gatherer | ==> |  Queue   | ==> | Processor | ==>  ...  ==> | Processor |
+// | (Parser) |     | Mem/File |      -----------                 -----------
+//  ----------       ---------            ||                          ||
+//                                        \/	                      \/
+//                                    ---------------------------------------
+//                                   |             OutputEventContext        |
+//                                    ---------------------------------------
+//                                                                    ||
+//                                                                    \/
+//                                    -------------------       -------------
+//                                   | BatchOutputEvents | <== | BatchBuffer |
+//                                    -------------------       -------------
+//                                             ||
+//                                             \/
+//                                    -------------------
+//                                   |     Forwarder     | ==> Kakfa/OAP
+//                                    -------------------
+// 1. The Gatherer plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to InputEvent.
+//    If the event needs output, please tag it by the IsOutput

Review comment:
       When we support sampling, no output event type needs to be supported to keep the filter chain going. The Output options should be decided by the Processor. That means the input event should always false. 




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r531692867



##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,74 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following comments is to illustrate the relationship between different plugin interface in api package.

Review comment:
       Here is the detailed thinking on the plugins interface. 




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532207523



##########
File path: internal/satellite/registry/registry.go
##########
@@ -0,0 +1,168 @@
+// Licensed to 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. Apache Software Foundation (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 registry
+
+import (
+	"fmt"
+
+	"github.com/apache/skywalking-satellite/internal/pkg/api"
+)
+
+// The creator reg.
+type pluginRegistry struct {
+	collectorCreatorRegistry map[string]CollectorCreator
+	queueCreatorRegistry     map[string]QueueCreator
+	filterCreatorRegistry    map[string]FilterCreator
+	forwarderCreatorRegistry map[string]ForwarderCreator
+	parserCreatorRegistry    map[string]ParserCreator
+	clientCreatorRegistry    map[string]ClientCreator
+}
+
+// ClientCreator creates a Client according to the config.
+type ClientCreator func(config map[string]interface{}) (api.Collector, error)
+
+// CollectorCreator creates a Collector according to the config.
+type CollectorCreator func(config map[string]interface{}) (api.Collector, error)
+
+// QueueCreator creates a Queue according to the config.
+type QueueCreator func(config map[string]interface{}) (api.Queue, error)
+
+// FilterCreator creates a Filter according to the config.
+type FilterCreator func(config map[string]interface{}) (api.Filter, error)
+
+// ForwarderCreator creates a forwarder according to the config.
+type ForwarderCreator func(config map[string]interface{}) (api.Forwarder, error)
+
+// ParserCreator creates a parser according to the config.
+type ParserCreator func(config map[string]interface{}) (api.Parser, error)
+
+var reg *pluginRegistry
+
+// RegisterClient registers the clientType as ClientCreator.
+func RegisterClient(clientType string, creator ClientCreator) {
+	fmt.Printf("register client creator success : %s", clientType)

Review comment:
       the creator is a func to create a new instance, its name is stored in a map. I think that is enough to use. In the future code, I would implement plugins in ./plugins dir.  The plugin type would be as the next level dirs, such as collector, client, or queue. And the 3rd level would be the plugin name. Maybe I would like to implement a method to read the parent dir as the plugin name.




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] wu-sheng commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r534200309



##########
File path: docs/project_structue.md
##########
@@ -0,0 +1,63 @@
+# Project Structure
+- configs: Satellite configs.
+- internal: Core, Api, and common utils.
+- internal/pkg: Sharing with Core and Plugins, such as api and utils.
+- internal/satellite: The core of Satellite.
+- plugins: Contains all plugins.
+- plugins/{type}: Contains the plugins of this {type}. Satellite has 6 plugin types, which are collector, queue, parser, filter, client, and forward.
+- plugins/{type}/define{type}: Contains the plugin define.

Review comment:
       Why need duplicate `{type}`? And define is a verb, usually, a package name is a noun.




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] wu-sheng commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532207740



##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,94 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following graph illustrates the relationship between different plugin interface in api package.
+//
+//
+//                   Gatherer                                Processor
+//       -------------------------------      -------------------------------------------
+//      |  -----------       ---------   |   |  -----------                 -----------  |
+//      | | Collector | ==> |  Queue   | |==>| |  Filter   | ==>  ...  ==> |  Filter   | |
+//      | | (Parser)  |     | Mem/File | |   |  -----------                 -----------  |
+//      |  -----------       ----------  |   |      ||                          ||       |
+//       --------------------------------    |      \/	                        \/       |
+//                                           |  ---------------------------------------  |
+//                                           | |             OutputEventContext        | |
+//                                           |  ---------------------------------------  |
+//                                            -------------------------------------------
+//                             				   ||
+//                                             ||        --------------------------------
+//                                             ||     ->|         Sharing Client        |
+//                                             ||    |   --------------------------------
+//                                             ||    |
+//                                             \/    |            SegmentSender
+//                                            ---    |  ---------------------------------
+//                                           |   |   | |  -----------       -----------  |
+//                                           | D |   |-| |BatchBuffer| ==> | Forwarder | |
+//                                           | i |   | |  -----------       -----------  |
+//                                           | s |   |  ---------------------------------
+//                                           | p |   |
+//                                           | a |=> |              .......                 ===> Kafka/OAP
+//                                           | t |   |
+//                                           | c |   |             MeterSender
+//                                           | h |   | -----------------------------------
+//                                           | e |   -|  -------------       -----------  |
+//                                           | r |    | | BatchBuffer | ==> | Forwarder | |
+//                                           |   |    |  -------------       -----------  |
+//                                            ---      -----------------------------------
+//
+//
+// 1. The Collector plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to SerializationEvent that is supported
+//    to be stored in Queue.
+// 3. The Queue plugin stores the SerializationEvent. However, the serialization depends on
+//    the Queue implements. For example, the serialization is unnecessary when using a Memory
+//    Queue.
+// 4. The Filter plugin would pull an event from the Queue and process the event to create
+//    a new event. Next, the event is passed to the next filter to do the same things until
+//    the whole filters are performed. The events labeled with RemoteEvent type would be
+//    stored in the OutputEventContext. When the processing finished,
+//    the OutputEventContext. After processing, the events in OutputEventContext would be
+//    partitioned by the event type and sent to the different BatchBuffers, such as Segment
+//    BatchBuffer, Jvm BatchBuffer, and Meter BatchBuffer.
+// 5. When each BatchBuffer reaches its maximum capacity, the OutputEventContexts would be
+//    converted to BatchEvents and sent to Forwarder.
+// 6. The Follower would send BatchEvents and ack Queue when successful process this batch

Review comment:
       How to define the output events which could have suitable senders?




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] wu-sheng commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532207584



##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,94 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following graph illustrates the relationship between different plugin interface in api package.
+//
+//
+//                   Gatherer                                Processor
+//       -------------------------------      -------------------------------------------
+//      |  -----------       ---------   |   |  -----------                 -----------  |
+//      | | Collector | ==> |  Queue   | |==>| |  Filter   | ==>  ...  ==> |  Filter   | |
+//      | | (Parser)  |     | Mem/File | |   |  -----------                 -----------  |
+//      |  -----------       ----------  |   |      ||                          ||       |
+//       --------------------------------    |      \/	                        \/       |
+//                                           |  ---------------------------------------  |
+//                                           | |             OutputEventContext        | |
+//                                           |  ---------------------------------------  |
+//                                            -------------------------------------------
+//                             				   ||
+//                                             ||        --------------------------------
+//                                             ||     ->|         Sharing Client        |
+//                                             ||    |   --------------------------------
+//                                             ||    |
+//                                             \/    |            SegmentSender
+//                                            ---    |  ---------------------------------
+//                                           |   |   | |  -----------       -----------  |
+//                                           | D |   |-| |BatchBuffer| ==> | Forwarder | |
+//                                           | i |   | |  -----------       -----------  |
+//                                           | s |   |  ---------------------------------
+//                                           | p |   |
+//                                           | a |=> |              .......                 ===> Kafka/OAP
+//                                           | t |   |
+//                                           | c |   |             MeterSender
+//                                           | h |   | -----------------------------------
+//                                           | e |   -|  -------------       -----------  |
+//                                           | r |    | | BatchBuffer | ==> | Forwarder | |
+//                                           |   |    |  -------------       -----------  |
+//                                            ---      -----------------------------------
+//
+//
+// 1. The Collector plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to SerializationEvent that is supported
+//    to be stored in Queue.
+// 3. The Queue plugin stores the SerializationEvent. However, the serialization depends on
+//    the Queue implements. For example, the serialization is unnecessary when using a Memory
+//    Queue.
+// 4. The Filter plugin would pull an event from the Queue and process the event to create
+//    a new event. Next, the event is passed to the next filter to do the same things until
+//    the whole filters are performed. The events labeled with RemoteEvent type would be

Review comment:
       I think we should add an `output` catalog in the event directly, right? And every type as `output` should have a collection of forwarder(s), right?




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] wu-sheng commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532205352



##########
File path: internal/satellite/registry/registry.go
##########
@@ -0,0 +1,168 @@
+// Licensed to 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. Apache Software Foundation (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 registry
+
+import (
+	"fmt"
+
+	"github.com/apache/skywalking-satellite/internal/pkg/api"
+)
+
+// The creator reg.
+type pluginRegistry struct {
+	collectorCreatorRegistry map[string]CollectorCreator
+	queueCreatorRegistry     map[string]QueueCreator
+	filterCreatorRegistry    map[string]FilterCreator
+	forwarderCreatorRegistry map[string]ForwarderCreator
+	parserCreatorRegistry    map[string]ParserCreator
+	clientCreatorRegistry    map[string]ClientCreator
+}
+
+// ClientCreator creates a Client according to the config.
+type ClientCreator func(config map[string]interface{}) (api.Collector, error)

Review comment:
       ```suggestion
   type ClientCreator func(config map[string]interface{}) (api.ClientCreator, error)
   ```




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532040371



##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,74 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following comments is to illustrate the relationship between different plugin interface in api package.
+//
+//
+//                                                 Processors
+//                                   -----------------------------------------
+//  ---------        ---------        -----------                 -----------
+// | Gatherer | ==> |  Queue   | ==> | Processor | ==>  ...  ==> | Processor |
+// | (Parser) |     | Mem/File |      -----------                 -----------
+//  ----------       ---------            ||                          ||
+//                                        \/	                      \/
+//                                    ---------------------------------------
+//                                   |             OutputEventContext        |
+//                                    ---------------------------------------
+//                                                                    ||
+//                                                                    \/
+//                                    -------------------       -------------
+//                                   | BatchOutputEvents | <== | BatchBuffer |
+//                                    -------------------       -------------
+//                                             ||
+//                                             \/
+//                                    -------------------
+//                                   |     Forwarder     | ==> Kakfa/OAP
+//                                    -------------------
+// 1. The Gatherer plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to InputEvent.
+//    If the event needs output, please tag it by the IsOutput
+//    method.
+// 3. The Queue plugin would store the InputEvent. But different
+//    Queue would use different ways to store data, such as store
+//    bytes by serialization or keep original.
+// 4. The Processor plugin would pull the event from the Queue and
+//    process the event to create a new event. Next, the event is
+//    passed to the next processor to do the same things until the
+//    whole processors are performed. Similar to above, if any
+//    events need output, please mark. The events would be stored
+//    in the OutputEventContext. When the processing is finished,
+//    the OutputEventContext would be passed to the BatchBuffer.
+// 5. When BatchBuffer is full, the OutputEventContexts would be
+//    partitioned by event name and convert to BatchOutputEvents.
+// 6. The Follower would be ordered to send each partition in
+//    BatchOutputEvents in different ways, such as different gRPC
+//    endpoints.
+
+// ComponentPlugin is an interface to initialize the specific plugin.
+type ComponentPlugin interface {
+	io.Closer
+
+	// Init initialize the specific plugin and would return error when the configuration is error.
+	Init() error
+
+	// Run triggers the specific plugin to work.
+	Run()

Review comment:
       Init method means to create.  The run method means to start, such as build connection. The process or publish method is core processing when running.




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] wu-sheng commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532207651



##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,94 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following graph illustrates the relationship between different plugin interface in api package.
+//
+//
+//                   Gatherer                                Processor
+//       -------------------------------      -------------------------------------------
+//      |  -----------       ---------   |   |  -----------                 -----------  |
+//      | | Collector | ==> |  Queue   | |==>| |  Filter   | ==>  ...  ==> |  Filter   | |
+//      | | (Parser)  |     | Mem/File | |   |  -----------                 -----------  |
+//      |  -----------       ----------  |   |      ||                          ||       |
+//       --------------------------------    |      \/	                        \/       |
+//                                           |  ---------------------------------------  |
+//                                           | |             OutputEventContext        | |
+//                                           |  ---------------------------------------  |
+//                                            -------------------------------------------
+//                             				   ||
+//                                             ||        --------------------------------
+//                                             ||     ->|         Sharing Client        |
+//                                             ||    |   --------------------------------
+//                                             ||    |
+//                                             \/    |            SegmentSender
+//                                            ---    |  ---------------------------------
+//                                           |   |   | |  -----------       -----------  |
+//                                           | D |   |-| |BatchBuffer| ==> | Forwarder | |
+//                                           | i |   | |  -----------       -----------  |
+//                                           | s |   |  ---------------------------------
+//                                           | p |   |
+//                                           | a |=> |              .......                 ===> Kafka/OAP
+//                                           | t |   |
+//                                           | c |   |             MeterSender
+//                                           | h |   | -----------------------------------
+//                                           | e |   -|  -------------       -----------  |
+//                                           | r |    | | BatchBuffer | ==> | Forwarder | |
+//                                           |   |    |  -------------       -----------  |
+//                                            ---      -----------------------------------
+//
+//
+// 1. The Collector plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to SerializationEvent that is supported
+//    to be stored in Queue.
+// 3. The Queue plugin stores the SerializationEvent. However, the serialization depends on
+//    the Queue implements. For example, the serialization is unnecessary when using a Memory
+//    Queue.
+// 4. The Filter plugin would pull an event from the Queue and process the event to create
+//    a new event. Next, the event is passed to the next filter to do the same things until
+//    the whole filters are performed. The events labeled with RemoteEvent type would be
+//    stored in the OutputEventContext. When the processing finished,
+//    the OutputEventContext. After processing, the events in OutputEventContext would be
+//    partitioned by the event type and sent to the different BatchBuffers, such as Segment
+//    BatchBuffer, Jvm BatchBuffer, and Meter BatchBuffer.
+// 5. When each BatchBuffer reaches its maximum capacity, the OutputEventContexts would be

Review comment:
       Do we have a consuming-period for `BatchBuffer`?




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r533410861



##########
File path: internal/satellite/event/struectured_event.go
##########
@@ -0,0 +1,79 @@
+// Licensed to 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. Apache Software Foundation (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 event
+
+import (
+	"time"
+
+	"github.com/apache/skywalking-satellite/internal/pkg/api"
+)
+
+// StructuredInputEventToBytesFunc serialize event to bytes.
+type StructuredInputEventToBytesFunc func(event *StructuredInputEvent) []byte
+
+// BytesToStructuredInputEventFunc deserialize bytes to event.
+type BytesToStructuredInputEventFunc func(bytes []byte) *StructuredInputEvent
+
+// StructuredEvent works when the data is a struct type.
+type StructuredEvent struct {

Review comment:
       for example segment.proto




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] wu-sheng commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532206706



##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,94 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following graph illustrates the relationship between different plugin interface in api package.
+//
+//
+//                   Gatherer                                Processor
+//       -------------------------------      -------------------------------------------
+//      |  -----------       ---------   |   |  -----------                 -----------  |
+//      | | Collector | ==> |  Queue   | |==>| |  Filter   | ==>  ...  ==> |  Filter   | |
+//      | | (Parser)  |     | Mem/File | |   |  -----------                 -----------  |
+//      |  -----------       ----------  |   |      ||                          ||       |
+//       --------------------------------    |      \/	                        \/       |
+//                                           |  ---------------------------------------  |
+//                                           | |             OutputEventContext        | |
+//                                           |  ---------------------------------------  |
+//                                            -------------------------------------------
+//                             				   ||
+//                                             ||        --------------------------------
+//                                             ||     ->|         Sharing Client        |
+//                                             ||    |   --------------------------------
+//                                             ||    |
+//                                             \/    |            SegmentSender
+//                                            ---    |  ---------------------------------
+//                                           |   |   | |  -----------       -----------  |
+//                                           | D |   |-| |BatchBuffer| ==> | Forwarder | |
+//                                           | i |   | |  -----------       -----------  |
+//                                           | s |   |  ---------------------------------
+//                                           | p |   |
+//                                           | a |=> |              .......                 ===> Kafka/OAP
+//                                           | t |   |
+//                                           | c |   |             MeterSender
+//                                           | h |   | -----------------------------------
+//                                           | e |   -|  -------------       -----------  |
+//                                           | r |    | | BatchBuffer | ==> | Forwarder | |
+//                                           |   |    |  -------------       -----------  |
+//                                            ---      -----------------------------------
+//
+//
+// 1. The Collector plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to SerializationEvent that is supported
+//    to be stored in Queue.
+// 3. The Queue plugin stores the SerializationEvent. However, the serialization depends on

Review comment:
       ```suggestion
   // 3. The Queue plugin stores the SerializationEvent. However, whether serializing depends on
   ```




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532225945



##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,94 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following graph illustrates the relationship between different plugin interface in api package.
+//
+//
+//                   Gatherer                                Processor
+//       -------------------------------      -------------------------------------------
+//      |  -----------       ---------   |   |  -----------                 -----------  |
+//      | | Collector | ==> |  Queue   | |==>| |  Filter   | ==>  ...  ==> |  Filter   | |
+//      | | (Parser)  |     | Mem/File | |   |  -----------                 -----------  |
+//      |  -----------       ----------  |   |      ||                          ||       |
+//       --------------------------------    |      \/	                        \/       |
+//                                           |  ---------------------------------------  |
+//                                           | |             OutputEventContext        | |
+//                                           |  ---------------------------------------  |
+//                                            -------------------------------------------
+//                             				   ||
+//                                             ||        --------------------------------
+//                                             ||     ->|         Sharing Client        |
+//                                             ||    |   --------------------------------
+//                                             ||    |
+//                                             \/    |            SegmentSender
+//                                            ---    |  ---------------------------------
+//                                           |   |   | |  -----------       -----------  |
+//                                           | D |   |-| |BatchBuffer| ==> | Forwarder | |
+//                                           | i |   | |  -----------       -----------  |
+//                                           | s |   |  ---------------------------------
+//                                           | p |   |
+//                                           | a |=> |              .......                 ===> Kafka/OAP
+//                                           | t |   |
+//                                           | c |   |             MeterSender
+//                                           | h |   | -----------------------------------
+//                                           | e |   -|  -------------       -----------  |
+//                                           | r |    | | BatchBuffer | ==> | Forwarder | |
+//                                           |   |    |  -------------       -----------  |
+//                                            ---      -----------------------------------
+//
+//
+// 1. The Collector plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to SerializationEvent that is supported
+//    to be stored in Queue.
+// 3. The Queue plugin stores the SerializationEvent. However, the serialization depends on
+//    the Queue implements. For example, the serialization is unnecessary when using a Memory
+//    Queue.
+// 4. The Filter plugin would pull an event from the Queue and process the event to create
+//    a new event. Next, the event is passed to the next filter to do the same things until
+//    the whole filters are performed. The events labeled with RemoteEvent type would be

Review comment:
       add comments in plugin.go




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r533412645



##########
File path: internal/pkg/api/event.go
##########
@@ -0,0 +1,95 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import (
+	"fmt"
+	"time"
+)
+
+// The event type.
+const (
+	// Mapping to the type supported by SkyWalking OAP.
+	_ EventType = iota
+	MetricsEvent
+	ProfilingEvent
+	SegmentEvent
+	ManagementEvent
+	MeterEvent
+	LogEvent
+)
+
+type EventType int32
+
+// Event that implement this interface would be allowed to transmit in the Satellite.
+type Event interface {
+	// Name returns the event name.
+	Name() string
+
+	// Meta is a pair of key and value to record meta data, such as labels.
+	Meta() map[string]string
+
+	// Data returns the wrapped data.
+	Data() interface{}
+
+	// Time returns the event time.
+	Time() time.Time
+
+	// Type is to distinguish different events.
+	Type() EventType

Review comment:
       in OutputEventContext




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp edited a comment on pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp edited a comment on pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#issuecomment-734920733


   > 3\. Why Output named as `BatchOutputEvents`? Batch or no batch depends on filter works. You could have several inputs to one output, vice versa.
   
   The Batch is pointed to the BatchBuffer context in the design doc.
   The core structure of BatchEvents is  *map[string][]Event*, which has many slices partition by event name.
   In the first release version,  Satellite would more forward rather than processing. So, the Sender would be independent in Log, Metrics, and Trace. In the future, some advanced features would break the current mod, such as extract Metrics from Log. SO, The Sender Layer would be a shared layer to avoid make duplicate connections.


----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r534208738



##########
File path: docs/project_structue.md
##########
@@ -0,0 +1,63 @@
+# Project Structure
+- configs: Satellite configs.
+- internal: Core, Api, and common utils.
+- internal/pkg: Sharing with Core and Plugins, such as api and utils.
+- internal/satellite: The core of Satellite.
+- plugins: Contains all plugins.
+- plugins/{type}: Contains the plugins of this {type}. Satellite has 6 plugin types, which are collector, queue, parser, filter, client, and forward.
+- plugins/{type}/define{type}: Contains the plugin define.

Review comment:
       already fixed 




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] surlymo closed pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
surlymo closed pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5


   


----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532220022



##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,94 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following graph illustrates the relationship between different plugin interface in api package.
+//
+//
+//                   Gatherer                                Processor
+//       -------------------------------      -------------------------------------------
+//      |  -----------       ---------   |   |  -----------                 -----------  |
+//      | | Collector | ==> |  Queue   | |==>| |  Filter   | ==>  ...  ==> |  Filter   | |
+//      | | (Parser)  |     | Mem/File | |   |  -----------                 -----------  |
+//      |  -----------       ----------  |   |      ||                          ||       |
+//       --------------------------------    |      \/	                        \/       |
+//                                           |  ---------------------------------------  |
+//                                           | |             OutputEventContext        | |
+//                                           |  ---------------------------------------  |
+//                                            -------------------------------------------
+//                             				   ||
+//                                             ||        --------------------------------
+//                                             ||     ->|         Sharing Client        |
+//                                             ||    |   --------------------------------
+//                                             ||    |
+//                                             \/    |            SegmentSender
+//                                            ---    |  ---------------------------------
+//                                           |   |   | |  -----------       -----------  |
+//                                           | D |   |-| |BatchBuffer| ==> | Forwarder | |
+//                                           | i |   | |  -----------       -----------  |
+//                                           | s |   |  ---------------------------------
+//                                           | p |   |
+//                                           | a |=> |              .......                 ===> Kafka/OAP
+//                                           | t |   |
+//                                           | c |   |             MeterSender
+//                                           | h |   | -----------------------------------
+//                                           | e |   -|  -------------       -----------  |
+//                                           | r |    | | BatchBuffer | ==> | Forwarder | |
+//                                           |   |    |  -------------       -----------  |
+//                                            ---      -----------------------------------
+//
+//
+// 1. The Collector plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to SerializationEvent that is supported
+//    to be stored in Queue.
+// 3. The Queue plugin stores the SerializationEvent. However, the serialization depends on
+//    the Queue implements. For example, the serialization is unnecessary when using a Memory
+//    Queue.
+// 4. The Filter plugin would pull an event from the Queue and process the event to create

Review comment:
       right, would polish the 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



[GitHub] [skywalking-satellite] wu-sheng commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r534203882



##########
File path: docs/project_structue.md
##########
@@ -0,0 +1,63 @@
+# Project Structure
+- configs: Satellite configs.
+- internal: Core, Api, and common utils.
+- internal/pkg: Sharing with Core and Plugins, such as api and utils.
+- internal/satellite: The core of Satellite.
+- plugins: Contains all plugins.
+- plugins/{type}: Contains the plugins of this {type}. Satellite has 6 plugin types, which are collector, queue, parser, filter, client, and forward.
+- plugins/{type}/define{type}: Contains the plugin define.

Review comment:
       What conflict?




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532560110



##########
File path: internal/pkg/api/event.go
##########
@@ -0,0 +1,103 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import (
+	"fmt"
+	"time"
+)
+
+// The event type.
+const (
+	// Mapping to the type supported by SkyWalking OAP.
+	_ EventType = iota
+	MetricsEvent
+	ProfilingEvent
+	SegmentEvent
+	ManagementEvent
+	MeterEvent
+	LogEvent
+)
+
+type EventType int32
+
+// Event that implement this interface would be allowed to transmit in the Satellite.
+type Event interface {
+	// Name returns the event name.
+	Name() string
+
+	// Meta is a pair of key and value to record meta data, such as labels.
+	Meta() map[string]string
+
+	// Data returns the wrapped data.
+	Data() interface{}
+
+	// Time returns the event time.
+	Time() time.Time
+
+	// Type is a identifier to distinguish different events.

Review comment:
       for distinguishing different events  to use different forwarders, such as log forwarder and metrics forwarder




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] wu-sheng commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532205557



##########
File path: internal/satellite/registry/registry.go
##########
@@ -0,0 +1,168 @@
+// Licensed to 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. Apache Software Foundation (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 registry
+
+import (
+	"fmt"
+
+	"github.com/apache/skywalking-satellite/internal/pkg/api"
+)
+
+// The creator reg.
+type pluginRegistry struct {
+	collectorCreatorRegistry map[string]CollectorCreator
+	queueCreatorRegistry     map[string]QueueCreator
+	filterCreatorRegistry    map[string]FilterCreator
+	forwarderCreatorRegistry map[string]ForwarderCreator
+	parserCreatorRegistry    map[string]ParserCreator
+	clientCreatorRegistry    map[string]ClientCreator
+}
+
+// ClientCreator creates a Client according to the config.
+type ClientCreator func(config map[string]interface{}) (api.Collector, error)
+
+// CollectorCreator creates a Collector according to the config.
+type CollectorCreator func(config map[string]interface{}) (api.Collector, error)
+
+// QueueCreator creates a Queue according to the config.
+type QueueCreator func(config map[string]interface{}) (api.Queue, error)
+
+// FilterCreator creates a Filter according to the config.
+type FilterCreator func(config map[string]interface{}) (api.Filter, error)
+
+// ForwarderCreator creates a forwarder according to the config.
+type ForwarderCreator func(config map[string]interface{}) (api.Forwarder, error)
+
+// ParserCreator creates a parser according to the config.
+type ParserCreator func(config map[string]interface{}) (api.Parser, error)
+
+var reg *pluginRegistry
+
+// RegisterClient registers the clientType as ClientCreator.
+func RegisterClient(clientType string, creator ClientCreator) {
+	fmt.Printf("register client creator success : %s", clientType)

Review comment:
       `register client creator success`, this is not a correct statement`. I think you mean, 
   `Create %s creator register successfully`? Or reword it a little.
   Also, does the creator have a name to identify?




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] wu-sheng commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532002585



##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,74 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following comments is to illustrate the relationship between different plugin interface in api package.
+//
+//
+//                                                 Processors
+//                                   -----------------------------------------
+//  ---------        ---------        -----------                 -----------
+// | Gatherer | ==> |  Queue   | ==> | Processor | ==>  ...  ==> | Processor |
+// | (Parser) |     | Mem/File |      -----------                 -----------
+//  ----------       ---------            ||                          ||
+//                                        \/	                      \/
+//                                    ---------------------------------------
+//                                   |             OutputEventContext        |
+//                                    ---------------------------------------
+//                                                                    ||
+//                                                                    \/
+//                                    -------------------       -------------
+//                                   | BatchOutputEvents | <== | BatchBuffer |
+//                                    -------------------       -------------
+//                                             ||
+//                                             \/
+//                                    -------------------
+//                                   |     Forwarder     | ==> Kakfa/OAP
+//                                    -------------------
+// 1. The Gatherer plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to InputEvent.
+//    If the event needs output, please tag it by the IsOutput

Review comment:
       > If the event needs output
   
   What event doesn't need output? Could we be clear about the case?

##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,74 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following comments is to illustrate the relationship between different plugin interface in api package.
+//
+//
+//                                                 Processors
+//                                   -----------------------------------------
+//  ---------        ---------        -----------                 -----------
+// | Gatherer | ==> |  Queue   | ==> | Processor | ==>  ...  ==> | Processor |
+// | (Parser) |     | Mem/File |      -----------                 -----------
+//  ----------       ---------            ||                          ||
+//                                        \/	                      \/
+//                                    ---------------------------------------
+//                                   |             OutputEventContext        |
+//                                    ---------------------------------------
+//                                                                    ||
+//                                                                    \/
+//                                    -------------------       -------------
+//                                   | BatchOutputEvents | <== | BatchBuffer |
+//                                    -------------------       -------------
+//                                             ||
+//                                             \/
+//                                    -------------------
+//                                   |     Forwarder     | ==> Kakfa/OAP
+//                                    -------------------
+// 1. The Gatherer plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to InputEvent.
+//    If the event needs output, please tag it by the IsOutput
+//    method.
+// 3. The Queue plugin would store the InputEvent. But different
+//    Queue would use different ways to store data, such as store
+//    bytes by serialization or keep original.
+// 4. The Processor plugin would pull the event from the Queue and

Review comment:
       Why processor is a plugin system? From my understanding, the processor should be a driving system, the filter itself is the unit to be combined. Please correct me if I am wrong.

##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,74 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following comments is to illustrate the relationship between different plugin interface in api package.
+//
+//
+//                                                 Processors
+//                                   -----------------------------------------
+//  ---------        ---------        -----------                 -----------
+// | Gatherer | ==> |  Queue   | ==> | Processor | ==>  ...  ==> | Processor |
+// | (Parser) |     | Mem/File |      -----------                 -----------
+//  ----------       ---------            ||                          ||
+//                                        \/	                      \/
+//                                    ---------------------------------------
+//                                   |             OutputEventContext        |
+//                                    ---------------------------------------
+//                                                                    ||
+//                                                                    \/
+//                                    -------------------       -------------
+//                                   | BatchOutputEvents | <== | BatchBuffer |
+//                                    -------------------       -------------
+//                                             ||
+//                                             \/
+//                                    -------------------
+//                                   |     Forwarder     | ==> Kakfa/OAP
+//                                    -------------------
+// 1. The Gatherer plugin would fetch or receive the input data.

Review comment:
       Why naming gatherer as parser interface as the core API? I think the gatherer implementation has the full responsibility to enqueue the fetched/received data. Could you give a more clear explanation?
   Gatherer usually requires a client or server. You should provide a dependency injection mechanism, in order to share things like gRPC server, HTTP server, etc, not explicitly creating its own.

##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,74 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following comments is to illustrate the relationship between different plugin interface in api package.
+//
+//
+//                                                 Processors
+//                                   -----------------------------------------
+//  ---------        ---------        -----------                 -----------
+// | Gatherer | ==> |  Queue   | ==> | Processor | ==>  ...  ==> | Processor |
+// | (Parser) |     | Mem/File |      -----------                 -----------
+//  ----------       ---------            ||                          ||
+//                                        \/	                      \/
+//                                    ---------------------------------------
+//                                   |             OutputEventContext        |
+//                                    ---------------------------------------
+//                                                                    ||
+//                                                                    \/
+//                                    -------------------       -------------
+//                                   | BatchOutputEvents | <== | BatchBuffer |
+//                                    -------------------       -------------
+//                                             ||
+//                                             \/
+//                                    -------------------
+//                                   |     Forwarder     | ==> Kakfa/OAP
+//                                    -------------------
+// 1. The Gatherer plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to InputEvent.
+//    If the event needs output, please tag it by the IsOutput
+//    method.
+// 3. The Queue plugin would store the InputEvent. But different
+//    Queue would use different ways to store data, such as store
+//    bytes by serialization or keep original.
+// 4. The Processor plugin would pull the event from the Queue and
+//    process the event to create a new event. Next, the event is
+//    passed to the next processor to do the same things until the
+//    whole processors are performed. Similar to above, if any
+//    events need output, please mark. The events would be stored
+//    in the OutputEventContext. When the processing is finished,
+//    the OutputEventContext would be passed to the BatchBuffer.
+// 5. When BatchBuffer is full, the OutputEventContexts would be

Review comment:
       `When BatchBuffer is full` <- Is this the correct statement? Isn't it a queue? 

##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,74 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following comments is to illustrate the relationship between different plugin interface in api package.
+//
+//
+//                                                 Processors
+//                                   -----------------------------------------
+//  ---------        ---------        -----------                 -----------
+// | Gatherer | ==> |  Queue   | ==> | Processor | ==>  ...  ==> | Processor |
+// | (Parser) |     | Mem/File |      -----------                 -----------
+//  ----------       ---------            ||                          ||
+//                                        \/	                      \/
+//                                    ---------------------------------------
+//                                   |             OutputEventContext        |
+//                                    ---------------------------------------
+//                                                                    ||
+//                                                                    \/
+//                                    -------------------       -------------
+//                                   | BatchOutputEvents | <== | BatchBuffer |
+//                                    -------------------       -------------
+//                                             ||
+//                                             \/
+//                                    -------------------
+//                                   |     Forwarder     | ==> Kakfa/OAP
+//                                    -------------------
+// 1. The Gatherer plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to InputEvent.
+//    If the event needs output, please tag it by the IsOutput
+//    method.
+// 3. The Queue plugin would store the InputEvent. But different

Review comment:
       For a queue, why need input and output? Our queue doesn't support the lambda function system, which means, the input is the output. The whole point of the queue is buffering and avoiding data-lost, right?

##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,74 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following comments is to illustrate the relationship between different plugin interface in api package.
+//
+//
+//                                                 Processors
+//                                   -----------------------------------------
+//  ---------        ---------        -----------                 -----------
+// | Gatherer | ==> |  Queue   | ==> | Processor | ==>  ...  ==> | Processor |
+// | (Parser) |     | Mem/File |      -----------                 -----------
+//  ----------       ---------            ||                          ||
+//                                        \/	                      \/
+//                                    ---------------------------------------
+//                                   |             OutputEventContext        |
+//                                    ---------------------------------------
+//                                                                    ||
+//                                                                    \/
+//                                    -------------------       -------------
+//                                   | BatchOutputEvents | <== | BatchBuffer |
+//                                    -------------------       -------------
+//                                             ||
+//                                             \/
+//                                    -------------------
+//                                   |     Forwarder     | ==> Kakfa/OAP
+//                                    -------------------
+// 1. The Gatherer plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to InputEvent.
+//    If the event needs output, please tag it by the IsOutput
+//    method.
+// 3. The Queue plugin would store the InputEvent. But different
+//    Queue would use different ways to store data, such as store
+//    bytes by serialization or keep original.
+// 4. The Processor plugin would pull the event from the Queue and
+//    process the event to create a new event. Next, the event is
+//    passed to the next processor to do the same things until the

Review comment:
       What do you mean by the next processor? Do you plan to support multiple consumers of gatherer queue?




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp edited a comment on pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp edited a comment on pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#issuecomment-737255710


   > Why named package like `defineclient`? What does this mean?
   
   move plugin API to plugin dir ? I do this to avoid packet conflicts ,Do you have any other naming suggestions?  could I merge? 


----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] surlymo commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
surlymo commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r533917767



##########
File path: internal/satellite/registry/registry.go
##########
@@ -20,102 +20,111 @@ package registry
 import (
 	"fmt"
 	"reflect"
+	"sync"
 
 	"github.com/apache/skywalking-satellite/internal/pkg/api"
 )
 
-// The creator registry.
 // All plugins is wrote in ./plugins dir. The plugin type would be as the next level dirs,
 // such as collector, client, or queue. And the 3rd level is the plugin name, that is also
 // used as key in pluginRegistry.
-type pluginRegistry struct {
-	collectorRegistry  map[string]api.Collector
-	queueRegistry      map[string]api.Queue
-	filterRegistry     map[string]api.Filter
-	forwarderRegistry  map[string]api.Forwarder
-	parserRegistry     map[string]api.Parser
-	clientRegistry     map[string]api.Client
-	fallbackerRegistry map[string]api.Fallbacker
-}
 
 // reg is the global plugin registry
-var reg *pluginRegistry
+var reg map[reflect.Type]map[string]interface{}
+var lock sync.Mutex
 
-// Plugin type.
+// Supported plugin types
 var (
 	collectorType  = reflect.TypeOf((*api.Collector)(nil)).Elem()
 	queueType      = reflect.TypeOf((*api.Queue)(nil)).Elem()
 	filterType     = reflect.TypeOf((*api.Filter)(nil)).Elem()
-	forwardType    = reflect.TypeOf((*api.Forwarder)(nil)).Elem()
+	forwarderType  = reflect.TypeOf((*api.Forwarder)(nil)).Elem()
 	parserType     = reflect.TypeOf((*api.Parser)(nil)).Elem()
 	clientType     = reflect.TypeOf((*api.Client)(nil)).Elem()
 	fallbackerType = reflect.TypeOf((*api.Fallbacker)(nil)).Elem()
 )
 
 func init() {
-	reg = &pluginRegistry{}
-	reg.collectorRegistry = make(map[string]api.Collector)
-	reg.queueRegistry = make(map[string]api.Queue)
-	reg.filterRegistry = make(map[string]api.Filter)
-	reg.forwarderRegistry = make(map[string]api.Forwarder)
-	reg.parserRegistry = make(map[string]api.Parser)
-	reg.clientRegistry = make(map[string]api.Client)
-	reg.fallbackerRegistry = make(map[string]api.Fallbacker)
+	reg = make(map[reflect.Type]map[string]interface{})
+	reg[collectorType] = make(map[string]interface{})
+	reg[queueType] = make(map[string]interface{})
+	reg[filterType] = make(map[string]interface{})
+	reg[forwarderType] = make(map[string]interface{})
+	reg[parserType] = make(map[string]interface{})
+	reg[clientType] = make(map[string]interface{})
+	reg[fallbackerType] = make(map[string]interface{})
 }
 
 // RegisterPlugin registers the pluginType as plugin.
-func RegisterPlugin(pluginType string, plugin interface{}) {
+func RegisterPlugin(pluginName string, plugin interface{}) {
+	lock.Lock()
+	defer lock.Unlock()
 	t := reflect.TypeOf(plugin)
-	switch {
-	case t.Implements(collectorType):
-		fmt.Printf("register %s collector successfully ", t.String())
-		reg.collectorRegistry[pluginType] = plugin.(api.Collector)
-	case t.Implements(queueType):
-		fmt.Printf("register %s queue successfully ", t.String())
-		reg.queueRegistry[pluginType] = plugin.(api.Queue)
-	case t.Implements(filterType):
-		fmt.Printf("register %s filter successfully ", t.String())
-		reg.filterRegistry[pluginType] = plugin.(api.Filter)
-	case t.Implements(forwardType):
-		fmt.Printf("register %s forwarder successfully ", t.String())
-		reg.forwarderRegistry[pluginType] = plugin.(api.Forwarder)
-	case t.Implements(parserType):
-		fmt.Printf("register %s parser successfully ", t.String())
-		reg.parserRegistry[pluginType] = plugin.(api.Parser)
-	case t.Implements(clientType):
-		fmt.Printf("register %s client successfully ", t.String())
-		reg.clientRegistry[pluginType] = plugin.(api.Client)
-	case t.Implements(fallbackerType):
-		fmt.Printf("register %s fallbacker successfully ", t.String())
-		reg.fallbackerRegistry[pluginType] = plugin.(api.Fallbacker)
-	default:
-		fmt.Printf("this type is not supported to register : %s", t.String())
+	success := false
+	for pType, pReg := range reg {
+		if t.Implements(pType) {
+			pReg[pluginName] = plugin
+			fmt.Printf("register %s %s successfully ", pluginName, t.String())
+			success = true
+			break
+		}
+	}
+	if !success {
+		fmt.Printf("this type of %s is not supported to register : %s", pluginName, t.String())
 	}
 }
 
-func GetCollector(pluginType string) api.Collector {
-	return reg.collectorRegistry[pluginType]
+func GetCollector(pluginName string) api.Collector {
+	plugin := reg[collectorType][pluginName]
+	if plugin != nil {
+		return plugin.(api.Collector)
+	}
+	return nil
 }
 
-func GetQueue(pluginType string) api.Queue {
-	return reg.queueRegistry[pluginType]
+func GetQueue(pluginName string) api.Queue {
+	plugin := reg[queueType][pluginName]
+	if plugin != nil {
+		return plugin.(api.Queue)
+	}
+	return nil
 }
 
-func GetFilter(pluginType string) api.Filter {
-	return reg.filterRegistry[pluginType]
+func GetFilter(pluginName string) api.Filter {

Review comment:
       DRY

##########
File path: internal/satellite/registry/registry.go
##########
@@ -20,102 +20,111 @@ package registry
 import (
 	"fmt"
 	"reflect"
+	"sync"
 
 	"github.com/apache/skywalking-satellite/internal/pkg/api"
 )
 
-// The creator registry.
 // All plugins is wrote in ./plugins dir. The plugin type would be as the next level dirs,
 // such as collector, client, or queue. And the 3rd level is the plugin name, that is also
 // used as key in pluginRegistry.
-type pluginRegistry struct {
-	collectorRegistry  map[string]api.Collector
-	queueRegistry      map[string]api.Queue
-	filterRegistry     map[string]api.Filter
-	forwarderRegistry  map[string]api.Forwarder
-	parserRegistry     map[string]api.Parser
-	clientRegistry     map[string]api.Client
-	fallbackerRegistry map[string]api.Fallbacker
-}
 
 // reg is the global plugin registry
-var reg *pluginRegistry
+var reg map[reflect.Type]map[string]interface{}
+var lock sync.Mutex
 
-// Plugin type.
+// Supported plugin types
 var (
 	collectorType  = reflect.TypeOf((*api.Collector)(nil)).Elem()
 	queueType      = reflect.TypeOf((*api.Queue)(nil)).Elem()
 	filterType     = reflect.TypeOf((*api.Filter)(nil)).Elem()
-	forwardType    = reflect.TypeOf((*api.Forwarder)(nil)).Elem()
+	forwarderType  = reflect.TypeOf((*api.Forwarder)(nil)).Elem()
 	parserType     = reflect.TypeOf((*api.Parser)(nil)).Elem()
 	clientType     = reflect.TypeOf((*api.Client)(nil)).Elem()
 	fallbackerType = reflect.TypeOf((*api.Fallbacker)(nil)).Elem()
 )
 
 func init() {
-	reg = &pluginRegistry{}
-	reg.collectorRegistry = make(map[string]api.Collector)
-	reg.queueRegistry = make(map[string]api.Queue)
-	reg.filterRegistry = make(map[string]api.Filter)
-	reg.forwarderRegistry = make(map[string]api.Forwarder)
-	reg.parserRegistry = make(map[string]api.Parser)
-	reg.clientRegistry = make(map[string]api.Client)
-	reg.fallbackerRegistry = make(map[string]api.Fallbacker)
+	reg = make(map[reflect.Type]map[string]interface{})

Review comment:
       merge code above. and make sure NONE specific plugin type related code coupled with plugin management to guarantee low coupling design

##########
File path: internal/pkg/api/plugin.go
##########
@@ -78,6 +78,8 @@ import "io"
 // preparing phase, running phase, and closing phase. In the running phase, each plugin has
 // its own interface definition. However, the other three phases have to be defined uniformly.
 
+// NOTICE: Each plugin should have a unique method to distinguish from other plugins.

Review comment:
       why? and it looks like a nonexcutable require?




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] hanahmily commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
hanahmily commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532007749



##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,74 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following comments is to illustrate the relationship between different plugin interface in api package.
+//
+//
+//                                                 Processors
+//                                   -----------------------------------------
+//  ---------        ---------        -----------                 -----------
+// | Gatherer | ==> |  Queue   | ==> | Processor | ==>  ...  ==> | Processor |
+// | (Parser) |     | Mem/File |      -----------                 -----------
+//  ----------       ---------            ||                          ||
+//                                        \/	                      \/
+//                                    ---------------------------------------
+//                                   |             OutputEventContext        |
+//                                    ---------------------------------------
+//                                                                    ||
+//                                                                    \/
+//                                    -------------------       -------------
+//                                   | BatchOutputEvents | <== | BatchBuffer |
+//                                    -------------------       -------------
+//                                             ||
+//                                             \/
+//                                    -------------------
+//                                   |     Forwarder     | ==> Kakfa/OAP
+//                                    -------------------
+// 1. The Gatherer plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to InputEvent.
+//    If the event needs output, please tag it by the IsOutput
+//    method.
+// 3. The Queue plugin would store the InputEvent. But different
+//    Queue would use different ways to store data, such as store
+//    bytes by serialization or keep original.
+// 4. The Processor plugin would pull the event from the Queue and
+//    process the event to create a new event. Next, the event is
+//    passed to the next processor to do the same things until the
+//    whole processors are performed. Similar to above, if any
+//    events need output, please mark. The events would be stored
+//    in the OutputEventContext. When the processing is finished,
+//    the OutputEventContext would be passed to the BatchBuffer.
+// 5. When BatchBuffer is full, the OutputEventContexts would be
+//    partitioned by event name and convert to BatchOutputEvents.
+// 6. The Follower would be ordered to send each partition in
+//    BatchOutputEvents in different ways, such as different gRPC
+//    endpoints.
+
+// ComponentPlugin is an interface to initialize the specific plugin.
+type ComponentPlugin interface {
+	io.Closer
+
+	// Init initialize the specific plugin and would return error when the configuration is error.
+	Init() error
+
+	// Run triggers the specific plugin to work.
+	Run()

Review comment:
       What does a general `Run` function work for? I found other interfaces embed `ComponentPlugin` define some dedicated functions, for example, `Process`, `Publish` and etc. I have no idea of the role of the `Run` function.
   
   




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] wu-sheng commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532208893



##########
File path: internal/satellite/registry/registry.go
##########
@@ -0,0 +1,168 @@
+// Licensed to 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. Apache Software Foundation (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 registry
+
+import (
+	"fmt"
+
+	"github.com/apache/skywalking-satellite/internal/pkg/api"
+)
+
+// The creator reg.
+type pluginRegistry struct {
+	collectorCreatorRegistry map[string]CollectorCreator
+	queueCreatorRegistry     map[string]QueueCreator
+	filterCreatorRegistry    map[string]FilterCreator
+	forwarderCreatorRegistry map[string]ForwarderCreator
+	parserCreatorRegistry    map[string]ParserCreator
+	clientCreatorRegistry    map[string]ClientCreator
+}
+
+// ClientCreator creates a Client according to the config.
+type ClientCreator func(config map[string]interface{}) (api.Collector, error)
+
+// CollectorCreator creates a Collector according to the config.
+type CollectorCreator func(config map[string]interface{}) (api.Collector, error)
+
+// QueueCreator creates a Queue according to the config.
+type QueueCreator func(config map[string]interface{}) (api.Queue, error)
+
+// FilterCreator creates a Filter according to the config.
+type FilterCreator func(config map[string]interface{}) (api.Filter, error)
+
+// ForwarderCreator creates a forwarder according to the config.
+type ForwarderCreator func(config map[string]interface{}) (api.Forwarder, error)
+
+// ParserCreator creates a parser according to the config.
+type ParserCreator func(config map[string]interface{}) (api.Parser, error)
+
+var reg *pluginRegistry
+
+// RegisterClient registers the clientType as ClientCreator.
+func RegisterClient(clientType string, creator ClientCreator) {
+	fmt.Printf("register client creator success : %s", clientType)

Review comment:
       Once you could have clear log info, it is good to me.

##########
File path: internal/satellite/registry/registry.go
##########
@@ -0,0 +1,168 @@
+// Licensed to 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. Apache Software Foundation (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 registry
+
+import (
+	"fmt"
+
+	"github.com/apache/skywalking-satellite/internal/pkg/api"
+)
+
+// The creator reg.
+type pluginRegistry struct {
+	collectorCreatorRegistry map[string]CollectorCreator
+	queueCreatorRegistry     map[string]QueueCreator
+	filterCreatorRegistry    map[string]FilterCreator
+	forwarderCreatorRegistry map[string]ForwarderCreator
+	parserCreatorRegistry    map[string]ParserCreator
+	clientCreatorRegistry    map[string]ClientCreator
+}
+
+// ClientCreator creates a Client according to the config.
+type ClientCreator func(config map[string]interface{}) (api.Collector, error)
+
+// CollectorCreator creates a Collector according to the config.
+type CollectorCreator func(config map[string]interface{}) (api.Collector, error)
+
+// QueueCreator creates a Queue according to the config.
+type QueueCreator func(config map[string]interface{}) (api.Queue, error)
+
+// FilterCreator creates a Filter according to the config.
+type FilterCreator func(config map[string]interface{}) (api.Filter, error)
+
+// ForwarderCreator creates a forwarder according to the config.
+type ForwarderCreator func(config map[string]interface{}) (api.Forwarder, error)
+
+// ParserCreator creates a parser according to the config.
+type ParserCreator func(config map[string]interface{}) (api.Parser, error)
+
+var reg *pluginRegistry
+
+// RegisterClient registers the clientType as ClientCreator.
+func RegisterClient(clientType string, creator ClientCreator) {
+	fmt.Printf("register client creator success : %s", clientType)

Review comment:
       Once you could have clear log info, it is good for me.




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp removed a comment on pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp removed a comment on pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#issuecomment-737255037


   @wu-sheng Could I merge?


----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#issuecomment-737255710


   > Why named package like `defineclient`? What does this mean?
   
   move plugin API to plugin dir ? I do this to avoid packet conflicts ,Do you have any other naming suggestions?


----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r534210310



##########
File path: docs/project_structue.md
##########
@@ -0,0 +1,63 @@
+# Project Structure
+- configs: Satellite configs.
+- internal: Core, Api, and common utils.
+- internal/pkg: Sharing with Core and Plugins, such as api and utils.
+- internal/satellite: The core of Satellite.
+- plugins: Contains all plugins.
+- plugins/{type}: Contains the plugins of this {type}. Satellite has 6 plugin types, which are collector, queue, parser, filter, client, and forward.
+- plugins/{type}/define{type}: Contains the plugin define.

Review comment:
       > What conflict?
   
   It's my 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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r531692867



##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,74 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following comments is to illustrate the relationship between different plugin interface in api package.

Review comment:
       Here is the detailed thinking on the plugins interface. @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



[GitHub] [skywalking-satellite] surlymo commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
surlymo commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r533387287



##########
File path: internal/pkg/api/forwarder.go
##########
@@ -0,0 +1,42 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+//   Init()     Initiating stage: Init plugin by config
+//    ||
+//    \/
+//   Prepare()   Preparing stage: Prepare the Forwarder, such as get remote client.
+//    ||
+//    \/
+//   Forward()  Running stage: Forward the batch events
+//    ||
+//    \/
+//   Close()    Closing stage: Close the Collector, such as close connection with SkyWalking javaagent.
+
+// Forwarder is a plugin interface, that defines new forwarders.
+type Forwarder interface {
+	Initializer
+	Preparer
+	Closer
+
+	// Forward the batch events to the external services, such as Kafka MQ and SkyWalking OAP cluster.
+	Forward(batch BatchEvents)
+
+	// ForwardType returns the supporting event type that could be forwarded.
+	ForwardType()

Review comment:
       where is return type?

##########
File path: internal/satellite/event/struectured_event.go
##########
@@ -0,0 +1,79 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor

Review comment:
       typo file name

##########
File path: internal/pkg/api/event.go
##########
@@ -0,0 +1,95 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import (
+	"fmt"
+	"time"
+)
+
+// The event type.
+const (
+	// Mapping to the type supported by SkyWalking OAP.
+	_ EventType = iota
+	MetricsEvent
+	ProfilingEvent
+	SegmentEvent
+	ManagementEvent
+	MeterEvent
+	LogEvent
+)
+
+type EventType int32
+
+// Event that implement this interface would be allowed to transmit in the Satellite.
+type Event interface {
+	// Name returns the event name.
+	Name() string
+
+	// Meta is a pair of key and value to record meta data, such as labels.
+	Meta() map[string]string
+
+	// Data returns the wrapped data.
+	Data() interface{}
+
+	// Time returns the event time.
+	Time() time.Time
+
+	// Type is to distinguish different events.
+	Type() EventType

Review comment:
       where is id using as offset?




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#issuecomment-735708636


   > Could you consider to create `project_structue.md` doc, linked from readme? I think some of the comments are very suitable in the doc too.
   
   no problem would add it 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



[GitHub] [skywalking-satellite] wu-sheng merged pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
wu-sheng merged pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5


   


----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532222379



##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,94 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following graph illustrates the relationship between different plugin interface in api package.
+//
+//
+//                   Gatherer                                Processor
+//       -------------------------------      -------------------------------------------
+//      |  -----------       ---------   |   |  -----------                 -----------  |
+//      | | Collector | ==> |  Queue   | |==>| |  Filter   | ==>  ...  ==> |  Filter   | |
+//      | | (Parser)  |     | Mem/File | |   |  -----------                 -----------  |
+//      |  -----------       ----------  |   |      ||                          ||       |
+//       --------------------------------    |      \/	                        \/       |
+//                                           |  ---------------------------------------  |
+//                                           | |             OutputEventContext        | |
+//                                           |  ---------------------------------------  |
+//                                            -------------------------------------------
+//                             				   ||
+//                                             ||        --------------------------------
+//                                             ||     ->|         Sharing Client        |
+//                                             ||    |   --------------------------------
+//                                             ||    |
+//                                             \/    |            SegmentSender
+//                                            ---    |  ---------------------------------
+//                                           |   |   | |  -----------       -----------  |
+//                                           | D |   |-| |BatchBuffer| ==> | Forwarder | |
+//                                           | i |   | |  -----------       -----------  |
+//                                           | s |   |  ---------------------------------
+//                                           | p |   |
+//                                           | a |=> |              .......                 ===> Kafka/OAP
+//                                           | t |   |
+//                                           | c |   |             MeterSender
+//                                           | h |   | -----------------------------------
+//                                           | e |   -|  -------------       -----------  |
+//                                           | r |    | | BatchBuffer | ==> | Forwarder | |
+//                                           |   |    |  -------------       -----------  |
+//                                            ---      -----------------------------------
+//
+//
+// 1. The Collector plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to SerializationEvent that is supported
+//    to be stored in Queue.
+// 3. The Queue plugin stores the SerializationEvent. However, the serialization depends on
+//    the Queue implements. For example, the serialization is unnecessary when using a Memory
+//    Queue.
+// 4. The Filter plugin would pull an event from the Queue and process the event to create
+//    a new event. Next, the event is passed to the next filter to do the same things until
+//    the whole filters are performed. The events labeled with RemoteEvent type would be
+//    stored in the OutputEventContext. When the processing finished,
+//    the OutputEventContext. After processing, the events in OutputEventContext would be
+//    partitioned by the event type and sent to the different BatchBuffers, such as Segment
+//    BatchBuffer, Jvm BatchBuffer, and Meter BatchBuffer.
+// 5. When each BatchBuffer reaches its maximum capacity, the OutputEventContexts would be
+//    converted to BatchEvents and sent to Forwarder.
+// 6. The Follower would send BatchEvents and ack Queue when successful process this batch

Review comment:
       ![image](https://user-images.githubusercontent.com/31562192/100545597-8b182d00-3297-11eb-8bb6-242b0c90bb5f.png) this mapping would be contained in Dispatcher
   




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] wu-sheng commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532626566



##########
File path: internal/pkg/api/event.go
##########
@@ -0,0 +1,103 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import (
+	"fmt"
+	"time"
+)
+
+// The event type.
+const (
+	// Mapping to the type supported by SkyWalking OAP.
+	_ EventType = iota
+	MetricsEvent
+	ProfilingEvent
+	SegmentEvent
+	ManagementEvent
+	MeterEvent
+	LogEvent
+)
+
+type EventType int32
+
+// Event that implement this interface would be allowed to transmit in the Satellite.
+type Event interface {
+	// Name returns the event name.
+	Name() string
+
+	// Meta is a pair of key and value to record meta data, such as labels.
+	Meta() map[string]string
+
+	// Data returns the wrapped data.
+	Data() interface{}
+
+	// Time returns the event time.
+	Time() time.Time
+
+	// Type is a identifier to distinguish different events.

Review comment:
       > Need identifier for every single event?
   
   I think this is just a type, not an ID for every event.




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] wu-sheng commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532208042



##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,94 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following graph illustrates the relationship between different plugin interface in api package.
+//
+//
+//                   Gatherer                                Processor
+//       -------------------------------      -------------------------------------------
+//      |  -----------       ---------   |   |  -----------                 -----------  |
+//      | | Collector | ==> |  Queue   | |==>| |  Filter   | ==>  ...  ==> |  Filter   | |
+//      | | (Parser)  |     | Mem/File | |   |  -----------                 -----------  |
+//      |  -----------       ----------  |   |      ||                          ||       |
+//       --------------------------------    |      \/	                        \/       |
+//                                           |  ---------------------------------------  |
+//                                           | |             OutputEventContext        | |
+//                                           |  ---------------------------------------  |
+//                                            -------------------------------------------
+//                             				   ||
+//                                             ||        --------------------------------
+//                                             ||     ->|         Sharing Client        |
+//                                             ||    |   --------------------------------
+//                                             ||    |
+//                                             \/    |            SegmentSender
+//                                            ---    |  ---------------------------------
+//                                           |   |   | |  -----------       -----------  |
+//                                           | D |   |-| |BatchBuffer| ==> | Forwarder | |
+//                                           | i |   | |  -----------       -----------  |
+//                                           | s |   |  ---------------------------------
+//                                           | p |   |
+//                                           | a |=> |              .......                 ===> Kafka/OAP
+//                                           | t |   |
+//                                           | c |   |             MeterSender
+//                                           | h |   | -----------------------------------
+//                                           | e |   -|  -------------       -----------  |
+//                                           | r |    | | BatchBuffer | ==> | Forwarder | |
+//                                           |   |    |  -------------       -----------  |
+//                                            ---      -----------------------------------
+//
+//
+// 1. The Collector plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to SerializationEvent that is supported
+//    to be stored in Queue.
+// 3. The Queue plugin stores the SerializationEvent. However, the serialization depends on
+//    the Queue implements. For example, the serialization is unnecessary when using a Memory
+//    Queue.
+// 4. The Filter plugin would pull an event from the Queue and process the event to create
+//    a new event. Next, the event is passed to the next filter to do the same things until
+//    the whole filters are performed. The events labeled with RemoteEvent type would be
+//    stored in the OutputEventContext. When the processing finished,
+//    the OutputEventContext. After processing, the events in OutputEventContext would be
+//    partitioned by the event type and sent to the different BatchBuffers, such as Segment
+//    BatchBuffer, Jvm BatchBuffer, and Meter BatchBuffer.
+// 5. When each BatchBuffer reaches its maximum capacity, the OutputEventContexts would be
+//    converted to BatchEvents and sent to Forwarder.
+// 6. The Follower would send BatchEvents and ack Queue when successful process this batch

Review comment:
       Or could I say, a follower match one type of event?




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] surlymo commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
surlymo commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r533866102



##########
File path: internal/satellite/registry/registry.go
##########
@@ -0,0 +1,121 @@
+// Licensed to 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. Apache Software Foundation (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 registry
+
+import (
+	"fmt"
+	"reflect"
+
+	"github.com/apache/skywalking-satellite/internal/pkg/api"
+)
+
+// The creator registry.
+// All plugins is wrote in ./plugins dir. The plugin type would be as the next level dirs,
+// such as collector, client, or queue. And the 3rd level is the plugin name, that is also
+// used as key in pluginRegistry.
+type pluginRegistry struct {
+	collectorRegistry  map[string]api.Collector
+	queueRegistry      map[string]api.Queue
+	filterRegistry     map[string]api.Filter
+	forwarderRegistry  map[string]api.Forwarder
+	parserRegistry     map[string]api.Parser
+	clientRegistry     map[string]api.Client
+	fallbackerRegistry map[string]api.Fallbacker
+}
+
+// reg is the global plugin registry
+var reg *pluginRegistry
+
+// Plugin type.
+var (
+	collectorType  = reflect.TypeOf((*api.Collector)(nil)).Elem()
+	queueType      = reflect.TypeOf((*api.Queue)(nil)).Elem()
+	filterType     = reflect.TypeOf((*api.Filter)(nil)).Elem()
+	forwardType    = reflect.TypeOf((*api.Forwarder)(nil)).Elem()
+	parserType     = reflect.TypeOf((*api.Parser)(nil)).Elem()
+	clientType     = reflect.TypeOf((*api.Client)(nil)).Elem()
+	fallbackerType = reflect.TypeOf((*api.Fallbacker)(nil)).Elem()
+)
+
+func init() {
+	reg = &pluginRegistry{}
+	reg.collectorRegistry = make(map[string]api.Collector)
+	reg.queueRegistry = make(map[string]api.Queue)
+	reg.filterRegistry = make(map[string]api.Filter)
+	reg.forwarderRegistry = make(map[string]api.Forwarder)
+	reg.parserRegistry = make(map[string]api.Parser)
+	reg.clientRegistry = make(map[string]api.Client)
+	reg.fallbackerRegistry = make(map[string]api.Fallbacker)
+}
+
+// RegisterPlugin registers the pluginType as plugin.
+func RegisterPlugin(pluginType string, plugin interface{}) {
+	t := reflect.TypeOf(plugin)
+	switch {
+	case t.Implements(collectorType):

Review comment:
       i would like to see further abstraction rather than hard code when adding a new plugin

##########
File path: internal/satellite/registry/registry.go
##########
@@ -0,0 +1,121 @@
+// Licensed to 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. Apache Software Foundation (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 registry
+
+import (
+	"fmt"
+	"reflect"
+
+	"github.com/apache/skywalking-satellite/internal/pkg/api"
+)
+
+// The creator registry.
+// All plugins is wrote in ./plugins dir. The plugin type would be as the next level dirs,
+// such as collector, client, or queue. And the 3rd level is the plugin name, that is also
+// used as key in pluginRegistry.
+type pluginRegistry struct {
+	collectorRegistry  map[string]api.Collector
+	queueRegistry      map[string]api.Queue
+	filterRegistry     map[string]api.Filter
+	forwarderRegistry  map[string]api.Forwarder
+	parserRegistry     map[string]api.Parser
+	clientRegistry     map[string]api.Client
+	fallbackerRegistry map[string]api.Fallbacker
+}
+
+// reg is the global plugin registry
+var reg *pluginRegistry
+
+// Plugin type.
+var (
+	collectorType  = reflect.TypeOf((*api.Collector)(nil)).Elem()
+	queueType      = reflect.TypeOf((*api.Queue)(nil)).Elem()
+	filterType     = reflect.TypeOf((*api.Filter)(nil)).Elem()
+	forwardType    = reflect.TypeOf((*api.Forwarder)(nil)).Elem()
+	parserType     = reflect.TypeOf((*api.Parser)(nil)).Elem()
+	clientType     = reflect.TypeOf((*api.Client)(nil)).Elem()
+	fallbackerType = reflect.TypeOf((*api.Fallbacker)(nil)).Elem()
+)
+
+func init() {
+	reg = &pluginRegistry{}
+	reg.collectorRegistry = make(map[string]api.Collector)
+	reg.queueRegistry = make(map[string]api.Queue)
+	reg.filterRegistry = make(map[string]api.Filter)
+	reg.forwarderRegistry = make(map[string]api.Forwarder)
+	reg.parserRegistry = make(map[string]api.Parser)
+	reg.clientRegistry = make(map[string]api.Client)
+	reg.fallbackerRegistry = make(map[string]api.Fallbacker)
+}
+
+// RegisterPlugin registers the pluginType as plugin.
+func RegisterPlugin(pluginType string, plugin interface{}) {
+	t := reflect.TypeOf(plugin)
+	switch {
+	case t.Implements(collectorType):
+		fmt.Printf("register %s collector successfully ", t.String())

Review comment:
       PLUGIN mechanism should be based on a independent way to deal with INSERT, EXTRACT, INIT/LOAD LEVEL, LOG,  LIFECYCLE MANAGEMENT etc. SPI should be nothing related to specific plugin.

##########
File path: internal/satellite/registry/registry.go
##########
@@ -0,0 +1,121 @@
+// Licensed to 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. Apache Software Foundation (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 registry
+
+import (
+	"fmt"
+	"reflect"
+
+	"github.com/apache/skywalking-satellite/internal/pkg/api"
+)
+
+// The creator registry.
+// All plugins is wrote in ./plugins dir. The plugin type would be as the next level dirs,
+// such as collector, client, or queue. And the 3rd level is the plugin name, that is also
+// used as key in pluginRegistry.
+type pluginRegistry struct {
+	collectorRegistry  map[string]api.Collector
+	queueRegistry      map[string]api.Queue
+	filterRegistry     map[string]api.Filter
+	forwarderRegistry  map[string]api.Forwarder
+	parserRegistry     map[string]api.Parser
+	clientRegistry     map[string]api.Client
+	fallbackerRegistry map[string]api.Fallbacker
+}
+
+// reg is the global plugin registry
+var reg *pluginRegistry
+
+// Plugin type.
+var (
+	collectorType  = reflect.TypeOf((*api.Collector)(nil)).Elem()
+	queueType      = reflect.TypeOf((*api.Queue)(nil)).Elem()
+	filterType     = reflect.TypeOf((*api.Filter)(nil)).Elem()
+	forwardType    = reflect.TypeOf((*api.Forwarder)(nil)).Elem()
+	parserType     = reflect.TypeOf((*api.Parser)(nil)).Elem()
+	clientType     = reflect.TypeOf((*api.Client)(nil)).Elem()
+	fallbackerType = reflect.TypeOf((*api.Fallbacker)(nil)).Elem()
+)
+
+func init() {
+	reg = &pluginRegistry{}
+	reg.collectorRegistry = make(map[string]api.Collector)
+	reg.queueRegistry = make(map[string]api.Queue)
+	reg.filterRegistry = make(map[string]api.Filter)
+	reg.forwarderRegistry = make(map[string]api.Forwarder)
+	reg.parserRegistry = make(map[string]api.Parser)
+	reg.clientRegistry = make(map[string]api.Client)
+	reg.fallbackerRegistry = make(map[string]api.Fallbacker)
+}
+
+// RegisterPlugin registers the pluginType as plugin.
+func RegisterPlugin(pluginType string, plugin interface{}) {

Review comment:
       this means that plugin must be **SINGLETON** . PROTOTYPE plugins are not allowed this time. describe it obviously in file **plugin.go** 




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] wu-sheng commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532207256



##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,94 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following graph illustrates the relationship between different plugin interface in api package.
+//
+//
+//                   Gatherer                                Processor
+//       -------------------------------      -------------------------------------------
+//      |  -----------       ---------   |   |  -----------                 -----------  |
+//      | | Collector | ==> |  Queue   | |==>| |  Filter   | ==>  ...  ==> |  Filter   | |
+//      | | (Parser)  |     | Mem/File | |   |  -----------                 -----------  |
+//      |  -----------       ----------  |   |      ||                          ||       |
+//       --------------------------------    |      \/	                        \/       |
+//                                           |  ---------------------------------------  |
+//                                           | |             OutputEventContext        | |
+//                                           |  ---------------------------------------  |
+//                                            -------------------------------------------
+//                             				   ||
+//                                             ||        --------------------------------
+//                                             ||     ->|         Sharing Client        |
+//                                             ||    |   --------------------------------
+//                                             ||    |
+//                                             \/    |            SegmentSender
+//                                            ---    |  ---------------------------------
+//                                           |   |   | |  -----------       -----------  |
+//                                           | D |   |-| |BatchBuffer| ==> | Forwarder | |
+//                                           | i |   | |  -----------       -----------  |
+//                                           | s |   |  ---------------------------------
+//                                           | p |   |
+//                                           | a |=> |              .......                 ===> Kafka/OAP
+//                                           | t |   |
+//                                           | c |   |             MeterSender
+//                                           | h |   | -----------------------------------
+//                                           | e |   -|  -------------       -----------  |
+//                                           | r |    | | BatchBuffer | ==> | Forwarder | |
+//                                           |   |    |  -------------       -----------  |
+//                                            ---      -----------------------------------
+//
+//
+// 1. The Collector plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to SerializationEvent that is supported
+//    to be stored in Queue.
+// 3. The Queue plugin stores the SerializationEvent. However, the serialization depends on
+//    the Queue implements. For example, the serialization is unnecessary when using a Memory
+//    Queue.
+// 4. The Filter plugin would pull an event from the Queue and process the event to create

Review comment:
       I think consumer pull the data, filters are just processing data in a chain mode, right?




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] wu-sheng commented on pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#issuecomment-735669360


   Could you consider to create `project_structue.md` doc, linked from readme? I think some of the comments are very suitable in the doc too.


----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp edited a comment on pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp edited a comment on pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#issuecomment-736217624


   @wu-sheng @hanahmily @gxthrj @surlymo  thx for recheck again because some logic in Sender is changed. In the previous version, the ack operation could not be purposed.


----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r534211732



##########
File path: docs/project_structue.md
##########
@@ -0,0 +1,63 @@
+# Project Structure
+- configs: Satellite configs.
+- internal: Core, Api, and common utils.
+- internal/pkg: Sharing with Core and Plugins, such as api and utils.
+- internal/satellite: The core of Satellite.
+- plugins: Contains all plugins.
+- plugins/{type}: Contains the plugins of this {type}. Satellite has 6 plugin types, which are collector, queue, parser, filter, client, and forward.
+- plugins/{type}/define{type}: Contains the plugin define.

Review comment:
       could I merge it? According to chao's advice, the plugin was decoupled.




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532034993



##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,74 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following comments is to illustrate the relationship between different plugin interface in api package.
+//
+//
+//                                                 Processors
+//                                   -----------------------------------------
+//  ---------        ---------        -----------                 -----------
+// | Gatherer | ==> |  Queue   | ==> | Processor | ==>  ...  ==> | Processor |
+// | (Parser) |     | Mem/File |      -----------                 -----------
+//  ----------       ---------            ||                          ||
+//                                        \/	                      \/
+//                                    ---------------------------------------
+//                                   |             OutputEventContext        |
+//                                    ---------------------------------------
+//                                                                    ||
+//                                                                    \/
+//                                    -------------------       -------------
+//                                   | BatchOutputEvents | <== | BatchBuffer |
+//                                    -------------------       -------------
+//                                             ||
+//                                             \/
+//                                    -------------------
+//                                   |     Forwarder     | ==> Kakfa/OAP
+//                                    -------------------
+// 1. The Gatherer plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to InputEvent.
+//    If the event needs output, please tag it by the IsOutput
+//    method.
+// 3. The Queue plugin would store the InputEvent. But different
+//    Queue would use different ways to store data, such as store
+//    bytes by serialization or keep original.
+// 4. The Processor plugin would pull the event from the Queue and

Review comment:
       no problem, I'm going to follow the document naming




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532221356



##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,94 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following graph illustrates the relationship between different plugin interface in api package.
+//
+//
+//                   Gatherer                                Processor
+//       -------------------------------      -------------------------------------------
+//      |  -----------       ---------   |   |  -----------                 -----------  |
+//      | | Collector | ==> |  Queue   | |==>| |  Filter   | ==>  ...  ==> |  Filter   | |
+//      | | (Parser)  |     | Mem/File | |   |  -----------                 -----------  |
+//      |  -----------       ----------  |   |      ||                          ||       |
+//       --------------------------------    |      \/	                        \/       |
+//                                           |  ---------------------------------------  |
+//                                           | |             OutputEventContext        | |
+//                                           |  ---------------------------------------  |
+//                                            -------------------------------------------
+//                             				   ||
+//                                             ||        --------------------------------
+//                                             ||     ->|         Sharing Client        |
+//                                             ||    |   --------------------------------
+//                                             ||    |
+//                                             \/    |            SegmentSender
+//                                            ---    |  ---------------------------------
+//                                           |   |   | |  -----------       -----------  |
+//                                           | D |   |-| |BatchBuffer| ==> | Forwarder | |
+//                                           | i |   | |  -----------       -----------  |
+//                                           | s |   |  ---------------------------------
+//                                           | p |   |
+//                                           | a |=> |              .......                 ===> Kafka/OAP
+//                                           | t |   |
+//                                           | c |   |             MeterSender
+//                                           | h |   | -----------------------------------
+//                                           | e |   -|  -------------       -----------  |
+//                                           | r |    | | BatchBuffer | ==> | Forwarder | |
+//                                           |   |    |  -------------       -----------  |
+//                                            ---      -----------------------------------
+//
+//
+// 1. The Collector plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to SerializationEvent that is supported
+//    to be stored in Queue.
+// 3. The Queue plugin stores the SerializationEvent. However, the serialization depends on
+//    the Queue implements. For example, the serialization is unnecessary when using a Memory
+//    Queue.
+// 4. The Filter plugin would pull an event from the Queue and process the event to create
+//    a new event. Next, the event is passed to the next filter to do the same things until
+//    the whole filters are performed. The events labeled with RemoteEvent type would be
+//    stored in the OutputEventContext. When the processing finished,
+//    the OutputEventContext. After processing, the events in OutputEventContext would be
+//    partitioned by the event type and sent to the different BatchBuffers, such as Segment
+//    BatchBuffer, Jvm BatchBuffer, and Meter BatchBuffer.
+// 5. When each BatchBuffer reaches its maximum capacity, the OutputEventContexts would be

Review comment:
       yes, I would add 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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532039980



##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,74 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following comments is to illustrate the relationship between different plugin interface in api package.
+//
+//
+//                                                 Processors
+//                                   -----------------------------------------
+//  ---------        ---------        -----------                 -----------
+// | Gatherer | ==> |  Queue   | ==> | Processor | ==>  ...  ==> | Processor |
+// | (Parser) |     | Mem/File |      -----------                 -----------
+//  ----------       ---------            ||                          ||
+//                                        \/	                      \/
+//                                    ---------------------------------------
+//                                   |             OutputEventContext        |
+//                                    ---------------------------------------
+//                                                                    ||
+//                                                                    \/
+//                                    -------------------       -------------
+//                                   | BatchOutputEvents | <== | BatchBuffer |
+//                                    -------------------       -------------
+//                                             ||
+//                                             \/
+//                                    -------------------
+//                                   |     Forwarder     | ==> Kakfa/OAP
+//                                    -------------------
+// 1. The Gatherer plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to InputEvent.
+//    If the event needs output, please tag it by the IsOutput

Review comment:
       When we support sampling, no output needs to be supported. The Output options should be decided by the Processor. That means the input event should always false. 




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r534202928



##########
File path: docs/project_structue.md
##########
@@ -0,0 +1,63 @@
+# Project Structure
+- configs: Satellite configs.
+- internal: Core, Api, and common utils.
+- internal/pkg: Sharing with Core and Plugins, such as api and utils.
+- internal/satellite: The core of Satellite.
+- plugins: Contains all plugins.
+- plugins/{type}: Contains the plugins of this {type}. Satellite has 6 plugin types, which are collector, queue, parser, filter, client, and forward.
+- plugins/{type}/define{type}: Contains the plugin define.

Review comment:
       just to avoid package conflicts, let me try again.




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532225929



##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,94 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following graph illustrates the relationship between different plugin interface in api package.
+//
+//
+//                   Gatherer                                Processor
+//       -------------------------------      -------------------------------------------
+//      |  -----------       ---------   |   |  -----------                 -----------  |
+//      | | Collector | ==> |  Queue   | |==>| |  Filter   | ==>  ...  ==> |  Filter   | |
+//      | | (Parser)  |     | Mem/File | |   |  -----------                 -----------  |
+//      |  -----------       ----------  |   |      ||                          ||       |
+//       --------------------------------    |      \/	                        \/       |
+//                                           |  ---------------------------------------  |
+//                                           | |             OutputEventContext        | |
+//                                           |  ---------------------------------------  |
+//                                            -------------------------------------------
+//                             				   ||
+//                                             ||        --------------------------------
+//                                             ||     ->|         Sharing Client        |
+//                                             ||    |   --------------------------------
+//                                             ||    |
+//                                             \/    |            SegmentSender
+//                                            ---    |  ---------------------------------
+//                                           |   |   | |  -----------       -----------  |
+//                                           | D |   |-| |BatchBuffer| ==> | Forwarder | |
+//                                           | i |   | |  -----------       -----------  |
+//                                           | s |   |  ---------------------------------
+//                                           | p |   |
+//                                           | a |=> |              .......                 ===> Kafka/OAP
+//                                           | t |   |
+//                                           | c |   |             MeterSender
+//                                           | h |   | -----------------------------------
+//                                           | e |   -|  -------------       -----------  |
+//                                           | r |    | | BatchBuffer | ==> | Forwarder | |
+//                                           |   |    |  -------------       -----------  |
+//                                            ---      -----------------------------------
+//
+//
+// 1. The Collector plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to SerializationEvent that is supported
+//    to be stored in Queue.
+// 3. The Queue plugin stores the SerializationEvent. However, the serialization depends on
+//    the Queue implements. For example, the serialization is unnecessary when using a Memory
+//    Queue.
+// 4. The Filter plugin would pull an event from the Queue and process the event to create
+//    a new event. Next, the event is passed to the next filter to do the same things until
+//    the whole filters are performed. The events labeled with RemoteEvent type would be
+//    stored in the OutputEventContext. When the processing finished,
+//    the OutputEventContext. After processing, the events in OutputEventContext would be
+//    partitioned by the event type and sent to the different BatchBuffers, such as Segment
+//    BatchBuffer, Jvm BatchBuffer, and Meter BatchBuffer.
+// 5. When each BatchBuffer reaches its maximum capacity, the OutputEventContexts would be
+//    converted to BatchEvents and sent to Forwarder.
+// 6. The Follower would send BatchEvents and ack Queue when successful process this batch

Review comment:
       add comments in plugin.go




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532052890



##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,74 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following comments is to illustrate the relationship between different plugin interface in api package.
+//
+//
+//                                                 Processors
+//                                   -----------------------------------------
+//  ---------        ---------        -----------                 -----------
+// | Gatherer | ==> |  Queue   | ==> | Processor | ==>  ...  ==> | Processor |
+// | (Parser) |     | Mem/File |      -----------                 -----------
+//  ----------       ---------            ||                          ||
+//                                        \/	                      \/
+//                                    ---------------------------------------
+//                                   |             OutputEventContext        |
+//                                    ---------------------------------------
+//                                                                    ||
+//                                                                    \/
+//                                    -------------------       -------------
+//                                   | BatchOutputEvents | <== | BatchBuffer |
+//                                    -------------------       -------------
+//                                             ||
+//                                             \/
+//                                    -------------------
+//                                   |     Forwarder     | ==> Kakfa/OAP
+//                                    -------------------
+// 1. The Gatherer plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to InputEvent.
+//    If the event needs output, please tag it by the IsOutput

Review comment:
       That's only a switch case to support more cases.  Do u have other suggestions?




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] wu-sheng commented on pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#issuecomment-736515563


   @surlymo Could you recheck and confirm?


----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] wu-sheng commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532044255



##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,74 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following comments is to illustrate the relationship between different plugin interface in api package.
+//
+//
+//                                                 Processors
+//                                   -----------------------------------------
+//  ---------        ---------        -----------                 -----------
+// | Gatherer | ==> |  Queue   | ==> | Processor | ==>  ...  ==> | Processor |
+// | (Parser) |     | Mem/File |      -----------                 -----------
+//  ----------       ---------            ||                          ||
+//                                        \/	                      \/
+//                                    ---------------------------------------
+//                                   |             OutputEventContext        |
+//                                    ---------------------------------------
+//                                                                    ||
+//                                                                    \/
+//                                    -------------------       -------------
+//                                   | BatchOutputEvents | <== | BatchBuffer |
+//                                    -------------------       -------------
+//                                             ||
+//                                             \/
+//                                    -------------------
+//                                   |     Forwarder     | ==> Kakfa/OAP
+//                                    -------------------
+// 1. The Gatherer plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to InputEvent.
+//    If the event needs output, please tag it by the IsOutput

Review comment:
       Then we could have an output with nothing in the context, but in the real case, we will still prefer forwarding the source(s), isn't it?




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532381384



##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,94 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following graph illustrates the relationship between different plugin interface in api package.
+//
+//
+//                   Gatherer                                Processor
+//       -------------------------------      -------------------------------------------
+//      |  -----------       ---------   |   |  -----------                 -----------  |
+//      | | Collector | ==> |  Queue   | |==>| |  Filter   | ==>  ...  ==> |  Filter   | |
+//      | | (Parser)  |     | Mem/File | |   |  -----------                 -----------  |
+//      |  -----------       ----------  |   |      ||                          ||       |
+//       --------------------------------    |      \/	                        \/       |
+//                                           |  ---------------------------------------  |
+//                                           | |             OutputEventContext        | |
+//                                           |  ---------------------------------------  |
+//                                            -------------------------------------------
+//                             				   ||
+//                                             ||        --------------------------------
+//                                             ||     ->|         Sharing Client        |
+//                                             ||    |   --------------------------------
+//                                             ||    |
+//                                             \/    |            SegmentSender
+//                                            ---    |  ---------------------------------
+//                                           |   |   | |  -----------       -----------  |
+//                                           | D |   |-| |BatchBuffer| ==> | Forwarder | |
+//                                           | i |   | |  -----------       -----------  |
+//                                           | s |   |  ---------------------------------
+//                                           | p |   |
+//                                           | a |=> |              .......                 ===> Kafka/OAP
+//                                           | t |   |
+//                                           | c |   |             MeterSender
+//                                           | h |   | -----------------------------------
+//                                           | e |   -|  -------------       -----------  |
+//                                           | r |    | | BatchBuffer | ==> | Forwarder | |
+//                                           |   |    |  -------------       -----------  |
+//                                            ---      -----------------------------------
+//
+//
+// 1. The Collector plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to SerializationEvent that is supported
+//    to be stored in Queue.
+// 3. The Queue plugin stores the SerializationEvent. However, the serialization depends on
+//    the Queue implements. For example, the serialization is unnecessary when using a Memory
+//    Queue.
+// 4. The Filter plugin would pull an event from the Queue and process the event to create
+//    a new event. Next, the event is passed to the next filter to do the same things until
+//    the whole filters are performed. The events labeled with RemoteEvent type would be
+//    stored in the OutputEventContext. When the processing finished,
+//    the OutputEventContext. After processing, the events in OutputEventContext would be
+//    partitioned by the event type and sent to the different BatchBuffers, such as Segment
+//    BatchBuffer, Jvm BatchBuffer, and Meter BatchBuffer.
+// 5. When each BatchBuffer reaches its maximum capacity, the OutputEventContexts would be
+//    converted to BatchEvents and sent to Forwarder.
+// 6. The Follower would send BatchEvents and ack Queue when successful process this batch

Review comment:
       add eventType on forwarder




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp edited a comment on pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp edited a comment on pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#issuecomment-734920733


   > 3\. Why Output named as `BatchOutputEvents`? Batch or no batch depends on filter works. You could have several inputs to one output, vice versa.
   
   The Batch is pointed to the BatchBuffer context in the design doc.
   In the first release version,  Satellite would more forward rather than processing. So, the Sender would be independent in Log, Metrics, and Trace. In the future, some advanced features would break the current mod, such as extract Metrics from Log. SO, The Sender Layer would be a shared layer to avoid make duplicate connections.


----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] surlymo commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
surlymo commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r533392661



##########
File path: internal/satellite/event/unstruectured_event.go
##########
@@ -0,0 +1,79 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor

Review comment:
       typo filename

##########
File path: internal/satellite/registry/registry.go
##########
@@ -0,0 +1,194 @@
+// Licensed to 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. Apache Software Foundation (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 registry
+
+import (
+	"fmt"
+
+	"github.com/apache/skywalking-satellite/internal/pkg/api"
+)
+
+// The creator registry.
+// All plugins is wrote in ./plugins dir. The plugin type would be as the next level dirs,
+// such as collector, client, or queue. And the 3rd level is the plugin name, that is also
+// used as key in pluginRegistry.
+type pluginRegistry struct {
+	collectorCreatorRegistry  map[string]CollectorCreator
+	queueCreatorRegistry      map[string]QueueCreator
+	filterCreatorRegistry     map[string]FilterCreator
+	forwarderCreatorRegistry  map[string]ForwarderCreator
+	parserCreatorRegistry     map[string]ParserCreator
+	clientCreatorRegistry     map[string]ClientCreator
+	fallbackerCreatorRegistry map[string]FallbackerCreator
+}
+
+// FallbackerCreator creates a Fallbacker according to the config.
+type FallbackerCreator func(config map[string]interface{}) (api.Fallbacker, error)
+
+// ClientCreator creates a Client according to the config.
+type ClientCreator func(config map[string]interface{}) (api.Client, error)
+
+// CollectorCreator creates a Collector according to the config.
+type CollectorCreator func(config map[string]interface{}) (api.Collector, error)
+
+// QueueCreator creates a Queue according to the config.
+type QueueCreator func(config map[string]interface{}) (api.Queue, error)
+
+// FilterCreator creates a Filter according to the config.
+type FilterCreator func(config map[string]interface{}) (api.Filter, error)
+
+// ForwarderCreator creates a forwarder according to the config.
+type ForwarderCreator func(config map[string]interface{}) (api.Forwarder, error)
+
+// ParserCreator creates a parser according to the config.
+type ParserCreator func(config map[string]interface{}) (api.Parser, error)
+
+var reg *pluginRegistry
+
+// RegisterFallbacker registers the fallbackerType as RegisterFallbacker.
+func RegisterFallbacker(fallbackerType string, creator FallbackerCreator) {
+	fmt.Printf("Create %s fallbacker creator register successfully", fallbackerType)
+	reg.fallbackerCreatorRegistry[fallbackerType] = creator
+}
+
+// RegisterClient registers the clientType as ClientCreator.
+func RegisterClient(clientType string, creator ClientCreator) {
+	fmt.Printf("Create %s client creator register successfully", clientType)
+	reg.clientCreatorRegistry[clientType] = creator
+}
+
+// RegisterCollector registers the collectorType as CollectorCreator.
+func RegisterCollector(collectorType string, creator CollectorCreator) {
+	fmt.Printf("Create %s collector creator register successfully", collectorType)
+	reg.collectorCreatorRegistry[collectorType] = creator
+}
+
+// RegisterQueue registers the queueType as QueueCreator.
+func RegisterQueue(queueType string, creator QueueCreator) {
+	fmt.Printf("Create %s queue creator register successfully", queueType)

Review comment:
       why need creator? can we just use **Initializer** to finish the creation?

##########
File path: internal/satellite/registry/registry.go
##########
@@ -0,0 +1,194 @@
+// Licensed to 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. Apache Software Foundation (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 registry
+
+import (
+	"fmt"
+
+	"github.com/apache/skywalking-satellite/internal/pkg/api"
+)
+
+// The creator registry.
+// All plugins is wrote in ./plugins dir. The plugin type would be as the next level dirs,
+// such as collector, client, or queue. And the 3rd level is the plugin name, that is also
+// used as key in pluginRegistry.
+type pluginRegistry struct {
+	collectorCreatorRegistry  map[string]CollectorCreator
+	queueCreatorRegistry      map[string]QueueCreator
+	filterCreatorRegistry     map[string]FilterCreator
+	forwarderCreatorRegistry  map[string]ForwarderCreator
+	parserCreatorRegistry     map[string]ParserCreator
+	clientCreatorRegistry     map[string]ClientCreator
+	fallbackerCreatorRegistry map[string]FallbackerCreator
+}
+
+// FallbackerCreator creates a Fallbacker according to the config.
+type FallbackerCreator func(config map[string]interface{}) (api.Fallbacker, error)
+
+// ClientCreator creates a Client according to the config.
+type ClientCreator func(config map[string]interface{}) (api.Client, error)
+
+// CollectorCreator creates a Collector according to the config.
+type CollectorCreator func(config map[string]interface{}) (api.Collector, error)
+
+// QueueCreator creates a Queue according to the config.
+type QueueCreator func(config map[string]interface{}) (api.Queue, error)
+
+// FilterCreator creates a Filter according to the config.
+type FilterCreator func(config map[string]interface{}) (api.Filter, error)
+
+// ForwarderCreator creates a forwarder according to the config.
+type ForwarderCreator func(config map[string]interface{}) (api.Forwarder, error)
+
+// ParserCreator creates a parser according to the config.
+type ParserCreator func(config map[string]interface{}) (api.Parser, error)
+
+var reg *pluginRegistry
+
+// RegisterFallbacker registers the fallbackerType as RegisterFallbacker.
+func RegisterFallbacker(fallbackerType string, creator FallbackerCreator) {
+	fmt.Printf("Create %s fallbacker creator register successfully", fallbackerType)
+	reg.fallbackerCreatorRegistry[fallbackerType] = creator
+}
+
+// RegisterClient registers the clientType as ClientCreator.
+func RegisterClient(clientType string, creator ClientCreator) {
+	fmt.Printf("Create %s client creator register successfully", clientType)
+	reg.clientCreatorRegistry[clientType] = creator
+}
+
+// RegisterCollector registers the collectorType as CollectorCreator.
+func RegisterCollector(collectorType string, creator CollectorCreator) {
+	fmt.Printf("Create %s collector creator register successfully", collectorType)
+	reg.collectorCreatorRegistry[collectorType] = creator
+}
+
+// RegisterQueue registers the queueType as QueueCreator.
+func RegisterQueue(queueType string, creator QueueCreator) {
+	fmt.Printf("Create %s queue creator register successfully", queueType)
+	reg.queueCreatorRegistry[queueType] = creator
+}
+
+// RegisterFilter registers the filterType as FilterCreator.
+func RegisterFilter(filterType string, creator FilterCreator) {
+	fmt.Printf("Create %s filter creator register successfully", filterType)
+	reg.filterCreatorRegistry[filterType] = creator
+}
+
+// RegisterForwarder registers the forwarderType as forwarderCreator.
+func RegisterForwarder(forwarderType string, creator ForwarderCreator) {
+	fmt.Printf("Create %s forward creator register successfully", forwarderType)
+	reg.forwarderCreatorRegistry[forwarderType] = creator
+}
+
+// CreateClient creates a Client according to the clientType.
+func CreateClient(clientType string, config map[string]interface{}) (api.Client, error) {
+	if c, ok := reg.clientCreatorRegistry[clientType]; ok {
+		client, err := c(config)
+		if err != nil {
+			return nil, fmt.Errorf("create client failed: %v", err)
+		}
+		return client, nil
+	}
+	return nil, fmt.Errorf("unsupported client type: %v", clientType)
+}
+
+// CreateCollector creates a Collector according to the collectorType.
+func CreateCollector(collectorType string, config map[string]interface{}) (api.Collector, error) {
+	if c, ok := reg.collectorCreatorRegistry[collectorType]; ok {
+		collector, err := c(config)
+		if err != nil {
+			return nil, fmt.Errorf("create collector failed: %v", err)
+		}
+		return collector, nil
+	}
+	return nil, fmt.Errorf("unsupported collector type: %v", collectorType)
+}
+
+// CreateQueue creates a Queue according to the queueType.
+func CreateQueue(queueType string, config map[string]interface{}) (api.Queue, error) {

Review comment:
       DRY

##########
File path: internal/satellite/registry/registry.go
##########
@@ -0,0 +1,194 @@
+// Licensed to 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. Apache Software Foundation (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 registry

Review comment:
       refactor it with SPI abstraction.

##########
File path: internal/satellite/event/struectured_event.go
##########
@@ -0,0 +1,79 @@
+// Licensed to 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. Apache Software Foundation (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 event
+
+import (
+	"time"
+
+	"github.com/apache/skywalking-satellite/internal/pkg/api"
+)
+
+// StructuredInputEventToBytesFunc serialize event to bytes.
+type StructuredInputEventToBytesFunc func(event *StructuredInputEvent) []byte
+
+// BytesToStructuredInputEventFunc deserialize bytes to event.
+type BytesToStructuredInputEventFunc func(bytes []byte) *StructuredInputEvent
+
+// StructuredEvent works when the data is a struct type.
+type StructuredEvent struct {

Review comment:
       usage scenario?

##########
File path: internal/satellite/event/unstruectured_event.go
##########
@@ -0,0 +1,79 @@
+// Licensed to 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. Apache Software Foundation (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 event
+
+import (
+	"time"
+
+	"github.com/apache/skywalking-satellite/internal/pkg/api"
+)
+
+// UnstructuredInputEventToBytesFunc serialize event to bytes.
+type UnstructuredInputEventToBytesFunc func(event *UnstructuredInputEvent) []byte
+
+// BytesToStructuredInputEventFunc deserialize bytes to event.
+type BytesToUnstructuredInputEventFunc func(bytes []byte) *UnstructuredInputEvent
+
+// UnstructuredEvent works when the data is a map type.
+type UnstructuredEvent struct {

Review comment:
       DRY




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532221267



##########
File path: internal/pkg/api/plugin.go
##########
@@ -0,0 +1,94 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import "io"
+
+// The following graph illustrates the relationship between different plugin interface in api package.
+//
+//
+//                   Gatherer                                Processor
+//       -------------------------------      -------------------------------------------
+//      |  -----------       ---------   |   |  -----------                 -----------  |
+//      | | Collector | ==> |  Queue   | |==>| |  Filter   | ==>  ...  ==> |  Filter   | |
+//      | | (Parser)  |     | Mem/File | |   |  -----------                 -----------  |
+//      |  -----------       ----------  |   |      ||                          ||       |
+//       --------------------------------    |      \/	                        \/       |
+//                                           |  ---------------------------------------  |
+//                                           | |             OutputEventContext        | |
+//                                           |  ---------------------------------------  |
+//                                            -------------------------------------------
+//                             				   ||
+//                                             ||        --------------------------------
+//                                             ||     ->|         Sharing Client        |
+//                                             ||    |   --------------------------------
+//                                             ||    |
+//                                             \/    |            SegmentSender
+//                                            ---    |  ---------------------------------
+//                                           |   |   | |  -----------       -----------  |
+//                                           | D |   |-| |BatchBuffer| ==> | Forwarder | |
+//                                           | i |   | |  -----------       -----------  |
+//                                           | s |   |  ---------------------------------
+//                                           | p |   |
+//                                           | a |=> |              .......                 ===> Kafka/OAP
+//                                           | t |   |
+//                                           | c |   |             MeterSender
+//                                           | h |   | -----------------------------------
+//                                           | e |   -|  -------------       -----------  |
+//                                           | r |    | | BatchBuffer | ==> | Forwarder | |
+//                                           |   |    |  -------------       -----------  |
+//                                            ---      -----------------------------------
+//
+//
+// 1. The Collector plugin would fetch or receive the input data.
+// 2. The Parser plugin would parse the input data to SerializationEvent that is supported
+//    to be stored in Queue.
+// 3. The Queue plugin stores the SerializationEvent. However, the serialization depends on
+//    the Queue implements. For example, the serialization is unnecessary when using a Memory
+//    Queue.
+// 4. The Filter plugin would pull an event from the Queue and process the event to create
+//    a new event. Next, the event is passed to the next filter to do the same things until
+//    the whole filters are performed. The events labeled with RemoteEvent type would be

Review comment:
       Do u means the following things? 
   ![image](https://user-images.githubusercontent.com/31562192/100545366-77b89200-3296-11eb-8c76-eb60dd3a20bf.png)
   




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#issuecomment-736217624


   @wu-sheng @hanahmily @gxthrj @surlymo  thx for recheck again because some logic in Sender is changed.


----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#issuecomment-734920733


   > 3\. Why Output named as `BatchOutputEvents`? Batch or no batch depends on filter works. You could have several inputs to one output, vice versa.
   
   The Batch is pointed to the BatchBuffer context in the design doc.


----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp edited a comment on pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp edited a comment on pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#issuecomment-734922926


   > 2\. Event in the doc should be the data of the processor. From my understanding, there should be no explicit boundary between input and output, because every output could be another filter's input. I am curious how you plan to use the InputEvent and OutputEvent.
   
   Event is a global context, all Events were allowed to transfer in Satellite. InputEvent is a smaller concept for Gatherer. Because  Gatherer would put the data in Queue, InputEvent guarantees that Queue can be replaced in different plug-ins.  It's up to the plug-in to decide whether to take the original look or serialize to bytes.


----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] kezhenxu94 commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532362284



##########
File path: internal/pkg/api/event.go
##########
@@ -0,0 +1,103 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import (
+	"fmt"
+	"time"
+)
+
+// The event type.
+const (
+	// Mapping to the type supported by SkyWalking OAP.
+	_ EventType = iota
+	MetricsEvent
+	ProflingEvent

Review comment:
       typo
   
   ```suggestion
   	ProfilingEvent
   ```

##########
File path: internal/pkg/api/event.go
##########
@@ -0,0 +1,103 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import (
+	"fmt"
+	"time"
+)
+
+// The event type.
+const (
+	// Mapping to the type supported by SkyWalking OAP.
+	_ EventType = iota
+	MetricsEvent
+	ProflingEvent
+	SegmentEvent
+	ManagementEvent
+	MeterEvent
+	LogEvent
+)
+
+type EventType int32
+
+// Event that implement this interface would be allowed to transmit in the Satellite.
+type Event interface {
+	// Name is a identify to distinguish different events.
+	Name() string
+
+	// Meta is a pair of key and value to record meta data, such as labels.
+	Meta() map[string]string
+
+	// Data returns the wrappered data.
+	Data() interface{}
+
+	// Time returns the event time.
+	Time() time.Time
+
+	// Type returns the event type.
+	Type() EventType
+
+	// Remote means is a output event when returns true.
+	Remote() bool
+}
+
+// SerializationEvent is used in Collector to bridge Queue.
+type SerializationEvent interface {

Review comment:
       `SerializationEvent` -> `SerializableEvent`?

##########
File path: internal/pkg/api/event.go
##########
@@ -0,0 +1,103 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import (
+	"fmt"
+	"time"
+)
+
+// The event type.
+const (
+	// Mapping to the type supported by SkyWalking OAP.
+	_ EventType = iota
+	MetricsEvent
+	ProflingEvent
+	SegmentEvent
+	ManagementEvent
+	MeterEvent
+	LogEvent
+)
+
+type EventType int32
+
+// Event that implement this interface would be allowed to transmit in the Satellite.
+type Event interface {
+	// Name is a identify to distinguish different events.

Review comment:
       ```suggestion
   	// Name is an identifier to distinguish different events.
   ```

##########
File path: internal/pkg/api/event.go
##########
@@ -0,0 +1,103 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import (
+	"fmt"
+	"time"
+)
+
+// The event type.
+const (
+	// Mapping to the type supported by SkyWalking OAP.
+	_ EventType = iota
+	MetricsEvent
+	ProflingEvent
+	SegmentEvent
+	ManagementEvent
+	MeterEvent
+	LogEvent
+)
+
+type EventType int32
+
+// Event that implement this interface would be allowed to transmit in the Satellite.
+type Event interface {
+	// Name is a identify to distinguish different events.
+	Name() string
+
+	// Meta is a pair of key and value to record meta data, such as labels.
+	Meta() map[string]string
+
+	// Data returns the wrappered data.

Review comment:
       ```suggestion
   	// Data returns the wrapped data.
   ```

##########
File path: internal/pkg/api/event.go
##########
@@ -0,0 +1,103 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import (
+	"fmt"
+	"time"
+)
+
+// The event type.
+const (
+	// Mapping to the type supported by SkyWalking OAP.
+	_ EventType = iota
+	MetricsEvent
+	ProflingEvent
+	SegmentEvent
+	ManagementEvent
+	MeterEvent
+	LogEvent
+)
+
+type EventType int32
+
+// Event that implement this interface would be allowed to transmit in the Satellite.
+type Event interface {
+	// Name is a identify to distinguish different events.
+	Name() string

Review comment:
       If it's an identifier, maybe name it as `ID` is more semantic?




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r533412315



##########
File path: internal/pkg/api/forwarder.go
##########
@@ -0,0 +1,42 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+//   Init()     Initiating stage: Init plugin by config
+//    ||
+//    \/
+//   Prepare()   Preparing stage: Prepare the Forwarder, such as get remote client.
+//    ||
+//    \/
+//   Forward()  Running stage: Forward the batch events
+//    ||
+//    \/
+//   Close()    Closing stage: Close the Collector, such as close connection with SkyWalking javaagent.
+
+// Forwarder is a plugin interface, that defines new forwarders.
+type Forwarder interface {
+	Initializer
+	Preparer
+	Closer
+
+	// Forward the batch events to the external services, such as Kafka MQ and SkyWalking OAP cluster.
+	Forward(batch BatchEvents)
+
+	// ForwardType returns the supporting event type that could be forwarded.
+	ForwardType()

Review comment:
       sorry, adding it




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532369164



##########
File path: internal/pkg/api/event.go
##########
@@ -0,0 +1,103 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import (
+	"fmt"
+	"time"
+)
+
+// The event type.
+const (
+	// Mapping to the type supported by SkyWalking OAP.
+	_ EventType = iota
+	MetricsEvent
+	ProflingEvent
+	SegmentEvent
+	ManagementEvent
+	MeterEvent
+	LogEvent
+)
+
+type EventType int32
+
+// Event that implement this interface would be allowed to transmit in the Satellite.
+type Event interface {
+	// Name is a identify to distinguish different events.
+	Name() string

Review comment:
       Sorry for the comment, `type` is a sign to distinguish different events




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r534199243



##########
File path: docs/project_structue.md
##########
@@ -0,0 +1,63 @@
+# Project Structure
+- configs: Satellite configs.
+- internal: Core, Api, and common utils.
+- internal/pkg: Sharing with Core and Plugins, such as api and utils.
+- internal/satellite: The core of Satellite.
+- plugins: Contains all plugins.
+- plugins/{type}: Contains the plugins of this {type}. Satellite has 6 plugin types, which are collector, queue, parser, filter, client, and forward.
+- plugins/{type}/define{type}: Contains the plugin define.

Review comment:
       move  API defines to plugins dir




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] surlymo commented on a change in pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
surlymo commented on a change in pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#discussion_r532550268



##########
File path: internal/pkg/api/event.go
##########
@@ -0,0 +1,103 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import (
+	"fmt"
+	"time"
+)
+
+// The event type.
+const (
+	// Mapping to the type supported by SkyWalking OAP.
+	_ EventType = iota
+	MetricsEvent
+	ProfilingEvent
+	SegmentEvent
+	ManagementEvent
+	MeterEvent
+	LogEvent
+)
+
+type EventType int32
+
+// Event that implement this interface would be allowed to transmit in the Satellite.
+type Event interface {
+	// Name returns the event name.
+	Name() string
+
+	// Meta is a pair of key and value to record meta data, such as labels.
+	Meta() map[string]string
+
+	// Data returns the wrapped data.
+	Data() interface{}
+
+	// Time returns the event time.
+	Time() time.Time
+
+	// Type is a identifier to distinguish different events.
+	Type() EventType
+
+	// Remote means is a output event when returns true.
+	Remote() bool

Review comment:
       Use IsRemote?

##########
File path: internal/pkg/api/event.go
##########
@@ -0,0 +1,103 @@
+// Licensed to 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. Apache Software Foundation (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 api
+
+import (
+	"fmt"
+	"time"
+)
+
+// The event type.
+const (
+	// Mapping to the type supported by SkyWalking OAP.
+	_ EventType = iota
+	MetricsEvent
+	ProfilingEvent
+	SegmentEvent
+	ManagementEvent
+	MeterEvent
+	LogEvent
+)
+
+type EventType int32
+
+// Event that implement this interface would be allowed to transmit in the Satellite.
+type Event interface {
+	// Name returns the event name.
+	Name() string
+
+	// Meta is a pair of key and value to record meta data, such as labels.
+	Meta() map[string]string
+
+	// Data returns the wrapped data.
+	Data() interface{}
+
+	// Time returns the event time.
+	Time() time.Time
+
+	// Type is a identifier to distinguish different events.

Review comment:
       Need identifier for every single event? Just for offset recording 




----------------------------------------------------------------
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



[GitHub] [skywalking-satellite] EvanLjp commented on pull request #5: Add API && Plugin framework registry

Posted by GitBox <gi...@apache.org>.
EvanLjp commented on pull request #5:
URL: https://github.com/apache/skywalking-satellite/pull/5#issuecomment-737255037


   @wu-sheng Could I merge?


----------------------------------------------------------------
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