Skip to content

RHOBS-1623(chore): switch from global slog to contextual logging #147

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

Draft
slashpai wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

RHOBS-1623(chore): switch from global slog to contextual logging #147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e65f1ab
chore(logs): switch main and k8s route discovery to klog contextual l…
slashpai Jun 30, 2026
aa2c9cc
chore(logs): switch MCP server to klog contextual logging
slashpai Jun 30, 2026
913c74e
chore(logs): switch tool handlers and backend loaders to klog
slashpai Jun 30, 2026
e079d55
chore(logs): switch otelcol, auth, and toolset packages to klog
slashpai Jun 30, 2026
9238eab
chore: fix lint
slashpai Jul 1, 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
82 changes: 42 additions & 40 deletions cmd/obs-mcp/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,12 @@ import (
"flag"
"fmt"
"log"
"log/slog"
"os"
"slices"
"strings"

"github.com/modelcontextprotocol/go-sdk/mcp"
"github.com/prometheus/common/promslog"
"k8s.io/klog/v2"

"github.com/rhobs/obs-mcp/pkg/auth"
"github.com/rhobs/obs-mcp/pkg/k8s"
Expand Down Expand Up @@ -66,8 +65,10 @@ func main() {
os.Exit(0)
}

// Configure slog with specified log level
// Configure klog with specified log level
configureLogging(*logLevel)
ctx := context.Background()
logger := klog.FromContext(ctx)

// Parse and validate auth mode
parsedAuthMode, err := auth.ParseAuthMode(*authMode)
Expand All @@ -92,7 +93,7 @@ func main() {
metricsBackendURL := ""
metricsURLSource := ""
if slices.Contains(parsedToolsets, mcpserver.ToolsetMetrics) {
metricsBackendURL, metricsURLSource, err = determineMetricsBackendURL(parsedAuthMode, parsedMetricsBackend)
metricsBackendURL, metricsURLSource, err = determineMetricsBackendURL(ctx, parsedAuthMode, parsedMetricsBackend)
if err != nil {
log.Fatalf("%v", err)
}
Expand All @@ -101,7 +102,7 @@ func main() {
alertmanagerURL := ""
alertmanagerURLSource := ""
if slices.Contains(parsedToolsets, mcpserver.ToolsetMetrics) {
alertmanagerURL, alertmanagerURLSource, err = determineAlertmanagerURL(parsedAuthMode)
alertmanagerURL, alertmanagerURLSource, err = determineAlertmanagerURL(ctx, parsedAuthMode)
if err != nil {
log.Fatalf("%v", err)
}
Expand All @@ -111,7 +112,7 @@ func main() {
lokiResolvedURL := ""
lokiURLSource := ""
if slices.Contains(parsedToolsets, mcpserver.ToolsetLogs) {
lokiResolvedURL, lokiURLSource, err = determineLokiURL(parsedAuthMode, *lokiURL, *lokiUseRoute)
lokiResolvedURL, lokiURLSource, err = determineLokiURL(ctx, parsedAuthMode, *lokiURL, *lokiUseRoute)
if err != nil {
log.Fatalf("%v", err)
}
Expand Down Expand Up @@ -155,7 +156,7 @@ func main() {
tempoResolvedURL := ""
tempoURLSource := ""
if slices.Contains(parsedToolsets, mcpserver.ToolsetTraces) {
tempoResolvedURL, tempoURLSource = determineTempoURL(*tempoURL)
tempoResolvedURL, tempoURLSource = determineTempoURL(ctx, *tempoURL)
}

// Create MCP options
Expand Down Expand Up @@ -184,7 +185,7 @@ func main() {
log.Fatalf("Failed to create MCP server: %v", err)
}

slog.Info("Starting server",
logger.Info("Starting server",
"toolsets", opts.Toolsets,
"auth_mode", opts.AuthMode,
"metrics_backend_url", opts.MetricsBackendURL,
Expand All @@ -201,14 +202,13 @@ func main() {
// Choose server mode based on flags
if *listen != "" {
// HTTP mode
ctx := context.Background()
if err := mcpserver.Serve(ctx, mcpServer, *listen, opts.AuthMode); err != nil {
log.Fatalf("HTTP server failed: %v", err)
}
} else {
// Start server on stdio (default mode)
transport := &mcp.StdioTransport{}
if _, err := mcpServer.Connect(context.Background(), transport, nil); err != nil {
if _, err := mcpServer.Connect(ctx, transport, nil); err != nil {
log.Fatalf("Server failed: %v", err)
}
}
Expand Down Expand Up @@ -241,16 +241,17 @@ func parseMetricsBackend(backend string) (k8s.MetricsBackend, error) {

// determineMetricsBackendURL determines the metrics backend URL based on auth mode and environment.
// Returns the resolved URL, a source description for logging, and an error if the configuration is invalid.
func determineMetricsBackendURL(authMode auth.AuthMode, backend k8s.MetricsBackend) (url, source string, err error) {
func determineMetricsBackendURL(ctx context.Context, authMode auth.AuthMode, backend k8s.MetricsBackend) (url, source string, err error) {
logger := klog.FromContext(ctx)
if prometheusURL := os.Getenv("PROMETHEUS_URL"); prometheusURL != "" {
return prometheusURL, "PROMETHEUS_URL env var", nil
}

if authMode == auth.AuthModeKubeConfig {
slog.Info("No PROMETHEUS_URL set, attempting route discovery", "backend", backend)
url, err := k8s.GetMetricsBackendURL(backend)
logger.Info("No PROMETHEUS_URL set, attempting route discovery", "backend", string(backend))
url, err := k8s.GetMetricsBackendURL(ctx, backend)
if err != nil {
slog.Warn("Route discovery failed, falling back to default", "err", err, "default", defaultPrometheusURL)
logger.Info("Route discovery failed, falling back to default", "err", err, "default", defaultPrometheusURL)
return defaultPrometheusURL, "default (route discovery failed)", nil
}
return url, "route discovery", nil
Expand All @@ -267,16 +268,17 @@ func determineMetricsBackendURL(authMode auth.AuthMode, backend k8s.MetricsBacke

// determineAlertmanagerURL determines the Alertmanager URL based on auth mode and environment.
// Returns the resolved URL, a source description for logging, and an error if the configuration is invalid.
func determineAlertmanagerURL(authMode auth.AuthMode) (url, source string, err error) {
func determineAlertmanagerURL(ctx context.Context, authMode auth.AuthMode) (url, source string, err error) {
logger := klog.FromContext(ctx)
if alertmanagerURL := os.Getenv("ALERTMANAGER_URL"); alertmanagerURL != "" {
return alertmanagerURL, "ALERTMANAGER_URL env var", nil
}

if authMode == auth.AuthModeKubeConfig {
slog.Info("No ALERTMANAGER_URL set, attempting route discovery")
url, err := k8s.GetAlertmanagerURL()
logger.Info("No ALERTMANAGER_URL set, attempting route discovery")
url, err := k8s.GetAlertmanagerURL(ctx)
if err != nil {
slog.Warn("Route discovery failed, falling back to default", "err", err, "default", defaultAlertmanagerURL)
logger.Info("Route discovery failed, falling back to default", "err", err, "default", defaultAlertmanagerURL)
return defaultAlertmanagerURL, "default (route discovery failed)", nil
}
return url, "route discovery", nil
Expand All @@ -289,29 +291,31 @@ func determineAlertmanagerURL(authMode auth.AuthMode) (url, source string, err e
)
}

func determineTempoURL(flagURL string) (url, source string) {
func determineTempoURL(ctx context.Context, flagURL string) (url, source string) {
logger := klog.FromContext(ctx)
if flagURL != "" {
return flagURL, "--traces.tempo-url flag"
}
if tempoURL := os.Getenv("TEMPO_URL"); tempoURL != "" {
return tempoURL, "TEMPO_URL env var"
}
slog.Info("No Tempo URL configured; Tempo tools require tempoNamespace+tempoName discovery parameters or explicit Tempo URL")
logger.Info("No Tempo URL configured; Tempo tools require tempoNamespace+tempoName discovery parameters or explicit Tempo URL")
return "", "unset"
}

func determineLokiURL(authMode auth.AuthMode, flagURL string, useRoute bool) (url, source string, err error) {
func determineLokiURL(ctx context.Context, authMode auth.AuthMode, flagURL string, useRoute bool) (url, source string, err error) {
logger := klog.FromContext(ctx)
if flagURL != "" {
return flagURL, "--loki-url flag", nil
}
if lokiURL := os.Getenv("LOKI_URL"); lokiURL != "" {
return lokiURL, "LOKI_URL env var", nil
}
if authMode == auth.AuthModeKubeConfig && !useRoute {
slog.Warn("No Loki URL configured, falling back to default", "default", defaultLokiURL)
logger.Info("No Loki URL configured, falling back to default", "default", defaultLokiURL)
return defaultLokiURL, "default", nil
}
slog.Warn("No Loki URL configured; Loki tools require lokiNamespace+lokiName discovery parameters or explicit Loki URL")
logger.Info("No Loki URL configured; Loki tools require lokiNamespace+lokiName discovery parameters or explicit Loki URL")
return "", "unset", nil
}

Expand All @@ -327,24 +331,22 @@ func isFlagExplicitlySet(name string) bool {
return found
}

// configureLogging sets up the slog logger with the specified log level
// configureLogging sets up klog with the specified log level
func configureLogging(levelStr string) {
level := promslog.NewLevel()
err := level.Set(levelStr)
if err != nil {
log.Fatal(err.Error())
}
fs := flag.NewFlagSet("klog", flag.ContinueOnError)
klog.InitFlags(fs)

format := promslog.NewFormat()
err = format.Set("logfmt")
if err != nil {
log.Fatal(err.Error())
var verbosity string
switch strings.ToLower(levelStr) {
case "debug":
verbosity = "4"
default:
verbosity = "0"
}
if err := fs.Set("v", verbosity); err != nil {
log.Fatalf("Failed to set klog verbosity: %v", err)
}
if err := fs.Set("logtostderr", "true"); err != nil {
log.Fatalf("Failed to set klog logtostderr: %v", err)
}

logger := promslog.New(&promslog.Config{
Level: level,
Format: format,
Style: promslog.GoKitStyle,
})
slog.SetDefault(logger)
}
19 changes: 10 additions & 9 deletions cmd/obs-mcp/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"context"
"testing"

"github.com/rhobs/obs-mcp/pkg/auth"
Expand Down Expand Up @@ -108,7 +109,7 @@ func TestDetermineMetricsBackendURL_RequiresURLForNonKubeconfigModes(t *testing.

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, _, err := determineMetricsBackendURL(tt.authMode, tt.backend)
_, _, err := determineMetricsBackendURL(context.Background(), tt.authMode, tt.backend)
if err == nil {
t.Errorf("expected error for auth mode %q without PROMETHEUS_URL, got nil", tt.authMode)
}
Expand All @@ -131,7 +132,7 @@ func TestDetermineMetricsBackendURL_EnvVarOverridesAll(t *testing.T) {

for _, authMode := range authModes {
t.Run(string(authMode), func(t *testing.T) {
url, source, err := determineMetricsBackendURL(authMode, k8s.MetricsBackendThanos)
url, source, err := determineMetricsBackendURL(context.Background(), authMode, k8s.MetricsBackendThanos)
if err != nil {
t.Errorf("unexpected error: %v", err)
}
Expand All @@ -148,7 +149,7 @@ func TestDetermineMetricsBackendURL_EnvVarOverridesAll(t *testing.T) {
func TestDetermineLokiURL(t *testing.T) {
t.Run("explicit flag wins", func(t *testing.T) {
t.Setenv("LOKI_URL", "http://from-env:3100")
got, source, err := determineLokiURL(auth.AuthModeHeader, "http://from-flag:3100", false)
got, source, err := determineLokiURL(context.Background(), auth.AuthModeHeader, "http://from-flag:3100", false)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
Expand All @@ -159,7 +160,7 @@ func TestDetermineLokiURL(t *testing.T) {

t.Run("env used when flag missing", func(t *testing.T) {
t.Setenv("LOKI_URL", "http://from-env:3100")
got, source, err := determineLokiURL(auth.AuthModeHeader, "", false)
got, source, err := determineLokiURL(context.Background(), auth.AuthModeHeader, "", false)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
Expand All @@ -170,7 +171,7 @@ func TestDetermineLokiURL(t *testing.T) {

t.Run("kubeconfig falls back to default", func(t *testing.T) {
t.Setenv("LOKI_URL", "")
got, source, err := determineLokiURL(auth.AuthModeKubeConfig, "", false)
got, source, err := determineLokiURL(context.Background(), auth.AuthModeKubeConfig, "", false)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
Expand All @@ -181,7 +182,7 @@ func TestDetermineLokiURL(t *testing.T) {

t.Run("non-kubeconfig without URL returns unset", func(t *testing.T) {
t.Setenv("LOKI_URL", "")
got, source, err := determineLokiURL(auth.AuthModeHeader, "", false)
got, source, err := determineLokiURL(context.Background(), auth.AuthModeHeader, "", false)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
Expand All @@ -194,23 +195,23 @@ func TestDetermineLokiURL(t *testing.T) {
func TestDetermineTempoURL(t *testing.T) {
t.Run("explicit flag wins", func(t *testing.T) {
t.Setenv("TEMPO_URL", "http://from-env:3200")
got, source := determineTempoURL("http://from-flag:3200")
got, source := determineTempoURL(context.Background(), "http://from-flag:3200")
if got != "http://from-flag:3200" || source != "--traces.tempo-url flag" {
t.Fatalf("unexpected result: %s (%s)", got, source)
}
})

t.Run("env used when flag missing", func(t *testing.T) {
t.Setenv("TEMPO_URL", "http://from-env:3200")
got, source := determineTempoURL("")
got, source := determineTempoURL(context.Background(), "")
if got != "http://from-env:3200" || source != "TEMPO_URL env var" {
t.Fatalf("unexpected result: %s (%s)", got, source)
}
})

t.Run("no URL returns unset", func(t *testing.T) {
t.Setenv("TEMPO_URL", "")
got, source := determineTempoURL("")
got, source := determineTempoURL(context.Background(), "")
if got != "" || source != "unset" {
t.Fatalf("unexpected result: %s (%s)", got, source)
}
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ require (
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
k8s.io/cli-runtime v0.36.2 // indirect
k8s.io/klog/v2 v2.140.0 // indirect
k8s.io/klog/v2 v2.140.0
k8s.io/kube-openapi v0.0.0-20260603220949-865597e52e25 // indirect
k8s.io/kubectl v0.36.2 // indirect
k8s.io/metrics v0.36.2 // indirect
Expand Down
15 changes: 8 additions & 7 deletions pkg/alertmanager/loader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@ package alertmanager
import (
"context"
"fmt"
"log/slog"
"net/http"
"net/url"
"strings"
"time"

"k8s.io/klog/v2"

httptransport "github.com/go-openapi/runtime/client"
"github.com/prometheus/alertmanager/api/v2/client"
"github.com/prometheus/alertmanager/api/v2/client/alert"
Expand Down Expand Up @@ -90,11 +91,11 @@ func (a *RealLoader) GetAlerts(ctx context.Context, active, silenced, inhibited,
resp, err := a.client.Alert.GetAlerts(params)
duration := time.Since(start)
if err != nil {
slog.Error("Backend call failed", "backend", "alertmanager", "operation", "alerts",
"duration_ms", duration.Milliseconds(), "error", err)
klog.FromContext(ctx).Error(err, "Backend call failed", "backend", "alertmanager", "operation", "alerts",
"duration_ms", duration.Milliseconds())
return nil, fmt.Errorf("error fetching alerts: %w", err)
}
slog.Debug("Backend call completed", "backend", "alertmanager", "operation", "alerts",
klog.FromContext(ctx).V(4).Info("Backend call completed", "backend", "alertmanager", "operation", "alerts",
"duration_ms", duration.Milliseconds(), "result_count", len(resp.Payload))

return resp.Payload, nil
Expand All @@ -111,11 +112,11 @@ func (a *RealLoader) GetSilences(ctx context.Context, filter []string) (models.G
resp, err := a.client.Silence.GetSilences(params)
duration := time.Since(start)
if err != nil {
slog.Error("Backend call failed", "backend", "alertmanager", "operation", "silences",
"duration_ms", duration.Milliseconds(), "error", err)
klog.FromContext(ctx).Error(err, "Backend call failed", "backend", "alertmanager", "operation", "silences",
"duration_ms", duration.Milliseconds())
return nil, fmt.Errorf("error fetching silences: %w", err)
}
slog.Debug("Backend call completed", "backend", "alertmanager", "operation", "silences",
klog.FromContext(ctx).V(4).Info("Backend call completed", "backend", "alertmanager", "operation", "silences",
"duration_ms", duration.Milliseconds(), "result_count", len(resp.Payload))

return resp.Payload, nil
Expand Down
Loading