Repository: prometheus/common
Pull request #746 (2025-01-13 18:50:43)

feat(promslog): implement reserved keys, rename duplicates
This commit adds support for "reserved" keys, where upon detecting a
duplicate log attribute key, it will rename the key to prevent
collisions/issues with core slog attribute keys in use.

The reserved keys are:
- `level`
- `source` (for standard slog style)
- `caller` (for legacy go-kit style)
- `time`   (for standard slog style)
- `ts`     (for legacy go-kit style)

By supporting reserved keys, we allow users of the library to use these
keys for their own supplemental log attributes.

Fixes: #745

Example test output:
```
=== RUN   TestReservedKeys/slog_log_style
time=2025-01-13T18:32:46.508Z level=INFO source=slog_test.go:212 msg="reserved keys test for slog_log_style" logged_level="surprise! I'm a string" logged_source="surprise! I'm a string" logged_time="surprise! I'm a string"

=== RUN   TestReservedKeys/go-kit_log_style
ts=2025-01-13T18:32:46.508Z level=info caller=slog_test.go:212 msg="reserved keys test for go-kit_log_style" logged_level="surprise! I'm a string" logged_caller="surprise! I'm a string" logged_ts="surprise! I'm a string"
```

Note: this implementation only technically removes duplicates of our
core reserved keys. If a user adds multiple instances of a reserved key,
the rest get renamed to `logged_$key`, which means there could be
duplicate attributes with `logged_$key`. This is mostly to simplify
implementation so we don't need to bother reference counting in the
replace attr function to do things like `logged_$key1`, `logged_$key2`,
etc.

Signed-off-by: TJ Hoplock <t.hoplock@gmail.com>


tjhop: Huh, I didn't get a lint error locally. Looking
tjhop: :rocket: 

Patch:
diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml
new file mode 100644
index 0000000..f7fc82f
--- /dev/null
+++ b/.github/workflows/codeql.yml
@@ -0,0 +1,50 @@
+name: "CodeQL"
+
+on:
+  push:
+    branches: [ "main" ]
+  pull_request:
+    branches: [ "main" ]
+  schedule:
+    - cron: '24 1 * * 5'
+
+permissions:
+  contents: read
+
+jobs:
+  analyze:
+    name: Analyze
+    runs-on: ubuntu-latest
+    timeout-minutes: 360
+    permissions:
+      # required for all workflows
+      security-events: write
+
+      # only required for workflows in private repositories
+      actions: read
+      contents: read
+
+    strategy:
+      fail-fast: false
+      matrix:
+        language: [ 'go' ]
+
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
+
+      # Initializes the CodeQL tools for scanning.
+      - name: Initialize CodeQL
+        uses: github/codeql-action/init@e5f05b81d5b6ff8cfa111c80c22c5fd02a384118 # v3.23.0
+        with:
+          languages: ${{ matrix.language }}
+
+      # Autobuild attempts to build any compiled languages (C/C++, C#, Go, Java, or Swift).
+      # If this step fails, then you should remove it and run the build manually (see below)
+      - name: Autobuild
+        uses: github/codeql-action/autobuild@e5f05b81d5b6ff8cfa111c80c22c5fd02a384118 # v3.23.0
+
+      - name: Perform CodeQL Analysis
+        uses: github/codeql-action/analyze@e5f05b81d5b6ff8cfa111c80c22c5fd02a384118 # v3.23.0
+        with:
+          category: "/language:${{matrix.language}}"
diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml
index 01b943b..0c00c41 100644
--- a/.github/workflows/golangci-lint.yml
+++ b/.github/workflows/golangci-lint.yml
@@ -36,4 +36,4 @@ jobs:
         uses: golangci/golangci-lint-action@971e284b6050e8a5849b72094c50ab08da042db8 # v6.1.1
         with:
           args: --verbose
