Skip to content

Commit 411cf30

Browse files
authored
Merge pull request #49 from thushan/feature/security-august-2025
feat: security & log consolidation
2 parents c025112 + bd579d7 commit 411cf30

File tree

10 files changed

+200
-59
lines changed

10 files changed

+200
-59
lines changed

docs/content/concepts/proxy-profiles.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ proxy:
4343
profile: "auto" # auto, streaming, or standard
4444
```
4545
46-
!!! note "Environment Variable Not Supported"
47-
The proxy profile can only be configured via the YAML configuration file. Environment variable override is not currently implemented.
46+
You can also use `OLLA_PROXY_PROFILE` environment variable to configure the Proxy Profile.
47+
4848
4949
## Profile Behaviour
5050

docs/content/configuration/reference.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,7 @@ OLLA_SERVER_REQUEST_LOGGING=true
396396
# Proxy settings
397397
OLLA_PROXY_ENGINE=olla
398398
OLLA_PROXY_LOAD_BALANCER=round-robin
399+
OLLA_PROXY_PROFILE=auto
399400
400401
# Logging
401402
OLLA_LOGGING_LEVEL=debug

internal/adapter/proxy/olla/service.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -388,13 +388,12 @@ func (s *Service) ProxyRequestToEndpoints(ctx context.Context, w http.ResponseWr
388388
reqCtx.targetURL = targetURL.String()
389389

390390
if ctxLogger != nil {
391-
ctxLogger.Info("Request dispatching to endpoint",
391+
ctxLogger.Info("Request dispatching",
392392
"endpoint", endpoint.Name,
393393
"target", stats.TargetUrl,
394-
"model", stats.Model,
395-
"request_id", middleware.GetRequestID(ctx))
394+
"model", stats.Model)
396395
} else {
397-
rlog.Info("Request dispatching to endpoint", "endpoint", endpoint.Name, "target", stats.TargetUrl, "model", stats.Model)
396+
rlog.Info("Request dispatching", "endpoint", endpoint.Name, "target", stats.TargetUrl, "model", stats.Model)
398397
}
399398

400399
// Create and prepare proxy request
@@ -592,9 +591,9 @@ func (s *Service) handleSuccessfulResponse(ctx context.Context, w http.ResponseW
592591
stats.EndTime = time.Now()
593592
stats.Latency = duration.Milliseconds()
594593

595-
// Log detailed completion metrics
594+
// Log detailed completion metrics at Debug level to reduce redundancy
596595
if ctxLogger != nil {
597-
ctxLogger.Info("Olla proxy request completed",
596+
ctxLogger.Debug("Olla proxy metrics",
598597
"endpoint", endpoint.Name,
599598
"latency_ms", stats.Latency,
600599
"processing_ms", stats.RequestProcessingMs,

internal/adapter/proxy/sherpa/service.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -204,13 +204,12 @@ func (s *Service) ProxyRequestToEndpoints(ctx context.Context, w http.ResponseWr
204204
stats.EndpointName = endpoint.Name
205205

206206
if ctxLogger != nil {
207-
ctxLogger.Info("Request dispatching to endpoint",
207+
ctxLogger.Info("Request dispatching",
208208
"endpoint", endpoint.Name,
209209
"target", stats.TargetUrl,
210-
"model", stats.Model,
211-
"request_id", middleware.GetRequestID(ctx))
210+
"model", stats.Model)
212211
} else {
213-
rlog.Info("Request dispatching to endpoint", "endpoint", endpoint.Name, "target", stats.TargetUrl, "model", stats.Model)
212+
rlog.Info("Request dispatching", "endpoint", endpoint.Name, "target", stats.TargetUrl, "model", stats.Model)
214213
}
215214

216215
s.Selector.IncrementConnections(endpoint)
@@ -312,9 +311,9 @@ func (s *Service) ProxyRequestToEndpoints(ctx context.Context, w http.ResponseWr
312311
stats.Latency = stats.EndTime.Sub(stats.StartTime).Milliseconds()
313312
stats.TotalBytes = bytesWritten
314313

315-
// Log detailed completion metrics
314+
// Log detailed completion metrics at Debug level to reduce redundancy
316315
if ctxLogger != nil {
317-
ctxLogger.Info("Sherpa proxy request completed",
316+
ctxLogger.Debug("Sherpa proxy metrics",
318317
"endpoint", endpoint.Name,
319318
"latency_ms", stats.Latency,
320319
"processing_ms", stats.RequestProcessingMs,

internal/app/handlers/handler_proxy.go

Lines changed: 61 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"net/http"
77
"time"
88

9+
"github.com/thushan/olla/internal/app/middleware"
910
"github.com/thushan/olla/internal/core/constants"
1011
"github.com/thushan/olla/internal/core/domain"
1112
"github.com/thushan/olla/internal/core/ports"
@@ -14,16 +15,17 @@ import (
1415
)
1516

1617
type proxyRequest struct {
17-
stats *ports.RequestStats
1818
requestLogger logger.StyledLogger
19+
stats *ports.RequestStats
20+
profile *domain.RequestProfile
1921
clientIP string
2022
targetPath string
21-
profile *domain.RequestProfile
2223
model string
2324
contentType string
2425
method string
2526
path string
2627
query string
28+
userAgent string
2729
contentLength int64
2830
}
2931

@@ -52,8 +54,19 @@ func (a *Application) proxyHandler(w http.ResponseWriter, r *http.Request) {
5254
}
5355

5456
func (a *Application) initializeProxyRequest(r *http.Request) *proxyRequest {
57+
// get the requestID from the middleware context first
58+
requestID := ""
59+
if id, ok := r.Context().Value(middleware.RequestIDKey).(string); ok {
60+
requestID = id
61+
}
62+
63+
// fallback to generating a new one otherwise
64+
if requestID == "" {
65+
requestID = util.GenerateRequestID()
66+
}
67+
5568
stats := &ports.RequestStats{
56-
RequestID: util.GenerateRequestID(),
69+
RequestID: requestID,
5770
StartTime: time.Now(),
5871
}
5972

@@ -65,6 +78,7 @@ func (a *Application) initializeProxyRequest(r *http.Request) *proxyRequest {
6578
path: r.URL.Path,
6679
query: r.URL.RawQuery,
6780
contentLength: r.ContentLength,
81+
userAgent: r.UserAgent(),
6882
}
6983
}
7084

@@ -121,34 +135,69 @@ func (a *Application) executeProxyRequest(ctx context.Context, w http.ResponseWr
121135
}
122136

123137
func (a *Application) logRequestStart(pr *proxyRequest, endpointCount int) {
138+
// Log essential operational info at INFO level
124139
logFields := []any{
125140
"client_ip", pr.clientIP,
126141
"method", pr.method,
127142
"path", pr.path,
128-
"target_path", pr.targetPath,
129143
"compatible_endpoints", endpointCount,
130-
"path_resolution_ms", pr.stats.PathResolutionMs,
131-
"query", pr.query,
132-
"content_type", pr.contentType,
133-
"content_length", pr.contentLength,
134144
}
135145

146+
// Add user agent if present
147+
if pr.userAgent != "" {
148+
logFields = append(logFields, "user_agent", pr.userAgent)
149+
}
150+
151+
// Add model if identified
136152
if pr.model != "" {
137153
logFields = append(logFields, "model", pr.model)
138154
}
139155

140-
pr.requestLogger.Info("Request started", logFields...)
156+
// Add content length if it's a POST/PUT with body
157+
if pr.contentLength > 0 {
158+
logFields = append(logFields, "content_length", pr.contentLength)
159+
}
160+
161+
pr.requestLogger.Info("Request received", logFields...)
162+
163+
// Log additional details at DEBUG level
164+
debugFields := []any{
165+
"target_path", pr.targetPath,
166+
"path_resolution_ms", pr.stats.PathResolutionMs,
167+
"query", pr.query,
168+
"content_type", pr.contentType,
169+
}
170+
171+
pr.requestLogger.Debug("Request details", debugFields...)
141172
}
142173

143174
func (a *Application) logRequestResult(pr *proxyRequest, err error) {
144175
duration := time.Since(pr.stats.StartTime)
145176

146-
logFields := a.buildLogFields(pr, duration)
147-
148177
if err != nil {
178+
logFields := a.buildLogFields(pr, duration)
149179
pr.requestLogger.Error("Request failed", append([]any{"error", err}, logFields...)...)
150180
} else {
151-
pr.requestLogger.Info("Request completed", logFields...)
181+
// Log essential completion info at INFO level
182+
infoFields := []any{
183+
"endpoint", pr.stats.EndpointName,
184+
"duration_ms", duration.Milliseconds(),
185+
"status", "completed",
186+
}
187+
188+
if pr.model != "" {
189+
infoFields = append(infoFields, "model", pr.model)
190+
}
191+
192+
if pr.stats.TotalBytes > 0 {
193+
infoFields = append(infoFields, "total_bytes", pr.stats.TotalBytes)
194+
}
195+
196+
pr.requestLogger.Info("Request completed", infoFields...)
197+
198+
// Log detailed metrics at DEBUG level
199+
debugFields := a.buildLogFields(pr, duration)
200+
pr.requestLogger.Debug("Request metrics", debugFields...)
152201
}
153202
}
154203

internal/app/handlers/handler_status_endpoints.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ const (
2020

2121
type EndpointSummary struct {
2222
Name string `json:"name"`
23-
URL string `json:"url"`
2423
Type string `json:"type"`
2524
Status string `json:"status"`
2625
LastModelSync string `json:"last_model_sync,omitempty"`
@@ -103,7 +102,6 @@ func (a *Application) buildEndpointSummaryOptimised(endpoint *domain.Endpoint, s
103102

104103
summary := EndpointSummary{
105104
Name: endpoint.Name,
106-
URL: url,
107105
Type: endpoint.Type,
108106
Status: endpoint.Status.String(),
109107
Priority: endpoint.Priority,

internal/app/middleware/logging.go

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"log/slog"
77
"net/http"
8+
"strings"
89
"time"
910

1011
"github.com/thushan/olla/internal/core/constants"
@@ -22,6 +23,15 @@ const (
2223
LoggerKey contextKey = "logger"
2324
)
2425

26+
// IsProxyRequest determines if a request is for the proxy endpoints
27+
// Used to decide logging levels to avoid redundancy with proxy handler logging
28+
func IsProxyRequest(path string) bool {
29+
// checks for proxy prefixes
30+
// /olla/ is the main proxy prefix
31+
return strings.Contains(path, constants.DefaultOllaProxyPathPrefix) ||
32+
(strings.HasPrefix(path, "/api/") && !strings.HasPrefix(path, "/api/v0/")) // /api/v0/ is internal
33+
}
34+
2535
// responseWriter wraps http.ResponseWriter to capture response size and status
2636
type responseWriter struct {
2737
http.ResponseWriter
@@ -89,7 +99,7 @@ func EnhancedLoggingMiddleware(styledLogger logger.StyledLogger) func(http.Handl
8999
ctx := context.WithValue(r.Context(), RequestIDKey, requestID)
90100

91101
// Create a base logger with request ID
92-
baseLogger := slog.Default().With("request_id", requestID)
102+
baseLogger := slog.Default().With(constants.ContextRequestIdKey, requestID)
93103
ctx = context.WithValue(ctx, LoggerKey, baseLogger)
94104

95105
// Add to response header for client tracking
@@ -98,29 +108,48 @@ func EnhancedLoggingMiddleware(styledLogger logger.StyledLogger) func(http.Handl
98108
// Wrap response writer to capture metrics
99109
wrapped := &responseWriter{ResponseWriter: w, status: 200}
100110

101-
// Log request start
102-
baseLogger.Info("Request started",
111+
// Log request start - use Debug for proxy requests to reduce noise
112+
// Proxy requests will log their own "Request received" at INFO level
113+
logFields := []any{
103114
"method", r.Method,
104115
"path", r.URL.Path,
105116
"remote_addr", r.RemoteAddr,
106117
"user_agent", r.UserAgent(),
107118
"request_bytes", requestSize,
108-
"request_size_formatted", formatBytes(requestSize))
119+
"request_size_formatted", formatBytes(requestSize),
120+
}
121+
122+
if IsProxyRequest(r.URL.Path) {
123+
// For proxy requests, just log at debug level since handler will log INFO
124+
baseLogger.Debug("HTTP request started", logFields...)
125+
} else {
126+
// For non-proxy requests (health, status, etc), log at INFO
127+
baseLogger.Info("Request started", logFields...)
128+
}
109129

110130
next.ServeHTTP(wrapped, r.WithContext(ctx))
111131

112132
duration := time.Since(start)
113133

114-
// Log request completion
115-
baseLogger.Info("Request completed",
134+
// Log request completion - use Debug for proxy requests to reduce noise
135+
completionFields := []any{
116136
"method", r.Method,
117137
"path", r.URL.Path,
118138
"status", wrapped.status,
119139
"duration_ms", duration.Milliseconds(),
120140
"duration_formatted", duration.String(),
121141
"request_bytes", requestSize,
122142
"response_bytes", wrapped.size,
123-
"size_flow", fmt.Sprintf("%s -> %s", formatBytes(requestSize), formatBytes(wrapped.size)))
143+
"size_flow", fmt.Sprintf("%s -> %s", formatBytes(requestSize), formatBytes(wrapped.size)),
144+
}
145+
146+
if IsProxyRequest(r.URL.Path) {
147+
// For proxy requests, just log at debug level since handler will log INFO
148+
baseLogger.Debug("HTTP request completed", completionFields...)
149+
} else {
150+
// For non-proxy requests, log at INFO
151+
baseLogger.Info("Request completed", completionFields...)
152+
}
124153
})
125154
}
126155
}
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
package middleware
2+
3+
import "testing"
4+
5+
func TestIsProxyRequest(t *testing.T) {
6+
tests := []struct {
7+
name string
8+
path string
9+
expected bool
10+
}{
11+
// Proxy requests
12+
{
13+
name: "olla proxy ollama path",
14+
path: "/olla/ollama/api/chat",
15+
expected: true,
16+
},
17+
{
18+
name: "olla proxy lmstudio path",
19+
path: "/olla/lmstudio/v1/models",
20+
expected: true,
21+
},
22+
{
23+
name: "olla proxy generic path",
24+
path: "/olla/proxy/some/endpoint",
25+
expected: true,
26+
},
27+
{
28+
name: "direct api path",
29+
path: "/api/chat",
30+
expected: true,
31+
},
32+
{
33+
name: "api models path",
34+
path: "/api/models",
35+
expected: true,
36+
},
37+
38+
// Non-proxy requests
39+
{
40+
name: "health check endpoint",
41+
path: "/internal/health",
42+
expected: false,
43+
},
44+
{
45+
name: "status endpoint",
46+
path: "/internal/status",
47+
expected: false,
48+
},
49+
{
50+
name: "version endpoint",
51+
path: "/version",
52+
expected: false,
53+
},
54+
{
55+
name: "root path",
56+
path: "/",
57+
expected: false,
58+
},
59+
{
60+
name: "internal api v0 models",
61+
path: "/api/v0/models",
62+
expected: false,
63+
},
64+
{
65+
name: "internal api v0 status",
66+
path: "/api/v0/status",
67+
expected: false,
68+
},
69+
}
70+
71+
for _, tt := range tests {
72+
t.Run(tt.name, func(t *testing.T) {
73+
result := IsProxyRequest(tt.path)
74+
if result != tt.expected {
75+
t.Errorf("IsProxyRequest(%q) = %v, want %v", tt.path, result, tt.expected)
76+
}
77+
})
78+
}
79+
}

0 commit comments

Comments
 (0)