From 3ad9e20ce6195f9e8846d5079a0d6c3fa43d0c52 Mon Sep 17 00:00:00 2001 From: jedi-tal Date: Fri, 14 Aug 2026 21:46:27 +0300 Subject: [PATCH] feat(prometheus): honor PROMETHEUS_URL as the default server URL `PROMETHEUS_URL` is documented in the README ("Default Prometheus server URL") but was never read: every prometheus_* handler hard-coded `http://localhost:9090` as the fallback for the optional `prometheus_url` parameter, and the Helm chart's `tools.prometheus.url` value was never passed to the container (#63, #36). The practical effect is that the ONLY way to reach a Prometheus that is not on localhost is for the model to pass `prometheus_url` on every single call. Because the parameter is optional and the schema advertises a plausible-looking default, models routinely omit it - the call then fails with "connection refused" against a port nothing serves inside the tool server's pod, and the agent burns a turn retrying. On one of our scheduled survey agents this wasted 12 of a 24-call budget in a single run. - read `PROMETHEUS_URL` (trimmed) as the default, falling back to `http://localhost:9090` when unset, so existing deployments are unchanged - build the `prometheus_url` parameter description from that same value, so the tool schema advertises the default a caller will actually get instead of always claiming localhost - render `PROMETHEUS_URL` in the chart from `tools.prometheus.url`, only when set; its previous default was schemeless (`prometheus.kagent.svc.cluster.local:9090`) and would have been rejected by `security.ValidateURL`, so it now defaults to empty = tool default The explicit `prometheus_url` parameter still wins when supplied. Closes #63 Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: jedi-tal --- README.md | 2 +- helm/kagent-tools/templates/deployment.yaml | 6 +++ helm/kagent-tools/values.yaml | 7 +++- pkg/prometheus/prometheus.go | 45 +++++++++++++++++---- pkg/prometheus/prometheus_test.go | 43 ++++++++++++++++++++ 5 files changed, 93 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 4b1c518..cb45c02 100644 --- a/README.md +++ b/README.md @@ -251,7 +251,7 @@ This Go implementation provides feature parity with the original Python tools wh Tools can be configured through environment variables: - `KUBECONFIG`: Kubernetes configuration file path -- `PROMETHEUS_URL`: Default Prometheus server URL +- `PROMETHEUS_URL`: Default Prometheus server URL for the `prometheus_*` tools, used when a call omits the optional `prometheus_url` parameter (which still wins when supplied). Must include the scheme; defaults to `http://localhost:9090`. In the Helm chart set `tools.prometheus.url`. - `GRAFANA_URL`: Default Grafana server URL - `GRAFANA_API_KEY`: Default Grafana API key diff --git a/helm/kagent-tools/templates/deployment.yaml b/helm/kagent-tools/templates/deployment.yaml index c92c9c6..a1ef9f2 100644 --- a/helm/kagent-tools/templates/deployment.yaml +++ b/helm/kagent-tools/templates/deployment.yaml @@ -95,6 +95,12 @@ spec: value: {{ .Values.otel.tracing.exporter.otlp.insecure | quote }} - name: TOKEN_PASSTHROUGH value: {{ (index .Values.tools "k8s" | default dict).tokenPassthrough | default false | quote }} + {{- with (index .Values.tools "prometheus" | default dict).url }} + # Default Prometheus server URL for the prometheus_* tools, used when a call omits + # the optional prometheus_url parameter. Unset -> the tool default (http://localhost:9090). + - name: PROMETHEUS_URL + value: {{ . | quote }} + {{- end }} {{- with .Values.tools.env }} {{- toYaml . | nindent 12 }} {{- end }} diff --git a/helm/kagent-tools/values.yaml b/helm/kagent-tools/values.yaml index b398a00..5d7f6b5 100644 --- a/helm/kagent-tools/values.yaml +++ b/helm/kagent-tools/values.yaml @@ -45,7 +45,12 @@ tools: # When false: kubectl uses in-cluster ServiceAccount. tokenPassthrough: false prometheus: - url: "prometheus.kagent.svc.cluster.local:9090" + # Default Prometheus server URL for the prometheus_* tools (rendered as PROMETHEUS_URL), + # used when a tool call omits the optional prometheus_url parameter. MUST include the + # scheme - the tools reject a URL that does not start with http:// or https://. + # Empty (the default) renders no env var, so the tools fall back to http://localhost:9090. + # e.g. "http://prometheus.kagent.svc.cluster.local:9090" + url: "" username: "" password: "" grafana: # kubectl port-forward svc/grafana 3000:3000 diff --git a/pkg/prometheus/prometheus.go b/pkg/prometheus/prometheus.go index c77e23d..1ddc181 100644 --- a/pkg/prometheus/prometheus.go +++ b/pkg/prometheus/prometheus.go @@ -7,6 +7,8 @@ import ( "io" "net/http" "net/url" + "os" + "strings" "time" "github.com/kagent-dev/tools/internal/errors" @@ -16,6 +18,33 @@ import ( "github.com/mark3labs/mcp-go/server" ) +// fallbackPrometheusURL is used when neither the prometheus_url parameter nor the +// PROMETHEUS_URL environment variable is set. +const fallbackPrometheusURL = "http://localhost:9090" + +// defaultPrometheusURL is the server URL used when a tool call omits the optional +// prometheus_url parameter. It reads the PROMETHEUS_URL environment variable, which the +// README already documents ("Default Prometheus server URL") but which was never honored. +// +// Without it, the only way to reach a non-localhost Prometheus is for the model to pass +// prometheus_url on every single call - and because the parameter is optional and its +// advertised default looks reasonable, models routinely omit it and the call fails with +// "connection refused" against a port nothing serves inside the tool server's pod. +// The env var lets an operator set the address once, per deployment. +func defaultPrometheusURL() string { + if url := strings.TrimSpace(os.Getenv("PROMETHEUS_URL")); url != "" { + return url + } + return fallbackPrometheusURL +} + +// prometheusURLDescription describes the prometheus_url parameter in the tool schema. +// It names the URL that is ACTUALLY used when the parameter is omitted, so a model reading +// the schema is not told the default is localhost when the deployment points somewhere else. +func prometheusURLDescription() string { + return fmt.Sprintf("Prometheus server URL (default: %s)", defaultPrometheusURL()) +} + // clientKey is the context key for the http client. type clientKey struct{} @@ -29,7 +58,7 @@ func getHTTPClient(ctx context.Context) *http.Client { // Prometheus tools using direct HTTP API calls func handlePrometheusQueryTool(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - prometheusURL := mcp.ParseString(request, "prometheus_url", "http://localhost:9090") + prometheusURL := mcp.ParseString(request, "prometheus_url", defaultPrometheusURL()) query := mcp.ParseString(request, "query", "") if query == "" { @@ -106,7 +135,7 @@ func handlePrometheusQueryTool(ctx context.Context, request mcp.CallToolRequest) } func handlePrometheusRangeQueryTool(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - prometheusURL := mcp.ParseString(request, "prometheus_url", "http://localhost:9090") + prometheusURL := mcp.ParseString(request, "prometheus_url", defaultPrometheusURL()) query := mcp.ParseString(request, "query", "") start := mcp.ParseString(request, "start", "") end := mcp.ParseString(request, "end", "") @@ -197,7 +226,7 @@ func handlePrometheusRangeQueryTool(ctx context.Context, request mcp.CallToolReq } func handlePrometheusLabelsQueryTool(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - prometheusURL := mcp.ParseString(request, "prometheus_url", "http://localhost:9090") + prometheusURL := mcp.ParseString(request, "prometheus_url", defaultPrometheusURL()) // Validate prometheus URL if err := security.ValidateURL(prometheusURL); err != nil { @@ -258,7 +287,7 @@ func handlePrometheusLabelsQueryTool(ctx context.Context, request mcp.CallToolRe } func handlePrometheusTargetsQueryTool(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { - prometheusURL := mcp.ParseString(request, "prometheus_url", "http://localhost:9090") + prometheusURL := mcp.ParseString(request, "prometheus_url", defaultPrometheusURL()) // Validate prometheus URL if err := security.ValidateURL(prometheusURL); err != nil { @@ -307,7 +336,7 @@ func RegisterTools(s *server.MCPServer, readOnly bool) { s.AddTool(mcp.NewTool("prometheus_query_tool", mcp.WithDescription("Execute a PromQL query against Prometheus"), mcp.WithString("query", mcp.Description("PromQL query to execute"), mcp.Required()), - mcp.WithString("prometheus_url", mcp.Description("Prometheus server URL (default: http://localhost:9090)")), + mcp.WithString("prometheus_url", mcp.Description(prometheusURLDescription())), ), telemetry.AdaptToolHandler(telemetry.WithTracing("prometheus_query_tool", handlePrometheusQueryTool))) s.AddTool(mcp.NewTool("prometheus_query_range_tool", @@ -316,17 +345,17 @@ func RegisterTools(s *server.MCPServer, readOnly bool) { mcp.WithString("start", mcp.Description("Start time (Unix timestamp or relative time)")), mcp.WithString("end", mcp.Description("End time (Unix timestamp or relative time)")), mcp.WithString("step", mcp.Description("Query resolution step (default: 15s)")), - mcp.WithString("prometheus_url", mcp.Description("Prometheus server URL (default: http://localhost:9090)")), + mcp.WithString("prometheus_url", mcp.Description(prometheusURLDescription())), ), telemetry.AdaptToolHandler(telemetry.WithTracing("prometheus_query_range_tool", handlePrometheusRangeQueryTool))) s.AddTool(mcp.NewTool("prometheus_label_names_tool", mcp.WithDescription("Get all available labels from Prometheus"), - mcp.WithString("prometheus_url", mcp.Description("Prometheus server URL (default: http://localhost:9090)")), + mcp.WithString("prometheus_url", mcp.Description(prometheusURLDescription())), ), telemetry.AdaptToolHandler(telemetry.WithTracing("prometheus_label_names_tool", handlePrometheusLabelsQueryTool))) s.AddTool(mcp.NewTool("prometheus_targets_tool", mcp.WithDescription("Get all Prometheus targets and their status"), - mcp.WithString("prometheus_url", mcp.Description("Prometheus server URL (default: http://localhost:9090)")), + mcp.WithString("prometheus_url", mcp.Description(prometheusURLDescription())), ), telemetry.AdaptToolHandler(telemetry.WithTracing("prometheus_targets_tool", handlePrometheusTargetsQueryTool))) s.AddTool(mcp.NewTool("prometheus_promql_tool", diff --git a/pkg/prometheus/prometheus_test.go b/pkg/prometheus/prometheus_test.go index 1e8ffc4..46ef48e 100644 --- a/pkg/prometheus/prometheus_test.go +++ b/pkg/prometheus/prometheus_test.go @@ -127,6 +127,49 @@ func TestGetHTTPClientDefault(t *testing.T) { assert.Equal(t, custom, getHTTPClient(ctx)) } +func TestDefaultPrometheusURL(t *testing.T) { + t.Run("falls back to localhost when PROMETHEUS_URL is unset", func(t *testing.T) { + t.Setenv("PROMETHEUS_URL", "") + assert.Equal(t, fallbackPrometheusURL, defaultPrometheusURL()) + assert.Contains(t, prometheusURLDescription(), fallbackPrometheusURL) + }) + + t.Run("honors PROMETHEUS_URL", func(t *testing.T) { + t.Setenv("PROMETHEUS_URL", " http://thanos-query.monitoring.svc.cluster.local:10902 ") + assert.Equal(t, "http://thanos-query.monitoring.svc.cluster.local:10902", defaultPrometheusURL()) + assert.Contains(t, prometheusURLDescription(), "http://thanos-query.monitoring.svc.cluster.local:10902") + }) +} + +// recordingRoundTripper records the URL of the last request it served. +type recordingRoundTripper struct { + lastURL string +} + +func (r *recordingRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + r.lastURL = req.URL.String() + return createMockResponse(200, `{"status":"success","data":{"resultType":"vector","result":[]}}`), nil +} + +// A call that omits the optional prometheus_url parameter must reach the server configured +// via PROMETHEUS_URL, not localhost. +func TestPrometheusURLDefaultsToEnv(t *testing.T) { + t.Setenv("PROMETHEUS_URL", "http://thanos-query.monitoring.svc.cluster.local:10902") + + rt := &recordingRoundTripper{} + ctx := contextWithMockClient(&http.Client{Transport: rt}) + + request := mcp.CallToolRequest{} + request.Params.Arguments = map[string]interface{}{"query": "up"} + + result, err := handlePrometheusQueryTool(ctx, request) + + assert.NoError(t, err) + assert.NotNil(t, result) + assert.False(t, result.IsError) + assert.Contains(t, rt.lastURL, "http://thanos-query.monitoring.svc.cluster.local:10902/api/v1/query") +} + // mockRoundTripper is used to mock HTTP responses for testing type mockRoundTripper struct { response *http.Response