Pr/har logging #1946

coderabbitai · 2026-05-07T06:09:10Z

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

claude CLI is not validated before use.

captain gets an install guard (command -v), but the claude CLI invoked on line 103 has no equivalent check. A missing claude binary will fail with an unhelpful command not found error instead of an actionable message.

🛡️ Proposed fix

.PHONY: PROPERTIES.md PROPERTIES.md: captain + `@command` -v claude >/dev/null || (echo "Claude CLI is required. Install it from https://docs.anthropic.com/en/docs/claude-code/cli-usage" && exit 1) claude -p --permission-mode acceptEdits --verbose --output-format stream-json --model sonnet \ "/properties-doc Refresh PROPERTIES.md and PROPERTIES.schema.json from the current source tree. Cross-reference ../incident-commander, ../config-db, ../canary-checker, ../flanksource-ui, and ../commons when present. Run in update mode and preserve hand-written prose sections." \ | captain

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

.PHONY: PROPERTIES.md

PROPERTIES.md: captain

claude -p --permission-mode acceptEdits --verbose --output-format stream-json --model sonnet \

"/properties-doc Refresh PROPERTIES.md and PROPERTIES.schema.json from the current source tree. Cross-reference ../incident-commander, ../config-db, ../canary-checker, ../flanksource-ui, and ../commons when present. Run in update mode and preserve hand-written prose sections." \

| captain

.PHONY: PROPERTIES.md

PROPERTIES.md: captain

`@command` -v claude >/dev/null || (echo "Claude CLI is required. Install it from https://docs.anthropic.com/en/docs/claude-code/cli-usage" && exit 1)

claude -p --permission-mode acceptEdits --verbose --output-format stream-json --model sonnet \

"/properties-doc Refresh PROPERTIES.md and PROPERTIES.schema.json from the current source tree. Cross-reference ../incident-commander, ../config-db, ../canary-checker, ../flanksource-ui, and ../commons when present. Run in update mode and preserve hand-written prose sections." \

| captain

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 101 - 105, The Makefile target PROPERTIES.md invokes the claude CLI without verifying it exists; add a pre-check (like using `command -v claude` or an equivalent guard) before the claude invocation in the PROPERTIES.md recipe so it prints a clear, actionable error and exits if claude is missing; update the PROPERTIES.md target (the recipe that runs claude -p ... | captain) to perform this validation and fail fast with a helpful message when claude is not found.

-Original file line number
+Diff line change
@@ Expand Up / @@ -23,3 +23,4 @@ generate-openapi @@
     ginkgo*.json
     migrate-e2e
     hack/generate-schemas/generate-schemas
+    .gavel/

-Original file line number
+Diff line change
@@ -1,6 +1,27 @@
     version: '3'
     tasks:
