Skip to content

fix(api-service): replace zap with logrus for unified logging #72

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
puzhen-ryan merged 3 commits into from
Mar 10, 2026
Merged

fix(api-service): replace zap with logrus for unified logging #72

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
64dfa83
fix(api-service): replace zap with logrus for unified logging
JacksonMei Mar 9, 2026
a91c46f
fix(api-service): use proper log levels instead of Printf
JacksonMei Mar 9, 2026
1291b1b
fix(api-service): set MaxSize=500MB and Compress=false for log rotation
JacksonMei Mar 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions api-service/controller/mcp_proxy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ package controller
import (
"api-service/constants"
"io"
"log"
"net/http"
"net/http/httputil"
"net/url"

"github.com/gin-gonic/gin"
log "github.com/sirupsen/logrus"
)

// MCP gateway constant definitions
Expand Down Expand Up @@ -182,7 +182,7 @@ func (g *MCPGateway) handleMCPSSEWithHeader(c *gin.Context) {
// Create request to MCP server
req, err := http.NewRequest(MethodGET, targetURL.String(), nil)
if err != nil {
log.Printf("Failed to create request: %v", err)
log.Errorf("Failed to create request: %v", err)
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to create request"})
return
}
Expand All @@ -198,19 +198,19 @@ func (g *MCPGateway) handleMCPSSEWithHeader(c *gin.Context) {
client := &http.Client{}
resp, err := client.Do(req)
if err != nil {
log.Printf("Failed to connect to MCP server (%s): %v", mcpServerURL, err)
log.Errorf("Failed to connect to MCP server (%s): %v", mcpServerURL, err)
c.JSON(http.StatusBadGateway, gin.H{"error": "Failed to connect to MCP server"})
return
}
defer func() {
if closeErr := resp.Body.Close(); closeErr != nil {
log.Printf("failed to close response body: %v", closeErr)
log.Warnf("failed to close response body: %v", closeErr)
}
}()

// Check response status
if resp.StatusCode != http.StatusOK {
log.Printf("MCP server (%s) returned status: %d", mcpServerURL, resp.StatusCode)
log.Warnf("MCP server (%s) returned status: %d", mcpServerURL, resp.StatusCode)
c.JSON(resp.StatusCode, gin.H{"error": "MCP server error"})
return
}
Expand All @@ -233,15 +233,15 @@ func (g *MCPGateway) handleMCPSSEWithHeader(c *gin.Context) {
n, err := resp.Body.Read(buf)
if err != nil {
if err != io.EOF {
log.Printf("Error reading from MCP server (%s): %v", mcpServerURL, err)
log.Errorf("Error reading from MCP server (%s): %v", mcpServerURL, err)
}
break
}

if n > 0 {
_, writeErr := c.Writer.Write(buf[:n])
if writeErr != nil {
log.Printf("Error writing to client: %v", writeErr)
log.Errorf("Error writing to client: %v", writeErr)
break
}

Expand Down Expand Up @@ -285,7 +285,7 @@ func (g *MCPGateway) handleMCPHTTPWithHeader(c *gin.Context) {

// Error handling
proxy.ErrorHandler = func(w http.ResponseWriter, r *http.Request, err error) {
log.Printf("Proxy error for server %s: %v", mcpServerURL, err)
log.Errorf("Proxy error for server %s: %v", mcpServerURL, err)
c.JSON(http.StatusBadGateway, gin.H{
"error": "Failed to forward request to MCP server",
"details": err.Error(),
Expand Down
2 changes: 0 additions & 2 deletions api-service/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ require (
github.com/prometheus/client_golang v1.14.0
github.com/sirupsen/logrus v1.9.3
github.com/spf13/pflag v1.0.6-0.20200504143853-81378bbcd8a1
go.uber.org/zap v1.27.0
gopkg.in/natefinch/lumberjack.v2 v2.2.1
)

Expand Down Expand Up @@ -47,7 +46,6 @@ require (
github.com/prometheus/procfs v0.8.0 // indirect
github.com/twitchyliquid64/golang-asm v0.15.1 // indirect
github.com/ugorji/go/codec v1.2.11 // indirect
go.uber.org/multierr v1.10.0 // indirect
golang.org/x/arch v0.3.0 // indirect
golang.org/x/crypto v0.13.0 // indirect
golang.org/x/net v0.15.0 // indirect
Expand Down
20 changes: 9 additions & 11 deletions api-service/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@ limitations under the License.
package main

import (
"log"
"net/http"
"runtime"
"time"

log "github.com/sirupsen/logrus"

"api-service/controller"
"api-service/metrics"
"api-service/middleware"
Expand Down Expand Up @@ -76,14 +77,9 @@ func main() {
// Register global metrics middleware
mainRouter.Use(middleware.MetricsMiddleware())

// Initialize logger
logger := middleware.InitLogger("")
defer func() {
if err := logger.Sync(); err != nil {
log.Printf("Failed to sync logger: %v", err)
}
}()
mainRouter.Use(middleware.LoggingMiddleware(logger))
// Initialize logger (logrus + lumberjack)
middleware.InitLogger("", "info")
mainRouter.Use(middleware.LoggingMiddleware())
// Main route configuration
var redisClient *service.RedisClient = nil
if redisAddr != "" {
Expand Down Expand Up @@ -138,10 +134,12 @@ func main() {
mainRouter.GET("/metrics", gin.WrapH(promhttp.Handler()))

// MCP dedicated routing engine
mcpLogger := middleware.InitLogger("/home/admin/logs/api-service-mcp.log")
// Note: MCP uses the same logrus global logger (writes to same log file)
// since logrus is a global singleton. For separate MCP log files,
// use a dedicated logrus instance in the future.
mcpRouter := gin.Default()
mcpRouter.Use(middleware.MetricsMiddleware())
mcpRouter.Use(middleware.LoggingMiddleware(mcpLogger))
mcpRouter.Use(middleware.LoggingMiddleware())
mcpGroup := mcpRouter.Group("/")
controller.NewMCPGateway(mcpGroup)

Expand Down
104 changes: 54 additions & 50 deletions api-service/middleware/logging.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,44 +20,41 @@ import (
"api-service/constants"
"bytes"
"io"
"net/http"
"os"
"time"

"github.com/gin-gonic/gin"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
log "github.com/sirupsen/logrus"
"gopkg.in/natefinch/lumberjack.v2"
)

// InitLogger initializes zap logger with log rotation
func InitLogger(logPath string) *zap.Logger {
const maxBodyLogSize = 2048 // 2KB body truncation limit

// InitLogger initializes logrus with lumberjack log rotation.
// logPath: log file path, empty means default /home/admin/logs/api-service.log
// logLevel: log level string (debug, info, warn, error), empty means info
func InitLogger(logPath, logLevel string) {
if logPath == "" {
logPath = "/home/admin/logs/api-service.log"
}
// Console encoder
consoleEncoder := zapcore.NewConsoleEncoder(zap.NewDevelopmentEncoderConfig())
// File encoder (JSON format)
fileEncoder := zapcore.NewJSONEncoder(zap.NewProductionEncoderConfig())
// Lumberjack log rotation configuration
logWriter := zapcore.AddSync(&lumberjack.Logger{
Filename: logPath, // Log file path
MaxSize: 100, // Maximum size of each log file (MB)
MaxBackups: 30, // Maximum number of old files to retain
MaxAge: 0, // Maximum age of old files in days (0 means permanent)
Compress: false, // Whether to compress old files

lv, err := log.ParseLevel(logLevel)
if err != nil {
lv = log.InfoLevel
}
log.SetLevel(lv)

log.SetFormatter(&log.TextFormatter{
TimestampFormat: "2006-01-02 15:04:05.000",
FullTimestamp: true,
})

log.SetOutput(&lumberjack.Logger{
Filename: logPath,
MaxSize: 500, // megabytes
MaxBackups: 10,
MaxAge: 7, // days
Compress: false,
})
// Console output (stdout)
consoleDebugging := zapcore.Lock(os.Stdout)
consoleEncoderConfig := zap.NewDevelopmentEncoderConfig()
consoleEncoderConfig.EncodeLevel = zapcore.CapitalColorLevelEncoder
consoleCore := zapcore.NewCore(consoleEncoder, consoleDebugging, zapcore.DebugLevel)
// File output
fileCore := zapcore.NewCore(fileEncoder, logWriter, zapcore.DebugLevel)
// Merge multiple cores (dual write)
core := zapcore.NewTee(consoleCore, fileCore)
logger := zap.New(core, zap.AddCaller(), zap.Development())
return logger
}

// ResponseWriter is a custom ResponseWriter for capturing response body
Expand All @@ -78,8 +75,8 @@ func (w ResponseWriter) WriteString(s string) (int, error) {
return w.ResponseWriter.WriteString(s)
}

// LoggingMiddleware creates logging middleware
func LoggingMiddleware(logger *zap.Logger) gin.HandlerFunc {
// LoggingMiddleware creates logging middleware using logrus
func LoggingMiddleware() gin.HandlerFunc {
return func(c *gin.Context) {

// Filter health check path, don't log
Expand Down Expand Up @@ -114,42 +111,49 @@ func LoggingMiddleware(logger *zap.Logger) gin.HandlerFunc {
// Get response status code
statusCode := c.Writer.Status()

// log redirect forward pod name and ip
safeHeaders := http.Header{}
safeHeaders.Set(constants.HeaderMCPServerURL, c.Request.Header.Get(constants.HeaderMCPServerURL))
safeHeaders.Set(constants.HeaderEnvInstanceID, c.Request.Header.Get(constants.HeaderEnvInstanceID))
// Log
fields := []zap.Field{
zap.String("method", c.Request.Method),
zap.String("path", c.Request.URL.Path),
zap.Any("header", safeHeaders),
zap.Int("status", statusCode),
zap.Duration("latency", latency),
zap.String("client_ip", c.ClientIP()),
// Build log fields
fields := log.Fields{
"method": c.Request.Method,
"path": c.Request.URL.Path,
"status": statusCode,
"latency": latency.String(),
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Logging latency as a string (latency.String()) is less flexible for analysis and filtering in log management systems compared to a numeric type. Logrus can handle time.Duration fields natively, typically logging them as an integer (nanoseconds). This allows for easier querying, sorting, and calculations on the latency value.

Suggested change
"latency": latency.String(),
"latency": latency,

"client_ip": c.ClientIP(),
"mcp_url": c.Request.Header.Get(constants.HeaderMCPServerURL),
"inst_id": c.Request.Header.Get(constants.HeaderEnvInstanceID),
}

// Add request body (if exists)
// Add request body (truncated)
if len(reqBody) > 0 {
fields = append(fields, zap.String("request_body", string(reqBody)))
fields["request_body"] = truncateString(string(reqBody), maxBodyLogSize)
}

// Add response body (if exists)
// Add response body (truncated)
if blw.body.Len() > 0 {
fields = append(fields, zap.String("response_body", blw.body.String()))
fields["response_body"] = truncateString(blw.body.String(), maxBodyLogSize)
}
Comment on lines +125 to 133
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-medium medium

The logging middleware captures and logs the entire request and response bodies (truncated to 2KB). These bodies can contain sensitive information such as authentication tokens, passwords, or PII. Specifically, the CreateEnvInstanceRequest includes environment_variables, which are often used to store secrets. Logging these secrets to a file increases the risk of exposure if the log files are compromised. It is recommended to implement a sanitization or redaction mechanism to remove sensitive fields from the request and response bodies before logging, or avoid logging the bodies entirely in production environments.


// Log error information (if any)
if len(c.Errors) > 0 {
fields = append(fields, zap.String("error", c.Errors.ByType(gin.ErrorTypePrivate).String()))
fields["error"] = c.Errors.ByType(gin.ErrorTypePrivate).String()
}

entry := log.WithFields(fields)

// Determine log level based on status code
if statusCode >= 500 {
logger.Error("API Error", fields...)
entry.Error("API Error")
} else if statusCode >= 400 {
logger.Warn("API Warning", fields...)
entry.Warn("API Warning")
} else {
logger.Info("API Access", fields...)
entry.Info("API Access")
}
}
}

// truncateString truncates a string to maxLen bytes
func truncateString(s string, maxLen int) string {
if len(s) > maxLen {
return s[:maxLen] + "...(truncated)"
}
Comment on lines +155 to +157
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current implementation of truncateString can corrupt multi-byte UTF-8 characters by slicing a string at an arbitrary byte index. This can lead to invalid characters in the logs, making them hard to read or process. A safer approach is to ensure the truncation happens at a rune boundary.

	if len(s) > maxLen {
		// To avoid cutting a multi-byte character in half, find the last valid start of a UTF-8 rune.
		end := maxLen
		for end > 0 && (s[end]&0xC0) == 0x80 { // is a continuation byte
			end--
		}
		return s[:end] + "...(truncated)"
	}

return s
}
6 changes: 3 additions & 3 deletions api-service/service/backend_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func (c *BackendClient) GetEnvByVersion(name, version string) (*backendmodel.Env
}
defer func() {
if closeErr := resp.Body.Close(); closeErr != nil {
log.Printf("failed to close response body: %v", closeErr)
log.Warnf("failed to close response body: %v", closeErr)
}
}()

Expand Down Expand Up @@ -183,7 +183,7 @@ func (c *BackendClient) ValidateToken(token string) (*backendmodel.Token, error)
}
defer func() {
if closeErr := resp.Body.Close(); closeErr != nil {
log.Printf("failed to close response body: %v", closeErr)
log.Warnf("failed to close response body: %v", closeErr)
}
}()

Expand Down Expand Up @@ -226,7 +226,7 @@ func (c *BackendClient) SearchDatasource(scenario, key string) (string, error) {
}
defer func() {
if closeErr := resp.Body.Close(); closeErr != nil {
log.Printf("failed to close response body: %v", closeErr)
log.Warnf("failed to close response body: %v", closeErr)
}
}()

Expand Down
Loading
Loading