[GitHub] [skywalking-satellite] kezhenxu94 commented on a change in pull request #11: add prometheus server

[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