From c213cdd6a287df9dca8163a025989872142b994d Mon Sep 17 00:00:00 2001 From: "923048992@qq.com" <923048992@qq.com> Date: Thu, 11 Jul 2024 18:55:57 +0800 Subject: [PATCH 1/3] add bpf log level getter Signed-off-by: 923048992@qq.com <923048992@qq.com> --- daemon/helper/helper.go | 35 ++++++++++++++++++++ daemon/manager/log/log.go | 50 ++++++++-------------------- pkg/status/status_server.go | 65 ++++++++++++++++++++++++++++++++----- 3 files changed, 106 insertions(+), 44 deletions(-) create mode 100644 daemon/helper/helper.go diff --git a/daemon/helper/helper.go b/daemon/helper/helper.go new file mode 100644 index 000000000..f2fecc354 --- /dev/null +++ b/daemon/helper/helper.go @@ -0,0 +1,35 @@ +package helper + +import ( + "encoding/json" + "fmt" + "io" + "net/http" +) + +func GetJson(url string, val any) { + resp, err := http.Get(url) + if err != nil { + fmt.Printf("Error making GET request: %v\n", err) + return + } + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + if err != nil { + fmt.Printf("Error reading response body: %v\n", err) + return + } + + if resp.StatusCode != http.StatusOK { + fmt.Printf("Error: received status code %d\n", resp.StatusCode) + fmt.Printf("Response body: %s\n", body) + return + } + + err = json.Unmarshal(body, val) + if err != nil { + fmt.Printf("Error unmarshaling response body: %v\n", err) + return + } +} diff --git a/daemon/manager/log/log.go b/daemon/manager/log/log.go index 0e28d8d43..73cbf6e48 100644 --- a/daemon/manager/log/log.go +++ b/daemon/manager/log/log.go @@ -27,7 +27,7 @@ import ( "github.com/spf13/cobra" - "kmesh.net/kmesh/pkg/logger" + "kmesh.net/kmesh/daemon/helper" "kmesh.net/kmesh/pkg/status" ) @@ -49,47 +49,25 @@ func NewCmd() *cobra.Command { return cmd } +func GetLoggerNames() { + url := status.GetLoggerURL() + var loggerNames []string + helper.GetJson(url, &loggerNames) + fmt.Printf("Existing Loggers:\n") + for _, logger := range loggerNames { + fmt.Printf("\t%s\n", logger) + } +} + func GetLoggerLevel(args []string) { - if len(args) != 1 { - names := logger.GetLoggerNames() - if len(names) > 0 { - fmt.Println("Existing loggers:") - for _, name := range names { - fmt.Println(name) - } - } else { - fmt.Println("No existing loggers.") - } + if len(args) == 0 { + GetLoggerNames() return } loggerName := args[0] url := status.GetLoggerURL() + "?name=" + loggerName - - resp, err := http.Get(url) - if err != nil { - fmt.Printf("Error making GET request: %v\n", err) - return - } - defer resp.Body.Close() - - body, err := io.ReadAll(resp.Body) - if err != nil { - fmt.Printf("Error reading response body: %v\n", err) - return - } - - if resp.StatusCode != http.StatusOK { - fmt.Printf("Error: received status code %d\n", resp.StatusCode) - fmt.Printf("Response body: %s\n", body) - return - } - var loggerInfo status.LoggerInfo - err = json.Unmarshal(body, &loggerInfo) - if err != nil { - fmt.Printf("Error unmarshaling response body: %v\n", err) - return - } + helper.GetJson(url, &loggerInfo) fmt.Printf("Logger Name: %s\n", loggerInfo.Name) fmt.Printf("Logger Level: %s\n", loggerInfo.Level) diff --git a/pkg/status/status_server.go b/pkg/status/status_server.go index ffebe6b54..77938409b 100644 --- a/pkg/status/status_server.go +++ b/pkg/status/status_server.go @@ -52,7 +52,6 @@ const ( patternConfigDumpWorkload = configDumpPrefix + "/workload" patternReadyProbe = "/debug/ready" patternLoggers = "/debug/loggers" - patternBpfLogLevel = "/debug/bpfLogLevel/" bpfLoggerName = "bpf" @@ -168,17 +167,40 @@ func (s *Server) loggersHandler(w http.ResponseWriter, r *http.Request) { } } +func (s *Server) getLoggerNames(w http.ResponseWriter, r *http.Request) { + loggerNames := append(logger.GetLoggerNames(), bpfLoggerName) + data, err := json.MarshalIndent(&loggerNames, "", " ") + if err != nil { + log.Errorf("Failed to marshal logger names: %v", err) + w.WriteHeader(http.StatusInternalServerError) + return + } + _, _ = w.Write(data) +} + func (s *Server) getLoggerLevel(w http.ResponseWriter, r *http.Request) { loggerName := r.URL.Query().Get("name") - loggerLevel, err := logger.GetLoggerLevel(loggerName) - if err != nil { - w.WriteHeader(http.StatusBadRequest) - fmt.Fprintf(w, "\t%v\n", err) + if loggerName == "" { + s.getLoggerNames(w, r) return } - loggerInfo := LoggerInfo{ - Name: loggerName, - Level: loggerLevel.String(), + var loggerInfo *LoggerInfo + if loggerName != bpfLoggerName { + loggerLevel, err := logger.GetLoggerLevel(loggerName) + if err != nil { + w.WriteHeader(http.StatusBadRequest) + fmt.Fprintf(w, "\t%v\n", err) + return + } + loggerInfo = &LoggerInfo{ + Name: loggerName, + Level: loggerLevel.String(), + } + } else { + loggerInfo = s.getBpfLogLevel(w) + if loggerInfo == nil { + return + } } data, err := json.MarshalIndent(&loggerInfo, "", " ") if err != nil { @@ -288,6 +310,33 @@ func (s *Server) readyProbe(w http.ResponseWriter, r *http.Request) { _, _ = w.Write([]byte("OK")) } +func (s *Server) getBpfLogLevel(w http.ResponseWriter) *LoggerInfo { + key := uint32(0) + value := uint32(0) + if err := s.bpfLogLevelMap.Lookup(&key, &value); err != nil { + http.Error(w, fmt.Sprintf("get log level error: %v", err), http.StatusBadRequest) + return nil + } + + logLevelMap := map[int]string{ + constants.BPF_LOG_ERR: "error", + constants.BPF_LOG_WARN: "warn", + constants.BPF_LOG_INFO: "info", + constants.BPF_LOG_DEBUG: "debug", + } + + loggerLevel, exists := logLevelMap[int(value)] + if !exists { + http.Error(w, fmt.Sprintf("unexpected invalid log level: %d", value), http.StatusInternalServerError) + return nil + } + + return &LoggerInfo{ + Name: bpfLoggerName, + Level: loggerLevel, + } +} + func (s *Server) setBpfLogLevel(w http.ResponseWriter, levelStr string) { level, err := strconv.Atoi(levelStr) if err != nil { From 3add8f48c73cd13402932cc16b29d9f2f0c53e4a Mon Sep 17 00:00:00 2001 From: "923048992@qq.com" <923048992@qq.com> Date: Thu, 11 Jul 2024 20:01:08 +0800 Subject: [PATCH 2/3] optimize logic and add some more uts Signed-off-by: 923048992@qq.com <923048992@qq.com> --- daemon/helper/helper.go | 35 -------------- daemon/manager/log/log.go | 33 +++++++++++-- pkg/status/status_server.go | 16 +++---- pkg/status/status_server_test.go | 82 ++++++++++++++++++-------------- 4 files changed, 83 insertions(+), 83 deletions(-) delete mode 100644 daemon/helper/helper.go diff --git a/daemon/helper/helper.go b/daemon/helper/helper.go deleted file mode 100644 index f2fecc354..000000000 --- a/daemon/helper/helper.go +++ /dev/null @@ -1,35 +0,0 @@ -package helper - -import ( - "encoding/json" - "fmt" - "io" - "net/http" -) - -func GetJson(url string, val any) { - resp, err := http.Get(url) - if err != nil { - fmt.Printf("Error making GET request: %v\n", err) - return - } - defer resp.Body.Close() - - body, err := io.ReadAll(resp.Body) - if err != nil { - fmt.Printf("Error reading response body: %v\n", err) - return - } - - if resp.StatusCode != http.StatusOK { - fmt.Printf("Error: received status code %d\n", resp.StatusCode) - fmt.Printf("Response body: %s\n", body) - return - } - - err = json.Unmarshal(body, val) - if err != nil { - fmt.Printf("Error unmarshaling response body: %v\n", err) - return - } -} diff --git a/daemon/manager/log/log.go b/daemon/manager/log/log.go index 73cbf6e48..51e928b86 100644 --- a/daemon/manager/log/log.go +++ b/daemon/manager/log/log.go @@ -26,8 +26,6 @@ import ( "strings" "github.com/spf13/cobra" - - "kmesh.net/kmesh/daemon/helper" "kmesh.net/kmesh/pkg/status" ) @@ -49,10 +47,37 @@ func NewCmd() *cobra.Command { return cmd } +func GetJson(url string, val any) { + resp, err := http.Get(url) + if err != nil { + fmt.Printf("Error making GET request(%s): %v\n", url, err) + return + } + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + if err != nil { + fmt.Printf("Error reading response body(%s): %v\n", url, err) + return + } + + if resp.StatusCode != http.StatusOK { + fmt.Printf("Error: received status code %d\n", resp.StatusCode) + fmt.Printf("Response body: %s\n", body) + return + } + + err = json.Unmarshal(body, val) + if err != nil { + fmt.Printf("Error unmarshaling response body: %v\n", err) + return + } +} + func GetLoggerNames() { url := status.GetLoggerURL() var loggerNames []string - helper.GetJson(url, &loggerNames) + GetJson(url, &loggerNames) fmt.Printf("Existing Loggers:\n") for _, logger := range loggerNames { fmt.Printf("\t%s\n", logger) @@ -67,7 +92,7 @@ func GetLoggerLevel(args []string) { loggerName := args[0] url := status.GetLoggerURL() + "?name=" + loggerName var loggerInfo status.LoggerInfo - helper.GetJson(url, &loggerInfo) + GetJson(url, &loggerInfo) fmt.Printf("Logger Name: %s\n", loggerInfo.Name) fmt.Printf("Logger Level: %s\n", loggerInfo.Level) diff --git a/pkg/status/status_server.go b/pkg/status/status_server.go index 77938409b..18457bd79 100644 --- a/pkg/status/status_server.go +++ b/pkg/status/status_server.go @@ -197,8 +197,10 @@ func (s *Server) getLoggerLevel(w http.ResponseWriter, r *http.Request) { Level: loggerLevel.String(), } } else { - loggerInfo = s.getBpfLogLevel(w) - if loggerInfo == nil { + var err error + loggerInfo, err = s.getBpfLogLevel() + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) return } } @@ -310,12 +312,11 @@ func (s *Server) readyProbe(w http.ResponseWriter, r *http.Request) { _, _ = w.Write([]byte("OK")) } -func (s *Server) getBpfLogLevel(w http.ResponseWriter) *LoggerInfo { +func (s *Server) getBpfLogLevel() (*LoggerInfo, error) { key := uint32(0) value := uint32(0) if err := s.bpfLogLevelMap.Lookup(&key, &value); err != nil { - http.Error(w, fmt.Sprintf("get log level error: %v", err), http.StatusBadRequest) - return nil + return nil, fmt.Errorf("get log level error: %v", err) } logLevelMap := map[int]string{ @@ -327,14 +328,13 @@ func (s *Server) getBpfLogLevel(w http.ResponseWriter) *LoggerInfo { loggerLevel, exists := logLevelMap[int(value)] if !exists { - http.Error(w, fmt.Sprintf("unexpected invalid log level: %d", value), http.StatusInternalServerError) - return nil + return nil, fmt.Errorf("unexpected invalid log level: %d", value) } return &LoggerInfo{ Name: bpfLoggerName, Level: loggerLevel, - } + }, nil } func (s *Server) setBpfLogLevel(w http.ResponseWriter, levelStr string) { diff --git a/pkg/status/status_server_test.go b/pkg/status/status_server_test.go index e51afc569..81411338e 100644 --- a/pkg/status/status_server_test.go +++ b/pkg/status/status_server_test.go @@ -22,10 +22,12 @@ import ( "net/http" "net/http/httptest" "net/netip" + "sort" "strconv" "testing" "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" "istio.io/istio/pilot/test/util" "kmesh.net/kmesh/api/v2/workloadapi" @@ -53,32 +55,35 @@ func TestServer_getLoggerLevel(t *testing.T) { w := httptest.NewRecorder() server.getLoggerLevel(w, req) - if w.Code != http.StatusOK { - t.Errorf("Expected status code %d, but got %d", http.StatusOK, w.Code) - } + assert.Equal(t, http.StatusOK, w.Code) var loggerInfo LoggerInfo err := json.Unmarshal(w.Body.Bytes(), &loggerInfo) - if err != nil { - t.Errorf("Unexpected error: %v", err) - } + assert.Nil(t, err) expectedLoggerLevel, err := logger.GetLoggerLevel(loggerName) - if err != nil { - t.Errorf("Unexpected error: %v", err) - } + assert.Nil(t, err) - if expectedLoggerLevel.String() != loggerInfo.Level { - t.Errorf("Wrong logger level, expected %s, but got %s", expectedLoggerLevel.String(), loggerInfo.Level) - } - - if loggerName != loggerInfo.Name { - t.Errorf("Wrong logger name, expected %s, but got %s", loggerName, loggerInfo.Name) - } + assert.Equal(t, loggerInfo.Level, expectedLoggerLevel.String()) + assert.Equal(t, loggerInfo.Name, loggerName) } + + req := httptest.NewRequest(http.MethodGet, patternLoggers, nil) + w := httptest.NewRecorder() + server.getLoggerLevel(w, req) + + assert.Equal(t, http.StatusOK, w.Code) + expectedLoggerNames := append(logger.GetLoggerNames(), bpfLoggerName) + var actualLoggerNames []string + err := json.Unmarshal(w.Body.Bytes(), &actualLoggerNames) + assert.Nil(t, err) + + sort.Strings(expectedLoggerNames) + sort.Strings(actualLoggerNames) + assert.Equal(t, expectedLoggerNames, actualLoggerNames) } -func TestServer_setBpfLevel(t *testing.T) { +func TestServer_getAndSetBpfLevel(t *testing.T) { // Test in two modes configs := []options.BpfConfig{{ Mode: "ads", @@ -126,17 +131,30 @@ func TestServer_setBpfLevel(t *testing.T) { w := httptest.NewRecorder() server.setLoggerLevel(w, req) - if w.Code != http.StatusOK { - t.Errorf("Expected status code %d, but got %d", http.StatusOK, w.Code) - } - + assert.Equal(t, http.StatusOK, w.Code) server.bpfLogLevelMap.Lookup(&key, &actualLoggerLevel) - - if actualLoggerLevel != expectedLoggerLevel { - t.Errorf("Wrong logger level, expected %d, but got %d", expectedLoggerLevel, actualLoggerLevel) - } + assert.Equal(t, expectedLoggerLevel, actualLoggerLevel) } } + + // test get bpf log level + getLoggerUrl := patternLoggers + "?name=" + bpfLoggerName + req := httptest.NewRequest(http.MethodGet, getLoggerUrl, nil) + w := httptest.NewRecorder() + server.getLoggerLevel(w, req) + + var ( + actualLoggerInfo LoggerInfo + expectedLoggerInfo *LoggerInfo + ) + err := json.Unmarshal(w.Body.Bytes(), &actualLoggerInfo) + assert.Nil(t, err) + + expectedLoggerInfo, err = server.getBpfLogLevel() + assert.Nil(t, err) + assert.NotNil(t, expectedLoggerInfo) + assert.Equal(t, expectedLoggerInfo.Level, actualLoggerInfo.Level) + assert.Equal(t, expectedLoggerInfo.Name, actualLoggerInfo.Name) }) } } @@ -171,18 +189,10 @@ func TestServer_setLoggerLevel(t *testing.T) { w := httptest.NewRecorder() server.setLoggerLevel(w, req) - if w.Code != http.StatusOK { - t.Errorf("Expected status code %d, but got %d", http.StatusOK, w.Code) - } - + assert.Equal(t, http.StatusOK, w.Code) actualLoggerLevel, err := logger.GetLoggerLevel(loggerName) - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - - if actualLoggerLevel.String() != loggerInfo.Level { - t.Errorf("Wrong logger level, expected %s, but got %s", loggerInfo.Level, actualLoggerLevel.String()) - } + assert.Nil(t, err) + assert.Equal(t, loggerInfo.Level, actualLoggerLevel.String()) } } } From 6ac828c5c88aa17763472879928f80e414451f66 Mon Sep 17 00:00:00 2001 From: "923048992@qq.com" <923048992@qq.com> Date: Thu, 11 Jul 2024 20:08:59 +0800 Subject: [PATCH 3/3] fix lint Signed-off-by: 923048992@qq.com <923048992@qq.com> --- daemon/manager/log/log.go | 1 + 1 file changed, 1 insertion(+) diff --git a/daemon/manager/log/log.go b/daemon/manager/log/log.go index 51e928b86..5e359a916 100644 --- a/daemon/manager/log/log.go +++ b/daemon/manager/log/log.go @@ -26,6 +26,7 @@ import ( "strings" "github.com/spf13/cobra" + "kmesh.net/kmesh/pkg/status" )