+      # test:
+      #   desc: |
+      #     Run the unit test suite via gavel, mirroring the `test` job in
+      #     .github/workflows/test.yaml. Excludes the heavy benchmark, generator,
+      #     spec, and e2e packages so it matches CI scope.
+      #
+      #     Anything after `--` is passed straight through to gavel, so:
+      #       task test -- --focus "MyFlow"
+      #       task test -- ./query --extra-args=--ginkgo.label-filter=focus
+      #       task test -- --dry-run
+      #     Without extra args, runs every unit package matching CI.
+      #   cmds:
+      #     - |
+      #       gavel test --timeout 30m --test-timeout 15m \
+      #         --ignore ./bench \
+      #         --ignore ./hack \
+      #         --ignore ./specs \
+      #         --ignore ./tests/e2e \
+      #         --ignore ./tests/e2e-blobs \
+      #         {{if .CLI_ARGS}}{{.CLI_ARGS}}{{else}}./...{{end}}
       test:migrate:
         desc: |
           Mirrors the `migrate` job in .github/workflows/test.yaml using an
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -4,7 +4,6 @@ import ( @@
     	"crypto/tls"
     	"fmt"
     	"net/http"
-    	"os"
     	"github.com/aws/aws-sdk-go-v2/aws"
     	"github.com/aws/aws-sdk-go-v2/config"
@@ Expand All / @@ -14,7 +13,6 @@ import ( @@
     	"github.com/flanksource/duty/context"
     	"github.com/flanksource/duty/models"
     	"github.com/flanksource/duty/types"
-    	"github.com/henvic/httpretty"
     )
     // +kubebuilder:object:generate=true
@@ Expand Down Expand Up @@
     	tr = &http.Transport{
     		TLSClientConfig: &tls.Config{InsecureSkipVerify: t.SkipTLSVerify},
     	}
-    	harCollector := o.HARCollector
-    	if harCollector == nil {
-    		harCollector = ctx.HARCollector()
-    	}
-    	if harCollector != nil {
-    		tr = harCollector.Middleware()(tr)
-    	}
-    	if ctx.IsTrace() && harCollector == nil {
-    		httplogger := &httpretty.Logger{
-    			Time:           true,
-    			TLS:            ctx.Logger.IsLevelEnabled(7),
-    			RequestHeader:  true,
-    			RequestBody:    ctx.Logger.IsLevelEnabled(8),
-    			ResponseHeader: true,
-    			ResponseBody:   ctx.Logger.IsLevelEnabled(9),
-    			Colors:         true,
-    			Formatters:     []httpretty.Formatter{&httpretty.JSONFormatter{}},
-    		}
-    		httplogger.SetOutput(os.Stderr)
-    		tr = httplogger.RoundTripper(tr)
-    	}
+    	tr = applyHTTPObservability(ctx, "aws", tr, o.HARCollector)
     	options := []func(*config.LoadOptions) error{
     		config.WithHTTPClient(&http.Client{Transport: tr}),
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -3,6 +3,7 @@ package connection @@
     import (
     	"encoding/base64"
     	"fmt"
+    	"net/http"
     	"github.com/flanksource/duty/context"
     	dutyKube "github.com/flanksource/duty/kubernetes"
@@ Expand Down Expand Up @@
     	if err != nil {
     		return nil, nil, fmt.Errorf("failed to create REST config for cluster resource: %w", err)
     	}
+    	o := types.NewClientOptions(opts...)
+    	if middleware := httpObservabilityMiddleware(ctx, "kubernetes", o.HARCollector); middleware != nil {
+    		clusterResourceRestConfig.WrapTransport = func(rt http.RoundTripper) http.RoundTripper {
+    			return middleware(rt)
+    		}
+    	}
     	clientset, err := kubernetes.NewForConfig(clusterResourceRestConfig)
     	if err != nil {
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pr/har logging #1946

Uh oh!

Diff view

Diff view

There are no files selected for viewing

coderabbitai Bot May 7, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

-Original file line number
+Diff line change
@@ Expand Up / @@ -3,8 +3,17 @@ package connection @@
     import (
     	"encoding/json"
     	"fmt"
+    	netHTTP "net/http"
+    	"net/url"
+    	"strings"
     	"time"
+    	"github.com/flanksource/commons/har"
+    	commonsHTTP "github.com/flanksource/commons/http"
+    	"github.com/flanksource/commons/http/middlewares"
+    	"github.com/flanksource/commons/logger"
+    	"github.com/flanksource/commons/logger/httpretty"
+    	"github.com/flanksource/commons/properties"
     	"github.com/patrickmn/go-cache"
     )
@@ Expand All @@
     		return fmt.Sprintf("%s-%s-%s", cloud, m, identifiers)
     	}
     }
+    type observabilityContext interface {
+    	HARCollector() *har.Collector
+    	EffectiveHARCollector(feature string, explicit *har.Collector) *har.Collector
+    	EffectiveHARLevel(feature string) (logger.LogLevel, string)
+    	HTTPLoggingContent(feature string) (bool, bool)
+    }
+    func effectiveHARCollector(ctx any, feature string, explicit *har.Collector) *har.Collector {
+    	if c, ok := ctx.(observabilityContext); ok {
+    		return c.EffectiveHARCollector(feature, explicit)
+    	}
+    	return explicit
+    }
+    func applyHTTPObservability(ctx any, feature string, base netHTTP.RoundTripper, explicit *har.Collector) netHTTP.RoundTripper {
+    	if base == nil {
+    		base = netHTTP.DefaultTransport
+    	}
+    	if middleware := harCollectorMiddleware(ctx, feature, explicit); middleware != nil {
+    		base = middleware(base)
+    	}
+    	if c, ok := ctx.(observabilityContext); ok {
+    		headers, bodies := c.HTTPLoggingContent(feature)
+    		base = httpLoggerWithContent(base, headers, bodies)
+    	}
+    	return base
+    }
+    func httpObservabilityMiddleware(ctx any, feature string, explicit *har.Collector) middlewares.Middleware {
+    	if effectiveHARCollector(ctx, feature, explicit) != nil {
+    		return func(rt netHTTP.RoundTripper) netHTTP.RoundTripper {
+    			return applyHTTPObservability(ctx, feature, rt, explicit)
+    		}
+    	}
+    	if c, ok := ctx.(observabilityContext); ok {
+    		headers, _ := c.HTTPLoggingContent(feature)
+    		if headers {
+    			return func(rt netHTTP.RoundTripper) netHTTP.RoundTripper {
+    				return applyHTTPObservability(ctx, feature, rt, explicit)
+    			}
+    		}
+    	}
+    	return nil
+    }
+    func applyHTTPClientObservability(ctx any, feature string, client *commonsHTTP.Client, explicit *har.Collector) middlewares.Middleware {
+    	if client == nil {
+    		return nil
+    	}
+    	var tokenTransport middlewares.Middleware
+    	level := logger.Info
+    	if c, ok := ctx.(observabilityContext); ok {
+    		level, _ = c.EffectiveHARLevel(feature)
+    	}
+    	if explicit != nil && level < logger.Debug {
+    		level = logger.Trace
+    	}
+    	if collector := effectiveHARCollector(ctx, feature, explicit); collector != nil && level >= logger.Debug {
+    		if level >= logger.Trace {
+    			client.HARCollector(collector)
+    		} else {
+    			middleware := metadataHARMiddleware(collector)
+    			client.Use(middleware)
+    			tokenTransport = middleware
+    		}
+    	}
+    	if c, ok := ctx.(observabilityContext); ok {
+    		headers, bodies := c.HTTPLoggingContent(feature)
+    		if headers {
+    			logMiddleware := func(rt netHTTP.RoundTripper) netHTTP.RoundTripper {
+    				return httpLoggerWithContent(rt, headers, bodies)
+    			}
+    			client.Use(logMiddleware)
+    			if tokenTransport == nil {
+    				tokenTransport = logMiddleware
+    			} else {
+    				existing := tokenTransport
+    				tokenTransport = func(rt netHTTP.RoundTripper) netHTTP.RoundTripper {
+    					return logMiddleware(existing(rt))
+    				}
+    			}
+    		}
+    	}
+    	return tokenTransport
+    }
+    func harTokenTransport(ctx any, feature string, explicit *har.Collector) middlewares.Middleware {
+    	return func(rt netHTTP.RoundTripper) netHTTP.RoundTripper {
+    		return applyHTTPObservability(ctx, feature, rt, explicit)
+    	}
+    }
+    func harCollectorMiddleware(ctx any, feature string, explicit *har.Collector) middlewares.Middleware {
+    	level := logger.Info
+    	if c, ok := ctx.(observabilityContext); ok {
+    		level, _ = c.EffectiveHARLevel(feature)
+    	}
+    	if explicit != nil && level < logger.Debug {
+    		level = logger.Trace
+    	}
+    	collector := effectiveHARCollector(ctx, feature, explicit)
+    	if collector == nil || level < logger.Debug {
+    		return nil
+    	}
+    	if level >= logger.Trace {
+    		return collector.Middleware()
+    	}
+    	return metadataHARMiddleware(collector)
+    }
+    func httpLoggerWithContent(rt netHTTP.RoundTripper, headers, bodies bool) netHTTP.RoundTripper {
+    	if !headers {
+    		return rt
+    	}
+    	l := &httpretty.Logger{
+    		Time:            true,
+    		TLS:             true,
+    		Auth:            true,
+    		RequestHeader:   true,
+    		RequestBody:     bodies,
+    		ResponseHeader:  true,
+    		ResponseBody:    bodies,
+    		Colors:          true,
+    		Formatters:      []httpretty.Formatter{&httpretty.JSONFormatter{}},
+    		MaxResponseBody: int64(properties.Int(4*1024, "http.log.response.body.length")),
+    	}
+    	l.SkipHeader(logger.SensitiveHeaders)
+    	return l.RoundTripper(rt)
+    }
+    func metadataHARMiddleware(collector *har.Collector) middlewares.Middleware {
+    	return func(next netHTTP.RoundTripper) netHTTP.RoundTripper {
+    		return middlewares.RoundTripperFunc(func(req *netHTTP.Request) (*netHTTP.Response, error) {
+    			started := time.Now()
+    			entry := &har.Entry{
+    				StartedDateTime: started.UTC().Format(time.RFC3339),
+    				Request: har.Request{
+    					Method:      req.Method,
+    					URL:         req.URL.String(),
+    					HTTPVersion: harHTTPVersion(req.Proto),
+    					Cookies:     []har.Cookie{},
+    					Headers:     toHARHeaders(logger.SanitizeHeaders(req.Header)),
+    					QueryString: toHARQueryString(req.URL.Query()),
+    					HeadersSize: -1,
+    					BodySize:    -1,
+    				},
+    			}
+    			waitStart := time.Now()
+    			resp, err := next.RoundTrip(req)
+    			waitMs := float64(time.Since(waitStart).Microseconds()) / 1000.0
+    			entry.Timings = har.Timings{Wait: waitMs}
+    			entry.Time = waitMs
+    			if resp != nil {
+    				entry.Response = har.Response{
+    					Status:      resp.StatusCode,
+    					StatusText:  resp.Status,
+    					HTTPVersion: harHTTPVersion(resp.Proto),
+    					Cookies:     []har.Cookie{},
+    					Headers:     toHARHeaders(logger.SanitizeHeaders(resp.Header)),
+    					Content:     har.Content{Size: -1},
+    					RedirectURL: "",
+    					HeadersSize: -1,
+    					BodySize:    -1,
+    				}
+    			} else {
+    				// Transport error: no response object. Use -1 sentinels (HAR spec
+    				// for "size unknown") so consumers don't read Status=0 as a
+    				// successful empty response.
+    				entry.Response = har.Response{
+    					Cookies:     []har.Cookie{},
+    					Headers:     []har.Header{},
+    					Content:     har.Content{Size: -1},
+    					HeadersSize: -1,
+    					BodySize:    -1,
+    				}
+    			}
+    			collector.Add(entry)
+    			return resp, err
+    		})
+    	}
+    }
+    func toHARHeaders(h netHTTP.Header) []har.Header {
+    	headers := make([]har.Header, 0, len(h))
+    	for name, vals := range h {
+    		for _, v := range vals {
+    			headers = append(headers, har.Header{Name: name, Value: v})
+    		}
+    	}
+    	return headers
+    }
+    func toHARQueryString(q url.Values) []har.QueryString {
+    	qs := make([]har.QueryString, 0, len(q))
+    	for k, vs := range q {
+    		for _, v := range vs {
+    			qs = append(qs, har.QueryString{Name: k, Value: v})
+    		}
+    	}
+    	return qs
+    }
+    func harHTTPVersion(proto string) string {
+    	if strings.TrimSpace(proto) == "" {
+    		return "HTTP/1.1"
+    	}
+    	return proto
+    }

Pr/har logging #1946

Uh oh!

Pr/har logging #1946

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

coderabbitai Bot May 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!