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/28 05:24:09 UTC
[GitHub] [skywalking-satellite] EvanLjp opened a new pull request #11: add prometheus server and polish api
EvanLjp opened a new pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11
add prometheus telemetry
----------------------------------------------------------------
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 #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549628395
##########
File path: configs/satellite_config.yaml
##########
@@ -17,23 +17,30 @@
logger:
log_pattern: "%time [%level][%field] - %msg"
- time_pattern: "2006-01-02 15:04:05.001"
+ time_pattern: "2006-01-02 15:04:05.000"
level: "info"
+telemetry:
+ namespace: namespaces
+ service: service1
+ instance: instance1
+
sharing:
+ common_config:
+ pipe_name: sharing
clients:
- plugin_name: "grpc-client"
k: v
servers:
- plugin_name: "grpc-server"
k: v
-namespaces:
+pipes:
Review comment:
Do you update the doc? I think the `namespace` has been mentioned there.
----------------------------------------------------------------
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 #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#issuecomment-751918222
> In this PR, he's using a third-party plugin instead of this bash to install protoc
I think 3rd party github action has been blocked by INFRA. @hanahmily
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-satellite] wu-sheng merged pull request #11: Satellite Self-Observability
Posted by GitBox <gi...@apache.org>.
wu-sheng merged pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11
----------------------------------------------------------------
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 #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549631324
##########
File path: configs/satellite_config.yaml
##########
@@ -17,23 +17,30 @@
logger:
log_pattern: "%time [%level][%field] - %msg"
- time_pattern: "2006-01-02 15:04:05.001"
+ time_pattern: "2006-01-02 15:04:05.000"
level: "info"
+telemetry:
+ namespace: namespaces
+ service: service1
+ instance: instance1
+
sharing:
+ common_config:
+ pipe_name: sharing
clients:
- plugin_name: "grpc-client"
k: v
servers:
- plugin_name: "grpc-server"
k: v
-namespaces:
+pipes:
Review comment:
sorry, 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] EvanLjp removed a comment on pull request #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
EvanLjp removed a comment on pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#issuecomment-752012785
@gxthrj @nic-chen @kezhenxu94 how about to use init() in logger? can't use init() in Logger brings up a lot of chore things.
----------------------------------------------------------------
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 #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549384425
##########
File path: tools/install_protoc.sh
##########
@@ -0,0 +1,32 @@
+#!/usr/bin/env bash
+
+#
+# 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.
+#
+
+
+if [[ "$(uname)"=="Darwin" || "$(expr substr $(uname -s) 1 10)"=="MINGW32_NT" ]];then
Review comment:
I think he is setting CI? Which only include Ubuntu for now? We should set up the MacOS for CI testing too.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-satellite] wu-sheng commented on a change in pull request #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549628139
##########
File path: plugins/server/prometheus/Prometheus_test.go
##########
@@ -0,0 +1,66 @@
+// 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 prometheus
+
+import (
+ "fmt"
+ "net/http"
+ "reflect"
+ "testing"
+ "time"
+
+ "github.com/apache/skywalking-satellite/internal/pkg/log"
+ "github.com/apache/skywalking-satellite/internal/pkg/plugin"
+ "github.com/apache/skywalking-satellite/internal/satellite/telemetry"
+ "github.com/apache/skywalking-satellite/plugins/server/api"
+)
+
+func initPrometheusServer(cfg plugin.Config) (*Server, error) {
+ log.Init(&log.LoggerConfig{})
+ telemetry.Init(new(telemetry.Config))
+ plugin.RegisterPluginCategory(reflect.TypeOf((*api.Server)(nil)).Elem())
+ plugin.RegisterPlugin(new(Server))
+ cfg[plugin.NameField] = "prometheus-server"
+ q := api.GetServer(cfg)
+ if q == nil {
+ return nil, fmt.Errorf("cannot get a default config mmap queue from the registry")
+ }
+ if err := q.Prepare(); err != nil {
+ return nil, fmt.Errorf("queue cannot initialize: %v", err)
+ }
+ return q.(*Server), nil
+}
+
+func TestServer_Start(t *testing.T) {
+ server, err := initPrometheusServer(make(plugin.Config))
+ if err != nil {
+ t.Fatalf("cannot init the prometheus server: %v", err)
+ }
+ _ = server.Start()
+ time.Sleep(time.Second)
+ response, err := http.Get("http://127.0.0.1" + server.Address + server.Endpoint)
Review comment:
Got 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] kezhenxu94 commented on a change in pull request #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549366373
##########
File path: internal/pkg/plugin/registry.go
##########
@@ -100,4 +102,16 @@ func initializing(plugin Plugin, cfg Config) {
if err := v.Unmarshal(plugin); err != nil {
panic(fmt.Errorf("cannot inject the config to the %s plugin, the error is %v", plugin.Name(), err))
}
+ cf := reflect.ValueOf(plugin).Elem().FieldByName(config.CommonFieldsName)
+ if !cf.IsValid() {
+ panic("%s plugin must have a field named CommonField.")
Review comment:
```suggestion
panic(fmt.Errorf("%s plugin must have a field named CommonField.", plugin.Name()))
```
##########
File path: internal/pkg/plugin/registry.go
##########
@@ -47,12 +49,12 @@ func RegisterPlugin(plugin Plugin) {
for pCategory, pReg := range reg {
if v.Type().Implements(pCategory) {
pReg[plugin.Name()] = v
- fmt.Printf("register %s %s successfully ", plugin.Name(), v.Type().String())
+ fmt.Printf("register %s %s successfully\n", plugin.Name(), v.Type().String())
Review comment:
I didn't notice this before, but `fmt.Printf` doesn't allow users to specify verbosity of logs, many other places are using `fmt.Printf` too
##########
File path: internal/pkg/plugin/registry.go
##########
@@ -100,4 +102,16 @@ func initializing(plugin Plugin, cfg Config) {
if err := v.Unmarshal(plugin); err != nil {
panic(fmt.Errorf("cannot inject the config to the %s plugin, the error is %v", plugin.Name(), err))
}
+ cf := reflect.ValueOf(plugin).Elem().FieldByName(config.CommonFieldsName)
+ if !cf.IsValid() {
+ panic("%s plugin must have a field named CommonField.")
+ }
+ for i := 0; i < cf.NumField(); i++ {
+ tagVal := cf.Type().Field(i).Tag.Get("mapstructure")
Review comment:
Use constant `TagName`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-satellite] kezhenxu94 commented on a change in pull request #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549518833
##########
File path: .github/workflows/build-and-test.yaml
##########
@@ -36,12 +36,6 @@ jobs:
with:
go-version: ${{ matrix.go-version }}
- - name: Install Protoc
- uses: arduino/setup-protoc@v1
Review comment:
> @kezhenxu94 Could we use a third part plugin now?
This kind of actions are blocked, unless we get enough reasons to persuade the infra team to unblock 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] wu-sheng commented on a change in pull request #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549342997
##########
File path: configs/satellite_config.yaml
##########
@@ -17,23 +17,30 @@
logger:
log_pattern: "%time [%level][%field] - %msg"
- time_pattern: "2006-01-02 15:04:05.001"
+ time_pattern: "2006-01-02 15:04:05.000"
level: "info"
+telemetry:
+ namespace: namespaces
+ service: service1
+ instance: instance1
+
sharing:
+ common_config:
+ pipe_name: sharing
clients:
- plugin_name: "grpc-client"
k: v
servers:
- plugin_name: "grpc-server"
k: v
-namespaces:
+pipes:
Review comment:
What does `pipes` mean?
----------------------------------------------------------------
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 #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549351835
##########
File path: plugins/server/prometheus/Prometheus_test.go
##########
@@ -0,0 +1,66 @@
+// 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 prometheus
+
+import (
+ "fmt"
+ "net/http"
+ "reflect"
+ "testing"
+ "time"
+
+ "github.com/apache/skywalking-satellite/internal/pkg/log"
+ "github.com/apache/skywalking-satellite/internal/pkg/plugin"
+ "github.com/apache/skywalking-satellite/internal/satellite/telemetry"
+ "github.com/apache/skywalking-satellite/plugins/server/api"
+)
+
+func initPrometheusServer(cfg plugin.Config) (*Server, error) {
+ log.Init(&log.LoggerConfig{})
+ telemetry.Init(new(telemetry.Config))
+ plugin.RegisterPluginCategory(reflect.TypeOf((*api.Server)(nil)).Elem())
+ plugin.RegisterPlugin(new(Server))
+ cfg[plugin.NameField] = "prometheus-server"
+ q := api.GetServer(cfg)
+ if q == nil {
+ return nil, fmt.Errorf("cannot get a default config mmap queue from the registry")
+ }
+ if err := q.Prepare(); err != nil {
+ return nil, fmt.Errorf("queue cannot initialize: %v", err)
+ }
+ return q.(*Server), nil
+}
+
+func TestServer_Start(t *testing.T) {
+ server, err := initPrometheusServer(make(plugin.Config))
+ if err != nil {
+ t.Fatalf("cannot init the prometheus server: %v", err)
+ }
+ _ = server.Start()
+ time.Sleep(time.Second)
+ response, err := http.Get("http://127.0.0.1" + server.Address + server.Endpoint)
Review comment:
You have only tested the Prometheus endpoint is open, right?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-satellite] gxthrj commented on a change in pull request #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549653711
##########
File path: internal/satellite/module/gatherer/receiver_gatherer.go
##########
@@ -41,51 +44,75 @@ type ReceiverGatherer struct {
// self components
outputChannel chan *queue.SequenceEvent
+ // metrics
+ receiveCounter *prometheus.CounterVec
+ queueOutputCounter *prometheus.CounterVec
}
func (r *ReceiverGatherer) Prepare() error {
- log.Logger.Infof("receiver gatherer module of %s namespace is preparing", r.config.NamespaceName)
- r.runningReceiver.RegisterHandler(r.runningServer)
+ log.Logger.Infof("receiver gatherer module of %s namespace is preparing", r.config.PipeName)
+ r.runningReceiver.RegisterHandler(r.runningServer.GetServer())
if err := r.runningQueue.Initialize(); err != nil {
- log.Logger.Infof("the %s queue of %s namespace was failed to initialize", r.runningQueue.Name(), r.config.NamespaceName)
+ log.Logger.Infof("the %s queue of %s namespace was failed to initialize", r.runningQueue.Name(), r.config.PipeName)
return err
}
+ r.receiveCounter = prometheus.NewCounterVec(prometheus.CounterOpts{
Review comment:
> the feature is to monitor self status
OK, This is the metric for satellite self.
I mistakenly thought that this is an extension of Prometheus Gather.
----------------------------------------------------------------
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 #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549676680
##########
File path: internal/satellite/module/gatherer/receiver_gatherer.go
##########
@@ -41,51 +44,75 @@ type ReceiverGatherer struct {
// self components
outputChannel chan *queue.SequenceEvent
+ // metrics
+ receiveCounter *prometheus.CounterVec
+ queueOutputCounter *prometheus.CounterVec
}
func (r *ReceiverGatherer) Prepare() error {
- log.Logger.Infof("receiver gatherer module of %s namespace is preparing", r.config.NamespaceName)
- r.runningReceiver.RegisterHandler(r.runningServer)
+ log.Logger.Infof("receiver gatherer module of %s namespace is preparing", r.config.PipeName)
+ r.runningReceiver.RegisterHandler(r.runningServer.GetServer())
if err := r.runningQueue.Initialize(); err != nil {
- log.Logger.Infof("the %s queue of %s namespace was failed to initialize", r.runningQueue.Name(), r.config.NamespaceName)
+ log.Logger.Infof("the %s queue of %s namespace was failed to initialize", r.runningQueue.Name(), r.config.PipeName)
return err
}
+ r.receiveCounter = prometheus.NewCounterVec(prometheus.CounterOpts{
Review comment:
@EvanLjp I think you should rename this PR to `Satellite Self-Observability` :) I used to think this is for scratch from prometheus endpoint.
----------------------------------------------------------------
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 #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549632698
##########
File path: plugins/server/prometheus/Prometheus_test.go
##########
@@ -0,0 +1,66 @@
+// 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 prometheus
+
+import (
+ "fmt"
+ "net/http"
+ "reflect"
+ "testing"
+ "time"
+
+ "github.com/apache/skywalking-satellite/internal/pkg/log"
+ "github.com/apache/skywalking-satellite/internal/pkg/plugin"
+ "github.com/apache/skywalking-satellite/internal/satellite/telemetry"
+ "github.com/apache/skywalking-satellite/plugins/server/api"
+)
+
+func initPrometheusServer(cfg plugin.Config) (*Server, error) {
Review comment:
could set custom values
----------------------------------------------------------------
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 #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549590767
##########
File path: internal/satellite/module/gatherer/receiver_gatherer.go
##########
@@ -41,51 +44,75 @@ type ReceiverGatherer struct {
// self components
outputChannel chan *queue.SequenceEvent
+ // metrics
+ receiveCounter *prometheus.CounterVec
+ queueOutputCounter *prometheus.CounterVec
}
func (r *ReceiverGatherer) Prepare() error {
- log.Logger.Infof("receiver gatherer module of %s namespace is preparing", r.config.NamespaceName)
- r.runningReceiver.RegisterHandler(r.runningServer)
+ log.Logger.Infof("receiver gatherer module of %s namespace is preparing", r.config.PipeName)
+ r.runningReceiver.RegisterHandler(r.runningServer.GetServer())
if err := r.runningQueue.Initialize(); err != nil {
- log.Logger.Infof("the %s queue of %s namespace was failed to initialize", r.runningQueue.Name(), r.config.NamespaceName)
+ log.Logger.Infof("the %s queue of %s namespace was failed to initialize", r.runningQueue.Name(), r.config.PipeName)
return err
}
+ r.receiveCounter = prometheus.NewCounterVec(prometheus.CounterOpts{
Review comment:
I am not very clear about this. Should the metrics here be defined in plugins ? Or we supports prometheus in `Gather` by default?
----------------------------------------------------------------
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 #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549622400
##########
File path: internal/pkg/plugin/definition.go
##########
@@ -33,12 +41,24 @@ type SharingPlugin interface {
// Prepare the sharing plugins, such as build the connection with the external services.
Prepare() error
+ // Start a server to receive the input APM data.
+ Start() error
// Close the sharing plugin.
Close() error
}
// Config is used to initialize the DefaultInitializingPlugin.
type Config map[string]interface{}
-// NameField is a required field in Config.
-const NameField = "plugin_name"
+// GetPluginName returns a plugin name generated by the packages.
+func GetPluginName(p Plugin) string {
Review comment:
Use the directory structure to ensure that the plug-in name is unique
----------------------------------------------------------------
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 #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549621857
##########
File path: configs/satellite_config.yaml
##########
@@ -17,23 +17,30 @@
logger:
log_pattern: "%time [%level][%field] - %msg"
- time_pattern: "2006-01-02 15:04:05.001"
+ time_pattern: "2006-01-02 15:04:05.000"
level: "info"
+telemetry:
+ namespace: namespaces
+ service: service1
+ instance: instance1
+
sharing:
+ common_config:
+ pipe_name: sharing
clients:
- plugin_name: "grpc-client"
k: v
servers:
- plugin_name: "grpc-server"
k: v
-namespaces:
+pipes:
Review comment:
the namespace concept is similar to the Kubernetes namespaces, so I change the original namespaces concept to the new name call pipe. Each pipe has one gatherer, one processor, and one sender.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-satellite] kezhenxu94 commented on a change in pull request #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549519221
##########
File path: tools/install_protoc.sh
##########
@@ -0,0 +1,32 @@
+#!/usr/bin/env bash
+
+#
+# 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.
+#
+
+
+if [[ "$(uname)"=="Darwin" || "$(expr substr $(uname -s) 1 10)"=="MINGW32_NT" ]];then
Review comment:
> I think he is setting CI? Which only include Ubuntu for now? We should set up the MacOS for CI testing too.
Locally, when you run `make deps` to setup the dev env, this is executed and if you didn't install protoc correctly, it fails.
----------------------------------------------------------------
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 #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549633934
##########
File path: .github/workflows/build-and-test.yaml
##########
@@ -36,12 +36,6 @@ jobs:
with:
go-version: ${{ matrix.go-version }}
- - name: Install Protoc
- uses: arduino/setup-protoc@v1
Review comment:
@hanahmily i use shell to install protoc
----------------------------------------------------------------
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 #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549339002
##########
File path: dist/licenses/LICENSE-prometheus-client-golang.txt
##########
@@ -0,0 +1,201 @@
+ Apache License
Review comment:
Apache 2.0 license is not required to be copied. If NOTICE exists, you need to copy the content into `dist/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] hanahmily commented on a change in pull request #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
hanahmily commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549503637
##########
File path: plugins/server/prometheus/Prometheus_test.go
##########
@@ -0,0 +1,66 @@
+// 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 prometheus
+
+import (
+ "fmt"
+ "net/http"
+ "reflect"
+ "testing"
+ "time"
+
+ "github.com/apache/skywalking-satellite/internal/pkg/log"
+ "github.com/apache/skywalking-satellite/internal/pkg/plugin"
+ "github.com/apache/skywalking-satellite/internal/satellite/telemetry"
+ "github.com/apache/skywalking-satellite/plugins/server/api"
+)
+
+func initPrometheusServer(cfg plugin.Config) (*Server, error) {
Review comment:
Could you use different endpoints and ports instead of defaults.
##########
File path: plugins/server/prometheus/Prometheus.go
##########
@@ -0,0 +1,83 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
Review comment:
Why the file name is a capical letter?
##########
File path: internal/satellite/telemetry/telemetry.go
##########
@@ -0,0 +1,53 @@
+// 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 telemetry
+
+import (
+ "github.com/prometheus/client_golang/prometheus"
+)
+
+// Registerer is the global metrics center for collecting the telemetry data in core modules or plugins.
+var (
+ registry *prometheus.Registry
+ Registerer prometheus.Registerer // The register is for adding metrics to the registry.
+ Gatherer prometheus.Gatherer // The gatherer is for fetching metrics from the registry.
+)
+
+// Config defines the common telemetry labels.
+type Config struct {
+ Service string `mapstructure:"service"` // The service name.
+ Namespace string `mapstructure:"namespace"` // The kubernetes namespace.
+ Pod string `mapstructure:"pod"` // The kubernetes pod name.
Review comment:
IIRC, we should not introduce Kubernetes notions into satellite since it's designed to deployed on VM as well.
How about using more general and SkyWalking-style concepts, for example, `Cluster`, `Service` and `Instance`
##########
File path: .github/workflows/build-and-test.yaml
##########
@@ -36,12 +36,6 @@ jobs:
with:
go-version: ${{ matrix.go-version }}
- - name: Install Protoc
- uses: arduino/setup-protoc@v1
Review comment:
@kezhenxu94 Could we use a third part plugin now?
##########
File path: tools/install_protoc.sh
##########
@@ -0,0 +1,32 @@
+#!/usr/bin/env bash
+
+#
+# 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.
+#
+
+
+if [[ "$(uname)"=="Darwin" || "$(expr substr $(uname -s) 1 10)"=="MINGW32_NT" ]];then
Review comment:
In this PR, he's using a third-party plugin instead of this bash to install protoc
----------------------------------------------------------------
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 #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
surlymo commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549363740
##########
File path: internal/pkg/plugin/definition.go
##########
@@ -33,12 +41,24 @@ type SharingPlugin interface {
// Prepare the sharing plugins, such as build the connection with the external services.
Prepare() error
+ // Start a server to receive the input APM data.
+ Start() error
// Close the sharing plugin.
Close() error
}
// Config is used to initialize the DefaultInitializingPlugin.
type Config map[string]interface{}
-// NameField is a required field in Config.
-const NameField = "plugin_name"
+// GetPluginName returns a plugin name generated by the packages.
+func GetPluginName(p Plugin) string {
+ t := reflect.TypeOf(p)
+ if t.Kind() == reflect.Ptr {
+ t = t.Elem()
+ }
+ arr := strings.Split(t.PkgPath(), "/")
+ if len(arr) < 2 {
Review comment:
magic num
##########
File path: internal/pkg/plugin/definition.go
##########
@@ -33,12 +41,24 @@ type SharingPlugin interface {
// Prepare the sharing plugins, such as build the connection with the external services.
Prepare() error
+ // Start a server to receive the input APM data.
+ Start() error
// Close the sharing plugin.
Close() error
}
// Config is used to initialize the DefaultInitializingPlugin.
type Config map[string]interface{}
-// NameField is a required field in Config.
-const NameField = "plugin_name"
+// GetPluginName returns a plugin name generated by the packages.
+func GetPluginName(p Plugin) string {
Review comment:
why not just return the string name?
##########
File path: plugins/fallbacker/timer/timer_fallbacker.go
##########
@@ -20,13 +20,15 @@ package timer
import (
"time"
- "github.com/apache/skywalking-satellite/internal/pkg/event"
+ "github.com/apache/skywalking-satellite/internal/pkg/config"
+ "github.com/apache/skywalking-satellite/internal/satellite/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 {
+ config.CommonFields
Review comment:
why all need this struct
----------------------------------------------------------------
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 #11: Satellite Self-Observability
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r550174195
##########
File path: tools/install_protoc.sh
##########
@@ -0,0 +1,32 @@
+#!/usr/bin/env bash
+
+#
+# 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.
+#
+
+
+if [[ "$(uname)"=="Darwin" || "$(expr substr $(uname -s) 1 10)"=="MINGW32_NT" ]];then
Review comment:
I tried, I have to install the protoc manually.
----------------------------------------------------------------
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 #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549341248
##########
File path: dist/LICENSE
##########
@@ -214,7 +214,7 @@ Apache 2.0 licenses
The following components are provided under the Apache License. See project link for details.
The text of each license is the standard Apache 2.0 license.
Review comment:
Note this sentence.
----------------------------------------------------------------
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 #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549623481
##########
File path: internal/satellite/telemetry/telemetry.go
##########
@@ -0,0 +1,53 @@
+// 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 telemetry
+
+import (
+ "github.com/prometheus/client_golang/prometheus"
+)
+
+// Registerer is the global metrics center for collecting the telemetry data in core modules or plugins.
+var (
+ registry *prometheus.Registry
+ Registerer prometheus.Registerer // The register is for adding metrics to the registry.
+ Gatherer prometheus.Gatherer // The gatherer is for fetching metrics from the registry.
+)
+
+// Config defines the common telemetry labels.
+type Config struct {
+ Service string `mapstructure:"service"` // The service name.
+ Namespace string `mapstructure:"namespace"` // The kubernetes namespace.
+ Pod string `mapstructure:"pod"` // The kubernetes pod name.
Review comment:
no problem
----------------------------------------------------------------
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 #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549623048
##########
File path: plugins/fallbacker/timer/timer_fallbacker.go
##########
@@ -20,13 +20,15 @@ package timer
import (
"time"
- "github.com/apache/skywalking-satellite/internal/pkg/event"
+ "github.com/apache/skywalking-satellite/internal/pkg/config"
+ "github.com/apache/skywalking-satellite/internal/satellite/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 {
+ config.CommonFields
Review comment:
the common field is inject by boot process.
for example: a plugins want to know which pipe belong to, the commonField is for this.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-satellite] EvanLjp commented on pull request #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#issuecomment-752012785
@gxthrj @nic-chen @kezhenxu94 how about to use init() in logger? can't use init() in Logger brings up a lot of chore things.
----------------------------------------------------------------
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 #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549621933
##########
File path: plugins/server/prometheus/Prometheus_test.go
##########
@@ -0,0 +1,66 @@
+// 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 prometheus
+
+import (
+ "fmt"
+ "net/http"
+ "reflect"
+ "testing"
+ "time"
+
+ "github.com/apache/skywalking-satellite/internal/pkg/log"
+ "github.com/apache/skywalking-satellite/internal/pkg/plugin"
+ "github.com/apache/skywalking-satellite/internal/satellite/telemetry"
+ "github.com/apache/skywalking-satellite/plugins/server/api"
+)
+
+func initPrometheusServer(cfg plugin.Config) (*Server, error) {
+ log.Init(&log.LoggerConfig{})
+ telemetry.Init(new(telemetry.Config))
+ plugin.RegisterPluginCategory(reflect.TypeOf((*api.Server)(nil)).Elem())
+ plugin.RegisterPlugin(new(Server))
+ cfg[plugin.NameField] = "prometheus-server"
+ q := api.GetServer(cfg)
+ if q == nil {
+ return nil, fmt.Errorf("cannot get a default config mmap queue from the registry")
+ }
+ if err := q.Prepare(); err != nil {
+ return nil, fmt.Errorf("queue cannot initialize: %v", err)
+ }
+ return q.(*Server), nil
+}
+
+func TestServer_Start(t *testing.T) {
+ server, err := initPrometheusServer(make(plugin.Config))
+ if err != nil {
+ t.Fatalf("cannot init the prometheus server: %v", err)
+ }
+ _ = server.Start()
+ time.Sleep(time.Second)
+ response, err := http.Get("http://127.0.0.1" + server.Address + server.Endpoint)
Review comment:
yes
----------------------------------------------------------------
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 #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549624469
##########
File path: internal/satellite/module/gatherer/receiver_gatherer.go
##########
@@ -41,51 +44,75 @@ type ReceiverGatherer struct {
// self components
outputChannel chan *queue.SequenceEvent
+ // metrics
+ receiveCounter *prometheus.CounterVec
+ queueOutputCounter *prometheus.CounterVec
}
func (r *ReceiverGatherer) Prepare() error {
- log.Logger.Infof("receiver gatherer module of %s namespace is preparing", r.config.NamespaceName)
- r.runningReceiver.RegisterHandler(r.runningServer)
+ log.Logger.Infof("receiver gatherer module of %s namespace is preparing", r.config.PipeName)
+ r.runningReceiver.RegisterHandler(r.runningServer.GetServer())
if err := r.runningQueue.Initialize(); err != nil {
- log.Logger.Infof("the %s queue of %s namespace was failed to initialize", r.runningQueue.Name(), r.config.NamespaceName)
+ log.Logger.Infof("the %s queue of %s namespace was failed to initialize", r.runningQueue.Name(), r.config.PipeName)
return err
}
+ r.receiveCounter = prometheus.NewCounterVec(prometheus.CounterOpts{
Review comment:
the feature is to monitor self status. Could you explain that again? I'm not very clear
----------------------------------------------------------------
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 #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
EvanLjp commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549633128
##########
File path: plugins/server/prometheus/Prometheus_test.go
##########
@@ -0,0 +1,66 @@
+// 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 prometheus
+
+import (
+ "fmt"
+ "net/http"
+ "reflect"
+ "testing"
+ "time"
+
+ "github.com/apache/skywalking-satellite/internal/pkg/log"
+ "github.com/apache/skywalking-satellite/internal/pkg/plugin"
+ "github.com/apache/skywalking-satellite/internal/satellite/telemetry"
+ "github.com/apache/skywalking-satellite/plugins/server/api"
+)
+
+func initPrometheusServer(cfg plugin.Config) (*Server, error) {
Review comment:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-satellite] kezhenxu94 commented on a change in pull request #11: add prometheus server
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #11:
URL: https://github.com/apache/skywalking-satellite/pull/11#discussion_r549375978
##########
File path: tools/install_protoc.sh
##########
@@ -0,0 +1,32 @@
+#!/usr/bin/env bash
+
+#
+# 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.
+#
+
+
+if [[ "$(uname)"=="Darwin" || "$(expr substr $(uname -s) 1 10)"=="MINGW32_NT" ]];then
Review comment:
I saw there is a binary for MacOS too, why not support MacOS here as well?
----------------------------------------------------------------
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