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/08 14:20:59 UTC
[GitHub] [skywalking-satellite] EvanLjp opened a new pull request #8: enhance the plugin mechanism
EvanLjp opened a new pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8
enhance the plugin mechanism, please look at 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] wu-sheng commented on a change in pull request #8: Add main structure
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r541863187
##########
File path: docs/design/plugin_structure.md
##########
@@ -0,0 +1,44 @@
+# plugin structure
+`Plugin is a common concept for Satellite, which is in all externsion plugins.`
+
+## Registration mechanism
+
+The Plugin registration mechanism in Satellite is similar to the SPI registration mechanism of Java.
+Plugin registration mechanism supports to register an interface and its implementation, that means different interfaces have different registration spaces.
+We can easily find the type of a specific plugin according to the interface and the plugin name and initialize it according to the type.
+
+structure:
+- code: `map[reflect.Type]map[string]reflect.Value`
+- meaning: `map[interface type]map[plugin name] plugin type`
+
+
+## Initialization mechanism
+
+Users can easily find a plugin type and initialize an empty plugin instance according to the previous registration mechanism. For setting up the configuration of the extension convenience, we define the initialization mechanism in Plugin structure.
+
+In the initialization mechanism, `the plugin category(interface)` and `the init config is required`.
+
+Initialize processing is like the following.
+
+1. Find the plugin name in the input config according to the fixed key `plugin_name`.
+2. Find plugin type according to the plugin category(interface) and the plugin name.
+3. Create an empty plugin.
+4. Initialize the plugin according to the merged config, which is created by the input config and the default config.
+
+
+
+## Plugin usage in Satellite
+Nowadays, the numbers of [sharing Plugin](module_design.md) is 2, and the numbers of the other [normal Plugin](module_design.md) is 7.
Review comment:
Don't put the number in the doc, it will change from time to time.
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539381778
##########
File path: plugins/fallbacker/api/fallbacker.go
##########
@@ -22,23 +22,24 @@ import (
"github.com/apache/skywalking-satellite/internal/pkg/event"
"github.com/apache/skywalking-satellite/internal/pkg/plugin"
+ "github.com/apache/skywalking-satellite/plugins/forwarder/api"
)
// Fallbacker is a plugin interface, that defines some fallback strategies.
type Fallbacker interface {
plugin.Plugin
-
// FallBack returns nil when finishing a successful process and returns a new Fallbacker when failure.
- FallBack(batch event.BatchEvents) Fallbacker
+ FallBack(batch event.BatchEvents, connection interface{}, forward api.ForwardFunc, callback DisconnectionCallback) Fallbacker
Review comment:
Always prefer `code` rather than `nil` to determine the behavior.
----------------------------------------------------------------
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 #8: Add main structure
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r541838271
##########
File path: docs/design/module_design.md
##########
@@ -0,0 +1,128 @@
+# Module Design
+## Namespace
+The namespace is an isolation concept in Satellite.
+Each namespace has one pipeline to process the telemetry data(metrics/traces/logs). Two namespaces are not sharing data.
+
+```
+ Satellite
+ ---------------------------------------------------------------------
+| ------------------------------------------- |
+| | Namespace | |
+| ------------------------------------------- |
+| ------------------------------------------- |
+| | Namespace | |
+| ------------------------------------------- |
+| ------------------------------------------- |
+| | Namespace | |
+| ------------------------------------------- |
+ ---------------------------------------------------------------------
+```
+## Modules
+There are 3 modules in one namespace, which are Gatherer, Processor, and Sender.
+
+- The Gatherer module is responsible for fetching or receiving data and pushing the data to Queue. So there are 2 kinds of Gatherer, which are ReceiverGatherer and FetcherGatherer.
+- The Processor module is responsible for reading data from the queue and processing data by a series of filter chains.
+- The Sender module is responsible for async processing and forwarding the data to the external services in the batch mode. After sending success, Sender would also acknowledge the offset of Queue in Gatherer.
+
+```
+ Namespace
+ --------------------------------------------------------------------
+| ---------- ----------- -------- |
+| | Gatherer | => | Processor | => | Sender | |
+| ---------- ----------- -------- |
+ --------------------------------------------------------------------
+```
+
+## Plugins
+
+Plugin is the minimal components in the module. There are 2 kinds of plugins in Satellite, which are sharing plugins and normal plugins.
Review comment:
Please consider renaming the `kind`. Such as, Sateliite has 2 plugin catalogs
----------------------------------------------------------------
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 #8: Add main structure
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r541828949
##########
File path: docs/design/module_design.md
##########
@@ -0,0 +1,127 @@
+# Module Design
+## Namespace
+Namespace is an isolation concept in Satellite. Each namespace has one agent to collect the APM data.
+So many agents would be running at the same time in Satellite.
Review comment:
APM data should be telemetry data.
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539420562
##########
File path: internal/pkg/plugin/registry.go
##########
@@ -18,62 +18,85 @@
package plugin
import (
+ "errors"
"fmt"
"reflect"
"sync"
)
-// 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.
-
-// reg is the global plugin registry
+// the global plugin registry
var (
Review comment:
Golang app has nothing like SPI in Java, a hardcode `if/else` section could take care of the register easily. Better to read, and easy to extend, no extra memory cost to maintain this register table.
----------------------------------------------------------------
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] gxthrj commented on a change in pull request #8: Add main structure
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r541840763
##########
File path: cmd/command.go
##########
@@ -0,0 +1,51 @@
+// 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 main
+
+import (
+ "github.com/urfave/cli/v2"
+
+ "github.com/apache/skywalking-satellite/internal/satellite/boot"
+ "github.com/apache/skywalking-satellite/internal/satellite/config"
+)
+
+var (
+ cmdStart = cli.Command{
+ Name: "start",
+ Usage: "start satellite",
+ Flags: []cli.Flag{
+ &cli.StringFlag{
+ Name: "config, c",
+ Usage: "Load configuration from `FILE`",
+ EnvVars: []string{"MOSN_CONFIG"},
Review comment:
mosn config?
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539076191
##########
File path: plugins/fallbacker/timer/timer_fallbacker.go
##########
@@ -0,0 +1,62 @@
+// 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 timer
+
+import (
+ "time"
+
+ "github.com/apache/skywalking-satellite/internal/pkg/event"
+ "github.com/apache/skywalking-satellite/plugins/forwarder/api"
+)
+
+// Fallbacker is a timer fallbacker when forward fails. `latencyFactor` is the standard retry duration,
+// and the time for each retry is expanded by 2 times until the number of retries reaches the maximum.
+type Fallbacker struct {
Review comment:
yes it is a plugin, api/xxx is interface , this is on timer package
----------------------------------------------------------------
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 #8: Add main structure
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r541832567
##########
File path: docs/design/module_structure.md
##########
@@ -1,18 +1,25 @@
# Module structure
+## Overview
Module is the core workers in Satellite. Module is constituted by the specific extension plugins.
-There are four modules in Satellite, which is ClientManager, Gatherer, Sender, and Processor.
+There are 3 modules in one agent, which are Gatherer, Processor, and Sender.
-Responsibilities:
+- The Gatherer module is responsible for fetching or receiving data and pushing the data to Queue. So there are 2 kinds of Gatherer, which are ReceiverGatherer and FetcherGatherer.
+- The Processor module is responsible for reading data from the queue and processing data by a series of filter chains.
+- The Sender module is responsible for async processing and forwarding the data to the external services in the batch mode. After sending success, Sender would also acknowledge the offset of Queue in Gatherer.
-- ClientManager: Maintain connection and monitor connection status
-- Sender: Sender data to the external services, such as Kafka and OAP
-- Gatherer: Gather the APM data from the other systems, such as fetch prometheus metrics.
-- Processor: Data processing to create new metrics data.
+```
+ Agent
Review comment:
would fix it by using one concept
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539405601
##########
File path: internal/pkg/plugin/registry.go
##########
@@ -18,62 +18,85 @@
package plugin
import (
+ "errors"
"fmt"
"reflect"
"sync"
)
-// 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.
-
-// reg is the global plugin registry
+// the global plugin registry
var (
Review comment:
Is this really required to be global? Take a look at Java version's `ModuleManager`. Once you accept the global singleton, the tests are sensitive about clearing this register table after every time, and you don't have a chance to run tests concurrently(even I don't know whether this is an option in 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] surlymo commented on a change in pull request #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
surlymo commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r540219743
##########
File path: internal/satellite/module/buffer/buffer.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 buffer
+
+import (
+ "sync"
+
+ "github.com/apache/skywalking-satellite/internal/pkg/event"
+ "github.com/apache/skywalking-satellite/internal/pkg/log"
+)
+
+// BatchBuffer is a buffer to cache the input data in Sender.
+type BatchBuffer struct {
+ sync.Mutex // local
+ buf []*event.OutputEventContext // cache
+ first int64 // the first OutputEventContext offset
+ last int64 // the last OutputEventContext offset
+ size int // usage size
+ cap int // the max capacity
+}
+
+// NewBuffer crate a new BatchBuffer according to the capacity param.
+func NewBatchBuffer(capacity int) *BatchBuffer {
+ return &BatchBuffer{
+ buf: make([]*event.OutputEventContext, capacity),
+ first: 0,
+ last: 0,
+ size: 0,
+ cap: capacity,
+ }
+}
+
+// Buf returns the cached data in BatchBuffer.
+func (b *BatchBuffer) Buf() []*event.OutputEventContext {
+ b.Lock()
Review comment:
so many locks. it may leads to the system slowing down ？
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539766312
##########
File path: internal/pkg/plugin/registry.go
##########
@@ -18,62 +18,85 @@
package plugin
import (
+ "errors"
"fmt"
"reflect"
"sync"
)
-// 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.
-
-// reg is the global plugin registry
+// the global plugin registry
var (
Review comment:
Satellite supports multi plugins ，so we support a registration mechanism to control this plugins. So there are 2 ways to register the plugins, which are auto registration and manual registration. Auto registration is relay on the init func in golang, that makes the codes more cleaner. However, it also makes the codes more complex. Another ways is manually registration, that would add few go files to control the plugins registration. This way would easier to read source code, but would add some files to control the registration. Which one should we choose？
----------------------------------------------------------------
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 #8: Add main structure
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#issuecomment-743975482
> The new structure seems fine, but the `lock` is still there.
>
> To all reviewers, I am going to OOO in the next 4 days, so, would not show up for reviewing details.
already remove the lock in the plugins, thx for your notice.
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539333349
##########
File path: plugins/collector/api/collector.go
##########
@@ -27,7 +27,7 @@ import (
// Init() Initial stage: Init plugin by config
// ||
// \/
-// Prepare() Preparing stage: Prepare the collector, such as build connection with SkyWalking javaagent.
+// Init() Preparing stage: Init the collector, such as build connection with SkyWalking javaagent.
Review comment:
Why duplicated `Init`? And `such as build connection with SkyWalking javaagent.` is wrong. I think you mean, set up the gRPC server for the agents?
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539424469
##########
File path: internal/pkg/plugin/registry.go
##########
@@ -18,62 +18,85 @@
package plugin
import (
+ "errors"
"fmt"
"reflect"
"sync"
)
-// 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.
-
-// reg is the global plugin registry
+// the global plugin registry
var (
Review comment:
the table only registers the reflect.type, I don't think this would cost resources for future extensions.
----------------------------------------------------------------
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] nic-chen commented on a change in pull request #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539781667
##########
File path: internal/pkg/plugin/registry.go
##########
@@ -18,62 +18,85 @@
package plugin
import (
+ "errors"
"fmt"
"reflect"
"sync"
)
-// 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.
-
-// reg is the global plugin registry
+// the global plugin registry
var (
Review comment:
> This leads to another question, why needs the `package init()` mechanism? All the plugins should be registered due to the configuration files or parameters, aren't they? Otherwise, you will cost the memory for nothing.
+1
----------------------------------------------------------------
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 #8: Add main structure
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#issuecomment-745781911
@gxthrj @wu-sheng @surlymo I think the codes have achieved a three-approved goal. Could u help me to approve it again, I would merge 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 merged pull request #8: Add main structure
Posted by GitBox <gi...@apache.org>.
EvanLjp merged pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8
----------------------------------------------------------------
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] gxthrj commented on a change in pull request #8: Add main structure
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r543965247
##########
File path: dist/licenses/LICENSE-cli
##########
@@ -0,0 +1,21 @@
+MIT License
+
+Copyright (c) 2016 Jeremy Saenz & Contributors
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.
Review comment:
Need newline at end of file
##########
File path: dist/licenses/LICENSE-viper
##########
@@ -0,0 +1,21 @@
+The MIT License (MIT)
+
+Copyright (c) 2014 Steve Francia
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.
Review comment:
ditto
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r541022794
##########
File path: internal/satellite/module/buffer/buffer.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 buffer
+
+import (
+ "sync"
+
+ "github.com/apache/skywalking-satellite/internal/pkg/event"
+ "github.com/apache/skywalking-satellite/internal/pkg/log"
+)
+
+// BatchBuffer is a buffer to cache the input data in Sender.
+type BatchBuffer struct {
+ sync.Mutex // local
+ buf []*event.OutputEventContext // cache
+ first int64 // the first OutputEventContext offset
+ last int64 // the last OutputEventContext offset
+ size int // usage size
+ cap int // the max capacity
+}
+
+// NewBuffer crate a new BatchBuffer according to the capacity param.
+func NewBatchBuffer(capacity int) *BatchBuffer {
+ return &BatchBuffer{
+ buf: make([]*event.OutputEventContext, capacity),
+ first: 0,
+ last: 0,
+ size: 0,
+ cap: capacity,
+ }
+}
+
+// Buf returns the cached data in BatchBuffer.
+func (b *BatchBuffer) Buf() []*event.OutputEventContext {
+ b.Lock()
Review comment:
Any update?
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539409693
##########
File path: internal/pkg/plugin/registry.go
##########
@@ -18,62 +18,85 @@
package plugin
import (
+ "errors"
"fmt"
"reflect"
"sync"
)
-// 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.
-
-// reg is the global plugin registry
+// the global plugin registry
var (
Review comment:
It should be global because the registered way is func init()
----------------------------------------------------------------
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 #8: Add main structure
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#issuecomment-743970639
The new structure seems fine, but the `lock` is still there.
To all reviewers, I am going to OOO in the next 4 days, so, would not show up for reviewing details.
----------------------------------------------------------------
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 #8: Add main structure
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r541832314
##########
File path: docs/design/module_structure.md
##########
@@ -1,18 +1,25 @@
# Module structure
+## Overview
Module is the core workers in Satellite. Module is constituted by the specific extension plugins.
-There are four modules in Satellite, which is ClientManager, Gatherer, Sender, and Processor.
+There are 3 modules in one agent, which are Gatherer, Processor, and Sender.
-Responsibilities:
+- The Gatherer module is responsible for fetching or receiving data and pushing the data to Queue. So there are 2 kinds of Gatherer, which are ReceiverGatherer and FetcherGatherer.
+- The Processor module is responsible for reading data from the queue and processing data by a series of filter chains.
+- The Sender module is responsible for async processing and forwarding the data to the external services in the batch mode. After sending success, Sender would also acknowledge the offset of Queue in Gatherer.
-- ClientManager: Maintain connection and monitor connection status
-- Sender: Sender data to the external services, such as Kafka and OAP
-- Gatherer: Gather the APM data from the other systems, such as fetch prometheus metrics.
-- Processor: Data processing to create new metrics data.
+```
+ Agent
Review comment:
Agent is a whole concept: Each namespace has one agent to collect the telemetry data. (module_design.md)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-satellite] wu-sheng commented on a change in pull request #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539768422
##########
File path: internal/pkg/plugin/registry.go
##########
@@ -18,62 +18,85 @@
package plugin
import (
+ "errors"
"fmt"
"reflect"
"sync"
)
-// 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.
-
-// reg is the global plugin registry
+// the global plugin registry
var (
Review comment:
Note, auto-registration would cost clear memory, even limited, our SLO is 20M for the whole satellite deployment, consider this in the decision.
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539418203
##########
File path: internal/pkg/plugin/registry.go
##########
@@ -18,62 +18,85 @@
package plugin
import (
+ "errors"
"fmt"
"reflect"
"sync"
)
-// 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.
-
-// reg is the global plugin registry
+// the global plugin registry
var (
Review comment:
This leads to another question, why needs the `package init()` mechanism? All the plugins should be registered due to the configuration files or parameters, aren't they? Otherwise, you will cost the memory for nothing.
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r540062049
##########
File path: plugins/collector/api/collector.go
##########
@@ -24,36 +24,22 @@ import (
"github.com/apache/skywalking-satellite/internal/pkg/plugin"
)
-// Init() Initial stage: Init plugin by config
-// ||
-// \/
-// Prepare() Preparing stage: Prepare the collector, such as build connection with SkyWalking javaagent.
-// ||
-// \/
-// Next() Running stage: When Collector collect a data, the data would be fetched by the upstream
-// || component through this method.
-// \/
-// Close() Closing stage: Close the Collector, such as close connection with SkyWalking javaagent.
-
// Collector is a plugin interface, that defines new collectors.
type Collector interface {
plugin.Plugin
-
- // Prepare creates a listen or reader to gather data.
- Prepare()
+ // Prepare creates a listener or reader to gather APM data, such as start a gRPC listener for Segment receiving.
+ Prepare() error
Review comment:
I don't think the upper layers need to care about protocols, they're just different plug-ins
----------------------------------------------------------------
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] gxthrj commented on a change in pull request #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539946046
##########
File path: docs/design/plugin_structure.md
##########
@@ -0,0 +1,47 @@
+# plugin structure
+`Plugin is a common concept for Satellite. Not only does the extension mechanism depend on Plugin, but the core modules also depend on Plugin`
+
+## Registration mechanism
+
+The Plugin registration mechanism in Satellite is similar to the SPI registration mechanism of Java.
+Plugin registration mechanism supports to register an interface and its implementation, that means different interfaces have different registration spaces.
+We can easily find the type of a specific plugin according to the interface and the plugin name and initialize it according to the type.
+
+structure:
+- code: map[reflect.Type]map[string]reflect.Value
+- meaning: map[`interface type`]map[`plugin name`] `plugin type`
+
+
+## Initialization mechanism
+
+Users can easily find a plugin type and initialize an empty plugin instance according to the previous registration mechanism. However, users often need an initialized plugin rather than a empty plugin. So we define the initialization mechanism in
+Plugin structure.
+
+In the initialization mechanism, the plugin category(interface) and the init config is required. Initialize processing is like the following.
+
+1. Find the plugin name in the input config.
+2. Find plugin type according to the plugin category(interface) and the plugin name.
+3. Create an empty plugin.
+4. Initialize the plugin according to the config.
+5. do some callback processing after initialized.
+
+
+## Plugin usage in Satellite
+Not only does the extension mechanism depend on Plugin, but the core modules also depend on Plugin in Satellite. We'll illustrate this with an example.
+
+- Core Module interface: module.Service
+ - the specific plugin: module.Gatherer
+ - the specific plugin: module.Processor
+ - the specific plugin: module.Sender
+ - the specific plugin: module.ClientManager
+- Extension:
+ - Collector interface
+ - the specific plugin: segment-receiver
Review comment:
Is the `fetcher` plugin also specified here?
----------------------------------------------------------------
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 #8: Add main structure
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r541844224
##########
File path: cmd/command.go
##########
@@ -0,0 +1,51 @@
+// 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 main
+
+import (
+ "github.com/urfave/cli/v2"
+
+ "github.com/apache/skywalking-satellite/internal/satellite/boot"
+ "github.com/apache/skywalking-satellite/internal/satellite/config"
+)
+
+var (
+ cmdStart = cli.Command{
+ Name: "start",
+ Usage: "start satellite",
+ Flags: []cli.Flag{
+ &cli.StringFlag{
+ Name: "config, c",
+ Usage: "Load configuration from `FILE`",
+ EnvVars: []string{"MOSN_CONFIG"},
Review comment:
sorry ,I learned the use of golang CLI through MOSN , already fix 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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r541528727
##########
File path: internal/satellite/module/buffer/buffer.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 buffer
+
+import (
+ "sync"
+
+ "github.com/apache/skywalking-satellite/internal/pkg/event"
+ "github.com/apache/skywalking-satellite/internal/pkg/log"
+)
+
+// BatchBuffer is a buffer to cache the input data in Sender.
+type BatchBuffer struct {
+ sync.Mutex // local
+ buf []*event.OutputEventContext // cache
+ first int64 // the first OutputEventContext offset
+ last int64 // the last OutputEventContext offset
+ size int // usage size
+ cap int // the max capacity
+}
+
+// NewBuffer crate a new BatchBuffer according to the capacity param.
+func NewBatchBuffer(capacity int) *BatchBuffer {
+ return &BatchBuffer{
+ buf: make([]*event.OutputEventContext, capacity),
+ first: 0,
+ last: 0,
+ size: 0,
+ cap: capacity,
+ }
+}
+
+// Buf returns the cached data in BatchBuffer.
+func (b *BatchBuffer) Buf() []*event.OutputEventContext {
+ b.Lock()
Review comment:
I am polishing it now, would push some code at night
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539403679
##########
File path: internal/pkg/plugin/registry.go
##########
@@ -18,62 +18,85 @@
package plugin
import (
+ "errors"
"fmt"
"reflect"
"sync"
)
-// 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.
-
-// reg is the global plugin registry
+// the global plugin registry
var (
- reg map[reflect.Type]map[string]reflect.Value
lock sync.Mutex
+ reg map[reflect.Type]map[string]reflect.Value
+ meta map[reflect.Type]*RegInfo
)
func init() {
reg = make(map[reflect.Type]map[string]reflect.Value)
+ meta = make(map[reflect.Type]*RegInfo)
}
-// Add new plugin category. The different plugin category could have same plugin names.
-func AddPluginCategory(pluginCategory reflect.Type) {
+// RegisterCategory register new plugin category with default InitializingFunc.
+// required:
+// pluginCategory: the plugin interface type.
+// Optional:
+// n: the plugin name finder,and the default value is defaultNameFinder
+// i, the plugin initializer, and the default value is defaultInitializing
+// c, the plugin initializer callback func, and the default value is defaultCallBack
Review comment:
Unmatched 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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539740741
##########
File path: internal/pkg/plugin/registry.go
##########
@@ -18,62 +18,85 @@
package plugin
import (
+ "errors"
"fmt"
"reflect"
"sync"
)
-// 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.
-
-// reg is the global plugin registry
+// the global plugin registry
var (
Review comment:
I know you are writing an SPI mechanism in the structure. Personally, I have concerns about that.
This thing makes the codes more complex than it needs to be, at the same time, w/o the dynamic link(I don't ask for implementing dynamic link) this mechanism provides no new feature.
This is the reason mostly.
Also, around the tech part
> In Go, the predefined init() function sets off a piece of code to run before any other part of your package. This code will execute as soon as the package is imported.
How could you sure the package is imported if the implementation in a random package? Or some packages the core doesn't know to access in the initializing stage?
___
In the basic idea, I don't want the go works like Java. @hanahmily @kezhenxu94 I hope to get your inputs.
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r541752083
##########
File path: plugins/collector/api/collector.go
##########
@@ -24,36 +24,22 @@ import (
"github.com/apache/skywalking-satellite/internal/pkg/plugin"
)
-// Init() Initial stage: Init plugin by config
-// ||
-// \/
-// Prepare() Preparing stage: Prepare the collector, such as build connection with SkyWalking javaagent.
-// ||
-// \/
-// Next() Running stage: When Collector collect a data, the data would be fetched by the upstream
-// || component through this method.
-// \/
-// Close() Closing stage: Close the Collector, such as close connection with SkyWalking javaagent.
-
// Collector is a plugin interface, that defines new collectors.
type Collector interface {
plugin.Plugin
-
- // Prepare creates a listen or reader to gather data.
- Prepare()
+ // Prepare creates a listener or reader to gather APM data, such as start a gRPC listener for Segment receiving.
+ Prepare() error
Review comment:
Take a look again. Related concepts have been added
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539804098
##########
File path: internal/pkg/plugin/registry.go
##########
@@ -18,62 +18,85 @@
package plugin
import (
+ "errors"
"fmt"
"reflect"
"sync"
)
-// 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.
-
-// reg is the global plugin registry
+// the global plugin registry
var (
Review comment:
ok, I would change to the manual way.
----------------------------------------------------------------
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 #8: Add main structure
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r541829059
##########
File path: docs/design/module_structure.md
##########
@@ -1,18 +1,25 @@
# Module structure
+## Overview
Module is the core workers in Satellite. Module is constituted by the specific extension plugins.
-There are four modules in Satellite, which is ClientManager, Gatherer, Sender, and Processor.
+There are 3 modules in one agent, which are Gatherer, Processor, and Sender.
-Responsibilities:
+- The Gatherer module is responsible for fetching or receiving data and pushing the data to Queue. So there are 2 kinds of Gatherer, which are ReceiverGatherer and FetcherGatherer.
+- The Processor module is responsible for reading data from the queue and processing data by a series of filter chains.
+- The Sender module is responsible for async processing and forwarding the data to the external services in the batch mode. After sending success, Sender would also acknowledge the offset of Queue in Gatherer.
-- ClientManager: Maintain connection and monitor connection status
-- Sender: Sender data to the external services, such as Kafka and OAP
-- Gatherer: Gather the APM data from the other systems, such as fetch prometheus metrics.
-- Processor: Data processing to create new metrics data.
+```
+ Agent
Review comment:
I think this should be `namespace`. Don't put 2 names in one concept.
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539076689
##########
File path: internal/pkg/plugin/definition.go
##########
@@ -0,0 +1,35 @@
+// 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 plugin
+
+// Plugin defines the plugin model in Satellite.
+type Plugin interface {
+ // Name returns the name of the specific plugin.
+ Name() string
+ // Description returns the description of the specific plugin.
+ Description() string
+}
+
+// NameFinderFunc is used to get the plugin name from different plugin configs.
+type NameFinderFunc func(config interface{}) string
+
+// InitializingFunc is used to initialize the specific plugin.
+type InitializingFunc func(plugin Plugin, config interface{})
+
+// CallbackFunc would be invoked after initializing.
Review comment:
the module and the extension plugin has different initial ways
----------------------------------------------------------------
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 #8: Add main structure
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r541874571
##########
File path: internal/pkg/log/log.go
##########
@@ -0,0 +1,146 @@
+// 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 log
+
+import (
+ "fmt"
+ "os"
+ "strings"
+ "sync"
+
+ "github.com/sirupsen/logrus"
+)
+
+// Default logger config.
+const (
+ defaultLogPattern = "%time [%level][%field] - %msg"
+ defaultTimePattern = "2006-01-02 15:04:05.001"
Review comment:
yes, it is the log pattern
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r541528727
##########
File path: internal/satellite/module/buffer/buffer.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 buffer
+
+import (
+ "sync"
+
+ "github.com/apache/skywalking-satellite/internal/pkg/event"
+ "github.com/apache/skywalking-satellite/internal/pkg/log"
+)
+
+// BatchBuffer is a buffer to cache the input data in Sender.
+type BatchBuffer struct {
+ sync.Mutex // local
+ buf []*event.OutputEventContext // cache
+ first int64 // the first OutputEventContext offset
+ last int64 // the last OutputEventContext offset
+ size int // usage size
+ cap int // the max capacity
+}
+
+// NewBuffer crate a new BatchBuffer according to the capacity param.
+func NewBatchBuffer(capacity int) *BatchBuffer {
+ return &BatchBuffer{
+ buf: make([]*event.OutputEventContext, capacity),
+ first: 0,
+ last: 0,
+ size: 0,
+ cap: capacity,
+ }
+}
+
+// Buf returns the cached data in BatchBuffer.
+func (b *BatchBuffer) Buf() []*event.OutputEventContext {
+ b.Lock()
Review comment:
I am polishing it now
----------------------------------------------------------------
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 #8: Add main structure
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r541829979
##########
File path: docs/design/plugin_structure.md
##########
@@ -27,20 +26,21 @@ Initialize processing is like the following.
4. Initialize the plugin according to the merged config, which is created by the input config and the default config.
+
## Plugin usage in Satellite
+Nowadays, we have 9 kinds of plugins in Satellite. 2 kinds of Plugins are the [sharing plugins](module_design.md) and the others are the [normal plugins](module_design.md).
Review comment:
The word `kinds` is over-used. Consider using `Catalog S` for sharding plugins and `Catalog N` for normal plugins?
----------------------------------------------------------------
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] gxthrj commented on a change in pull request #8: Add main structure
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r541841299
##########
File path: configs/satellite_config.yaml
##########
@@ -0,0 +1,69 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+#
Review comment:
duplecate headers
----------------------------------------------------------------
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] gxthrj commented on a change in pull request #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539948304
##########
File path: plugins/collector/api/collector.go
##########
@@ -24,36 +24,22 @@ import (
"github.com/apache/skywalking-satellite/internal/pkg/plugin"
)
-// Init() Initial stage: Init plugin by config
-// ||
-// \/
-// Prepare() Preparing stage: Prepare the collector, such as build connection with SkyWalking javaagent.
-// ||
-// \/
-// Next() Running stage: When Collector collect a data, the data would be fetched by the upstream
-// || component through this method.
-// \/
-// Close() Closing stage: Close the Collector, such as close connection with SkyWalking javaagent.
-
// Collector is a plugin interface, that defines new collectors.
type Collector interface {
plugin.Plugin
-
- // Prepare creates a listen or reader to gather data.
- Prepare()
+ // Prepare creates a listener or reader to gather APM data, such as start a gRPC listener for Segment receiving.
+ Prepare() error
Review comment:
If we need to implement a `http-receiver`, should we provide concrete implementation here?
##########
File path: plugins/collector/api/collector.go
##########
@@ -24,36 +24,22 @@ import (
"github.com/apache/skywalking-satellite/internal/pkg/plugin"
)
-// Init() Initial stage: Init plugin by config
-// ||
-// \/
-// Prepare() Preparing stage: Prepare the collector, such as build connection with SkyWalking javaagent.
-// ||
-// \/
-// Next() Running stage: When Collector collect a data, the data would be fetched by the upstream
-// || component through this method.
-// \/
-// Close() Closing stage: Close the Collector, such as close connection with SkyWalking javaagent.
-
// Collector is a plugin interface, that defines new collectors.
type Collector interface {
plugin.Plugin
-
- // Prepare creates a listen or reader to gather data.
- Prepare()
+ // Prepare creates a listener or reader to gather APM data, such as start a gRPC listener for Segment receiving.
+ Prepare() error
Review comment:
And do we need a type field to identify the `receiver` protocol, such as `http` `grpc`.
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539769232
##########
File path: internal/pkg/plugin/registry.go
##########
@@ -18,62 +18,85 @@
package plugin
import (
+ "errors"
"fmt"
"reflect"
"sync"
)
-// 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.
-
-// reg is the global plugin registry
+// the global plugin registry
var (
Review comment:
Please edit this comment to show your preference. @apache/skywalking-committers, especially reviewers, @surlymo @hanahmily @kezhenxu94 @dmsolr @zhangkewei @gxthrj @nic-chen @moonming
- +1 for Auto-registration.
- +1 for manual registration in go source codes. @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] EvanLjp commented on a change in pull request #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r540083308
##########
File path: docs/design/plugin_structure.md
##########
@@ -0,0 +1,47 @@
+# plugin structure
+`Plugin is a common concept for Satellite. Not only does the extension mechanism depend on Plugin, but the core modules also depend on Plugin`
+
+## Registration mechanism
+
+The Plugin registration mechanism in Satellite is similar to the SPI registration mechanism of Java.
+Plugin registration mechanism supports to register an interface and its implementation, that means different interfaces have different registration spaces.
+We can easily find the type of a specific plugin according to the interface and the plugin name and initialize it according to the type.
+
+structure:
+- code: map[reflect.Type]map[string]reflect.Value
+- meaning: map[`interface type`]map[`plugin name`] `plugin type`
+
+
+## Initialization mechanism
+
+Users can easily find a plugin type and initialize an empty plugin instance according to the previous registration mechanism. However, users often need an initialized plugin rather than a empty plugin. So we define the initialization mechanism in
+Plugin structure.
+
+In the initialization mechanism, the plugin category(interface) and the init config is required. Initialize processing is like the following.
+
+1. Find the plugin name in the input config.
+2. Find plugin type according to the plugin category(interface) and the plugin name.
+3. Create an empty plugin.
+4. Initialize the plugin according to the config.
+5. do some callback processing after initialized.
+
+
+## Plugin usage in Satellite
+Not only does the extension mechanism depend on Plugin, but the core modules also depend on Plugin in Satellite. We'll illustrate this with an example.
+
+- Core Module interface: module.Service
+ - the specific plugin: module.Gatherer
+ - the specific plugin: module.Processor
+ - the specific plugin: module.Sender
+ - the specific plugin: module.ClientManager
+- Extension:
+ - Collector interface
+ - the specific plugin: segment-receiver
Review comment:
fetcher is a kind of collector.
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
surlymo commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539806389
##########
File path: internal/pkg/plugin/registry.go
##########
@@ -18,62 +18,85 @@
package plugin
import (
+ "errors"
"fmt"
"reflect"
"sync"
)
-// 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.
-
-// reg is the global plugin registry
+// the global plugin registry
var (
Review comment:
+1 for manual registration in go source codes
i think either is ok. auto-registration like spring annotation declaration in java, manual registration like spring xml definetion. i prefer the latter.
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#issuecomment-741544045
First, please fix Makefile, `make all` doesn't work, due to no `build` task.
----------------------------------------------------------------
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 #8: Add main structure
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r541828790
##########
File path: docs/design/module_design.md
##########
@@ -0,0 +1,127 @@
+# Module Design
+## Namespace
+Namespace is an isolation concept in Satellite. Each namespace has one agent to collect the APM data.
+So many agents would be running at the same time in Satellite.
+```
+ Satellite
+ ---------------------------------------------------------------------
+| ------------------------------------------- |
+| | Agent | |
+| ------------------------------------------- |
+| ------------------------------------------- |
+| | Agent | |
+| ------------------------------------------- |
+| ------------------------------------------- |
+| | Agent | |
+| ------------------------------------------- |
+ ---------------------------------------------------------------------
+```
+## Modules
+There are 3 modules in one agent, which are Gatherer, Processor, and Sender.
+
+- The Gatherer module is responsible for fetching or receiving data and pushing the data to Queue. So there are 2 kinds of Gatherer, which are ReceiverGatherer and FetcherGatherer.
+- The Processor module is responsible for reading data from the queue and processing data by a series of filter chains.
+- The Sender module is responsible for async processing and forwarding the data to the external services in the batch mode. After sending success, Sender would also acknowledge the offset of Queue in Gatherer.
+
+```
+ Agent
+ --------------------------------------------------------------------
+| ---------- ----------- -------- |
+| | Gatherer | => | Processor | => | Sender | |
+| ---------- ----------- -------- |
+ --------------------------------------------------------------------
+```
+
+## Plugins
+
+Plugin is the minimal components in the module. There are 2 kinds of plugins in Satellite, which are sharing plugins and normal plugins.
+
+- a sharing plugin instance could be sharing with multiple modules in the different namespaces.
+- a normal plugin instance is only be used in a fixed module of the fixed namespaces.
+
+### Sharing plugin
+Nowadays, there are 2 sharing plugins in Satellite, which are server plugins and client plugins. The reason why they are sharing plugins is to reduce the resource cost in connection. Server plugins are sharing with the ReceiverGatherer modules in the different namespaces to receive the external requests. And the client plugins is sharing with the Sender modules in the different namespaces to connect with external services, such as Kafka and OAP.
+
+```
+ Sharing Server Sharing Client
+ --------------------------------------------------------------------
+| ------------------ ----------- -------- |
+| | ReceiverGatherer | => | Processor | => | Sender | |
+| ------------------ ----------- -------- |
+ --------------------------------------------------------------------
+ --------------------------------------------------------------------
+| ------------------ ----------- -------- |
+| | ReceiverGatherer | => | Processor | => | Sender | |
+| ------------------ ----------- -------- |
+ --------------------------------------------------------------------
+ --------------------------------------------------------------------
+| ------------------ ----------- -------- |
+| | ReceiverGatherer | => | Processor | => | Sender | |
+| ------------------ ----------- -------- |
+ --------------------------------------------------------------------
+```
+
+### Normal plugin
+There are 7 normal plugins in Satellite, which are Receiver, Fetcher, Queue, Parser, Filter, Forwarder, and Fallbacker.
Review comment:
```suggestion
There are 7 kinds of normal plugins in Satellite, which are Receiver, Fetcher, Queue, Parser, Filter, Forwarder, and Fallbacker.
```
Same as above.
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#issuecomment-742524870
The new plugin design makes me more confortable now. Thanks for the refactoring, @EvanLjp
----------------------------------------------------------------
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 #8: Add main structure
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r541830173
##########
File path: docs/design/plugin_structure.md
##########
@@ -27,20 +26,21 @@ Initialize processing is like the following.
4. Initialize the plugin according to the merged config, which is created by the input config and the default config.
+
## Plugin usage in Satellite
+Nowadays, we have 9 kinds of plugins in Satellite. 2 kinds of Plugins are the [sharing plugins](module_design.md) and the others are the [normal plugins](module_design.md).
+
+
- Extension:
- - Collector interface
- - the specific plugin: segment-receiver
- - Queue interface
- - the specific plugin: mmap-queue
- - Filter interface
- - the specific plugin: sampling-filter
- - Client interface
- - the specific plugin: gRpc-client
- - Fallbacker interface
- - the specific plugin: timer-fallbacker
- - Forwarder interface
- - the specific plugin: grpc-segment-forwarder
- - Parser interface
- - the specific plugin: csv-parser
+ - sharing plugins
+ - Server interface
+ - Client interface
Review comment:
Let's don't use `interface`, this is not a design concept, this is the code. You called them as plugins in other parts of the 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 commented on a change in pull request #8: Add main structure
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r541874450
##########
File path: docs/design/plugin_structure.md
##########
@@ -0,0 +1,44 @@
+# plugin structure
+`Plugin is a common concept for Satellite, which is in all externsion plugins.`
+
+## Registration mechanism
+
+The Plugin registration mechanism in Satellite is similar to the SPI registration mechanism of Java.
+Plugin registration mechanism supports to register an interface and its implementation, that means different interfaces have different registration spaces.
+We can easily find the type of a specific plugin according to the interface and the plugin name and initialize it according to the type.
+
+structure:
+- code: `map[reflect.Type]map[string]reflect.Value`
+- meaning: `map[interface type]map[plugin name] plugin type`
+
+
+## Initialization mechanism
+
+Users can easily find a plugin type and initialize an empty plugin instance according to the previous registration mechanism. For setting up the configuration of the extension convenience, we define the initialization mechanism in Plugin structure.
+
+In the initialization mechanism, `the plugin category(interface)` and `the init config is required`.
+
+Initialize processing is like the following.
+
+1. Find the plugin name in the input config according to the fixed key `plugin_name`.
+2. Find plugin type according to the plugin category(interface) and the plugin name.
+3. Create an empty plugin.
+4. Initialize the plugin according to the merged config, which is created by the input config and the default config.
+
+
+
+## Plugin usage in Satellite
+Nowadays, the numbers of [sharing Plugin](module_design.md) is 2, and the numbers of the other [normal Plugin](module_design.md) is 7.
Review comment:
okay
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539433237
##########
File path: internal/pkg/plugin/registry.go
##########
@@ -18,62 +18,85 @@
package plugin
import (
+ "errors"
"fmt"
"reflect"
"sync"
)
-// 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.
-
-// reg is the global plugin registry
+// the global plugin registry
var (
Review comment:
there is a demo to compute the size of (reflect.type), the size is only few Bytes, https://stackoverflow.com/questions/31338588/generic-function-to-get-size-of-any-structure-in-golang
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
surlymo commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539059956
##########
File path: plugins/fallbacker/timer/timer_fallbacker.go
##########
@@ -0,0 +1,62 @@
+// 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 timer
+
+import (
+ "time"
+
+ "github.com/apache/skywalking-satellite/internal/pkg/event"
+ "github.com/apache/skywalking-satellite/plugins/forwarder/api"
+)
+
+// Fallbacker is a timer fallbacker when forward fails. `latencyFactor` is the standard retry duration,
+// and the time for each retry is expanded by 2 times until the number of retries reaches the maximum.
+type Fallbacker struct {
Review comment:
i think fallbacker should be a plugin?
##########
File path: plugins/fallbacker/timer/timer_fallbacker.go
##########
@@ -0,0 +1,62 @@
+// 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 timer
+
+import (
+ "time"
+
+ "github.com/apache/skywalking-satellite/internal/pkg/event"
+ "github.com/apache/skywalking-satellite/plugins/forwarder/api"
+)
+
+// Fallbacker is a timer fallbacker when forward fails. `latencyFactor` is the standard retry duration,
+// and the time for each retry is expanded by 2 times until the number of retries reaches the maximum.
+type Fallbacker struct {
+ maxTimes int `mapstructure:"max_times"`
+ latencyFactor int `mapstructure:"latency_factor"`
+}
+
+func (t *Fallbacker) Name() string {
+ return "timer-fallbacker"
+}
+
+func (t *Fallbacker) Description() string {
+ return "this is a timer trigger when forward fails."
+}
+
+func (t *Fallbacker) DefaultConfig() string {
+ return `
+max_times: 3
+latency_factor: 2000
+`
+}
+
+func (t *Fallbacker) FallBack(batch event.BatchEvents, connection interface{}, forward api.ForwardFunc) {
Review comment:
do not make func name the same as type name
##########
File path: internal/pkg/plugin/definition.go
##########
@@ -0,0 +1,35 @@
+// 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 plugin
+
+// Plugin defines the plugin model in Satellite.
+type Plugin interface {
+ // Name returns the name of the specific plugin.
+ Name() string
+ // Description returns the description of the specific plugin.
+ Description() string
+}
+
+// NameFinderFunc is used to get the plugin name from different plugin configs.
+type NameFinderFunc func(config interface{}) string
+
+// InitializingFunc is used to initialize the specific plugin.
+type InitializingFunc func(plugin Plugin, config interface{})
+
+// CallbackFunc would be invoked after initializing.
Review comment:
why split it from initializingfunc？show some use cases?
----------------------------------------------------------------
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 #8: Add main structure
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r541832275
##########
File path: configs/satellite_config.yaml
##########
@@ -0,0 +1,69 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+logger:
+ log_pattern: "%time [%level][%field] - %msg"
+ time_pattern: "2006-01-02 15:04:05.001"
+ level: "info"
+
+sharing:
+ clients:
+ - plugin_name: "grpc-client"
+ k: v
+ servers:
+ - plugin_name: "grpc-server"
+ k: v
+agents:
+ - common_config:
+ namespace: namespace1
Review comment:
```suggestion
namespace:
- common_config:
id: namespace1
```
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
surlymo commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539238627
##########
File path: docs/design/plugin_structure.md
##########
@@ -0,0 +1,47 @@
+# plugin structure
+`Plugin is a common concept for Satellite. Not only does the extension mechanism depend on Plugin, but the core modules also depend on Plugin`
+
+## Registration mechanism
+
+The Plugin registration mechanism in Satellite is similar to the SPI registration mechanism of Java.
+Plugin registration mechanism supports to register an interface and its implementation, that means different interfaces have different registration spaces.
+We can easily find the type of a specific plugin according to the interface and the plugin name and initialize it according to the type.
+
+structure:
+- code: map[reflect.Type]map[string]reflect.Value
+- meaning: map[`interface type`]map[`plugin name`] `plugin type`
+
+
+## Initialization mechanism
+
+Users can easily find a plugin type and initialize an empty plugin instance according to the previous registration mechanism. However, users often need an initialized plugin rather than a empty plugin. So we define the initialization mechanism in
+Plugin structure.
+
+In the initialization mechanism, the plugin category(interface) and the init config is required. Initialize processing is like the following.
+
+1. Find the plugin name in the input config.
+2. Find plugin type according to the plugin category(interface) and the plugin name.
+3. Create an empty plugin.
+4. Initialize the plugin according to the config.
+5. do some callback processing after initialized.
+
+
+## Plugin usage in Satellite
+Not only does the extension mechanism depend on Plugin, but the core modules also depend on Plugin in Satellite. We'll illustrate this with an example.
+
+- Core Module interface: module.Service
Review comment:
it seems simpler if merging defaultplugin & service design. plugin only have four layers:
plugin basic init -> lifecycle management -> specific plugin interface -> specific plugin implements
----------------------------------------------------------------
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 #8: Add main structure
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r541870930
##########
File path: internal/pkg/log/log.go
##########
@@ -0,0 +1,146 @@
+// 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 log
+
+import (
+ "fmt"
+ "os"
+ "strings"
+ "sync"
+
+ "github.com/sirupsen/logrus"
+)
+
+// Default logger config.
+const (
+ defaultLogPattern = "%time [%level][%field] - %msg"
+ defaultTimePattern = "2006-01-02 15:04:05.001"
Review comment:
Is this a pattern?
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r540222153
##########
File path: internal/satellite/module/buffer/buffer.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 buffer
+
+import (
+ "sync"
+
+ "github.com/apache/skywalking-satellite/internal/pkg/event"
+ "github.com/apache/skywalking-satellite/internal/pkg/log"
+)
+
+// BatchBuffer is a buffer to cache the input data in Sender.
+type BatchBuffer struct {
+ sync.Mutex // local
+ buf []*event.OutputEventContext // cache
+ first int64 // the first OutputEventContext offset
+ last int64 // the last OutputEventContext offset
+ size int // usage size
+ cap int // the max capacity
+}
+
+// NewBuffer crate a new BatchBuffer according to the capacity param.
+func NewBatchBuffer(capacity int) *BatchBuffer {
+ return &BatchBuffer{
+ buf: make([]*event.OutputEventContext, capacity),
+ first: 0,
+ last: 0,
+ size: 0,
+ cap: capacity,
+ }
+}
+
+// Buf returns the cached data in BatchBuffer.
+func (b *BatchBuffer) Buf() []*event.OutputEventContext {
+ b.Lock()
Review comment:
Yes, I want to say this. Is this really necessary for using lock even in the register? Who will use concurrency-registration?
----------------------------------------------------------------
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 #8: Add main structure
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r541832275
##########
File path: configs/satellite_config.yaml
##########
@@ -0,0 +1,69 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+logger:
+ log_pattern: "%time [%level][%field] - %msg"
+ time_pattern: "2006-01-02 15:04:05.001"
+ level: "info"
+
+sharing:
+ clients:
+ - plugin_name: "grpc-client"
+ k: v
+ servers:
+ - plugin_name: "grpc-server"
+ k: v
+agents:
+ - common_config:
+ namespace: namespace1
Review comment:
```suggestion
namespace:
- common_config:
name: namespace1
```
----------------------------------------------------------------
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] gxthrj commented on a change in pull request #8: Add main structure
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r541840381
##########
File path: Makefile
##########
@@ -70,3 +70,12 @@ verify: clean license lint test
.PHONY: clean
clean: tools
-rm -rf coverage.txt
+
+.PHONY: build
+build: deps windows linux darwin
+
+
+.PHONY: $(PLATFORMS)
+$(PLATFORMS):
+ mkdir -p $(OUT_DIR)
+ GOOS=$(os) GOARCH=$(ARCH) $(GO_BUILD) $(GO_BUILD_FLAGS) -ldflags "$(GO_BUILD_LDFLAGS)" -o $(OUT_DIR)/$(BINARY)-$(VERSION)-$(os)-$(ARCH) ./cmd
Review comment:
No newline at end of file.
----------------------------------------------------------------
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 #8: Add main structure
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r541828174
##########
File path: docs/design/module_design.md
##########
@@ -0,0 +1,127 @@
+# Module Design
+## Namespace
+Namespace is an isolation concept in Satellite. Each namespace has one agent to collect the APM data.
+So many agents would be running at the same time in Satellite.
Review comment:
```suggestion
The namespace is an isolation concept in Satellite. Each namespace has one pipeline to process the telemetry data(metrics/traces/logs). Two namespaces are not sharing data.
```
I think you mean this, please update the doc accordingly.
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539376098
##########
File path: plugins/collector/api/collector.go
##########
@@ -27,7 +27,7 @@ import (
// Init() Initial stage: Init plugin by config
// ||
// \/
-// Prepare() Preparing stage: Prepare the collector, such as build connection with SkyWalking javaagent.
+// Init() Preparing stage: Init the collector, such as build connection with SkyWalking javaagent.
Review comment:
it is a mistake when use goland refactor
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539749557
##########
File path: internal/pkg/plugin/registry.go
##########
@@ -18,62 +18,85 @@
package plugin
import (
+ "errors"
"fmt"
"reflect"
"sync"
)
-// 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.
-
-// reg is the global plugin registry
+// the global plugin registry
var (
Review comment:
if a specific plugin want to register itself to the registry, it must invoke the register method in its init func. That purpose the specific plugin must import the registry package. In the golang, the import package init would be first invoked. The reason why i want to use init func is it is hard to control the used plugins by manual registration.
----------------------------------------------------------------
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] gxthrj commented on a change in pull request #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539934763
##########
File path: docs/design/plugin_structure.md
##########
@@ -0,0 +1,47 @@
+# plugin structure
+`Plugin is a common concept for Satellite. Not only does the extension mechanism depend on Plugin, but the core modules also depend on Plugin`
+
+## Registration mechanism
+
+The Plugin registration mechanism in Satellite is similar to the SPI registration mechanism of Java.
+Plugin registration mechanism supports to register an interface and its implementation, that means different interfaces have different registration spaces.
+We can easily find the type of a specific plugin according to the interface and the plugin name and initialize it according to the type.
+
+structure:
+- code: map[reflect.Type]map[string]reflect.Value
Review comment:
`[]` is a markdown keyword that needs to be escaped
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539406752
##########
File path: plugins/collector/api/collector.go
##########
@@ -24,36 +24,22 @@ import (
"github.com/apache/skywalking-satellite/internal/pkg/plugin"
)
-// Init() Initial stage: Init plugin by config
-// ||
-// \/
-// Prepare() Preparing stage: Prepare the collector, such as build connection with SkyWalking javaagent.
-// ||
-// \/
-// Next() Running stage: When Collector collect a data, the data would be fetched by the upstream
-// || component through this method.
-// \/
-// Close() Closing stage: Close the Collector, such as close connection with SkyWalking javaagent.
-
// Collector is a plugin interface, that defines new collectors.
type Collector interface {
plugin.Plugin
-
- // Prepare creates a listen or reader to gather data.
- Prepare()
+ // Prepare creates a listener or reader to gather APM data, such as build connection with SkyWalking javaagent.
Review comment:
`such as build connection with SkyWalking javaagent.` still a wrong statement, you can't connect to javaagent.
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539429228
##########
File path: internal/pkg/plugin/registry.go
##########
@@ -18,62 +18,85 @@
package plugin
import (
+ "errors"
"fmt"
"reflect"
"sync"
)
-// 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.
-
-// reg is the global plugin registry
+// the global plugin registry
var (
Review comment:
A similar SPI mechanism is implemented, I don't think hard code to register plugins is a good way, Similar patterns exist in many places, such as mosn, beats, and etc... . In the original version, I also experimented with the golang dynamic link(.so mechanism), but it was difficult to use
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#issuecomment-743805675
@wu-sheng @gxthrj @gxthrj refactoring(add server concept) & polish codes & add docs please read doc first, thx
----------------------------------------------------------------
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 #8: Add main structure
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r541828732
##########
File path: docs/design/module_design.md
##########
@@ -0,0 +1,127 @@
+# Module Design
+## Namespace
+Namespace is an isolation concept in Satellite. Each namespace has one agent to collect the APM data.
+So many agents would be running at the same time in Satellite.
+```
+ Satellite
+ ---------------------------------------------------------------------
+| ------------------------------------------- |
+| | Agent | |
+| ------------------------------------------- |
+| ------------------------------------------- |
+| | Agent | |
+| ------------------------------------------- |
+| ------------------------------------------- |
+| | Agent | |
+| ------------------------------------------- |
+ ---------------------------------------------------------------------
+```
+## Modules
+There are 3 modules in one agent, which are Gatherer, Processor, and Sender.
+
+- The Gatherer module is responsible for fetching or receiving data and pushing the data to Queue. So there are 2 kinds of Gatherer, which are ReceiverGatherer and FetcherGatherer.
+- The Processor module is responsible for reading data from the queue and processing data by a series of filter chains.
+- The Sender module is responsible for async processing and forwarding the data to the external services in the batch mode. After sending success, Sender would also acknowledge the offset of Queue in Gatherer.
+
+```
+ Agent
+ --------------------------------------------------------------------
+| ---------- ----------- -------- |
+| | Gatherer | => | Processor | => | Sender | |
+| ---------- ----------- -------- |
+ --------------------------------------------------------------------
+```
+
+## Plugins
+
+Plugin is the minimal components in the module. There are 2 kinds of plugins in Satellite, which are sharing plugins and normal plugins.
+
+- a sharing plugin instance could be sharing with multiple modules in the different namespaces.
+- a normal plugin instance is only be used in a fixed module of the fixed namespaces.
+
+### Sharing plugin
+Nowadays, there are 2 sharing plugins in Satellite, which are server plugins and client plugins. The reason why they are sharing plugins is to reduce the resource cost in connection. Server plugins are sharing with the ReceiverGatherer modules in the different namespaces to receive the external requests. And the client plugins is sharing with the Sender modules in the different namespaces to connect with external services, such as Kafka and OAP.
Review comment:
I think this is not `2 sharding plugins`. You mean there are 2 kinds of sharding plugins?
----------------------------------------------------------------
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 #8: enhance the plugin mechanism
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8:
URL: https://github.com/apache/skywalking-satellite/pull/8#discussion_r539769232
##########
File path: internal/pkg/plugin/registry.go
##########
@@ -18,62 +18,85 @@
package plugin
import (
+ "errors"
"fmt"
"reflect"
"sync"
)
-// 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.
-
-// reg is the global plugin registry
+// the global plugin registry
var (
Review comment:
Please edit this comment to show your preference. @apache/skywalking-committers, especially reviewers, @surlymo @hanahmily @kezhenxu94 @dmsolr @zhangkewei @gxthrj @nic-chen @moonming
- +1 for Auto-registration.
- +1 for manual registration in go source codes. @wu-sheng @nic-chen
----------------------------------------------------------------
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