-          version: v1.62.0
+          version: v1.63.4
diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml
new file mode 100644
index 0000000..4793ce7
--- /dev/null
+++ b/.github/workflows/scorecard.yml
@@ -0,0 +1,56 @@
+name: Scorecard supply-chain security
+
+on:
+  # For Branch-Protection check. Only the default branch is supported. See
+  # https://github.com/ossf/scorecard/blob/main/docs/checks.md#branch-protection
+  branch_protection_rule:
+  # To guarantee Maintained check is occasionally updated. See
+  # https://github.com/ossf/scorecard/blob/main/docs/checks.md#maintained
+  schedule:
+    - cron: '26 18 * * 2'
+  push:
+    branches: [ "main" ]
+
+# Declare default permissions as read only.
+permissions: read-all
+
+jobs:
+  analysis:
+    name: Scorecard analysis
+    runs-on: ubuntu-latest
+    permissions:
+      # Needed to upload the results to code-scanning dashboard.
+      security-events: write
+      # Needed to publish results and get a badge (see publish_results below).
+      id-token: write
+      # Uncomment the permissions below if installing in a private repository.
+      # contents: read
+      # actions: read
+
+    steps:
+      - name: "Checkout code"
+        uses: actions/checkout@93ea575cb5d8a053eaa0ac8fa3b40d7e05a33cc8 # v3.1.0
+        with:
+          persist-credentials: false
+
+      - name: "Run analysis"
+        uses: ossf/scorecard-action@e38b1902ae4f44df626f11ba0734b14fb91f8f86 # v2.1.2
+        with:
+          results_file: results.sarif
+          results_format: sarif
+          publish_results: true
+
+      # Upload the results as artifacts (optional). Commenting out will disable uploads of run results in SARIF
+      # format to the repository Actions tab.
+      - name: "Upload artifact"
+        uses: actions/upload-artifact@3cea5372237819ed00197afe530f5a7ea3e805c8 # v3.1.0
+        with:
+          name: SARIF file
+          path: results.sarif
+          retention-days: 5
+
+      # Upload the results to GitHub's code scanning dashboard.
+      - name: "Upload to code-scanning"
+        uses: github/codeql-action/upload-sarif@17573ee1cc1b9d061760f3a006fc4aac4f944fd5 # v2.2.4
+        with:
+          sarif_file: results.sarif
diff --git a/Makefile.common b/Makefile.common
index fc47bdb..d1576bb 100644
--- a/Makefile.common
+++ b/Makefile.common
@@ -61,7 +61,7 @@ PROMU_URL     := https://github.com/prometheus/promu/releases/download/v$(PROMU_
 SKIP_GOLANGCI_LINT :=
 GOLANGCI_LINT :=
 GOLANGCI_LINT_OPTS ?=
-GOLANGCI_LINT_VERSION ?= v1.62.0
+GOLANGCI_LINT_VERSION ?= v1.63.4
 # golangci-lint only supports linux, darwin and windows platforms on i386/amd64/arm64.
 # windows isn't included here because of the path separator being different.
 ifeq ($(GOHOSTOS),$(filter $(GOHOSTOS),linux darwin))
diff --git a/README.md b/README.md
index 954cc91..f7d5342 100644
--- a/README.md
+++ b/README.md
@@ -1,5 +1,7 @@
 # Common
 ![circleci](https://circleci.com/gh/prometheus/common/tree/main.svg?style=shield)
+[![OpenSSF Scorecard](https://api.securityscorecards.dev/projects/github.com/prometheus/common/badge)](https://securityscorecards.dev/viewer/?uri=github.com/prometheus/common)
+
 
 This repository contains Go libraries that are shared across Prometheus
 components and libraries. They are considered internal to Prometheus, without
diff --git a/config/headers.go b/config/headers.go
index 7276742..9beaae2 100644
--- a/config/headers.go
+++ b/config/headers.go
@@ -24,9 +24,9 @@ import (
 	"strings"
 )
 
-// reservedHeaders that change the connection, are set by Prometheus, or can
+// ReservedHeaders that change the connection, are set by Prometheus, or can
 // be changed otherwise.
-var reservedHeaders = map[string]struct{}{
+var ReservedHeaders = map[string]struct{}{
 	"Authorization":                       {},
 	"Host":                                {},
 	"Content-Encoding":                    {},
@@ -72,7 +72,7 @@ func (h *Headers) SetDirectory(dir string) {
 // Validate validates the Headers config.
 func (h *Headers) Validate() error {
 	for n := range h.Headers {
-		if _, ok := reservedHeaders[http.CanonicalHeaderKey(n)]; ok {
+		if _, ok := ReservedHeaders[http.CanonicalHeaderKey(n)]; ok {
 			return fmt.Errorf("setting header %q is not allowed", http.CanonicalHeaderKey(n))
 		}
 	}
diff --git a/config/http_config.go b/config/http_config.go
index 57ec252..6380908 100644
--- a/config/http_config.go
+++ b/config/http_config.go
@@ -52,7 +52,8 @@ var (
 		http2Enabled:      true,
 		// 5 minutes is typically above the maximum sane scrape interval. So we can
 		// use keepalive for all configurations.
-		idleConnTimeout: 5 * time.Minute,
+		idleConnTimeout:  5 * time.Minute,
+		newTLSConfigFunc: NewTLSConfigWithContext,
 	}
 )
 
@@ -452,8 +453,12 @@ func (a *BasicAuth) UnmarshalYAML(unmarshal func(interface{}) error) error {
 // by net.Dialer.
 type DialContextFunc func(context.Context, string, string) (net.Conn, error)
 
+// NewTLSConfigFunc returns tls.Config.
+type NewTLSConfigFunc func(context.Context, *TLSConfig, ...TLSConfigOption) (*tls.Config, error)
+
 type httpClientOptions struct {
 	dialContextFunc   DialContextFunc
+	newTLSConfigFunc  NewTLSConfigFunc
 	keepAlivesEnabled bool
 	http2Enabled      bool
 	idleConnTimeout   time.Duration
@@ -473,13 +478,23 @@ func (f httpClientOptionFunc) applyToHTTPClientOptions(options *httpClientOption
 	f(options)
 }
 
-// WithDialContextFunc allows you to override func gets used for the actual dialing. The default is `net.Dialer.DialContext`.
+// WithDialContextFunc allows you to override the func gets used for the dialing.
+// The default is `net.Dialer.DialContext`.
 func WithDialContextFunc(fn DialContextFunc) HTTPClientOption {
 	return httpClientOptionFunc(func(opts *httpClientOptions) {
 		opts.dialContextFunc = fn
 	})
 }
 
+// WithNewTLSConfigFunc allows you to override the func that creates the TLS config
+// from the prometheus http config.
+// The default is `NewTLSConfigWithContext`.
+func WithNewTLSConfigFunc(newTLSConfigFunc NewTLSConfigFunc) HTTPClientOption {
+	return httpClientOptionFunc(func(opts *httpClientOptions) {
+		opts.newTLSConfigFunc = newTLSConfigFunc
+	})
+}
+
 // WithKeepAlivesDisabled allows to disable HTTP keepalive.
 func WithKeepAlivesDisabled() HTTPClientOption {
 	return httpClientOptionFunc(func(opts *httpClientOptions) {
@@ -670,7 +685,7 @@ func NewRoundTripperFromConfigWithContext(ctx context.Context, cfg HTTPClientCon
 		return rt, nil
 	}
 
-	tlsConfig, err := NewTLSConfig(&cfg.TLSConfig, WithSecretManager(opts.secretManager))
+	tlsConfig, err := opts.newTLSConfigFunc(ctx, &cfg.TLSConfig, WithSecretManager(opts.secretManager))
 	if err != nil {
 		return nil, err
 	}
@@ -679,6 +694,7 @@ func NewRoundTripperFromConfigWithContext(ctx context.Context, cfg HTTPClientCon
 	if err != nil {
 		return nil, err
 	}
+
 	if tlsSettings.immutable() {
 		// No need for a RoundTripper that reloads the files automatically.
 		return newRT(tlsConfig)
diff --git a/promslog/slog.go b/promslog/slog.go
index 6e8fbab..11c33fb 100644
--- a/promslog/slog.go
+++ b/promslog/slog.go
@@ -25,6 +25,7 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
+	"time"
 )
 
 type LogStyle string
@@ -32,6 +33,8 @@ type LogStyle string
 const (
 	SlogStyle  LogStyle = "slog"
 	GoKitStyle LogStyle = "go-kit"
+
+	reservedKeyPrefix = "logged_"
 )
 
 var (
@@ -43,26 +46,51 @@ var (
 	goKitStyleReplaceAttrFunc = func(groups []string, a slog.Attr) slog.Attr {
 		key := a.Key
 		switch key {
-		case slog.TimeKey:
-			a.Key = "ts"
-
-			// This timestamp format differs from RFC3339Nano by using .000 instead
-			// of .999999999 which changes the timestamp from 9 variable to 3 fixed
-			// decimals (.130 instead of .130987456).
-			t := a.Value.Time()
-			a.Value = slog.StringValue(t.UTC().Format("2006-01-02T15:04:05.000Z07:00"))
-		case slog.SourceKey:
-			a.Key = "caller"
-			src, _ := a.Value.Any().(*slog.Source)
+		case slog.TimeKey, "ts":
+			if t, ok := a.Value.Any().(time.Time); ok {
+				a.Key = "ts"
 
-			switch callerAddFunc {
-			case true:
-				a.Value = slog.StringValue(filepath.Base(src.File) + "(" + filepath.Base(src.Function) + "):" + strconv.Itoa(src.Line))
-			default:
-				a.Value = slog.StringValue(filepath.Base(src.File) + ":" + strconv.Itoa(src.Line))
+				// This timestamp format differs from RFC3339Nano by using .000 instead
+				// of .999999999 which changes the timestamp from 9 variable to 3 fixed
+				// decimals (.130 instead of .130987456).
+				a.Value = slog.StringValue(t.UTC().Format("2006-01-02T15:04:05.000Z07:00"))
+			} else {
+				// If we can't cast the any from the value to a
+				// time.Time, it means the caller logged
+				// another attribute with a key of `ts`.
+				// Prevent duplicate keys (necessary for proper
+				// JSON) by renaming the key to `logged_ts`.
+				a.Key = reservedKeyPrefix + key
+			}
+		case slog.SourceKey, "caller":
+			if src, ok := a.Value.Any().(*slog.Source); ok {
+				a.Key = "caller"
+				switch callerAddFunc {
+				case true:
+					a.Value = slog.StringValue(filepath.Base(src.File) + "(" + filepath.Base(src.Function) + "):" + strconv.Itoa(src.Line))
+				default:
+					a.Value = slog.StringValue(filepath.Base(src.File) + ":" + strconv.Itoa(src.Line))
+				}
+			} else {
+				// If we can't cast the any from the value to
+				// an *slog.Source, it means the caller logged
+				// another attribute with a key of `caller`.
+				// Prevent duplicate keys (necessary for proper
+				// JSON) by renaming the key to
+				// `logged_caller`.
+				a.Key = reservedKeyPrefix + key
 			}
 		case slog.LevelKey:
-			a.Value = slog.StringValue(strings.ToLower(a.Value.String()))
+			if lvl, ok := a.Value.Any().(slog.Level); ok {
+				a.Value = slog.StringValue(strings.ToLower(lvl.String()))
+			} else {
+				// If we can't cast the any from the value to
+				// an slog.Level, it means the caller logged
+				// another attribute with a key of `level`.
+				// Prevent duplicate keys (necessary for proper
+				// JSON) by renaming the key to `logged_level`.
+				a.Key = reservedKeyPrefix + key
+			}
 		default:
 		}
 
@@ -72,11 +100,38 @@ var (
 		key := a.Key
 		switch key {
 		case slog.TimeKey:
-			t := a.Value.Time()
-			a.Value = slog.TimeValue(t.UTC())
+			if t, ok := a.Value.Any().(time.Time); ok {
+				a.Value = slog.TimeValue(t.UTC())
+			} else {
+				// If we can't cast the any from the value to a
+				// time.Time, it means the caller logged
+				// another attribute with a key of `time`.
+				// Prevent duplicate keys (necessary for proper
+				// JSON) by renaming the key to `logged_time`.
+				a.Key = reservedKeyPrefix + key
+			}
 		case slog.SourceKey:
-			src, _ := a.Value.Any().(*slog.Source)
-			a.Value = slog.StringValue(filepath.Base(src.File) + ":" + strconv.Itoa(src.Line))
+			if src, ok := a.Value.Any().(*slog.Source); ok {
+				a.Value = slog.StringValue(filepath.Base(src.File) + ":" + strconv.Itoa(src.Line))
+			} else {
+				// If we can't cast the any from the value to
+				// an *slog.Source, it means the caller logged
+				// another attribute with a key of `source`.
+				// Prevent duplicate keys (necessary for proper
+				// JSON) by renaming the key to
+				// `logged_source`.
+				a.Key = reservedKeyPrefix + key
+			}
+		case slog.LevelKey:
+			if _, ok := a.Value.Any().(slog.Level); !ok {
+				// If we can't cast the any from the value to
+				// an slog.Level, it means the caller logged
+				// another attribute with a key of `level`.
+				// Prevent duplicate keys (necessary for proper
+				// JSON) by renaming the key to
+				// `logged_level`.
+				a.Key = reservedKeyPrefix + key
+			}
 		default:
 		}
 


Test patch:
diff --git a/config/headers_test.go b/config/headers_test.go
index 39c6f9f..c807fbc 100644
--- a/config/headers_test.go
+++ b/config/headers_test.go
@@ -22,10 +22,10 @@ import (
 )
 
 func TestReservedHeaders(t *testing.T) {
-	for k := range reservedHeaders {
+	for k := range ReservedHeaders {
 		l := http.CanonicalHeaderKey(k)
 		if k != l {
-			t.Errorf("reservedHeaders keys should be lowercase: got %q, expected %q", k, http.CanonicalHeaderKey(k))
+			t.Errorf("ReservedHeaders keys should be lowercase: got %q, expected %q", k, http.CanonicalHeaderKey(k))
 		}
 	}
 }
diff --git a/promslog/slog_test.go b/promslog/slog_test.go
index fc824e0..6da14d9 100644
--- a/promslog/slog_test.go
+++ b/promslog/slog_test.go
@@ -188,3 +188,42 @@ func TestTruncateSourceFileName_GoKitStyle(t *testing.T) {
 		t.Errorf("Expected no directory separators in caller, got: %s", output)
 	}
 }
+
+func TestReservedKeys(t *testing.T) {
+	var buf bytes.Buffer
+	reservedKeyTestVal := "surprise! I'm a string"
+
+	tests := map[string]struct {
+		logStyle  LogStyle
+		levelKey  string
+		sourceKey string
+		timeKey   string
+	}{
+		"slog_log_style":   {logStyle: SlogStyle, levelKey: "level", sourceKey: "source", timeKey: "time"},
+		"go-kit_log_style": {logStyle: GoKitStyle, levelKey: "level", sourceKey: "caller", timeKey: "ts"},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			buf.Reset() // Ensure buf is reset prior to tests
+			config := &Config{Writer: &buf, Style: tc.logStyle}
+			logger := New(config)
+
+			logger.LogAttrs(context.Background(),
+				slog.LevelInfo,
+				"reserved keys test for "+name,
+				slog.String(tc.levelKey, reservedKeyTestVal),
+				slog.String(tc.sourceKey, reservedKeyTestVal),
+				slog.String(tc.timeKey, reservedKeyTestVal),
+			)
+
+			output := buf.String()
+			require.Containsf(t, output, fmt.Sprintf("%s%s=\"%s\"", reservedKeyPrefix, tc.levelKey, reservedKeyTestVal), "Expected duplicate level key to be renamed")
+			require.Containsf(t, output, fmt.Sprintf("%s%s=\"%s\"", reservedKeyPrefix, tc.sourceKey, reservedKeyTestVal), "Expected duplicate source key to be renamed")
+			require.Containsf(t, output, fmt.Sprintf("%s%s=\"%s\"", reservedKeyPrefix, tc.timeKey, reservedKeyTestVal), "Expected duplicate time key to be renamed")
+
+			// Print logs for humans to see, if needed.
+			fmt.Println(buf.String())
+		})
+	}
+}