diff --git a/.circleci/config.yml b/.circleci/config.yml index 8f035514d..23a770e5b 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -93,15 +93,14 @@ workflows: matrix: parameters: go_version: - - "1.21" - - "1.22" - - "1.23" + - "1.24" + - "1.25" - test-assets: name: assets-go-<< matrix.go_version >> matrix: parameters: go_version: - - "1.23" + - "1.25" - style: name: style - go_version: "1.23" + go_version: "1.25" diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml new file mode 100644 index 000000000..f7fc82f37 --- /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 0c00c410a..1816a556b 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -25,15 +25,17 @@ jobs: steps: - name: Checkout repository uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + persist-credentials: false - name: Install Go - uses: actions/setup-go@3041bf56c941b39c61721a86cd11f3bb1338122a # v5.2.0 + uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0 with: - go-version: 1.23.x + go-version: 1.25.x - name: Install snmp_exporter/generator dependencies run: sudo apt-get update && sudo apt-get -y install libsnmp-dev if: github.repository == 'prometheus/snmp_exporter' - name: Lint - uses: golangci/golangci-lint-action@971e284b6050e8a5849b72094c50ab08da042db8 # v6.1.1 + uses: golangci/golangci-lint-action@4afd733a84b1f43292c63897423277bb7f4313a9 # v8.0.0 with: args: --verbose - version: v1.63.4 + version: v2.2.1 diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml new file mode 100644 index 000000000..e7437b3f6 --- /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@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + 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/.golangci.yml b/.golangci.yml index e2f3e9459..02bf7fb0f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,46 +1,188 @@ -issues: - max-issues-per-linter: 0 - max-same-issues: 0 +version: "2" linters: + # Keep this list sorted alphabetically enable: - - errcheck + - depguard - errorlint - - gofumpt - - goimports - - gosimple + - exptostd + - fatcontext + - gocritic + - godot - govet - - ineffassign + - loggercheck - misspell + - nilnesserr + # TODO(bwplotka): Enable once https://github.com/golangci/golangci-lint/issues/3228 is fixed. + # - nolintlint - perfsprint + - predeclared - revive - - staticcheck + - sloglint - testifylint + - unconvert - unused -linters-settings: - goimports: - local-prefixes: github.com/prometheus/common - perfsprint: - # Optimizes even if it requires an int or uint type cast. - int-conversion: true - # Optimizes into `err.Error()` even if it is only equivalent for non-nil errors. - err-error: true - # Optimizes `fmt.Errorf`. - errorf: true - # Optimizes `fmt.Sprintf` with only one argument. - sprintf1: true - # Optimizes into strings concatenation. - strconcat: false - revive: + - usestdlibvars + - whitespace + exclusions: + generated: lax + presets: + - comments + - common-false-positives + - legacy + - std-error-handling + paths: + - third_party$ + - builtin$ + - examples$ rules: - # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#unused-parameter - - name: unused-parameter - severity: warning - disabled: true - testifylint: - disable: - - go-require - enable-all: true - formatter: - require-f-funcs: true + - linters: + - errcheck + # Taken from the default exclusions in v1. + text: Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked + - linters: + - govet + # We use many Seek methods that do not follow the usual pattern. + text: "stdmethods: method Seek.* should have signature Seek" + - linters: + - revive + # We have stopped at some point to write doc comments on exported symbols. + # TODO(beorn7): Maybe we should enforce this again? + text: exported (.+) should have comment( \(or a comment on this block\))? or be unexported + - linters: + - gocritic + text: "appendAssign" + - linters: + - errcheck + path: _test.go + - linters: + - errorlint + path: "tsdb/head_wal.go" + - linters: + - godot + source: "^// ===" + warn-unused: true + settings: + depguard: + rules: + main: + deny: + - pkg: "github.com/go-kit/kit/log" + desc: "Use github.com/go-kit/log instead of github.com/go-kit/kit/log" + - pkg: "io/ioutil" + desc: "Use corresponding 'os' or 'io' functions instead." + - pkg: "github.com/pkg/errors" + desc: "Use 'errors' or 'fmt' instead of github.com/pkg/errors" + - pkg: "gzip" + desc: "Use github.com/klauspost/compress instead of gzip" + - pkg: "zlib" + desc: "Use github.com/klauspost/compress instead of zlib" + - pkg: "golang.org/x/exp/slices" + desc: "Use 'slices' instead." + errcheck: + exclude-functions: + # Don't flag lines such as "io.Copy(io.Discard, resp.Body)". + - io.Copy + # The next two are used in HTTP handlers, any error is handled by the server itself. + - io.WriteString + - (net/http.ResponseWriter).Write + # No need to check for errors on server's shutdown. + - (*net/http.Server).Shutdown + # Never check for rollback errors as Rollback() is called when a previous error was detected. + - (github.com/prometheus/prometheus/storage.Appender).Rollback + govet: + disable: + - shadow + - fieldalignment + enable-all: true + perfsprint: + # Optimizes even if it requires an int or uint type cast. + int-conversion: true + # Optimizes into `err.Error()` even if it is only equivalent for non-nil errors. + err-error: true + # Optimizes `fmt.Errorf`. + errorf: true + # Optimizes `fmt.Sprintf` with only one argument. + sprintf1: true + # Optimizes into strings concatenation. + strconcat: false + revive: + # By default, revive will enable only the linting rules that are named in the configuration file. + # So, it's needed to explicitly enable all required rules here. + rules: + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md + - name: blank-imports + - name: comment-spacings + - name: context-as-argument + arguments: + # Allow functions with test or bench signatures. + - allowTypesBefore: '*testing.T,testing.TB' + - name: context-keys-type + - name: dot-imports + - name: early-return + arguments: + - "preserveScope" + # A lot of false positives: incorrectly identifies channel draining as "empty code block". + # See https://github.com/mgechev/revive/issues/386 + - name: empty-block + disabled: true + - name: error-naming + - name: error-return + - name: error-strings + - name: errorf + - name: exported + - name: increment-decrement + - name: indent-error-flow + arguments: + - "preserveScope" + - name: package-comments + # TODO(beorn7): Currently, we have a lot of missing package doc comments. Maybe we should have them. + disabled: true + - name: range + - name: receiver-naming + - name: redefines-builtin-id + - name: superfluous-else + arguments: + - "preserveScope" + - name: time-naming + - name: unexported-return + - name: unreachable-code + - name: unused-parameter + - name: unused-receiver + - name: var-declaration + - name: var-naming + testifylint: + enable-all: true + disable: + - float-compare + - go-require + formatter: + require-f-funcs: true +issues: + max-issues-per-linter: 0 + max-same-issues: 0 +output: + show-stats: false run: - timeout: 5m \ No newline at end of file + timeout: 15m +formatters: + enable: + - gci + - gofumpt + - goimports + settings: + gci: + sections: + - standard + - default + - prefix(github.com/prometheus/common) + gofumpt: + extra-rules: true + goimports: + local-prefixes: + - github.com/prometheus/common + exclusions: + generated: lax + paths: + - third_party$ + - builtin$ + - examples$ diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 000000000..9f96ecbe3 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,81 @@ +# Changelog + +## main / unreleased + +### What's Changed + +## v0.67.1 / 2025-10-07 + +## What's Changed +* Remove VERSION file to avoid Go conflict error in https://github.com/prometheus/common/pull/853 + +**Full Changelog**: https://github.com/prometheus/common/compare/v0.67.0...v0.67.1 + +## v0.67.0 / 2025-10-07 + +## What's Changed +* Create CHANGELOG.md for easier communication of library changes, especially possible breaking changes. by @ywwg in https://github.com/prometheus/common/pull/833 +* model: New test for validation with dots by @m1k1o in https://github.com/prometheus/common/pull/759 +* expfmt: document NewTextParser as required by @burgerdev in https://github.com/prometheus/common/pull/842 +* expfmt: Add support for float histograms and gauge histograms by @beorn7 in https://github.com/prometheus/common/pull/843 +* Updated minimum Go version to 1.24.0, updated Go dependecies by @SuperQ in https://github.com/prometheus/common/pull/849 + +## New Contributors +* @m1k1o made their first contribution in https://github.com/prometheus/common/pull/759 +* @burgerdev made their first contribution in https://github.com/prometheus/common/pull/842 + +**Full Changelog**: https://github.com/prometheus/common/compare/v0.66.1...v0.67.0 + +## v0.66.1 / 2025-09-05 + +This release has no functional changes, it just drops the dependencies `github.com/grafana/regexp` and `go.uber.org/atomic` and replaces `gopkg.in/yaml.v2` with `go.yaml.in/yaml/v2` (a drop-in replacement). + +### What's Changed +* Revert "Use github.com/grafana/regexp instead of regexp" by @aknuds1 in https://github.com/prometheus/common/pull/835 +* Move to supported version of yaml parser by @dims in https://github.com/prometheus/common/pull/834 +* Revert "Use go.uber.org/atomic instead of sync/atomic (#825)" by @aknuds1 in https://github.com/prometheus/common/pull/838 + +**Full Changelog**: https://github.com/prometheus/common/compare/v1.20.99...v0.66.1 + +## v0.66.0 / 2025-09-02 + +### ⚠️ Breaking Changes ⚠️ + +* A default-constructed TextParser will be invalid. It must have a valid `scheme` set, so users should use the NewTextParser function to create a valid TextParser. Otherwise parsing will panic with "Invalid name validation scheme requested: unset". + +### What's Changed +* model: add constants for type and unit labels. by @bwplotka in https://github.com/prometheus/common/pull/801 +* model.ValidationScheme: Support encoding as YAML by @aknuds1 in https://github.com/prometheus/common/pull/799 +* fix(promslog): always print time.Duration values as go duration strings by @tjhop in https://github.com/prometheus/common/pull/798 +* Add `ValidationScheme` methods `IsValidMetricName` and `IsValidLabelName` by @aknuds1 in https://github.com/prometheus/common/pull/806 +* Fix delimited proto not escaped correctly by @thampiotr in https://github.com/prometheus/common/pull/809 +* Decoder: Remove use of global name validation and add validation by @ywwg in https://github.com/prometheus/common/pull/808 +* ValidationScheme implements pflag.Value and json.Marshaler/Unmarshaler interfaces by @juliusmh in https://github.com/prometheus/common/pull/807 +* expfmt: Add NewTextParser function by @aknuds1 in https://github.com/prometheus/common/pull/816 + +* Enable the godot linter by @aknuds1 in https://github.com/prometheus/common/pull/821 +* Enable usestdlibvars linter by @aknuds1 in https://github.com/prometheus/common/pull/820 +* Enable unconvert linter by @aknuds1 in https://github.com/prometheus/common/pull/819 +* Enable the fatcontext linter by @aknuds1 in https://github.com/prometheus/common/pull/822 +* Enable gocritic linter by @aknuds1 in https://github.com/prometheus/common/pull/818 +* Use go.uber.org/atomic instead of sync/atomic by @aknuds1 in https://github.com/prometheus/common/pull/825 +* Enable revive rule unused-parameter by @aknuds1 in https://github.com/prometheus/common/pull/824 +* Enable revive rules by @aknuds1 in https://github.com/prometheus/common/pull/823 +* Synchronize common files from prometheus/prometheus by @prombot in https://github.com/prometheus/common/pull/802 +* Synchronize common files from prometheus/prometheus by @prombot in https://github.com/prometheus/common/pull/803 +* Sync .golangci.yml with prometheus/prometheus by @aknuds1 in https://github.com/prometheus/common/pull/817 +* ci: update upload-actions by @ywwg in https://github.com/prometheus/common/pull/814 +* docs: fix typo in expfmt.Negotiate by @wmcram in https://github.com/prometheus/common/pull/813 +* build(deps): bump golang.org/x/net from 0.40.0 to 0.41.0 by @dependabot[bot] in https://github.com/prometheus/common/pull/800 +* build(deps): bump golang.org/x/net from 0.41.0 to 0.42.0 by @dependabot[bot] in https://github.com/prometheus/common/pull/810 +* build(deps): bump github.com/stretchr/testify from 1.10.0 to 1.11.1 in /assets by @dependabot[bot] in https://github.com/prometheus/common/pull/826 +* build(deps): bump google.golang.org/protobuf from 1.36.6 to 1.36.8 by @dependabot[bot] in https://github.com/prometheus/common/pull/830 +* build(deps): bump golang.org/x/net from 0.42.0 to 0.43.0 by @dependabot[bot] in https://github.com/prometheus/common/pull/829 +* build(deps): bump github.com/stretchr/testify from 1.10.0 to 1.11.1 by @dependabot[bot] in https://github.com/prometheus/common/pull/827 + +### New Contributors +* @aknuds1 made their first contribution in https://github.com/prometheus/common/pull/799 +* @thampiotr made their first contribution in https://github.com/prometheus/common/pull/809 +* @wmcram made their first contribution in https://github.com/prometheus/common/pull/813 +* @juliusmh made their first contribution in https://github.com/prometheus/common/pull/807 + diff --git a/Makefile.common b/Makefile.common index d1576bb31..1f4c9025a 100644 --- a/Makefile.common +++ b/Makefile.common @@ -61,7 +61,8 @@ PROMU_URL := https://github.com/prometheus/promu/releases/download/v$(PROMU_ SKIP_GOLANGCI_LINT := GOLANGCI_LINT := GOLANGCI_LINT_OPTS ?= -GOLANGCI_LINT_VERSION ?= v1.63.4 +GOLANGCI_LINT_VERSION ?= v2.2.1 +GOLANGCI_FMT_OPTS ?= # 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)) @@ -138,7 +139,7 @@ common-deps: update-go-deps: @echo ">> updating Go dependencies" @for m in $$($(GO) list -mod=readonly -m -f '{{ if and (not .Indirect) (not .Main)}}{{.Path}}{{end}}' all); do \ - $(GO) get -d $$m; \ + $(GO) get $$m; \ done $(GO) mod tidy @@ -156,9 +157,13 @@ $(GOTEST_DIR): @mkdir -p $@ .PHONY: common-format -common-format: +common-format: $(GOLANGCI_LINT) @echo ">> formatting code" $(GO) fmt $(pkgs) +ifdef GOLANGCI_LINT + @echo ">> formatting code with golangci-lint" + $(GOLANGCI_LINT) fmt $(GOLANGCI_FMT_OPTS) +endif .PHONY: common-vet common-vet: @@ -248,8 +253,8 @@ $(PROMU): cp $(PROMU_TMP)/promu-$(PROMU_VERSION).$(GO_BUILD_PLATFORM)/promu $(FIRST_GOPATH)/bin/promu rm -r $(PROMU_TMP) -.PHONY: proto -proto: +.PHONY: common-proto +common-proto: @echo ">> generating code from proto files" @./scripts/genproto.sh diff --git a/README.md b/README.md index 954cc91b4..f7d5342dc 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/RELEASE.md b/RELEASE.md index ba63828e1..42e76cafa 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -11,7 +11,16 @@ With those steps done, you can proceed to cut a release. ## How to cut an individual release -There is no automated process for cutting a release in `prometheus/common`. A manual release using GitHub's release feature via [this link](https://github.com/prometheus/prometheus/releases/new) is the best way to go. The tag name must be prefixed with a `v` e.g. `v0.53.0` and then you can use the "Generate release notes" button to generate the release note automagically ✨. No need to create a discussion or mark it a pre-release, please do mark it as the latest release if needed. +There is no automated process for cutting a release in `prometheus/common`. +The primary trigger for announcing a release is pushing a new version tag. + +NOTE: As soon as a new tag is created, many downstream projects will automatically create pull requests to update their dependency of prom/common. +Make sure the release is ready to go, with an updated changelog including notices of any breaking changes, before pushing a new tag. + +Here are the basic steps: + +1. Update CHANGELOG.md, applying the new version number (this time including the `v` prefix, e.g. `v0.53.0`) and date to the changes listed under ``## main / unreleased`, and commit those changes to the main branch. +2. Use GitHub's release feature via [this link](https://github.com/prometheus/prometheus/releases/new) to apply a new tag. The tag name must be prefixed with a `v` e.g. `v0.53.0` and then use the "Generate release notes" button to generate the release notes automagically ✨. No need to create a discussion or mark it a pre-release, please do make sure it is marked as the latest release. ## Versioning strategy diff --git a/assets/go.mod b/assets/go.mod index 6e1afe3f6..33f87bcea 100644 --- a/assets/go.mod +++ b/assets/go.mod @@ -1,8 +1,8 @@ module github.com/prometheus/common/assets -go 1.21 +go 1.24.0 -require github.com/stretchr/testify v1.10.0 +require github.com/stretchr/testify v1.11.1 require ( github.com/davecgh/go-spew v1.1.1 // indirect diff --git a/assets/go.sum b/assets/go.sum index 713a0b4f0..c4c1710c4 100644 --- a/assets/go.sum +++ b/assets/go.sum @@ -2,8 +2,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= -github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/config/config.go b/config/config.go index 7588da555..ff54cdd82 100644 --- a/config/config.go +++ b/config/config.go @@ -30,7 +30,7 @@ type Secret string // MarshalSecretValue if set to true will expose Secret type // through the marshal interfaces. Useful for outside projects // that load and marshal the Prometheus config. -var MarshalSecretValue bool = false +var MarshalSecretValue = false // MarshalYAML implements the yaml.Marshaler interface for Secrets. func (s Secret) MarshalYAML() (interface{}, error) { diff --git a/config/config_test.go b/config/config_test.go index f62b266af..9b0d0480a 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -21,7 +21,7 @@ import ( "testing" "github.com/stretchr/testify/require" - "gopkg.in/yaml.v2" + "go.yaml.in/yaml/v2" ) func TestJSONMarshalSecret(t *testing.T) { diff --git a/config/headers.go b/config/headers.go index 7276742ec..9beaae26c 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/headers_test.go b/config/headers_test.go index 39c6f9ff3..c807fbc3b 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/config/http_config.go b/config/http_config.go index 63809083a..4e5ff92a2 100644 --- a/config/http_config.go +++ b/config/http_config.go @@ -32,11 +32,11 @@ import ( "time" conntrack "github.com/mwitkow/go-conntrack" + "go.yaml.in/yaml/v2" "golang.org/x/net/http/httpproxy" "golang.org/x/net/http2" "golang.org/x/oauth2" "golang.org/x/oauth2/clientcredentials" - "gopkg.in/yaml.v2" ) var ( @@ -72,7 +72,7 @@ var TLSVersions = map[string]TLSVersion{ func (tv *TLSVersion) UnmarshalYAML(unmarshal func(interface{}) error) error { var s string - err := unmarshal((*string)(&s)) + err := unmarshal(&s) if err != nil { return err } @@ -225,7 +225,7 @@ func (u *URL) UnmarshalJSON(data []byte) error { // MarshalJSON implements the json.Marshaler interface for URL. func (u URL) MarshalJSON() ([]byte, error) { if u.URL != nil { - return json.Marshal(u.URL.String()) + return json.Marshal(u.String()) } return []byte("null"), nil } @@ -245,13 +245,13 @@ type OAuth2 struct { ProxyConfig `yaml:",inline"` } -// UnmarshalYAML implements the yaml.Unmarshaler interface +// UnmarshalYAML implements the yaml.Unmarshaler interface. func (o *OAuth2) UnmarshalYAML(unmarshal func(interface{}) error) error { type plain OAuth2 if err := unmarshal((*plain)(o)); err != nil { return err } - return o.ProxyConfig.Validate() + return o.Validate() } // UnmarshalJSON implements the json.Marshaler interface for URL. @@ -260,7 +260,7 @@ func (o *OAuth2) UnmarshalJSON(data []byte) error { if err := json.Unmarshal(data, (*plain)(o)); err != nil { return err } - return o.ProxyConfig.Validate() + return o.Validate() } // SetDirectory joins any relative file paths with dir. @@ -346,7 +346,7 @@ func nonZeroCount[T comparable](values ...T) int { var zero T for _, value := range values { if value != zero { - count += 1 + count++ } } return count @@ -363,7 +363,7 @@ func (c *HTTPClientConfig) Validate() error { if (c.BasicAuth != nil || c.OAuth2 != nil) && (len(c.BearerToken) > 0 || len(c.BearerTokenFile) > 0) { return errors.New("at most one of basic_auth, oauth2, bearer_token & bearer_token_file must be configured") } - if c.BasicAuth != nil && nonZeroCount(string(c.BasicAuth.Username) != "", c.BasicAuth.UsernameFile != "", c.BasicAuth.UsernameRef != "") > 1 { + if c.BasicAuth != nil && nonZeroCount(c.BasicAuth.Username != "", c.BasicAuth.UsernameFile != "", c.BasicAuth.UsernameRef != "") > 1 { return errors.New("at most one of basic_auth username, username_file & username_ref must be configured") } if c.BasicAuth != nil && nonZeroCount(string(c.BasicAuth.Password) != "", c.BasicAuth.PasswordFile != "", c.BasicAuth.PasswordRef != "") > 1 { @@ -423,7 +423,7 @@ func (c *HTTPClientConfig) Validate() error { return nil } -// UnmarshalYAML implements the yaml.Unmarshaler interface +// UnmarshalYAML implements the yaml.Unmarshaler interface. func (c *HTTPClientConfig) UnmarshalYAML(unmarshal func(interface{}) error) error { type plain HTTPClientConfig *c = DefaultHTTPClientConfig @@ -542,8 +542,14 @@ func (s *secretManagerOption) applyToTLSConfigOptions(opts *tlsConfigOptions) { opts.secretManager = s.secretManager } +// SecretManagerOption is an option for providing a SecretManager. +type SecretManagerOption interface { + TLSConfigOption + HTTPClientOption +} + // WithSecretManager allows setting the secret manager. -func WithSecretManager(manager SecretManager) *secretManagerOption { +func WithSecretManager(manager SecretManager) SecretManagerOption { return &secretManagerOption{ secretManager: manager, } @@ -604,8 +610,8 @@ func NewRoundTripperFromConfigWithContext(ctx context.Context, cfg HTTPClientCon // The only timeout we care about is the configured scrape timeout. // It is applied on request. So we leave out any timings here. var rt http.RoundTripper = &http.Transport{ - Proxy: cfg.ProxyConfig.Proxy(), - ProxyConnectHeader: cfg.ProxyConfig.GetProxyConnectHeader(), + Proxy: cfg.Proxy(), + ProxyConnectHeader: cfg.GetProxyConnectHeader(), MaxIdleConns: 20000, MaxIdleConnsPerHost: 1000, // see https://github.com/golang/go/issues/13801 DisableKeepAlives: !opts.keepAlivesEnabled, @@ -726,11 +732,11 @@ func (s *InlineSecret) Fetch(context.Context) (string, error) { return s.text, nil } -func (s *InlineSecret) Description() string { +func (*InlineSecret) Description() string { return "inline" } -func (s *InlineSecret) Immutable() bool { +func (*InlineSecret) Immutable() bool { return true } @@ -742,7 +748,7 @@ func NewFileSecret(file string) *FileSecret { return &FileSecret{file: file} } -func (s *FileSecret) Fetch(ctx context.Context) (string, error) { +func (s *FileSecret) Fetch(context.Context) (string, error) { fileBytes, err := os.ReadFile(s.file) if err != nil { return "", fmt.Errorf("unable to read file %s: %w", s.file, err) @@ -754,7 +760,7 @@ func (s *FileSecret) Description() string { return "file " + s.file } -func (s *FileSecret) Immutable() bool { +func (*FileSecret) Immutable() bool { return false } @@ -772,7 +778,7 @@ func (s *refSecret) Description() string { return "ref " + s.ref } -func (s *refSecret) Immutable() bool { +func (*refSecret) Immutable() bool { return false } @@ -914,8 +920,8 @@ func (rt *oauth2RoundTripper) newOauth2TokenSource(req *http.Request, secret str tlsTransport := func(tlsConfig *tls.Config) (http.RoundTripper, error) { return &http.Transport{ TLSClientConfig: tlsConfig, - Proxy: rt.config.ProxyConfig.Proxy(), - ProxyConnectHeader: rt.config.ProxyConfig.GetProxyConnectHeader(), + Proxy: rt.config.Proxy(), + ProxyConnectHeader: rt.config.GetProxyConnectHeader(), DisableKeepAlives: !rt.opts.keepAlivesEnabled, MaxIdleConns: 20, MaxIdleConnsPerHost: 1, // see https://github.com/golang/go/issues/13801 @@ -1224,7 +1230,7 @@ func (c *TLSConfig) getClientCertificate(ctx context.Context, secretManager Secr } } - keySecret, err := toSecret(secretManager, Secret(c.Key), c.KeyFile, c.KeyRef) + keySecret, err := toSecret(secretManager, c.Key, c.KeyFile, c.KeyRef) if err != nil { return nil, fmt.Errorf("unable to use client key: %w", err) } @@ -1362,9 +1368,9 @@ func (t *tlsRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { } t.mtx.RLock() - equal := bytes.Equal(caHash[:], t.hashCAData) && - bytes.Equal(certHash[:], t.hashCertData) && - bytes.Equal(keyHash[:], t.hashKeyData) + equal := bytes.Equal(caHash, t.hashCAData) && + bytes.Equal(certHash, t.hashCertData) && + bytes.Equal(keyHash, t.hashKeyData) rt := t.rt t.mtx.RUnlock() if equal { @@ -1387,9 +1393,9 @@ func (t *tlsRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { t.mtx.Lock() t.rt = rt - t.hashCAData = caHash[:] - t.hashCertData = certHash[:] - t.hashKeyData = keyHash[:] + t.hashCAData = caHash + t.hashCertData = certHash + t.hashKeyData = keyHash t.mtx.Unlock() return rt.RoundTrip(req) @@ -1508,7 +1514,7 @@ func (c *ProxyConfig) Proxy() (fn func(*http.Request) (*url.URL, error)) { } return } - if c.ProxyURL.URL != nil && c.ProxyURL.URL.String() != "" { + if c.ProxyURL.URL != nil && c.ProxyURL.String() != "" { if c.NoProxy == "" { c.proxyFunc = http.ProxyURL(c.ProxyURL.URL) return diff --git a/config/http_config_test.go b/config/http_config_test.go index 58d13b0dc..c8853592e 100644 --- a/config/http_config_test.go +++ b/config/http_config_test.go @@ -36,7 +36,7 @@ import ( "time" "github.com/stretchr/testify/require" - "gopkg.in/yaml.v2" + "go.yaml.in/yaml/v2" ) const ( @@ -181,7 +181,7 @@ func TestNewClientFromConfig(t *testing.T) { InsecureSkipVerify: true, }, }, - handler: func(w http.ResponseWriter, r *http.Request) { + handler: func(w http.ResponseWriter, _ *http.Request) { fmt.Fprint(w, ExpectedMessage) }, }, @@ -195,7 +195,7 @@ func TestNewClientFromConfig(t *testing.T) { InsecureSkipVerify: false, }, }, - handler: func(w http.ResponseWriter, r *http.Request) { + handler: func(w http.ResponseWriter, _ *http.Request) { fmt.Fprint(w, ExpectedMessage) }, }, @@ -369,13 +369,14 @@ func TestNewClientFromConfig(t *testing.T) { }, handler: func(w http.ResponseWriter, r *http.Request) { username, password, ok := r.BasicAuth() - if !ok { + switch { + case !ok: fmt.Fprintf(w, "The Authorization header wasn't set") - } else if ExpectedUsername != username { + case ExpectedUsername != username: fmt.Fprintf(w, "The expected username (%s) differs from the obtained username (%s).", ExpectedUsername, username) - } else if ExpectedPassword != password { + case ExpectedPassword != password: fmt.Fprintf(w, "The expected password (%s) differs from the obtained password (%s).", ExpectedPassword, password) - } else { + default: fmt.Fprint(w, ExpectedMessage) } }, @@ -705,7 +706,7 @@ func TestBearerAuthRoundTripper(t *testing.T) { // Normal flow. bearerAuthRoundTripper := NewAuthorizationCredentialsRoundTripper("Bearer", NewInlineSecret(BearerToken), fakeRoundTripper) - request, _ := http.NewRequest("GET", "/hitchhiker", nil) + request, _ := http.NewRequest(http.MethodGet, "/hitchhiker", nil) request.Header.Set("User-Agent", "Douglas Adams mind") _, err := bearerAuthRoundTripper.RoundTrip(request) if err != nil { @@ -714,7 +715,7 @@ func TestBearerAuthRoundTripper(t *testing.T) { // Should honor already Authorization header set. bearerAuthRoundTripperShouldNotModifyExistingAuthorization := NewAuthorizationCredentialsRoundTripper("Bearer", NewInlineSecret(newBearerToken), fakeRoundTripper) - request, _ = http.NewRequest("GET", "/hitchhiker", nil) + request, _ = http.NewRequest(http.MethodGet, "/hitchhiker", nil) request.Header.Set("Authorization", ExpectedBearer) _, err = bearerAuthRoundTripperShouldNotModifyExistingAuthorization.RoundTrip(request) if err != nil { @@ -733,7 +734,7 @@ func TestBearerAuthFileRoundTripper(t *testing.T) { // Normal flow. bearerAuthRoundTripper := NewAuthorizationCredentialsRoundTripper("Bearer", &FileSecret{file: BearerTokenFile}, fakeRoundTripper) - request, _ := http.NewRequest("GET", "/hitchhiker", nil) + request, _ := http.NewRequest(http.MethodGet, "/hitchhiker", nil) request.Header.Set("User-Agent", "Douglas Adams mind") _, err := bearerAuthRoundTripper.RoundTrip(request) if err != nil { @@ -742,7 +743,7 @@ func TestBearerAuthFileRoundTripper(t *testing.T) { // Should honor already Authorization header set. bearerAuthRoundTripperShouldNotModifyExistingAuthorization := NewAuthorizationCredentialsRoundTripper("Bearer", &FileSecret{file: MissingBearerTokenFile}, fakeRoundTripper) - request, _ = http.NewRequest("GET", "/hitchhiker", nil) + request, _ = http.NewRequest(http.MethodGet, "/hitchhiker", nil) request.Header.Set("Authorization", ExpectedBearer) _, err = bearerAuthRoundTripperShouldNotModifyExistingAuthorization.RoundTrip(request) if err != nil { @@ -932,7 +933,7 @@ type secretManager struct { data map[string]string } -func (m *secretManager) Fetch(ctx context.Context, secretRef string) (string, error) { +func (m *secretManager) Fetch(_ context.Context, secretRef string) (string, error) { secretData, ok := m.data[secretRef] if !ok { return "", fmt.Errorf("unknown secret %s", secretRef) @@ -1043,7 +1044,7 @@ func TestTLSRoundTripper(t *testing.T) { ca, cert, key := filepath.Join(tmpDir, "ca"), filepath.Join(tmpDir, "cert"), filepath.Join(tmpDir, "key") - handler := func(w http.ResponseWriter, r *http.Request) { + handler := func(w http.ResponseWriter, _ *http.Request) { fmt.Fprint(w, ExpectedMessage) } testServer, err := newTestServer(handler) @@ -1161,7 +1162,7 @@ func TestTLSRoundTripper(t *testing.T) { } func TestTLSRoundTripper_Inline(t *testing.T) { - handler := func(w http.ResponseWriter, r *http.Request) { + handler := func(w http.ResponseWriter, _ *http.Request) { fmt.Fprint(w, ExpectedMessage) } testServer, err := newTestServer(handler) @@ -1283,7 +1284,7 @@ func TestTLSRoundTripperRaces(t *testing.T) { ca, cert, key := filepath.Join(tmpDir, "ca"), filepath.Join(tmpDir, "cert"), filepath.Join(tmpDir, "key") - handler := func(w http.ResponseWriter, r *http.Request) { + handler := func(w http.ResponseWriter, _ *http.Request) { fmt.Fprint(w, ExpectedMessage) } testServer, err := newTestServer(handler) @@ -1401,7 +1402,7 @@ type roundTrip struct { theError error } -func (rt *roundTrip) RoundTrip(r *http.Request) (*http.Response, error) { +func (rt *roundTrip) RoundTrip(*http.Request) (*http.Response, error) { return rt.theResponse, rt.theError } @@ -1748,7 +1749,7 @@ func TestUnmarshalEmptyURL(t *testing.T) { } } -// checks if u equals to &url.URL{} +// checks if u equals to &url.URL{}. func isEmptyNonNilURL(u *url.URL) bool { return u != nil && *u == url.URL{} } @@ -1874,7 +1875,7 @@ func TestModifyTLSCertificates(t *testing.T) { defer os.RemoveAll(tmpDir) ca, cert, key := filepath.Join(tmpDir, "ca"), filepath.Join(tmpDir, "cert"), filepath.Join(tmpDir, "key") - handler := func(w http.ResponseWriter, r *http.Request) { + handler := func(w http.ResponseWriter, _ *http.Request) { fmt.Fprint(w, ExpectedMessage) } testServer, err := newTestServer(handler) @@ -2104,7 +2105,7 @@ no_proxy: promcon.io,cncf.io`, proxyServer.URL), os.Setenv("NO_PROXY", tc.noProxyEnv) } - req := httptest.NewRequest("GET", tc.targetURL, nil) + req := httptest.NewRequest(http.MethodGet, tc.targetURL, nil) proxyFunc := proxyConfig.Proxy() resultURL, err := proxyFunc(req) diff --git a/config/tls_config_test.go b/config/tls_config_test.go index b43a1a166..f02c87fdd 100644 --- a/config/tls_config_test.go +++ b/config/tls_config_test.go @@ -26,7 +26,7 @@ import ( "testing" "github.com/stretchr/testify/require" - "gopkg.in/yaml.v2" + "go.yaml.in/yaml/v2" ) // LoadTLSConfig parses the given file into a tls.Config. diff --git a/expfmt/bench_test.go b/expfmt/bench_test.go index 4f691b310..1acb19965 100644 --- a/expfmt/bench_test.go +++ b/expfmt/bench_test.go @@ -22,13 +22,14 @@ import ( "os" "testing" - "google.golang.org/protobuf/encoding/protodelim" - dto "github.com/prometheus/client_model/go" "github.com/stretchr/testify/require" + "google.golang.org/protobuf/encoding/protodelim" + + "github.com/prometheus/common/model" ) -var parser TextParser +var parser = TextParser{scheme: model.UTF8Validation} // Benchmarks to show how much penalty text format parsing actually inflicts. // diff --git a/expfmt/decode.go b/expfmt/decode.go index 1448439b7..8f8dc65d3 100644 --- a/expfmt/decode.go +++ b/expfmt/decode.go @@ -70,19 +70,34 @@ func ResponseFormat(h http.Header) Format { return FmtUnknown } -// NewDecoder returns a new decoder based on the given input format. -// If the input format does not imply otherwise, a text format decoder is returned. +// NewDecoder returns a new decoder based on the given input format. Metric +// names are validated based on the provided Format -- if the format requires +// escaping, raditional Prometheues validity checking is used. Otherwise, names +// are checked for UTF-8 validity. Supported formats include delimited protobuf +// and Prometheus text format. For historical reasons, this decoder fallbacks +// to classic text decoding for any other format. This decoder does not fully +// support OpenMetrics although it may often succeed due to the similarities +// between the formats. This decoder may not support the latest features of +// Prometheus text format and is not intended for high-performance applications. +// See: https://github.com/prometheus/common/issues/812 func NewDecoder(r io.Reader, format Format) Decoder { + scheme := model.LegacyValidation + if format.ToEscapingScheme() == model.NoEscaping { + scheme = model.UTF8Validation + } switch format.FormatType() { case TypeProtoDelim: - return &protoDecoder{r: bufio.NewReader(r)} + return &protoDecoder{r: bufio.NewReader(r), s: scheme} + case TypeProtoText, TypeProtoCompact: + return &errDecoder{err: fmt.Errorf("format %s not supported for decoding", format)} } - return &textDecoder{r: r} + return &textDecoder{r: r, s: scheme} } // protoDecoder implements the Decoder interface for protocol buffers. type protoDecoder struct { r protodelim.Reader + s model.ValidationScheme } // Decode implements the Decoder interface. @@ -93,7 +108,7 @@ func (d *protoDecoder) Decode(v *dto.MetricFamily) error { if err := opts.UnmarshalFrom(d.r, v); err != nil { return err } - if !model.IsValidMetricName(model.LabelValue(v.GetName())) { + if !d.s.IsValidMetricName(v.GetName()) { return fmt.Errorf("invalid metric name %q", v.GetName()) } for _, m := range v.GetMetric() { @@ -107,7 +122,7 @@ func (d *protoDecoder) Decode(v *dto.MetricFamily) error { if !model.LabelValue(l.GetValue()).IsValid() { return fmt.Errorf("invalid label value %q", l.GetValue()) } - if !model.LabelName(l.GetName()).IsValid() { + if !d.s.IsValidLabelName(l.GetName()) { return fmt.Errorf("invalid label name %q", l.GetName()) } } @@ -115,10 +130,20 @@ func (d *protoDecoder) Decode(v *dto.MetricFamily) error { return nil } +// errDecoder is an error-state decoder that always returns the same error. +type errDecoder struct { + err error +} + +func (d *errDecoder) Decode(*dto.MetricFamily) error { + return d.err +} + // textDecoder implements the Decoder interface for the text protocol. type textDecoder struct { r io.Reader fams map[string]*dto.MetricFamily + s model.ValidationScheme err error } @@ -126,7 +151,7 @@ type textDecoder struct { func (d *textDecoder) Decode(v *dto.MetricFamily) error { if d.err == nil { // Read all metrics in one shot. - var p TextParser + p := NewTextParser(d.s) d.fams, d.err = p.TextToMetricFamilies(d.r) // If we don't get an error, store io.EOF for the end. if d.err == nil { @@ -195,7 +220,7 @@ func extractSamples(f *dto.MetricFamily, o *DecodeOptions) (model.Vector, error) return extractSummary(o, f), nil case dto.MetricType_UNTYPED: return extractUntyped(o, f), nil - case dto.MetricType_HISTOGRAM: + case dto.MetricType_HISTOGRAM, dto.MetricType_GAUGE_HISTOGRAM: return extractHistogram(o, f), nil } return nil, fmt.Errorf("expfmt.extractSamples: unknown metric family type %v", f.GetType()) @@ -378,9 +403,13 @@ func extractHistogram(o *DecodeOptions, f *dto.MetricFamily) model.Vector { infSeen = true } + v := q.GetCumulativeCountFloat() + if v <= 0 { + v = float64(q.GetCumulativeCount()) + } samples = append(samples, &model.Sample{ Metric: model.Metric(lset), - Value: model.SampleValue(q.GetCumulativeCount()), + Value: model.SampleValue(v), Timestamp: timestamp, }) } @@ -403,9 +432,13 @@ func extractHistogram(o *DecodeOptions, f *dto.MetricFamily) model.Vector { } lset[model.MetricNameLabel] = model.LabelValue(f.GetName() + "_count") + v := m.Histogram.GetSampleCountFloat() + if v <= 0 { + v = float64(m.Histogram.GetSampleCount()) + } count := &model.Sample{ Metric: model.Metric(lset), - Value: model.SampleValue(m.Histogram.GetSampleCount()), + Value: model.SampleValue(v), Timestamp: timestamp, } samples = append(samples, count) diff --git a/expfmt/decode_test.go b/expfmt/decode_test.go index 10b12b667..baa761db1 100644 --- a/expfmt/decode_test.go +++ b/expfmt/decode_test.go @@ -80,7 +80,10 @@ mf2 4 ) dec := &SampleDecoder{ - Dec: &textDecoder{r: strings.NewReader(in)}, + Dec: &textDecoder{ + s: model.UTF8Validation, + r: strings.NewReader(in), + }, Opts: &DecodeOptions{ Timestamp: ts, }, @@ -361,7 +364,7 @@ func TestProtoDecoder(t *testing.T) { for i, scenario := range scenarios { dec := &SampleDecoder{ - Dec: &protoDecoder{r: strings.NewReader(scenario.in)}, + Dec: &protoDecoder{r: strings.NewReader(scenario.in), s: model.LegacyValidation}, Opts: &DecodeOptions{ Timestamp: testTime, }, @@ -369,7 +372,6 @@ func TestProtoDecoder(t *testing.T) { var all model.Vector for { - model.NameValidationScheme = model.LegacyValidation var smpls model.Vector err := dec.Decode(&smpls) if err != nil && errors.Is(err, io.EOF) { @@ -377,9 +379,8 @@ func TestProtoDecoder(t *testing.T) { } if scenario.legacyNameFail { require.Errorf(t, err, "Expected error when decoding without UTF-8 support enabled but got none") - model.NameValidationScheme = model.UTF8Validation dec = &SampleDecoder{ - Dec: &protoDecoder{r: strings.NewReader(scenario.in)}, + Dec: &protoDecoder{r: strings.NewReader(scenario.in), s: model.UTF8Validation}, Opts: &DecodeOptions{ Timestamp: testTime, }, diff --git a/expfmt/encode.go b/expfmt/encode.go index d7f3d76f5..73c24dfbc 100644 --- a/expfmt/encode.go +++ b/expfmt/encode.go @@ -18,14 +18,12 @@ import ( "io" "net/http" + "github.com/munnerz/goautoneg" + dto "github.com/prometheus/client_model/go" "google.golang.org/protobuf/encoding/protodelim" "google.golang.org/protobuf/encoding/prototext" "github.com/prometheus/common/model" - - "github.com/munnerz/goautoneg" - - dto "github.com/prometheus/client_model/go" ) // Encoder types encode metric families into an underlying wire protocol. @@ -61,7 +59,7 @@ func (ec encoderCloser) Close() error { // appropriate accepted type is found, FmtText is returned (which is the // Prometheus text format). This function will never negotiate FmtOpenMetrics, // as the support is still experimental. To include the option to negotiate -// FmtOpenMetrics, use NegotiateOpenMetrics. +// FmtOpenMetrics, use NegotiateIncludingOpenMetrics. func Negotiate(h http.Header) Format { escapingScheme := Format(fmt.Sprintf("; escaping=%s", Format(model.NameEscapingScheme.String()))) for _, ac := range goautoneg.ParseAccept(h.Get(hdrAccept)) { @@ -153,7 +151,7 @@ func NewEncoder(w io.Writer, format Format, options ...EncoderOption) Encoder { case TypeProtoDelim: return encoderCloser{ encode: func(v *dto.MetricFamily) error { - _, err := protodelim.MarshalTo(w, v) + _, err := protodelim.MarshalTo(w, model.EscapeMetricFamily(v, escapingScheme)) return err }, close: func() error { return nil }, diff --git a/expfmt/encode_test.go b/expfmt/encode_test.go index d91faaab6..6a801629a 100644 --- a/expfmt/encode_test.go +++ b/expfmt/encode_test.go @@ -18,11 +18,12 @@ import ( "net/http" "testing" + dto "github.com/prometheus/client_model/go" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" "github.com/prometheus/common/model" - - dto "github.com/prometheus/client_model/go" ) func TestNegotiate(t *testing.T) { @@ -97,7 +98,7 @@ func TestNegotiate(t *testing.T) { } } -func TestNegotiateOpenMetrics(t *testing.T) { +func TestNegotiateIncludingOpenMetrics(t *testing.T) { acceptValuePrefix := "application/vnd.google.protobuf;proto=io.prometheus.client.MetricFamily" tests := []struct { name string @@ -309,8 +310,32 @@ foo_metric 1.234 } func TestEscapedEncode(t *testing.T) { - var buff bytes.Buffer - delimEncoder := NewEncoder(&buff, FmtProtoDelim+"; escaping=underscores") + tests := []struct { + name string + format Format + }{ + { + name: "ProtoDelim", + format: FmtProtoDelim, + }, + { + name: "ProtoDelim with escaping underscores", + format: FmtProtoDelim + "; escaping=underscores", + }, + { + name: "ProtoCompact", + format: FmtProtoCompact, + }, + { + name: "ProtoText", + format: FmtProtoText, + }, + { + name: "Text", + format: FmtText, + }, + } + metric := &dto.MetricFamily{ Name: proto.String("foo.metric"), Type: dto.MetricType_UNTYPED.Enum(), @@ -334,61 +359,109 @@ func TestEscapedEncode(t *testing.T) { }, } - err := delimEncoder.Encode(metric) - if err != nil { - t.Errorf("unexpected error during encode: %s", err.Error()) - } - - out := buff.Bytes() - if len(out) == 0 { - t.Errorf("expected the output bytes buffer to be non-empty") - } - - buff.Reset() - - compactEncoder := NewEncoder(&buff, FmtProtoCompact) - err = compactEncoder.Encode(metric) - if err != nil { - t.Errorf("unexpected error during encode: %s", err.Error()) - } - - out = buff.Bytes() - if len(out) == 0 { - t.Errorf("expected the output bytes buffer to be non-empty") - } - - buff.Reset() + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buff bytes.Buffer + encoder := NewEncoder(&buff, tt.format) + err := encoder.Encode(metric) + require.NoError(t, err) - protoTextEncoder := NewEncoder(&buff, FmtProtoText) - err = protoTextEncoder.Encode(metric) - if err != nil { - t.Errorf("unexpected error during encode: %s", err.Error()) - } - - out = buff.Bytes() - if len(out) == 0 { - t.Errorf("expected the output bytes buffer to be non-empty") + s := buff.String() + assert.NotContains(t, s, "foo.metric") + assert.Contains(t, s, "foo_metric") + assert.NotContains(t, s, "dotted.label.name") + assert.Contains(t, s, "dotted_label_name") + assert.Contains(t, s, "my.label.value") + }) } +} - buff.Reset() - - textEncoder := NewEncoder(&buff, FmtText) - err = textEncoder.Encode(metric) - if err != nil { - t.Errorf("unexpected error during encode: %s", err.Error()) +func TestDottedEncode(t *testing.T) { + //nolint:staticcheck + model.NameValidationScheme = model.UTF8Validation + metric := &dto.MetricFamily{ + Name: proto.String("foo.metric"), + Type: dto.MetricType_COUNTER.Enum(), + Metric: []*dto.Metric{ + { + Counter: &dto.Counter{ + Value: proto.Float64(1.234), + }, + }, + { + Label: []*dto.LabelPair{ + { + Name: proto.String("dotted.label.name"), + Value: proto.String("my.label.value"), + }, + }, + Counter: &dto.Counter{ + Value: proto.Float64(8), + }, + }, + }, } - out = buff.Bytes() - if len(out) == 0 { - t.Errorf("expected the output bytes buffer to be non-empty") + scenarios := []struct { + format Format + expectMetricName string + expectLabelName string + }{ + { + format: FmtProtoDelim, + expectMetricName: "foo_metric", + expectLabelName: "dotted_label_name", + }, + { + format: FmtProtoDelim.WithEscapingScheme(model.NoEscaping), + expectMetricName: "foo.metric", + expectLabelName: "dotted.label.name", + }, + { + format: FmtProtoDelim.WithEscapingScheme(model.DotsEscaping), + expectMetricName: "foo_dot_metric", + expectLabelName: "dotted_dot_label_dot_name", + }, + { + format: FmtText, + expectMetricName: "foo_metric", + expectLabelName: "dotted_label_name", + }, + { + format: FmtText.WithEscapingScheme(model.NoEscaping), + expectMetricName: "foo.metric", + expectLabelName: "dotted.label.name", + }, + { + format: FmtText.WithEscapingScheme(model.DotsEscaping), + expectMetricName: "foo_dot_metric", + expectLabelName: "dotted_dot_label_dot_name", + }, + // common library does not support proto text or open metrics parsing so we + // do not test those here. } - expected := `# TYPE foo_metric untyped -foo_metric 1.234 -foo_metric{dotted_label_name="my.label.value"} 8 -` + for i, scenario := range scenarios { + out := bytes.NewBuffer(make([]byte, 0)) + enc := NewEncoder(out, scenario.format) + err := enc.Encode(metric) + if err != nil { + t.Errorf("%d. error: %s", i, err) + continue + } - if string(out) != expected { - t.Errorf("expected TextEncoder to return %s, but got %s instead", expected, string(out)) + dec := NewDecoder(bytes.NewReader(out.Bytes()), scenario.format) + var gotFamily dto.MetricFamily + err = dec.Decode(&gotFamily) + if err != nil { + t.Errorf("%v: unexpected error during decode: %s", scenario.format, err.Error()) + } + if gotFamily.GetName() != scenario.expectMetricName { + t.Errorf("%v: incorrect encoded metric name, want %v, got %v", scenario.format, scenario.expectMetricName, gotFamily.GetName()) + } + lName := gotFamily.GetMetric()[1].Label[0].GetName() + if lName != scenario.expectLabelName { + t.Errorf("%v: incorrect encoded label name, want %v, got %v", scenario.format, scenario.expectLabelName, lName) + } } } diff --git a/expfmt/expfmt.go b/expfmt/expfmt.go index b26886560..c34c7de43 100644 --- a/expfmt/expfmt.go +++ b/expfmt/expfmt.go @@ -36,9 +36,11 @@ const ( ProtoType = `application/vnd.google.protobuf` ProtoProtocol = `io.prometheus.client.MetricFamily` // Deprecated: Use expfmt.NewFormat(expfmt.TypeProtoCompact) instead. - ProtoFmt = ProtoType + "; proto=" + ProtoProtocol + ";" - OpenMetricsType = `application/openmetrics-text` + ProtoFmt = ProtoType + "; proto=" + ProtoProtocol + ";" + OpenMetricsType = `application/openmetrics-text` + //nolint:revive // Allow for underscores. OpenMetricsVersion_0_0_1 = "0.0.1" + //nolint:revive // Allow for underscores. OpenMetricsVersion_1_0_0 = "1.0.0" // The Content-Type values for the different wire protocols. Do not do direct @@ -54,8 +56,10 @@ const ( // Deprecated: Use expfmt.NewFormat(expfmt.TypeProtoCompact) instead. FmtProtoCompact Format = ProtoFmt + ` encoding=compact-text` // Deprecated: Use expfmt.NewFormat(expfmt.TypeOpenMetrics) instead. + //nolint:revive // Allow for underscores. FmtOpenMetrics_1_0_0 Format = OpenMetricsType + `; version=` + OpenMetricsVersion_1_0_0 + `; charset=utf-8` // Deprecated: Use expfmt.NewFormat(expfmt.TypeOpenMetrics) instead. + //nolint:revive // Allow for underscores. FmtOpenMetrics_0_0_1 Format = OpenMetricsType + `; version=` + OpenMetricsVersion_0_0_1 + `; charset=utf-8` ) @@ -188,8 +192,8 @@ func (f Format) FormatType() FormatType { // Format contains a escaping=allow-utf-8 term, it will select NoEscaping. If a valid // "escaping" term exists, that will be used. Otherwise, the global default will // be returned. -func (format Format) ToEscapingScheme() model.EscapingScheme { - for _, p := range strings.Split(string(format), ";") { +func (f Format) ToEscapingScheme() model.EscapingScheme { + for _, p := range strings.Split(string(f), ";") { toks := strings.Split(p, "=") if len(toks) != 2 { continue diff --git a/expfmt/expfmt_test.go b/expfmt/expfmt_test.go index d9373bcf3..7ac054f23 100644 --- a/expfmt/expfmt_test.go +++ b/expfmt/expfmt_test.go @@ -16,14 +16,12 @@ package expfmt import ( "testing" - "github.com/prometheus/common/model" - "github.com/stretchr/testify/require" + + "github.com/prometheus/common/model" ) -// Test Format to Escapting Scheme conversion -// Path: expfmt/expfmt_test.go -// Compare this snippet from expfmt/expfmt.go: +// Test Format to Escaping Scheme conversion. func TestToFormatType(t *testing.T) { tests := []struct { format Format diff --git a/expfmt/fuzz.go b/expfmt/fuzz.go index dfac962a4..0290f6abc 100644 --- a/expfmt/fuzz.go +++ b/expfmt/fuzz.go @@ -17,7 +17,11 @@ package expfmt -import "bytes" +import ( + "bytes" + + "github.com/prometheus/common/model" +) // Fuzz text metric parser with with github.com/dvyukov/go-fuzz: // @@ -26,9 +30,8 @@ import "bytes" // // Further input samples should go in the folder fuzz/corpus. func Fuzz(in []byte) int { - parser := TextParser{} + parser := NewTextParser(model.UTF8Validation) _, err := parser.TextToMetricFamilies(bytes.NewReader(in)) - if err != nil { return 0 } diff --git a/expfmt/openmetrics_create.go b/expfmt/openmetrics_create.go index a21ed4ec1..8c8bbaa62 100644 --- a/expfmt/openmetrics_create.go +++ b/expfmt/openmetrics_create.go @@ -22,11 +22,10 @@ import ( "strconv" "strings" + dto "github.com/prometheus/client_model/go" "google.golang.org/protobuf/types/known/timestamppb" "github.com/prometheus/common/model" - - dto "github.com/prometheus/client_model/go" ) type encoderOption struct { @@ -209,6 +208,8 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...E n, err = w.WriteString(" unknown\n") case dto.MetricType_HISTOGRAM: n, err = w.WriteString(" histogram\n") + case dto.MetricType_GAUGE_HISTOGRAM: + n, err = w.WriteString(" gaugehistogram\n") default: return written, fmt.Errorf("unknown metric type %s", metricType.String()) } @@ -249,7 +250,7 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...E // Finally the samples, one line for each. if metricType == dto.MetricType_COUNTER && strings.HasSuffix(name, "_total") { - compliantName = compliantName + "_total" + compliantName += "_total" } for _, metric := range in.Metric { switch metricType { @@ -326,7 +327,7 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...E createdTsBytesWritten, err = writeOpenMetricsCreated(w, compliantName, "", metric, "", 0, metric.Summary.GetCreatedTimestamp()) n += createdTsBytesWritten } - case dto.MetricType_HISTOGRAM: + case dto.MetricType_HISTOGRAM, dto.MetricType_GAUGE_HISTOGRAM: if metric.Histogram == nil { return written, fmt.Errorf( "expected histogram in metric %s %s", compliantName, metric, @@ -334,6 +335,12 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...E } infSeen := false for _, b := range metric.Histogram.Bucket { + if b.GetCumulativeCountFloat() > 0 { + return written, fmt.Errorf( + "OpenMetrics v1.0 does not support float histogram %s %s", + compliantName, metric, + ) + } n, err = writeOpenMetricsSample( w, compliantName, "_bucket", metric, model.BucketLabel, b.GetUpperBound(), @@ -355,6 +362,9 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...E 0, metric.Histogram.GetSampleCount(), true, nil, ) + // We do not check for a float sample count here + // because we will check for it below (and error + // out if needed). written += n if err != nil { return @@ -369,6 +379,12 @@ func MetricFamilyToOpenMetrics(out io.Writer, in *dto.MetricFamily, options ...E if err != nil { return } + if metric.Histogram.GetSampleCountFloat() > 0 { + return written, fmt.Errorf( + "OpenMetrics v1.0 does not support float histogram %s %s", + compliantName, metric, + ) + } n, err = writeOpenMetricsSample( w, compliantName, "_count", metric, "", 0, 0, metric.Histogram.GetSampleCount(), true, @@ -477,7 +493,7 @@ func writeOpenMetricsNameAndLabelPairs( if name != "" { // If the name does not pass the legacy validity check, we must put the // metric name inside the braces, quoted. - if !model.IsValidLegacyMetricName(name) { + if !model.LegacyValidation.IsValidMetricName(name) { metricInsideBraces = true err := w.WriteByte(separator) written++ @@ -641,11 +657,11 @@ func writeExemplar(w enhancedWriter, e *dto.Exemplar) (int, error) { if err != nil { return written, err } - err = (*e).Timestamp.CheckValid() + err = e.Timestamp.CheckValid() if err != nil { return written, err } - ts := (*e).Timestamp.AsTime() + ts := e.Timestamp.AsTime() // TODO(beorn7): Format this directly from components of ts to // avoid overflow/underflow and precision issues of the float // conversion. diff --git a/expfmt/openmetrics_create_test.go b/expfmt/openmetrics_create_test.go index a81bfed3f..cd941f844 100644 --- a/expfmt/openmetrics_create_test.go +++ b/expfmt/openmetrics_create_test.go @@ -20,11 +20,10 @@ import ( "testing" "time" - "google.golang.org/protobuf/proto" - "google.golang.org/protobuf/types/known/timestamppb" - dto "github.com/prometheus/client_model/go" "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/timestamppb" "github.com/prometheus/common/model" ) @@ -744,6 +743,50 @@ request_duration_microseconds_count 2693 # UNIT some_measure_seconds seconds some_measure_seconds_total{labelname="val1",basename="basevalue"} 42.0 some_measure_seconds_total{labelname="val2",basename="basevalue"} 0.23 1.23456789e+06 +`, + }, + // 11: Gauge histogram. + { + in: &dto.MetricFamily{ + Name: proto.String("name"), + Help: proto.String("doc string"), + Type: dto.MetricType_GAUGE_HISTOGRAM.Enum(), + Metric: []*dto.Metric{ + { + Histogram: &dto.Histogram{ + SampleCount: proto.Uint64(2693), + SampleSum: proto.Float64(1756047.3), + Bucket: []*dto.Bucket{ + { + UpperBound: proto.Float64(100), + CumulativeCount: proto.Uint64(123), + }, + { + UpperBound: proto.Float64(120), + CumulativeCount: proto.Uint64(412), + }, + { + UpperBound: proto.Float64(144), + CumulativeCount: proto.Uint64(592), + }, + { + UpperBound: proto.Float64(172.8), + CumulativeCount: proto.Uint64(1524), + }, + }, + }, + }, + }, + }, + out: `# HELP name doc string +# TYPE name gaugehistogram +name_bucket{le="100.0"} 123 +name_bucket{le="120.0"} 412 +name_bucket{le="144.0"} 592 +name_bucket{le="172.8"} 1524 +name_bucket{le="+Inf"} 2693 +name_sum 1.7560473e+06 +name_count 2693 `, }, } @@ -904,6 +947,47 @@ func TestOpenMetricsCreateError(t *testing.T) { }, err: "expected counter in metric", }, + // 2: Float histogram. + { + in: &dto.MetricFamily{ + Name: proto.String("name"), + Help: proto.String("doc string"), + Type: dto.MetricType_GAUGE_HISTOGRAM.Enum(), + Metric: []*dto.Metric{ + { + Histogram: &dto.Histogram{ + // Note that it is enough to fill the float fields even + // if the values are integers. + SampleCountFloat: proto.Float64(2693), + SampleSum: proto.Float64(1756047.3), + Bucket: []*dto.Bucket{ + { + UpperBound: proto.Float64(100), + CumulativeCountFloat: proto.Float64(123), + }, + { + UpperBound: proto.Float64(120), + CumulativeCountFloat: proto.Float64(412), + }, + { + UpperBound: proto.Float64(144), + CumulativeCountFloat: proto.Float64(592), + }, + { + UpperBound: proto.Float64(172.8), + CumulativeCountFloat: proto.Float64(1524), + }, + { + UpperBound: proto.Float64(math.Inf(+1)), + CumulativeCountFloat: proto.Float64(2693), + }, + }, + }, + }, + }, + }, + err: "OpenMetrics v1.0 does not support float histogram name histogram", + }, } for i, scenario := range scenarios { diff --git a/expfmt/text_create.go b/expfmt/text_create.go index 4b86434b3..7e1d23cab 100644 --- a/expfmt/text_create.go +++ b/expfmt/text_create.go @@ -22,9 +22,9 @@ import ( "strings" "sync" - "github.com/prometheus/common/model" - dto "github.com/prometheus/client_model/go" + + "github.com/prometheus/common/model" ) // enhancedWriter has all the enhanced write functions needed here. bufio.Writer @@ -151,7 +151,10 @@ func MetricFamilyToText(out io.Writer, in *dto.MetricFamily) (written int, err e n, err = w.WriteString(" summary\n") case dto.MetricType_UNTYPED: n, err = w.WriteString(" untyped\n") - case dto.MetricType_HISTOGRAM: + case dto.MetricType_HISTOGRAM, dto.MetricType_GAUGE_HISTOGRAM: + // The classic Prometheus text format has no notion of a gauge + // histogram. We render a gauge histogram in the same way as a + // regular histogram. n, err = w.WriteString(" histogram\n") default: return written, fmt.Errorf("unknown metric type %s", metricType.String()) @@ -223,7 +226,7 @@ func MetricFamilyToText(out io.Writer, in *dto.MetricFamily) (written int, err e w, name, "_count", metric, "", 0, float64(metric.Summary.GetSampleCount()), ) - case dto.MetricType_HISTOGRAM: + case dto.MetricType_HISTOGRAM, dto.MetricType_GAUGE_HISTOGRAM: if metric.Histogram == nil { return written, fmt.Errorf( "expected histogram in metric %s %s", name, metric, @@ -231,10 +234,14 @@ func MetricFamilyToText(out io.Writer, in *dto.MetricFamily) (written int, err e } infSeen := false for _, b := range metric.Histogram.Bucket { + v := b.GetCumulativeCountFloat() + if v == 0 { + v = float64(b.GetCumulativeCount()) + } n, err = writeSample( w, name, "_bucket", metric, model.BucketLabel, b.GetUpperBound(), - float64(b.GetCumulativeCount()), + v, ) written += n if err != nil { @@ -245,10 +252,14 @@ func MetricFamilyToText(out io.Writer, in *dto.MetricFamily) (written int, err e } } if !infSeen { + v := metric.Histogram.GetSampleCountFloat() + if v == 0 { + v = float64(metric.Histogram.GetSampleCount()) + } n, err = writeSample( w, name, "_bucket", metric, model.BucketLabel, math.Inf(+1), - float64(metric.Histogram.GetSampleCount()), + v, ) written += n if err != nil { @@ -263,10 +274,11 @@ func MetricFamilyToText(out io.Writer, in *dto.MetricFamily) (written int, err e if err != nil { return } - n, err = writeSample( - w, name, "_count", metric, "", 0, - float64(metric.Histogram.GetSampleCount()), - ) + v := metric.Histogram.GetSampleCountFloat() + if v == 0 { + v = float64(metric.Histogram.GetSampleCount()) + } + n, err = writeSample(w, name, "_count", metric, "", 0, v) default: return written, fmt.Errorf( "unexpected type in metric %s %s", name, metric, @@ -354,7 +366,7 @@ func writeNameAndLabelPairs( if name != "" { // If the name does not pass the legacy validity check, we must put the // metric name inside the braces. - if !model.IsValidLegacyMetricName(name) { + if !model.LegacyValidation.IsValidMetricName(name) { metricInsideBraces = true err := w.WriteByte(separator) written++ @@ -498,7 +510,7 @@ func writeInt(w enhancedWriter, i int64) (int, error) { // writeName writes a string as-is if it complies with the legacy naming // scheme, or escapes it in double quotes if not. func writeName(w enhancedWriter, name string) (int, error) { - if model.IsValidLegacyMetricName(name) { + if model.LegacyValidation.IsValidMetricName(name) { return w.WriteString(name) } var written int diff --git a/expfmt/text_create_test.go b/expfmt/text_create_test.go index bb1c8f77a..dc214966b 100644 --- a/expfmt/text_create_test.go +++ b/expfmt/text_create_test.go @@ -19,10 +19,9 @@ import ( "strings" "testing" - "google.golang.org/protobuf/proto" - dto "github.com/prometheus/client_model/go" "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" "github.com/prometheus/common/model" ) @@ -384,6 +383,186 @@ request_duration_microseconds_count 2693 out: `# HELP name doc string # TYPE name counter name -Inf +`, + }, + // 8: Float histogram with +Inf. + { + in: &dto.MetricFamily{ + Name: proto.String("name"), + Help: proto.String("doc string"), + Type: dto.MetricType_HISTOGRAM.Enum(), + Metric: []*dto.Metric{ + { + Histogram: &dto.Histogram{ + SampleCountFloat: proto.Float64(2693.123), + SampleSum: proto.Float64(1756047.3), + Bucket: []*dto.Bucket{ + { + UpperBound: proto.Float64(100), + CumulativeCountFloat: proto.Float64(123.123), + }, + { + UpperBound: proto.Float64(120), + CumulativeCountFloat: proto.Float64(412), + }, + { + UpperBound: proto.Float64(144), + CumulativeCountFloat: proto.Float64(592), + }, + { + UpperBound: proto.Float64(172.8), + CumulativeCountFloat: proto.Float64(1524), + }, + { + UpperBound: proto.Float64(math.Inf(+1)), + CumulativeCountFloat: proto.Float64(2693.123), + }, + }, + }, + }, + }, + }, + out: `# HELP name doc string +# TYPE name histogram +name_bucket{le="100"} 123.123 +name_bucket{le="120"} 412 +name_bucket{le="144"} 592 +name_bucket{le="172.8"} 1524 +name_bucket{le="+Inf"} 2693.123 +name_sum 1.7560473e+06 +name_count 2693.123 +`, + }, + // 9: Float histogram without +Inf. + { + in: &dto.MetricFamily{ + Name: proto.String("name"), + Help: proto.String("doc string"), + Type: dto.MetricType_HISTOGRAM.Enum(), + Metric: []*dto.Metric{ + { + Histogram: &dto.Histogram{ + SampleCountFloat: proto.Float64(2693.123), + SampleSum: proto.Float64(1756047.3), + Bucket: []*dto.Bucket{ + { + UpperBound: proto.Float64(100), + CumulativeCountFloat: proto.Float64(123.123), + }, + { + UpperBound: proto.Float64(120), + CumulativeCountFloat: proto.Float64(412), + }, + { + UpperBound: proto.Float64(144), + CumulativeCountFloat: proto.Float64(592), + }, + { + UpperBound: proto.Float64(172.8), + CumulativeCountFloat: proto.Float64(1524), + }, + }, + }, + }, + }, + }, + out: `# HELP name doc string +# TYPE name histogram +name_bucket{le="100"} 123.123 +name_bucket{le="120"} 412 +name_bucket{le="144"} 592 +name_bucket{le="172.8"} 1524 +name_bucket{le="+Inf"} 2693.123 +name_sum 1.7560473e+06 +name_count 2693.123 +`, + }, + // 10: Gauge histogram (rendered as regular histogram) + { + in: &dto.MetricFamily{ + Name: proto.String("name"), + Help: proto.String("doc string"), + Type: dto.MetricType_GAUGE_HISTOGRAM.Enum(), + Metric: []*dto.Metric{ + { + Histogram: &dto.Histogram{ + SampleCount: proto.Uint64(2693), + SampleSum: proto.Float64(1756047.3), + Bucket: []*dto.Bucket{ + { + UpperBound: proto.Float64(100), + CumulativeCount: proto.Uint64(123), + }, + { + UpperBound: proto.Float64(120), + CumulativeCount: proto.Uint64(412), + }, + { + UpperBound: proto.Float64(144), + CumulativeCount: proto.Uint64(592), + }, + { + UpperBound: proto.Float64(172.8), + CumulativeCount: proto.Uint64(1524), + }, + }, + }, + }, + }, + }, + out: `# HELP name doc string +# TYPE name histogram +name_bucket{le="100"} 123 +name_bucket{le="120"} 412 +name_bucket{le="144"} 592 +name_bucket{le="172.8"} 1524 +name_bucket{le="+Inf"} 2693 +name_sum 1.7560473e+06 +name_count 2693 +`, + }, + // 11: Gauge float histogram (rendered as regular histogram) + { + in: &dto.MetricFamily{ + Name: proto.String("name"), + Help: proto.String("doc string"), + Type: dto.MetricType_GAUGE_HISTOGRAM.Enum(), + Metric: []*dto.Metric{ + { + Histogram: &dto.Histogram{ + SampleCountFloat: proto.Float64(2693.123), + SampleSum: proto.Float64(1756047.3), + Bucket: []*dto.Bucket{ + { + UpperBound: proto.Float64(100), + CumulativeCountFloat: proto.Float64(123.123), + }, + { + UpperBound: proto.Float64(120), + CumulativeCountFloat: proto.Float64(412), + }, + { + UpperBound: proto.Float64(144), + CumulativeCountFloat: proto.Float64(592), + }, + { + UpperBound: proto.Float64(172.8), + CumulativeCountFloat: proto.Float64(1524), + }, + }, + }, + }, + }, + }, + out: `# HELP name doc string +# TYPE name histogram +name_bucket{le="100"} 123.123 +name_bucket{le="120"} 412 +name_bucket{le="144"} 592 +name_bucket{le="172.8"} 1524 +name_bucket{le="+Inf"} 2693.123 +name_sum 1.7560473e+06 +name_count 2693.123 `, }, } diff --git a/expfmt/text_parse.go b/expfmt/text_parse.go index b4607fe4d..00c8841a1 100644 --- a/expfmt/text_parse.go +++ b/expfmt/text_parse.go @@ -48,8 +48,10 @@ func (e ParseError) Error() string { return fmt.Sprintf("text format parsing error in line %d: %s", e.Line, e.Msg) } -// TextParser is used to parse the simple and flat text-based exchange format. Its -// zero value is ready to use. +// TextParser is used to parse the simple and flat text-based exchange format. +// +// TextParser instances must be created with NewTextParser, the zero value of +// TextParser is invalid. type TextParser struct { metricFamiliesByName map[string]*dto.MetricFamily buf *bufio.Reader // Where the parsed input is read through. @@ -78,6 +80,14 @@ type TextParser struct { // These indicate if the metric name from the current line being parsed is inside // braces and if that metric name was found respectively. currentMetricIsInsideBraces, currentMetricInsideBracesIsPresent bool + // scheme sets the desired ValidationScheme for names. Defaults to the invalid + // UnsetValidation. + scheme model.ValidationScheme +} + +// NewTextParser returns a new TextParser with the provided nameValidationScheme. +func NewTextParser(nameValidationScheme model.ValidationScheme) TextParser { + return TextParser{scheme: nameValidationScheme} } // TextToMetricFamilies reads 'in' as the simple and flat text-based exchange @@ -121,11 +131,47 @@ func (p *TextParser) TextToMetricFamilies(in io.Reader) (map[string]*dto.MetricF if p.err != nil && errors.Is(p.err, io.EOF) { p.parseError("unexpected end of input stream") } + for _, histogramMetric := range p.histograms { + normalizeHistogram(histogramMetric.GetHistogram()) + } return p.metricFamiliesByName, p.err } +// normalizeHistogram makes sure that all the buckets and the count in each +// histogram is either completely float or completely integer. +func normalizeHistogram(histogram *dto.Histogram) { + if histogram == nil { + return + } + anyFloats := false + if histogram.GetSampleCountFloat() != 0 { + anyFloats = true + } else { + for _, b := range histogram.GetBucket() { + if b.GetCumulativeCountFloat() != 0 { + anyFloats = true + break + } + } + } + if !anyFloats { + return + } + if histogram.GetSampleCountFloat() == 0 { + histogram.SampleCountFloat = proto.Float64(float64(histogram.GetSampleCount())) + histogram.SampleCount = nil + } + for _, b := range histogram.GetBucket() { + if b.GetCumulativeCountFloat() == 0 { + b.CumulativeCountFloat = proto.Float64(float64(b.GetCumulativeCount())) + b.CumulativeCount = nil + } + } +} + func (p *TextParser) reset(in io.Reader) { p.metricFamiliesByName = map[string]*dto.MetricFamily{} + p.currentLabelPairs = nil if p.buf == nil { p.buf = bufio.NewReader(in) } else { @@ -216,6 +262,9 @@ func (p *TextParser) startComment() stateFn { return nil } p.setOrCreateCurrentMF() + if p.err != nil { + return nil + } if p.skipBlankTab(); p.err != nil { return nil // Unexpected end of input. } @@ -244,6 +293,9 @@ func (p *TextParser) readingMetricName() stateFn { return nil } p.setOrCreateCurrentMF() + if p.err != nil { + return nil + } // Now is the time to fix the type if it hasn't happened yet. if p.currentMF.Type == nil { p.currentMF.Type = dto.MetricType_UNTYPED.Enum() @@ -266,7 +318,9 @@ func (p *TextParser) readingLabels() stateFn { // Summaries/histograms are special. We have to reset the // currentLabels map, currentQuantile and currentBucket before starting to // read labels. - if p.currentMF.GetType() == dto.MetricType_SUMMARY || p.currentMF.GetType() == dto.MetricType_HISTOGRAM { + if p.currentMF.GetType() == dto.MetricType_SUMMARY || + p.currentMF.GetType() == dto.MetricType_HISTOGRAM || + p.currentMF.GetType() == dto.MetricType_GAUGE_HISTOGRAM { p.currentLabels = map[string]string{} p.currentLabels[string(model.MetricNameLabel)] = p.currentMF.GetName() p.currentQuantile = math.NaN() @@ -311,6 +365,9 @@ func (p *TextParser) startLabelName() stateFn { switch p.currentByte { case ',': p.setOrCreateCurrentMF() + if p.err != nil { + return nil + } if p.currentMF.Type == nil { p.currentMF.Type = dto.MetricType_UNTYPED.Enum() } @@ -319,6 +376,10 @@ func (p *TextParser) startLabelName() stateFn { return p.startLabelName case '}': p.setOrCreateCurrentMF() + if p.err != nil { + p.currentLabelPairs = nil + return nil + } if p.currentMF.Type == nil { p.currentMF.Type = dto.MetricType_UNTYPED.Enum() } @@ -341,25 +402,32 @@ func (p *TextParser) startLabelName() stateFn { p.currentLabelPair = &dto.LabelPair{Name: proto.String(p.currentToken.String())} if p.currentLabelPair.GetName() == string(model.MetricNameLabel) { p.parseError(fmt.Sprintf("label name %q is reserved", model.MetricNameLabel)) + p.currentLabelPairs = nil + return nil + } + if !p.scheme.IsValidLabelName(p.currentLabelPair.GetName()) { + p.parseError(fmt.Sprintf("invalid label name %q", p.currentLabelPair.GetName())) + p.currentLabelPairs = nil return nil } // Special summary/histogram treatment. Don't add 'quantile' and 'le' // labels to 'real' labels. - if !(p.currentMF.GetType() == dto.MetricType_SUMMARY && p.currentLabelPair.GetName() == model.QuantileLabel) && - !(p.currentMF.GetType() == dto.MetricType_HISTOGRAM && p.currentLabelPair.GetName() == model.BucketLabel) { + if (p.currentMF.GetType() != dto.MetricType_SUMMARY || p.currentLabelPair.GetName() != model.QuantileLabel) && + ((p.currentMF.GetType() != dto.MetricType_HISTOGRAM && + p.currentMF.GetType() != dto.MetricType_GAUGE_HISTOGRAM) || + p.currentLabelPair.GetName() != model.BucketLabel) { p.currentLabelPairs = append(p.currentLabelPairs, p.currentLabelPair) } // Check for duplicate label names. labels := make(map[string]struct{}) for _, l := range p.currentLabelPairs { lName := l.GetName() - if _, exists := labels[lName]; !exists { - labels[lName] = struct{}{} - } else { + if _, exists := labels[lName]; exists { p.parseError(fmt.Sprintf("duplicate label names for metric %q", p.currentMF.GetName())) p.currentLabelPairs = nil return nil } + labels[lName] = struct{}{} } return p.startLabelValue } @@ -398,7 +466,7 @@ func (p *TextParser) startLabelValue() stateFn { } } // Similar special treatment of histograms. - if p.currentMF.GetType() == dto.MetricType_HISTOGRAM { + if p.currentMF.GetType() == dto.MetricType_HISTOGRAM || p.currentMF.GetType() == dto.MetricType_GAUGE_HISTOGRAM { if p.currentLabelPair.GetName() == model.BucketLabel { if p.currentBucket, p.err = parseFloat(p.currentLabelPair.GetValue()); p.err != nil { // Create a more helpful error message. @@ -440,7 +508,8 @@ func (p *TextParser) readingValue() stateFn { // When we are here, we have read all the labels, so for the // special case of a summary/histogram, we can finally find out // if the metric already exists. - if p.currentMF.GetType() == dto.MetricType_SUMMARY { + switch p.currentMF.GetType() { + case dto.MetricType_SUMMARY: signature := model.LabelsToSignature(p.currentLabels) if summary := p.summaries[signature]; summary != nil { p.currentMetric = summary @@ -448,7 +517,7 @@ func (p *TextParser) readingValue() stateFn { p.summaries[signature] = p.currentMetric p.currentMF.Metric = append(p.currentMF.Metric, p.currentMetric) } - } else if p.currentMF.GetType() == dto.MetricType_HISTOGRAM { + case dto.MetricType_HISTOGRAM, dto.MetricType_GAUGE_HISTOGRAM: signature := model.LabelsToSignature(p.currentLabels) if histogram := p.histograms[signature]; histogram != nil { p.currentMetric = histogram @@ -456,7 +525,7 @@ func (p *TextParser) readingValue() stateFn { p.histograms[signature] = p.currentMetric p.currentMF.Metric = append(p.currentMF.Metric, p.currentMetric) } - } else { + default: p.currentMF.Metric = append(p.currentMF.Metric, p.currentMetric) } if p.readTokenUntilWhitespace(); p.err != nil { @@ -494,24 +563,38 @@ func (p *TextParser) readingValue() stateFn { }, ) } - case dto.MetricType_HISTOGRAM: + case dto.MetricType_HISTOGRAM, dto.MetricType_GAUGE_HISTOGRAM: // *sigh* if p.currentMetric.Histogram == nil { p.currentMetric.Histogram = &dto.Histogram{} } switch { case p.currentIsHistogramCount: - p.currentMetric.Histogram.SampleCount = proto.Uint64(uint64(value)) + if uintValue := uint64(value); value == float64(uintValue) { + p.currentMetric.Histogram.SampleCount = proto.Uint64(uintValue) + } else { + if value < 0 { + p.parseError(fmt.Sprintf("negative count for histogram %q", p.currentMF.GetName())) + return nil + } + p.currentMetric.Histogram.SampleCountFloat = proto.Float64(value) + } case p.currentIsHistogramSum: p.currentMetric.Histogram.SampleSum = proto.Float64(value) case !math.IsNaN(p.currentBucket): - p.currentMetric.Histogram.Bucket = append( - p.currentMetric.Histogram.Bucket, - &dto.Bucket{ - UpperBound: proto.Float64(p.currentBucket), - CumulativeCount: proto.Uint64(uint64(value)), - }, - ) + b := &dto.Bucket{ + UpperBound: proto.Float64(p.currentBucket), + } + if uintValue := uint64(value); value == float64(uintValue) { + b.CumulativeCount = proto.Uint64(uintValue) + } else { + if value < 0 { + p.parseError(fmt.Sprintf("negative bucket population for histogram %q", p.currentMF.GetName())) + return nil + } + b.CumulativeCountFloat = proto.Float64(value) + } + p.currentMetric.Histogram.Bucket = append(p.currentMetric.Histogram.Bucket, b) } default: p.err = fmt.Errorf("unexpected type for metric name %q", p.currentMF.GetName()) @@ -574,10 +657,18 @@ func (p *TextParser) readingType() stateFn { if p.readTokenUntilNewline(false); p.err != nil { return nil // Unexpected end of input. } - metricType, ok := dto.MetricType_value[strings.ToUpper(p.currentToken.String())] + typ := strings.ToUpper(p.currentToken.String()) // Tolerate any combination of upper and lower case. + metricType, ok := dto.MetricType_value[typ] // Tolerate "gauge_histogram" (not originally part of the text format). if !ok { - p.parseError(fmt.Sprintf("unknown metric type %q", p.currentToken.String())) - return nil + // We also want to tolerate "gaugehistogram" to mark a gauge + // histogram, because that string is used in OpenMetrics. Note, + // however, that gauge histograms do not officially exist in the + // classic text format. + if typ != "GAUGEHISTOGRAM" { + p.parseError(fmt.Sprintf("unknown metric type %q", p.currentToken.String())) + return nil + } + metricType = int32(dto.MetricType_GAUGE_HISTOGRAM) } p.currentMF.Type = dto.MetricType(metricType).Enum() return p.startOfLine @@ -805,6 +896,10 @@ func (p *TextParser) setOrCreateCurrentMF() { p.currentIsHistogramCount = false p.currentIsHistogramSum = false name := p.currentToken.String() + if !p.scheme.IsValidMetricName(name) { + p.parseError(fmt.Sprintf("invalid metric name %q", name)) + return + } if p.currentMF = p.metricFamiliesByName[name]; p.currentMF != nil { return } @@ -823,7 +918,8 @@ func (p *TextParser) setOrCreateCurrentMF() { } histogramName := histogramMetricName(name) if p.currentMF = p.metricFamiliesByName[histogramName]; p.currentMF != nil { - if p.currentMF.GetType() == dto.MetricType_HISTOGRAM { + if p.currentMF.GetType() == dto.MetricType_HISTOGRAM || + p.currentMF.GetType() == dto.MetricType_GAUGE_HISTOGRAM { if isCount(name) { p.currentIsHistogramCount = true } diff --git a/expfmt/text_parse_test.go b/expfmt/text_parse_test.go index fac60ba6f..edcee13a0 100644 --- a/expfmt/text_parse_test.go +++ b/expfmt/text_parse_test.go @@ -21,8 +21,27 @@ import ( dto "github.com/prometheus/client_model/go" "google.golang.org/protobuf/proto" + + "github.com/prometheus/common/model" ) +func TestNewTextParser(t *testing.T) { + p := NewTextParser(model.UTF8Validation) + if p.scheme != model.UTF8Validation { + t.Errorf("expected NewTextParser to return a TextParser with scheme %s - got %s", model.UTF8Validation, p.scheme) + } + + p = NewTextParser(model.LegacyValidation) + if p.scheme != model.LegacyValidation { + t.Errorf("expected NewTextParser to return a TextParser with scheme %s - got %s", model.LegacyValidation, p.scheme) + } + + p = NewTextParser(model.UnsetValidation) + if p.scheme != model.UnsetValidation { + t.Errorf("expected NewTextParser to return a TextParser with scheme %s - got %s", model.UnsetValidation, p.scheme) + } +} + func testTextParse(t testing.TB) { scenarios := []struct { in string @@ -191,10 +210,10 @@ my_summary{n1="val3", quantile="0.2"} 4711 my_summary{n1="val1",n2="val2",quantile="-12.34",} NaN # some # funny comments -# HELP +# HELP # HELP # HELP my_summary -# HELP my_summary +# HELP my_summary `, out: []*dto.MetricFamily{ { @@ -637,6 +656,307 @@ request_duration_microseconds_count 2693 }, }, }, + // 12: A full float histogram. + { + in: ` +# HELP request_duration_microseconds The response latency. +# TYPE request_duration_microseconds histogram +request_duration_microseconds_bucket{le="100"} 123.5 +request_duration_microseconds_bucket{le="120"} 412.6 +request_duration_microseconds_bucket{le="144"} 592.7 +request_duration_microseconds_bucket{le="172.8"} 1524.8 +request_duration_microseconds_bucket{le="+Inf"} 2693.9 +request_duration_microseconds_sum 1.7560473e+06 +request_duration_microseconds_count 2693.9 +`, + out: []*dto.MetricFamily{ + { + Name: proto.String("request_duration_microseconds"), + Help: proto.String("The response latency."), + Type: dto.MetricType_HISTOGRAM.Enum(), + Metric: []*dto.Metric{ + { + Histogram: &dto.Histogram{ + SampleCountFloat: proto.Float64(2693.9), + SampleSum: proto.Float64(1756047.3), + Bucket: []*dto.Bucket{ + { + UpperBound: proto.Float64(100), + CumulativeCountFloat: proto.Float64(123.5), + }, + { + UpperBound: proto.Float64(120), + CumulativeCountFloat: proto.Float64(412.6), + }, + { + UpperBound: proto.Float64(144), + CumulativeCountFloat: proto.Float64(592.7), + }, + { + UpperBound: proto.Float64(172.8), + CumulativeCountFloat: proto.Float64(1524.8), + }, + { + UpperBound: proto.Float64(math.Inf(+1)), + CumulativeCountFloat: proto.Float64(2693.9), + }, + }, + }, + }, + }, + }, + }, + }, + // 13: A float histogram where only the count is really a float. + { + in: ` +# HELP request_duration_microseconds The response latency. +# TYPE request_duration_microseconds histogram +request_duration_microseconds_bucket{le="100"} 123 +request_duration_microseconds_bucket{le="120"} 412 +request_duration_microseconds_bucket{le="144"} 592 +request_duration_microseconds_bucket{le="172.8"} 1524 +request_duration_microseconds_sum 1.7560473e+06 +request_duration_microseconds_count 2693.9 +`, + out: []*dto.MetricFamily{ + { + Name: proto.String("request_duration_microseconds"), + Help: proto.String("The response latency."), + Type: dto.MetricType_HISTOGRAM.Enum(), + Metric: []*dto.Metric{ + { + Histogram: &dto.Histogram{ + SampleCountFloat: proto.Float64(2693.9), + SampleSum: proto.Float64(1756047.3), + Bucket: []*dto.Bucket{ + { + UpperBound: proto.Float64(100), + CumulativeCountFloat: proto.Float64(123), + }, + { + UpperBound: proto.Float64(120), + CumulativeCountFloat: proto.Float64(412), + }, + { + UpperBound: proto.Float64(144), + CumulativeCountFloat: proto.Float64(592), + }, + { + UpperBound: proto.Float64(172.8), + CumulativeCountFloat: proto.Float64(1524), + }, + }, + }, + }, + }, + }, + }, + }, + // 14: A float histogram where only one of the buckets is really a float. + { + in: ` +# HELP request_duration_microseconds The response latency. +# TYPE request_duration_microseconds histogram +request_duration_microseconds_bucket{le="100"} 123 +request_duration_microseconds_bucket{le="120"} 412 +request_duration_microseconds_bucket{le="144"} 592 +request_duration_microseconds_bucket{le="172.8"} 1524.8 +request_duration_microseconds_bucket{le="+Inf"} 2693 +request_duration_microseconds_sum 1.7560473e+06 +request_duration_microseconds_count 2693 +`, + out: []*dto.MetricFamily{ + { + Name: proto.String("request_duration_microseconds"), + Help: proto.String("The response latency."), + Type: dto.MetricType_HISTOGRAM.Enum(), + Metric: []*dto.Metric{ + { + Histogram: &dto.Histogram{ + SampleCountFloat: proto.Float64(2693), + SampleSum: proto.Float64(1756047.3), + Bucket: []*dto.Bucket{ + { + UpperBound: proto.Float64(100), + CumulativeCountFloat: proto.Float64(123), + }, + { + UpperBound: proto.Float64(120), + CumulativeCountFloat: proto.Float64(412), + }, + { + UpperBound: proto.Float64(144), + CumulativeCountFloat: proto.Float64(592), + }, + { + UpperBound: proto.Float64(172.8), + CumulativeCountFloat: proto.Float64(1524.8), + }, + { + UpperBound: proto.Float64(math.Inf(+1)), + CumulativeCountFloat: proto.Float64(2693), + }, + }, + }, + }, + }, + }, + }, + }, + // 15: A gauge histogram. + { + in: ` +# HELP request_duration_microseconds The response latency. +# TYPE request_duration_microseconds gaugehistogram +request_duration_microseconds_bucket{le="100"} 123 +request_duration_microseconds_bucket{le="120"} 412 +request_duration_microseconds_bucket{le="144"} 592 +request_duration_microseconds_bucket{le="172.8"} 1524 +request_duration_microseconds_bucket{le="+Inf"} 2693 +request_duration_microseconds_sum 1.7560473e+06 +request_duration_microseconds_count 2693 +`, + out: []*dto.MetricFamily{ + { + Name: proto.String("request_duration_microseconds"), + Help: proto.String("The response latency."), + Type: dto.MetricType_GAUGE_HISTOGRAM.Enum(), + Metric: []*dto.Metric{ + { + Histogram: &dto.Histogram{ + SampleCount: proto.Uint64(2693), + SampleSum: proto.Float64(1756047.3), + Bucket: []*dto.Bucket{ + { + UpperBound: proto.Float64(100), + CumulativeCount: proto.Uint64(123), + }, + { + UpperBound: proto.Float64(120), + CumulativeCount: proto.Uint64(412), + }, + { + UpperBound: proto.Float64(144), + CumulativeCount: proto.Uint64(592), + }, + { + UpperBound: proto.Float64(172.8), + CumulativeCount: proto.Uint64(1524), + }, + { + UpperBound: proto.Float64(math.Inf(+1)), + CumulativeCount: proto.Uint64(2693), + }, + }, + }, + }, + }, + }, + }, + }, + // 16: A gauge histogram with alternative spelling. + { + in: ` +# HELP request_duration_microseconds The response latency. +# TYPE request_duration_microseconds gauge_histogram +request_duration_microseconds_bucket{le="100"} 123 +request_duration_microseconds_bucket{le="120"} 412 +request_duration_microseconds_bucket{le="144"} 592 +request_duration_microseconds_bucket{le="172.8"} 1524 +request_duration_microseconds_bucket{le="+Inf"} 2693 +request_duration_microseconds_sum 1.7560473e+06 +request_duration_microseconds_count 2693 +`, + out: []*dto.MetricFamily{ + { + Name: proto.String("request_duration_microseconds"), + Help: proto.String("The response latency."), + Type: dto.MetricType_GAUGE_HISTOGRAM.Enum(), + Metric: []*dto.Metric{ + { + Histogram: &dto.Histogram{ + SampleCount: proto.Uint64(2693), + SampleSum: proto.Float64(1756047.3), + Bucket: []*dto.Bucket{ + { + UpperBound: proto.Float64(100), + CumulativeCount: proto.Uint64(123), + }, + { + UpperBound: proto.Float64(120), + CumulativeCount: proto.Uint64(412), + }, + { + UpperBound: proto.Float64(144), + CumulativeCount: proto.Uint64(592), + }, + { + UpperBound: proto.Float64(172.8), + CumulativeCount: proto.Uint64(1524), + }, + { + UpperBound: proto.Float64(math.Inf(+1)), + CumulativeCount: proto.Uint64(2693), + }, + }, + }, + }, + }, + }, + }, + }, + // 17: A float gauge histogram where only one of the buckets is really a float and with alternative spelling. + { + in: ` +# HELP request_duration_microseconds The response latency. +# TYPE request_duration_microseconds gauge_histogram +request_duration_microseconds_bucket{le="100"} 123 +request_duration_microseconds_bucket{le="120"} 412 +request_duration_microseconds_bucket{le="144"} 592 +request_duration_microseconds_bucket{le="172.8"} 1524.8 +request_duration_microseconds_bucket{le="+Inf"} 2693 +request_duration_microseconds_sum 1.7560473e+06 +request_duration_microseconds_count 2693 +`, + out: []*dto.MetricFamily{ + { + Name: proto.String("request_duration_microseconds"), + Help: proto.String("The response latency."), + Type: dto.MetricType_GAUGE_HISTOGRAM.Enum(), + Metric: []*dto.Metric{ + { + Histogram: &dto.Histogram{ + SampleCountFloat: proto.Float64(2693), + SampleSum: proto.Float64(1756047.3), + Bucket: []*dto.Bucket{ + { + UpperBound: proto.Float64(100), + CumulativeCountFloat: proto.Float64(123), + }, + { + UpperBound: proto.Float64(120), + CumulativeCountFloat: proto.Float64(412), + }, + { + UpperBound: proto.Float64(144), + CumulativeCountFloat: proto.Float64(592), + }, + { + UpperBound: proto.Float64(172.8), + CumulativeCountFloat: proto.Float64(1524.8), + }, + { + UpperBound: proto.Float64(math.Inf(+1)), + CumulativeCountFloat: proto.Float64(2693), + }, + }, + }, + }, + }, + }, + }, + }, } for i, scenario := range scenarios { @@ -682,20 +1002,22 @@ func BenchmarkTextParse(b *testing.B) { func testTextParseError(t testing.TB) { scenarios := []struct { - in string - err string + in string + errUTF8 string + // if errLegacy is blank, it is assumed to be the same as errUTF8 + errLegacy string }{ // 0: No new-line at end of input. { in: ` bla 3.14 blubber 42`, - err: "text format parsing error in line 3: unexpected end of input stream", + errUTF8: "text format parsing error in line 3: unexpected end of input stream", }, // 1: Invalid escape sequence in label value. { - in: `metric{label="\t"} 3.14`, - err: "text format parsing error in line 1: invalid escape sequence", + in: `metric{label="\t"} 3.14`, + errUTF8: "text format parsing error in line 1: invalid escape sequence", }, // 2: Newline in label value. { @@ -703,27 +1025,27 @@ blubber 42`, metric{label="new line"} 3.14 `, - err: `text format parsing error in line 2: label value "new" contains unescaped new-line`, + errUTF8: `text format parsing error in line 2: label value "new" contains unescaped new-line`, }, // 3: { - in: `metric{@="bla"} 3.14`, - err: "text format parsing error in line 1: invalid label name for metric", + in: `metric{@="bla"} 3.14`, + errUTF8: "text format parsing error in line 1: invalid label name for metric", }, // 4: { - in: `metric{__name__="bla"} 3.14`, - err: `text format parsing error in line 1: label name "__name__" is reserved`, + in: `metric{__name__="bla"} 3.14`, + errUTF8: `text format parsing error in line 1: label name "__name__" is reserved`, }, // 5: { - in: `metric{label+="bla"} 3.14`, - err: "text format parsing error in line 1: expected '=' after label name", + in: `metric{label+="bla"} 3.14`, + errUTF8: "text format parsing error in line 1: expected '=' after label name", }, // 6: { - in: `metric{label=bla} 3.14`, - err: "text format parsing error in line 1: expected '\"' at start of label value", + in: `metric{label=bla} 3.14`, + errUTF8: "text format parsing error in line 1: expected '\"' at start of label value", }, // 7: { @@ -731,30 +1053,30 @@ line"} 3.14 # TYPE metric summary metric{quantile="bla"} 3.14 `, - err: "text format parsing error in line 3: expected float as value for 'quantile' label", + errUTF8: "text format parsing error in line 3: expected float as value for 'quantile' label", }, // 8: { - in: `metric{label="bla"+} 3.14`, - err: "text format parsing error in line 1: unexpected end of label value", + in: `metric{label="bla"+} 3.14`, + errUTF8: "text format parsing error in line 1: unexpected end of label value", }, // 9: { in: `metric{label="bla"} 3.14 2.72 `, - err: "text format parsing error in line 1: expected integer as timestamp", + errUTF8: "text format parsing error in line 1: expected integer as timestamp", }, // 10: { in: `metric{label="bla"} 3.14 2 3 `, - err: "text format parsing error in line 1: spurious string after timestamp", + errUTF8: "text format parsing error in line 1: spurious string after timestamp", }, // 11: { in: `metric{label="bla"} blubb `, - err: "text format parsing error in line 1: expected float as value", + errUTF8: "text format parsing error in line 1: expected float as value", }, // 12: { @@ -762,7 +1084,7 @@ metric{quantile="bla"} 3.14 # HELP metric one # HELP metric two `, - err: "text format parsing error in line 3: second HELP line for metric name", + errUTF8: "text format parsing error in line 3: second HELP line for metric name", }, // 13: { @@ -770,7 +1092,7 @@ metric{quantile="bla"} 3.14 # TYPE metric counter # TYPE metric untyped `, - err: `text format parsing error in line 3: second TYPE line for metric name "metric", or TYPE reported after samples`, + errUTF8: `text format parsing error in line 3: second TYPE line for metric name "metric", or TYPE reported after samples`, }, // 14: { @@ -778,145 +1100,146 @@ metric{quantile="bla"} 3.14 metric 4.12 # TYPE metric counter `, - err: `text format parsing error in line 3: second TYPE line for metric name "metric", or TYPE reported after samples`, + errUTF8: `text format parsing error in line 3: second TYPE line for metric name "metric", or TYPE reported after samples`, }, - // 14: + // 15: { in: ` # TYPE metric bla `, - err: "text format parsing error in line 2: unknown metric type", + errUTF8: "text format parsing error in line 2: unknown metric type", }, - // 15: + // 16: { in: ` # TYPE met-ric `, - err: "text format parsing error in line 2: invalid metric name in comment", - }, - // 16: - { - in: `@invalidmetric{label="bla"} 3.14 2`, - err: "text format parsing error in line 1: invalid metric name", + errUTF8: "text format parsing error in line 2: invalid metric name in comment", }, // 17: { - in: `{label="bla"} 3.14 2`, - err: "text format parsing error in line 1: invalid metric name", + in: `@invalidmetric{label="bla"} 3.14 2`, + errUTF8: "text format parsing error in line 1: invalid metric name", }, // 18: + { + in: `{label="bla"} 3.14 2`, + errUTF8: "text format parsing error in line 1: invalid metric name", + }, + // 19: { in: ` # TYPE metric histogram metric_bucket{le="bla"} 3.14 `, - err: "text format parsing error in line 3: expected float as value for 'le' label", + errUTF8: "text format parsing error in line 3: expected float as value for 'le' label", }, - // 19: Invalid UTF-8 in label value. + // 20: Invalid UTF-8 in label value. { - in: "metric{l=\"\xbd\"} 3.14\n", - err: "text format parsing error in line 1: invalid label value \"\\xbd\"", + in: "metric{l=\"\xbd\"} 3.14\n", + errUTF8: "text format parsing error in line 1: invalid label value \"\\xbd\"", }, - // 20: Go 1.13 sometimes allows underscores in numbers. + // 21: Go 1.13 sometimes allows underscores in numbers. { - in: "foo 1_2\n", - err: "text format parsing error in line 1: expected float as value", + in: "foo 1_2\n", + errUTF8: "text format parsing error in line 1: expected float as value", }, - // 21: Go 1.13 supports hex floating point. + // 22: Go 1.13 supports hex floating point. { - in: "foo 0x1p-3\n", - err: "text format parsing error in line 1: expected float as value", + in: "foo 0x1p-3\n", + errUTF8: "text format parsing error in line 1: expected float as value", }, - // 22: Check for various other literals variants, just in case. + // 23: Check for various other literals variants, just in case. { - in: "foo 0x1P-3\n", - err: "text format parsing error in line 1: expected float as value", - }, - // 23: - { - in: "foo 0B1\n", - err: "text format parsing error in line 1: expected float as value", + in: "foo 0x1P-3\n", + errUTF8: "text format parsing error in line 1: expected float as value", }, // 24: { - in: "foo 0O1\n", - err: "text format parsing error in line 1: expected float as value", + in: "foo 0B1\n", + errUTF8: "text format parsing error in line 1: expected float as value", }, // 25: { - in: "foo 0X1\n", - err: "text format parsing error in line 1: expected float as value", + in: "foo 0O1\n", + errUTF8: "text format parsing error in line 1: expected float as value", }, // 26: { - in: "foo 0x1\n", - err: "text format parsing error in line 1: expected float as value", + in: "foo 0X1\n", + errUTF8: "text format parsing error in line 1: expected float as value", }, // 27: { - in: "foo 0b1\n", - err: "text format parsing error in line 1: expected float as value", + in: "foo 0x1\n", + errUTF8: "text format parsing error in line 1: expected float as value", }, // 28: { - in: "foo 0o1\n", - err: "text format parsing error in line 1: expected float as value", + in: "foo 0b1\n", + errUTF8: "text format parsing error in line 1: expected float as value", }, // 29: { - in: "foo 0x1\n", - err: "text format parsing error in line 1: expected float as value", + in: "foo 0o1\n", + errUTF8: "text format parsing error in line 1: expected float as value", }, // 30: { - in: "foo 0x1\n", - err: "text format parsing error in line 1: expected float as value", + in: "foo 0x1\n", + errUTF8: "text format parsing error in line 1: expected float as value", }, - // 31: Check histogram label. + // 31: + { + in: "foo 0x1\n", + errUTF8: "text format parsing error in line 1: expected float as value", + }, + // 32: Check histogram label. { in: ` # TYPE metric histogram metric_bucket{le="0x1p-3"} 3.14 `, - err: "text format parsing error in line 3: expected float as value for 'le' label", + errUTF8: "text format parsing error in line 3: expected float as value for 'le' label", }, - // 32: Check quantile label. + // 33: Check quantile label. { in: ` # TYPE metric summary metric{quantile="0x1p-3"} 3.14 `, - err: "text format parsing error in line 3: expected float as value for 'quantile' label", + errUTF8: "text format parsing error in line 3: expected float as value for 'quantile' label", }, - // 33: Check duplicate label. + // 34: Check duplicate label. { - in: `metric{label="bla",label="bla"} 3.14`, - err: "text format parsing error in line 1: duplicate label names for metric", + in: `metric{label="bla",label="bla"} 3.14`, + errUTF8: "text format parsing error in line 1: duplicate label names for metric", }, - // 34: Multiple quoted metric names. + // 35: Multiple quoted metric names. { - in: `{"one.name","another.name"} 3.14`, - err: "text format parsing error in line 1: multiple metric names", + in: `{"one.name","another.name"} 3.14`, + errUTF8: "text format parsing error in line 1: multiple metric names", + errLegacy: `text format parsing error in line 1: invalid metric name "one.name"`, }, - // 35: Invalid escape sequence in quoted metric name. + // 36: Invalid escape sequence in quoted metric name. { - in: `{"a\xc5z",label="bla"} 3.14`, - err: "text format parsing error in line 1: invalid escape sequence", + in: `{"a\xc5z",label="bla"} 3.14`, + errUTF8: "text format parsing error in line 1: invalid escape sequence", }, - // 36: Unexpected end of quoted metric name. + // 37: Unexpected end of quoted metric name. { - in: `{"metric.name".label="bla"} 3.14`, - err: "text format parsing error in line 1: unexpected end of metric name", + in: `{"metric.name".label="bla"} 3.14`, + errUTF8: "text format parsing error in line 1: unexpected end of metric name", }, - // 37: Invalid escape sequence in quoted metric name. + // 38: Invalid escape sequence in quoted metric name. { in: ` # TYPE "metric.name\t" counter {"metric.name\t",label="bla"} 3.14 `, - err: "text format parsing error in line 2: invalid escape sequence", + errUTF8: "text format parsing error in line 2: invalid escape sequence", }, - // 38: Newline in quoted metric name. + // 39: Newline in quoted metric name. { in: ` # TYPE "metric @@ -924,29 +1247,124 @@ name" counter {"metric name",label="bla"} 3.14 `, - err: `text format parsing error in line 2: metric name "metric" contains unescaped new-line`, + errUTF8: `text format parsing error in line 2: metric name "metric" contains unescaped new-line`, }, - // 39: Newline in quoted label name. + // 40: Newline in quoted label name. { in: ` {"metric.name","new line"="bla"} 3.14 `, - err: `text format parsing error in line 2: label name "new" contains unescaped new-line`, + errUTF8: `text format parsing error in line 2: label name "new" contains unescaped new-line`, + errLegacy: `text format parsing error in line 2: invalid metric name "metric.name"`, + }, + // 41: Dotted metric name fails legacy validation. + { + in: `{"metric.name",foo="bla"} 3.14 +`, + errUTF8: ``, + errLegacy: `text format parsing error in line 1: invalid metric name "metric.name"`, + }, + // 42: Dotted label name fails legacy validation. + { + in: `metric_name{"foo"="bar", "dotted.label"="bla"} 3.14 +`, + errUTF8: ``, + errLegacy: `text format parsing error in line 1: invalid label name "dotted.label"`, + }, + // 43: Histogram with negative count. + { + in: ` +# HELP request_duration_microseconds The response latency. +# TYPE request_duration_microseconds histogram +request_duration_microseconds_bucket{le="100"} 123 +request_duration_microseconds_bucket{le="120"} 412 +request_duration_microseconds_bucket{le="144"} 592 +request_duration_microseconds_bucket{le="172.8"} 1524 +request_duration_microseconds_bucket{le="+Inf"} 2693 +request_duration_microseconds_sum 1.7560473e+06 +request_duration_microseconds_count -2693 +`, + errUTF8: `text format parsing error in line 10: negative count for histogram "request_duration_microseconds"`, + }, + // 44: Histogram with negative bucket. + { + in: ` +# HELP request_duration_microseconds The response latency. +# TYPE request_duration_microseconds histogram +request_duration_microseconds_bucket{le="100"} 123 +request_duration_microseconds_bucket{le="120"} -412 +request_duration_microseconds_bucket{le="144"} 592 +request_duration_microseconds_bucket{le="172.8"} 1524 +request_duration_microseconds_bucket{le="+Inf"} 2693 +request_duration_microseconds_sum 1.7560473e+06 +request_duration_microseconds_count 2693 +`, + errUTF8: `text format parsing error in line 5: negative bucket population for histogram "request_duration_microseconds"`, + }, + // 45: Histogram with negative float count. + { + in: ` +# HELP request_duration_microseconds The response latency. +# TYPE request_duration_microseconds histogram +request_duration_microseconds_bucket{le="100"} 123 +request_duration_microseconds_bucket{le="120"} 412 +request_duration_microseconds_bucket{le="144"} 592 +request_duration_microseconds_bucket{le="172.8"} 1524 +request_duration_microseconds_bucket{le="+Inf"} 2693 +request_duration_microseconds_sum 1.7560473e+06 +request_duration_microseconds_count -2693.123 +`, + errUTF8: `text format parsing error in line 10: negative count for histogram "request_duration_microseconds"`, + }, + // 46: Histogram with negative float bucket. + { + in: ` +# HELP request_duration_microseconds The response latency. +# TYPE request_duration_microseconds histogram +request_duration_microseconds_bucket{le="100"} 123 +request_duration_microseconds_bucket{le="120"} -412.456 +request_duration_microseconds_bucket{le="144"} 592 +request_duration_microseconds_bucket{le="172.8"} 1524 +request_duration_microseconds_bucket{le="+Inf"} 2693 +request_duration_microseconds_sum 1.7560473e+06 +request_duration_microseconds_count 2693 +`, + errUTF8: `text format parsing error in line 5: negative bucket population for histogram "request_duration_microseconds"`, }, } for i, scenario := range scenarios { + parser.scheme = model.UTF8Validation _, err := parser.TextToMetricFamilies(strings.NewReader(scenario.in)) if err == nil { - t.Errorf("%d. expected error, got nil", i) - continue - } - if expected, got := scenario.err, err.Error(); strings.Index(got, expected) != 0 { + if scenario.errUTF8 != "" { + t.Errorf("%d. expected error, got nil", i) + } + } else if expected, got := scenario.errUTF8, err.Error(); strings.Index(got, expected) != 0 { t.Errorf( "%d. expected error starting with %q, got %q", i, expected, got, ) } + + parser.scheme = model.LegacyValidation + _, err = parser.TextToMetricFamilies(strings.NewReader(scenario.in)) + if err == nil { + if scenario.errLegacy != "" { + t.Errorf("%d. expected error, got nil", i) + } + } else { + expected := scenario.errUTF8 + if scenario.errLegacy != "" { + expected = scenario.errLegacy + } + if got := err.Error(); strings.Index(got, expected) != 0 { + t.Errorf( + "%d. expected error starting with %q, got %q", + i, expected, got, + ) + } + } } } @@ -962,7 +1380,7 @@ func BenchmarkParseError(b *testing.B) { func TestTextParserStartOfLine(t *testing.T) { t.Run("EOF", func(t *testing.T) { - p := TextParser{} + p := NewTextParser(model.UTF8Validation) in := strings.NewReader("") p.reset(in) fn := p.startOfLine() @@ -975,7 +1393,7 @@ func TestTextParserStartOfLine(t *testing.T) { }) t.Run("OtherError", func(t *testing.T) { - p := TextParser{} + p := NewTextParser(model.UTF8Validation) in := &errReader{err: errors.New("unexpected error")} p.reset(in) fn := p.startOfLine() @@ -992,6 +1410,6 @@ type errReader struct { err error } -func (r *errReader) Read(p []byte) (int, error) { +func (r *errReader) Read([]byte) (int, error) { return 0, r.err } diff --git a/go.mod b/go.mod index 4d62719bb..f0eb3d693 100644 --- a/go.mod +++ b/go.mod @@ -1,23 +1,23 @@ module github.com/prometheus/common -go 1.21 +go 1.24.0 require ( github.com/alecthomas/kingpin/v2 v2.4.0 - github.com/google/go-cmp v0.6.0 + github.com/google/go-cmp v0.7.0 github.com/julienschmidt/httprouter v1.3.0 github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f - github.com/prometheus/client_model v0.6.1 - github.com/stretchr/testify v1.10.0 - golang.org/x/net v0.33.0 - golang.org/x/oauth2 v0.24.0 - google.golang.org/protobuf v1.36.1 - gopkg.in/yaml.v2 v2.4.0 + github.com/prometheus/client_model v0.6.2 + github.com/stretchr/testify v1.11.1 + go.yaml.in/yaml/v2 v2.4.3 + golang.org/x/net v0.44.0 + golang.org/x/oauth2 v0.31.0 + google.golang.org/protobuf v1.36.10 ) require ( - github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137 // indirect + github.com/alecthomas/units v0.0.0-20240927000941-0f3dac36c52b // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/davecgh/go-spew v1.1.1 // indirect @@ -27,10 +27,48 @@ require ( github.com/prometheus/procfs v0.15.1 // indirect github.com/rogpeppe/go-internal v1.10.0 // indirect github.com/xhit/go-str2duration/v2 v2.1.0 // indirect - golang.org/x/sys v0.28.0 // indirect - golang.org/x/text v0.21.0 // indirect + golang.org/x/sys v0.36.0 // indirect + golang.org/x/text v0.29.0 // indirect gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) -retract v0.50.0 // Critical bug in counter suffixes, please read issue https://github.com/prometheus/common/issues/605 +retract ( + v1.20.99 // This tag is needed to retract accidental tags below, but is retracted directly. + v1.20.3 // Tags pushed accidentally, see https://github.com/prometheus/common/issues/831 + v1.20.3 + v1.20.2 + v1.20.1 + v1.20.0 + v1.19.1 + v1.19.0 + v1.18.0 + v1.17.0 + v1.16.0 + v1.15.1 + v1.15.0 + v1.14.0 + v1.13.1 + v1.13.0 + v1.12.2 + v1.12.1 + v1.12.0 + v1.11.1 + v1.11.0 + v1.10.0 + v1.9.0 + v1.8.0 + v1.7.1 + v1.7.0 + v1.6.0 + v1.5.1 + v1.5.0 + v1.4.1 + v1.4.0 + v1.3.0 + v1.2.1 + v1.2.0 + v1.1.0 + v1.0.0 + v0.50.0 // Critical bug in counter suffixes, please read issue https://github.com/prometheus/common/issues/605 +) diff --git a/go.sum b/go.sum index b5955f01f..3c5194f10 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,7 @@ github.com/alecthomas/kingpin/v2 v2.4.0 h1:f48lwail6p8zpO1bC4TxtqACaGqHYA22qkHjHpqDjYY= github.com/alecthomas/kingpin/v2 v2.4.0/go.mod h1:0gyi0zQnjuFk8xrkNKamJoyUo382HRL7ATRpFZCw6tE= -github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137 h1:s6gZFSlWYmbqAuRjVTiNNhvNRfY2Wxp9nhfyel4rklc= -github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137/go.mod h1:OMCwj8VM1Kc9e19TLln2VL61YJF0x1XFtfdL4JdbSyE= +github.com/alecthomas/units v0.0.0-20240927000941-0f3dac36c52b h1:mimo19zliBX/vSQ6PWWSL9lK8qwHozUj03+zLoEB8O0= +github.com/alecthomas/units v0.0.0-20240927000941-0f3dac36c52b/go.mod h1:fvzegU4vN3H1qMT+8wDmzjAcDONcgo2/SZ/TyfdUOFs= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= @@ -9,8 +9,8 @@ github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XL github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= -github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= +github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/jpillora/backoff v1.0.0 h1:uvFg412JmmHBHw7iwprIxkPMI+sGQ4kzOWsMeHnm2EA= github.com/jpillora/backoff v1.0.0/go.mod h1:J/6gKK9jxlEcS3zixgDgUAsiuZ7yrSoa/FX5e0EB2j4= github.com/julienschmidt/httprouter v1.3.0 h1:U0609e9tgbseu3rBINet9P48AI/D3oJs4dN7jwJOQ1U= @@ -31,33 +31,39 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/prometheus/client_golang v1.20.4 h1:Tgh3Yr67PaOv/uTqloMsCEdeuFTatm5zIq5+qNN23vI= github.com/prometheus/client_golang v1.20.4/go.mod h1:PIEt8X02hGcP8JWbeHyeZ53Y/jReSnHgO035n//V5WE= -github.com/prometheus/client_model v0.6.1 h1:ZKSh/rekM+n3CeS952MLRAdFwIKqeY8b62p8ais2e9E= -github.com/prometheus/client_model v0.6.1/go.mod h1:OrxVMOVHjw3lKMa8+x6HeMGkHMQyHDk9E3jmP2AmGiY= +github.com/prometheus/client_model v0.6.2 h1:oBsgwpGs7iVziMvrGhE53c/GrLUsZdHnqNwqPLxwZyk= +github.com/prometheus/client_model v0.6.2/go.mod h1:y3m2F6Gdpfy6Ut/GBsUqTWZqCUvMVzSfMLjcu6wAwpE= github.com/prometheus/procfs v0.15.1 h1:YagwOFzUgYfKKHX6Dr+sHT7km/hxC76UB0learggepc= github.com/prometheus/procfs v0.15.1/go.mod h1:fB45yRUv8NstnjriLhBQLuOUt+WW4BsoGhij/e3PBqk= github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjRBZyWFQ= github.com/rogpeppe/go-internal v1.10.0/go.mod h1:UQnix2H7Ngw/k4C5ijL5+65zddjncjaFoBhdsK/akog= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= -github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= -github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= +github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= +github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/xhit/go-str2duration/v2 v2.1.0 h1:lxklc02Drh6ynqX+DdPyp5pCKLUQpRT8bp8Ydu2Bstc= github.com/xhit/go-str2duration/v2 v2.1.0/go.mod h1:ohY8p+0f07DiV6Em5LKB0s2YpLtXVyJfNt1+BlmyAsU= -golang.org/x/net v0.33.0 h1:74SYHlV8BIgHIFC/LrYkOGIwL19eTYXQ5wc6TBuO36I= -golang.org/x/net v0.33.0/go.mod h1:HXLR5J+9DxmrqMwG9qjGCxZ+zKXxBru04zlTvWlWuN4= -golang.org/x/oauth2 v0.24.0 h1:KTBBxWqUa0ykRPLtV69rRto9TLXcqYkeswu48x/gvNE= -golang.org/x/oauth2 v0.24.0/go.mod h1:XYTD2NtWslqkgxebSiOHnXEap4TF09sJSc7H1sXbhtI= -golang.org/x/sys v0.28.0 h1:Fksou7UEQUWlKvIdsqzJmUmCX3cZuD2+P3XyyzwMhlA= -golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/text v0.21.0 h1:zyQAAkrwaneQ066sspRyJaG9VNi/YJ1NfzcGB3hZ/qo= -golang.org/x/text v0.21.0/go.mod h1:4IBbMaMmOPCJ8SecivzSH54+73PCFmPWxNTLm+vZkEQ= -google.golang.org/protobuf v1.36.1 h1:yBPeRvTftaleIgM3PZ/WBIZ7XM/eEYAaEyCwvyjq/gk= -google.golang.org/protobuf v1.36.1/go.mod h1:9fA7Ob0pmnwhb644+1+CVWFRbNajQ6iRojtC/QF5bRE= +go.yaml.in/yaml/v2 v2.4.3 h1:6gvOSjQoTB3vt1l+CU+tSyi/HOjfOjRLJ4YwYZGwRO0= +go.yaml.in/yaml/v2 v2.4.3/go.mod h1:zSxWcmIDjOzPXpjlTTbAsKokqkDNAVtZO0WOMiT90s8= +golang.org/x/net v0.44.0 h1:evd8IRDyfNBMBTTY5XRF1vaZlD+EmWx6x8PkhR04H/I= +golang.org/x/net v0.44.0/go.mod h1:ECOoLqd5U3Lhyeyo/QDCEVQ4sNgYsqvCZ722XogGieY= +golang.org/x/oauth2 v0.31.0 h1:8Fq0yVZLh4j4YA47vHKFTa9Ew5XIrCP8LC6UeNZnLxo= +golang.org/x/oauth2 v0.31.0/go.mod h1:lzm5WQJQwKZ3nwavOZ3IS5Aulzxi68dUSgRHujetwEA= +golang.org/x/sys v0.36.0 h1:KVRy2GtZBrk1cBYA7MKu5bEZFxQk4NIDV6RLVcC8o0k= +golang.org/x/sys v0.36.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= +golang.org/x/text v0.29.0 h1:1neNs90w9YzJ9BocxfsQNHKuAT4pkghyXc4nhZ6sJvk= +golang.org/x/text v0.29.0/go.mod h1:7MhJOA9CD2qZyOKYazxdYMF85OwPdEr9jTtBpO7ydH4= +google.golang.org/protobuf v1.36.10 h1:AYd7cD/uASjIL6Q9LiTjz8JLcrh/88q5UObnmY3aOOE= +google.golang.org/protobuf v1.36.10/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= -gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= -gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= -gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/model/alert.go b/model/alert.go index bd3a39e3e..460f554f2 100644 --- a/model/alert.go +++ b/model/alert.go @@ -65,7 +65,7 @@ func (a *Alert) Resolved() bool { return a.ResolvedAt(time.Now()) } -// ResolvedAt returns true off the activity interval ended before +// ResolvedAt returns true iff the activity interval ended before // the given timestamp. func (a *Alert) ResolvedAt(ts time.Time) bool { if a.EndsAt.IsZero() { diff --git a/model/labels.go b/model/labels.go index 73b7aa3e6..dfeb34be5 100644 --- a/model/labels.go +++ b/model/labels.go @@ -22,7 +22,7 @@ import ( ) const ( - // AlertNameLabel is the name of the label containing the an alert's name. + // AlertNameLabel is the name of the label containing the alert's name. AlertNameLabel = "alertname" // ExportedLabelPrefix is the prefix to prepend to the label names present in @@ -32,6 +32,12 @@ const ( // MetricNameLabel is the label name indicating the metric name of a // timeseries. MetricNameLabel = "__name__" + // MetricTypeLabel is the label name indicating the metric type of + // timeseries as per the PROM-39 proposal. + MetricTypeLabel = "__type__" + // MetricUnitLabel is the label name indicating the metric unit of + // timeseries as per the PROM-39 proposal. + MetricUnitLabel = "__unit__" // SchemeLabel is the name of the label that holds the scheme on which to // scrape a target. @@ -100,33 +106,21 @@ type LabelName string // IsValid returns true iff the name matches the pattern of LabelNameRE when // NameValidationScheme is set to LegacyValidation, or valid UTF-8 if // NameValidationScheme is set to UTF8Validation. +// +// Deprecated: This method should not be used and may be removed in the future. +// Use [ValidationScheme.IsValidLabelName] instead. func (ln LabelName) IsValid() bool { - if len(ln) == 0 { - return false - } - switch NameValidationScheme { - case LegacyValidation: - return ln.IsValidLegacy() - case UTF8Validation: - return utf8.ValidString(string(ln)) - default: - panic(fmt.Sprintf("Invalid name validation scheme requested: %d", NameValidationScheme)) - } + return NameValidationScheme.IsValidLabelName(string(ln)) } // IsValidLegacy returns true iff name matches the pattern of LabelNameRE for // legacy names. It does not use LabelNameRE for the check but a much faster // hardcoded implementation. +// +// Deprecated: This method should not be used and may be removed in the future. +// Use [LegacyValidation.IsValidLabelName] instead. func (ln LabelName) IsValidLegacy() bool { - if len(ln) == 0 { - return false - } - for i, b := range ln { - if !((b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z') || b == '_' || (b >= '0' && b <= '9' && i > 0)) { - return false - } - } - return true + return LegacyValidation.IsValidLabelName(string(ln)) } // UnmarshalYAML implements the yaml.Unmarshaler interface. diff --git a/model/labels_test.go b/model/labels_test.go index 233954326..31717b4e5 100644 --- a/model/labels_test.go +++ b/model/labels_test.go @@ -14,6 +14,7 @@ package model import ( + "fmt" "sort" "testing" ) @@ -90,9 +91,9 @@ func BenchmarkLabelValues(b *testing.B) { } } -func TestLabelNameIsValid(t *testing.T) { +func TestValidationScheme_IsLabelNameValid(t *testing.T) { scenarios := []struct { - ln LabelName + ln string legacyValid bool utf8Valid bool }{ @@ -141,20 +142,44 @@ func TestLabelNameIsValid(t *testing.T) { legacyValid: false, utf8Valid: false, }, + { + ln: "dot.in.name", + legacyValid: false, + utf8Valid: true, + }, + { + ln: "", + legacyValid: false, + utf8Valid: false, + }, } - for _, s := range scenarios { - NameValidationScheme = LegacyValidation - if s.ln.IsValid() != s.legacyValid { - t.Errorf("Expected %v for %q using legacy IsValid method", s.legacyValid, s.ln) - } - if LabelNameRE.MatchString(string(s.ln)) != s.legacyValid { - t.Errorf("Expected %v for %q using legacy regexp match", s.legacyValid, s.ln) - } - NameValidationScheme = UTF8Validation - if s.ln.IsValid() != s.utf8Valid { - t.Errorf("Expected %v for %q using UTF-8 IsValid method", s.legacyValid, s.ln) - } + t.Run(fmt.Sprintf("%s,%t,%t", s.ln, s.legacyValid, s.utf8Valid), func(t *testing.T) { + if LegacyValidation.IsValidLabelName(s.ln) != s.legacyValid { + t.Errorf("Expected %v for %q using LegacyValidation.IsValidLabelName", s.legacyValid, s.ln) + } + if LabelNameRE.MatchString(s.ln) != s.legacyValid { + t.Errorf("Expected %v for %q using legacy regexp match", s.legacyValid, s.ln) + } + if UTF8Validation.IsValidLabelName(s.ln) != s.utf8Valid { + t.Errorf("Expected %v for %q using UTF8Validation.IsValidLabelName", s.utf8Valid, s.ln) + } + + // Test deprecated functions. + origScheme := NameValidationScheme + t.Cleanup(func() { + NameValidationScheme = origScheme + }) + NameValidationScheme = LegacyValidation + labelName := LabelName(s.ln) + if labelName.IsValid() != s.legacyValid { + t.Errorf("Expected %v for %q using legacy IsValid method", s.legacyValid, s.ln) + } + NameValidationScheme = UTF8Validation + if labelName.IsValid() != s.utf8Valid { + t.Errorf("Expected %v for %q using UTF-8 IsValid method", s.utf8Valid, s.ln) + } + }) } } diff --git a/model/labelset.go b/model/labelset.go index d0ad88da3..9de47b256 100644 --- a/model/labelset.go +++ b/model/labelset.go @@ -114,10 +114,10 @@ func (ls LabelSet) Clone() LabelSet { } // Merge is a helper function to non-destructively merge two label sets. -func (l LabelSet) Merge(other LabelSet) LabelSet { - result := make(LabelSet, len(l)) +func (ls LabelSet) Merge(other LabelSet) LabelSet { + result := make(LabelSet, len(ls)) - for k, v := range l { + for k, v := range ls { result[k] = v } @@ -140,7 +140,7 @@ func (ls LabelSet) FastFingerprint() Fingerprint { } // UnmarshalJSON implements the json.Unmarshaler interface. -func (l *LabelSet) UnmarshalJSON(b []byte) error { +func (ls *LabelSet) UnmarshalJSON(b []byte) error { var m map[LabelName]LabelValue if err := json.Unmarshal(b, &m); err != nil { return err @@ -153,6 +153,6 @@ func (l *LabelSet) UnmarshalJSON(b []byte) error { return fmt.Errorf("%q is not a valid label name", ln) } } - *l = LabelSet(m) + *ls = LabelSet(m) return nil } diff --git a/model/metric.go b/model/metric.go index 5766107cf..3feebf328 100644 --- a/model/metric.go +++ b/model/metric.go @@ -14,6 +14,7 @@ package model import ( + "encoding/json" "errors" "fmt" "regexp" @@ -23,17 +24,30 @@ import ( "unicode/utf8" dto "github.com/prometheus/client_model/go" + "go.yaml.in/yaml/v2" "google.golang.org/protobuf/proto" ) var ( - // NameValidationScheme determines the method of name validation to be used by - // all calls to IsValidMetricName() and LabelName IsValid(). Setting UTF-8 - // mode in isolation from other components that don't support UTF-8 may result - // in bugs or other undefined behavior. This value can be set to - // LegacyValidation during startup if a binary is not UTF-8-aware binaries. To - // avoid need for locking, this value should be set once, ideally in an - // init(), before multiple goroutines are started. + // NameValidationScheme determines the global default method of the name + // validation to be used by all calls to IsValidMetricName() and LabelName + // IsValid(). + // + // Deprecated: This variable should not be used and might be removed in the + // far future. If you wish to stick to the legacy name validation use + // `IsValidLegacyMetricName()` and `LabelName.IsValidLegacy()` methods + // instead. This variable is here as an escape hatch for emergency cases, + // given the recent change from `LegacyValidation` to `UTF8Validation`, e.g., + // to delay UTF-8 migrations in time or aid in debugging unforeseen results of + // the change. In such a case, a temporary assignment to `LegacyValidation` + // value in the `init()` function in your main.go or so, could be considered. + // + // Historically we opted for a global variable for feature gating different + // validation schemes in operations that were not otherwise easily adjustable + // (e.g. Labels yaml unmarshaling). That could have been a mistake, a separate + // Labels structure or package might have been a better choice. Given the + // change was made and many upgraded the common already, we live this as-is + // with this warning and learning for the future. NameValidationScheme = UTF8Validation // NameEscapingScheme defines the default way that names will be escaped when @@ -50,16 +64,151 @@ var ( type ValidationScheme int const ( - // LegacyValidation is a setting that requirets that metric and label names + // UnsetValidation represents an undefined ValidationScheme. + // Should not be used in practice. + UnsetValidation ValidationScheme = iota + + // LegacyValidation is a setting that requires that all metric and label names // conform to the original Prometheus character requirements described by // MetricNameRE and LabelNameRE. - LegacyValidation ValidationScheme = iota + LegacyValidation // UTF8Validation only requires that metric and label names be valid UTF-8 // strings. UTF8Validation ) +var _ interface { + yaml.Marshaler + yaml.Unmarshaler + json.Marshaler + json.Unmarshaler + fmt.Stringer +} = new(ValidationScheme) + +// String returns the string representation of s. +func (s ValidationScheme) String() string { + switch s { + case UnsetValidation: + return "unset" + case LegacyValidation: + return "legacy" + case UTF8Validation: + return "utf8" + default: + panic(fmt.Errorf("unhandled ValidationScheme: %d", s)) + } +} + +// MarshalYAML implements the yaml.Marshaler interface. +func (s ValidationScheme) MarshalYAML() (any, error) { + switch s { + case UnsetValidation: + return "", nil + case LegacyValidation, UTF8Validation: + return s.String(), nil + default: + panic(fmt.Errorf("unhandled ValidationScheme: %d", s)) + } +} + +// UnmarshalYAML implements the yaml.Unmarshaler interface. +func (s *ValidationScheme) UnmarshalYAML(unmarshal func(any) error) error { + var scheme string + if err := unmarshal(&scheme); err != nil { + return err + } + return s.Set(scheme) +} + +// MarshalJSON implements the json.Marshaler interface. +func (s ValidationScheme) MarshalJSON() ([]byte, error) { + switch s { + case UnsetValidation: + return json.Marshal("") + case UTF8Validation, LegacyValidation: + return json.Marshal(s.String()) + default: + return nil, fmt.Errorf("unhandled ValidationScheme: %d", s) + } +} + +// UnmarshalJSON implements the json.Unmarshaler interface. +func (s *ValidationScheme) UnmarshalJSON(bytes []byte) error { + var repr string + if err := json.Unmarshal(bytes, &repr); err != nil { + return err + } + return s.Set(repr) +} + +// Set implements the pflag.Value interface. +func (s *ValidationScheme) Set(text string) error { + switch text { + case "": + // Don't change the value. + case LegacyValidation.String(): + *s = LegacyValidation + case UTF8Validation.String(): + *s = UTF8Validation + default: + return fmt.Errorf("unrecognized ValidationScheme: %q", text) + } + return nil +} + +// IsValidMetricName returns whether metricName is valid according to s. +func (s ValidationScheme) IsValidMetricName(metricName string) bool { + switch s { + case LegacyValidation: + if len(metricName) == 0 { + return false + } + for i, b := range metricName { + if !isValidLegacyRune(b, i) { + return false + } + } + return true + case UTF8Validation: + if len(metricName) == 0 { + return false + } + return utf8.ValidString(metricName) + default: + panic(fmt.Sprintf("Invalid name validation scheme requested: %s", s.String())) + } +} + +// IsValidLabelName returns whether labelName is valid according to s. +func (s ValidationScheme) IsValidLabelName(labelName string) bool { + switch s { + case LegacyValidation: + if len(labelName) == 0 { + return false + } + for i, b := range labelName { + // TODO: Apply De Morgan's law. Make sure there are tests for this. + if !((b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z') || b == '_' || (b >= '0' && b <= '9' && i > 0)) { //nolint:staticcheck + return false + } + } + return true + case UTF8Validation: + if len(labelName) == 0 { + return false + } + return utf8.ValidString(labelName) + default: + panic(fmt.Sprintf("Invalid name validation scheme requested: %s", s)) + } +} + +// Type implements the pflag.Value interface. +func (ValidationScheme) Type() string { + return "validationScheme" +} + type EscapingScheme int const ( @@ -89,7 +238,7 @@ const ( // Accept header, the default NameEscapingScheme will be used. EscapingKey = "escaping" - // Possible values for Escaping Key: + // Possible values for Escaping Key. AllowUTF8 = "allow-utf-8" // No escaping required. EscapeUnderscores = "underscores" EscapeDots = "dots" @@ -163,34 +312,22 @@ func (m Metric) FastFingerprint() Fingerprint { // IsValidMetricName returns true iff name matches the pattern of MetricNameRE // for legacy names, and iff it's valid UTF-8 if the UTF8Validation scheme is // selected. +// +// Deprecated: This function should not be used and might be removed in the future. +// Use [ValidationScheme.IsValidMetricName] instead. func IsValidMetricName(n LabelValue) bool { - switch NameValidationScheme { - case LegacyValidation: - return IsValidLegacyMetricName(string(n)) - case UTF8Validation: - if len(n) == 0 { - return false - } - return utf8.ValidString(string(n)) - default: - panic(fmt.Sprintf("Invalid name validation scheme requested: %d", NameValidationScheme)) - } + return NameValidationScheme.IsValidMetricName(string(n)) } // IsValidLegacyMetricName is similar to IsValidMetricName but always uses the // legacy validation scheme regardless of the value of NameValidationScheme. // This function, however, does not use MetricNameRE for the check but a much // faster hardcoded implementation. +// +// Deprecated: This function should not be used and might be removed in the future. +// Use [LegacyValidation.IsValidMetricName] instead. func IsValidLegacyMetricName(n string) bool { - if len(n) == 0 { - return false - } - for i, b := range n { - if !isValidLegacyRune(b, i) { - return false - } - } - return true + return LegacyValidation.IsValidMetricName(n) } // EscapeMetricFamily escapes the given metric names and labels with the given @@ -298,13 +435,14 @@ func EscapeName(name string, scheme EscapingScheme) string { case DotsEscaping: // Do not early return for legacy valid names, we still escape underscores. for i, b := range name { - if b == '_' { + switch { + case b == '_': escaped.WriteString("__") - } else if b == '.' { + case b == '.': escaped.WriteString("_dot_") - } else if isValidLegacyRune(b, i) { + case isValidLegacyRune(b, i): escaped.WriteRune(b) - } else { + default: escaped.WriteString("__") } } @@ -315,13 +453,14 @@ func EscapeName(name string, scheme EscapingScheme) string { } escaped.WriteString("U__") for i, b := range name { - if b == '_' { + switch { + case b == '_': escaped.WriteString("__") - } else if isValidLegacyRune(b, i) { + case isValidLegacyRune(b, i): escaped.WriteRune(b) - } else if !utf8.ValidRune(b) { + case !utf8.ValidRune(b): escaped.WriteString("_FFFD_") - } else { + default: escaped.WriteRune('_') escaped.WriteString(strconv.FormatInt(int64(b), 16)) escaped.WriteRune('_') @@ -333,7 +472,7 @@ func EscapeName(name string, scheme EscapingScheme) string { } } -// lower function taken from strconv.atoi +// lower function taken from strconv.atoi. func lower(c byte) byte { return c | ('x' - 'X') } @@ -397,11 +536,12 @@ func UnescapeName(name string, scheme EscapingScheme) string { } r := lower(escapedName[i]) utf8Val *= 16 - if r >= '0' && r <= '9' { + switch { + case r >= '0' && r <= '9': utf8Val += uint(r) - '0' - } else if r >= 'a' && r <= 'f' { + case r >= 'a' && r <= 'f': utf8Val += uint(r) - 'a' + 10 - } else { + default: return name } i++ diff --git a/model/metric_test.go b/model/metric_test.go index 6152c5481..8a8bec95b 100644 --- a/model/metric_test.go +++ b/model/metric_test.go @@ -14,11 +14,17 @@ package model import ( + "encoding/json" + "errors" + "fmt" + "strings" "testing" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" dto "github.com/prometheus/client_model/go" + "github.com/stretchr/testify/require" + "go.yaml.in/yaml/v2" "google.golang.org/protobuf/proto" ) @@ -89,9 +95,206 @@ func BenchmarkMetric(b *testing.B) { } } -func TestMetricNameIsLegacyValid(t *testing.T) { +func TestValidationScheme(t *testing.T) { + var scheme ValidationScheme + require.Equal(t, UnsetValidation, scheme) +} + +func TestValidationScheme_String(t *testing.T) { + for _, tc := range []struct { + name string + scheme ValidationScheme + want string + }{ + { + name: "Unset", + scheme: UnsetValidation, + want: "unset", + }, + { + name: "Legacy", + scheme: LegacyValidation, + want: "legacy", + }, + { + name: "UTF8", + scheme: UTF8Validation, + want: "utf8", + }, + } { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.want, tc.scheme.String()) + }) + } +} + +func TestValidationScheme_MarshalYAML(t *testing.T) { + for _, tc := range []struct { + name string + scheme ValidationScheme + want string + }{ + { + name: "Unset", + scheme: UnsetValidation, + want: `""`, + }, + { + name: "Legacy", + scheme: LegacyValidation, + want: "legacy", + }, + { + name: "UTF8", + scheme: UTF8Validation, + want: "utf8", + }, + } { + t.Run(tc.name, func(t *testing.T) { + marshaled, err := yaml.Marshal(tc.scheme) + require.NoError(t, err) + require.Equal(t, tc.want, strings.TrimSpace(string(marshaled))) + }) + } +} + +func TestValidationScheme_UnmarshalYAML(t *testing.T) { + for _, tc := range []struct { + name string + input string + want ValidationScheme + wantError error + }{ + { + name: "Unset empty input", + input: "", + want: UnsetValidation, + }, + { + name: "Unset quoted input", + input: `""`, + want: UnsetValidation, + }, + { + name: "Legacy", + input: "legacy", + want: LegacyValidation, + }, + { + name: "UTF8", + input: "utf8", + want: UTF8Validation, + }, + { + name: "Invalid", + input: "invalid", + wantError: errors.New(`unrecognized ValidationScheme: "invalid"`), + }, + } { + t.Run(tc.name, func(t *testing.T) { + scheme := UnsetValidation + err := yaml.Unmarshal([]byte(tc.input), &scheme) + if tc.wantError == nil { + require.NoError(t, err) + require.Equal(t, tc.want, scheme) + } else { + require.EqualError(t, err, tc.wantError.Error()) + } + }) + } +} + +func TestValidationScheme_UnmarshalJSON(t *testing.T) { + testCases := []struct { + name string + input string + want ValidationScheme + wantErr bool + }{ + { + name: "invalid", + input: `invalid`, + wantErr: true, + }, + { + name: "empty", + input: `""`, + want: UnsetValidation, + }, + { + name: "legacy validation", + input: `"legacy"`, + want: LegacyValidation, + }, + { + name: "utf8 validation", + input: `"utf8"`, + want: UTF8Validation, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var got ValidationScheme + err := json.Unmarshal([]byte(tc.input), &got) + if tc.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Equal(t, tc.want, got) + + output, err := json.Marshal(got) + require.NoError(t, err) + require.Equal(t, tc.input, string(output)) + }) + } +} + +func TestValidationScheme_Set(t *testing.T) { + testCases := []struct { + name string + input string + want ValidationScheme + wantErr bool + }{ + { + name: "invalid", + input: `invalid`, + wantErr: true, + }, + { + name: "empty", + input: ``, + want: UnsetValidation, + }, + { + name: "legacy validation", + input: `legacy`, + want: LegacyValidation, + }, + { + name: "utf8 validation", + input: `utf8`, + want: UTF8Validation, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var got ValidationScheme + err := got.Set(tc.input) + if tc.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Equal(t, tc.want, got) + }) + } +} + +func TestValidationScheme_IsMetricNameValid(t *testing.T) { scenarios := []struct { - mn LabelValue + mn string legacyValid bool utf8Valid bool }{ @@ -145,20 +348,41 @@ func TestMetricNameIsLegacyValid(t *testing.T) { legacyValid: false, utf8Valid: false, }, + { + mn: "dot.in.name", + legacyValid: false, + utf8Valid: true, + }, } - for _, s := range scenarios { - NameValidationScheme = LegacyValidation - if IsValidMetricName(s.mn) != s.legacyValid { - t.Errorf("Expected %v for %q using legacy IsValidMetricName method", s.legacyValid, s.mn) - } - if MetricNameRE.MatchString(string(s.mn)) != s.legacyValid { - t.Errorf("Expected %v for %q using regexp matching", s.legacyValid, s.mn) - } - NameValidationScheme = UTF8Validation - if IsValidMetricName(s.mn) != s.utf8Valid { - t.Errorf("Expected %v for %q using utf-8 IsValidMetricName method", s.legacyValid, s.mn) - } + t.Run(fmt.Sprintf("%s,%t,%t", s.mn, s.legacyValid, s.utf8Valid), func(t *testing.T) { + if LegacyValidation.IsValidMetricName(s.mn) != s.legacyValid { + t.Errorf("Expected %v for %q using LegacyValidation.IsValidMetricName", s.legacyValid, s.mn) + } + if MetricNameRE.MatchString(s.mn) != s.legacyValid { + t.Errorf("Expected %v for %q using regexp matching", s.legacyValid, s.mn) + } + if UTF8Validation.IsValidMetricName(s.mn) != s.utf8Valid { + t.Errorf("Expected %v for %q using UTF8Validation.IsValidMetricName", s.utf8Valid, s.mn) + } + + // Test deprecated functions. + if IsValidLegacyMetricName(s.mn) != s.legacyValid { + t.Errorf("Expected %v for %q using IsValidLegacyMetricNames", s.legacyValid, s.mn) + } + origScheme := NameValidationScheme + t.Cleanup(func() { + NameValidationScheme = origScheme + }) + NameValidationScheme = LegacyValidation + if IsValidMetricName(LabelValue(s.mn)) != s.legacyValid { + t.Errorf("Expected %v for %q using legacy IsValidMetricName", s.legacyValid, s.mn) + } + NameValidationScheme = UTF8Validation + if IsValidMetricName(LabelValue(s.mn)) != s.utf8Valid { + t.Errorf("Expected %v for %q using utf-8 IsValidMetricName method", s.utf8Valid, s.mn) + } + }) } } diff --git a/model/time.go b/model/time.go index 5727452c1..1730b0fdc 100644 --- a/model/time.go +++ b/model/time.go @@ -126,14 +126,14 @@ func (t *Time) UnmarshalJSON(b []byte) error { p := strings.Split(string(b), ".") switch len(p) { case 1: - v, err := strconv.ParseInt(string(p[0]), 10, 64) + v, err := strconv.ParseInt(p[0], 10, 64) if err != nil { return err } *t = Time(v * second) case 2: - v, err := strconv.ParseInt(string(p[0]), 10, 64) + v, err := strconv.ParseInt(p[0], 10, 64) if err != nil { return err } @@ -143,7 +143,7 @@ func (t *Time) UnmarshalJSON(b []byte) error { if prec < 0 { p[1] = p[1][:dotPrecision] } else if prec > 0 { - p[1] = p[1] + strings.Repeat("0", prec) + p[1] += strings.Repeat("0", prec) } va, err := strconv.ParseInt(p[1], 10, 32) @@ -170,15 +170,15 @@ func (t *Time) UnmarshalJSON(b []byte) error { // This type should not propagate beyond the scope of input/output processing. type Duration time.Duration -// Set implements pflag/flag.Value +// Set implements pflag/flag.Value. func (d *Duration) Set(s string) error { var err error *d, err = ParseDuration(s) return err } -// Type implements pflag.Value -func (d *Duration) Type() string { +// Type implements pflag.Value. +func (*Duration) Type() string { return "duration" } @@ -201,6 +201,7 @@ var unitMap = map[string]struct { // ParseDuration parses a string into a time.Duration, assuming that a year // always has 365d, a week always has 7d, and a day always has 24h. +// Negative durations are not supported. func ParseDuration(s string) (Duration, error) { switch s { case "0": @@ -253,18 +254,36 @@ func ParseDuration(s string) (Duration, error) { return 0, errors.New("duration out of range") } } + return Duration(dur), nil } +// ParseDurationAllowNegative is like ParseDuration but also accepts negative durations. +func ParseDurationAllowNegative(s string) (Duration, error) { + if s == "" || s[0] != '-' { + return ParseDuration(s) + } + + d, err := ParseDuration(s[1:]) + + return -d, err +} + func (d Duration) String() string { var ( - ms = int64(time.Duration(d) / time.Millisecond) - r = "" + ms = int64(time.Duration(d) / time.Millisecond) + r = "" + sign = "" ) + if ms == 0 { return "0s" } + if ms < 0 { + sign, ms = "-", -ms + } + f := func(unit string, mult int64, exact bool) { if exact && ms%mult != 0 { return @@ -286,7 +305,7 @@ func (d Duration) String() string { f("s", 1000, false) f("ms", 1, false) - return r + return sign + r } // MarshalJSON implements the json.Marshaler interface. diff --git a/model/time_test.go b/model/time_test.go index 70f383947..a4e9069f1 100644 --- a/model/time_test.go +++ b/model/time_test.go @@ -68,70 +68,171 @@ func TestDuration(t *testing.T) { } func TestParseDuration(t *testing.T) { - cases := []struct { - in string - out time.Duration + type testCase struct { + in string + out time.Duration + expectedString string + allowedNegative bool + } - expectedString string - }{ + baseCases := []testCase{ { - in: "0", - out: 0, - expectedString: "0s", - }, { - in: "0w", - out: 0, - expectedString: "0s", - }, { - in: "0s", - out: 0, - }, { - in: "324ms", - out: 324 * time.Millisecond, - }, { - in: "3s", - out: 3 * time.Second, - }, { - in: "5m", - out: 5 * time.Minute, - }, { - in: "1h", - out: time.Hour, - }, { - in: "4d", - out: 4 * 24 * time.Hour, - }, { - in: "4d1h", - out: 4*24*time.Hour + time.Hour, - }, { - in: "14d", - out: 14 * 24 * time.Hour, - expectedString: "2w", - }, { - in: "3w", - out: 3 * 7 * 24 * time.Hour, - }, { - in: "3w2d1h", - out: 3*7*24*time.Hour + 2*24*time.Hour + time.Hour, - expectedString: "23d1h", - }, { - in: "10y", - out: 10 * 365 * 24 * time.Hour, + in: "0", + out: 0, + expectedString: "0s", + allowedNegative: false, + }, + { + in: "0w", + out: 0, + expectedString: "0s", + allowedNegative: false, + }, + { + in: "0s", + out: 0, + expectedString: "", + allowedNegative: false, + }, + { + in: "324ms", + out: 324 * time.Millisecond, + expectedString: "", + allowedNegative: false, + }, + { + in: "3s", + out: 3 * time.Second, + expectedString: "", + allowedNegative: false, + }, + { + in: "5m", + out: 5 * time.Minute, + expectedString: "", + allowedNegative: false, + }, + { + in: "1h", + out: time.Hour, + expectedString: "", + allowedNegative: false, + }, + { + in: "4d", + out: 4 * 24 * time.Hour, + expectedString: "", + allowedNegative: false, + }, + { + in: "4d1h", + out: 4*24*time.Hour + time.Hour, + expectedString: "", + allowedNegative: false, + }, + { + in: "14d", + out: 14 * 24 * time.Hour, + expectedString: "2w", + allowedNegative: false, + }, + { + in: "3w", + out: 3 * 7 * 24 * time.Hour, + expectedString: "", + allowedNegative: false, + }, + { + in: "3w2d1h", + out: 3*7*24*time.Hour + 2*24*time.Hour + time.Hour, + expectedString: "23d1h", + allowedNegative: false, + }, + { + in: "10y", + out: 10 * 365 * 24 * time.Hour, + expectedString: "", + allowedNegative: false, }, } - for _, c := range cases { - d, err := ParseDuration(c.in) + negativeCases := []testCase{ + { + in: "-3s", + out: -3 * time.Second, + expectedString: "", + allowedNegative: true, + }, + { + in: "-5m", + out: -5 * time.Minute, + expectedString: "", + allowedNegative: true, + }, + { + in: "-1h", + out: -1 * time.Hour, + expectedString: "", + allowedNegative: true, + }, + { + in: "-2d", + out: -2 * 24 * time.Hour, + expectedString: "", + allowedNegative: true, + }, + { + in: "-1w", + out: -7 * 24 * time.Hour, + expectedString: "", + allowedNegative: true, + }, + { + in: "-3w2d1h", + out: -(3*7*24*time.Hour + 2*24*time.Hour + time.Hour), + expectedString: "-23d1h", + allowedNegative: true, + }, + { + in: "-10y", + out: -10 * 365 * 24 * time.Hour, + expectedString: "", + allowedNegative: true, + }, + } + + for _, c := range baseCases { + c.allowedNegative = true + negativeCases = append(negativeCases, c) + } + + allCases := append(baseCases, negativeCases...) + + for _, c := range allCases { + var ( + d Duration + err error + ) + + if c.allowedNegative { + d, err = ParseDurationAllowNegative(c.in) + } else { + d, err = ParseDuration(c.in) + } + if err != nil { t.Errorf("Unexpected error on input %q", c.in) } + if time.Duration(d) != c.out { t.Errorf("Expected %v but got %v", c.out, d) } + expectedString := c.expectedString - if c.expectedString == "" { + if expectedString == "" { expectedString = c.in } + if d.String() != expectedString { t.Errorf("Expected duration string %q but got %q", c.in, d.String()) } @@ -307,7 +408,6 @@ func TestParseBadDuration(t *testing.T) { cases := []string{ "1", "1y1m1d", - "-1w", "1.5d", "d", "294y", @@ -322,7 +422,6 @@ func TestParseBadDuration(t *testing.T) { if err == nil { t.Errorf("Expected error on input %s", c) } - } } diff --git a/model/value.go b/model/value.go index 8050637d8..a9995a37e 100644 --- a/model/value.go +++ b/model/value.go @@ -191,7 +191,8 @@ func (ss SampleStream) String() string { } func (ss SampleStream) MarshalJSON() ([]byte, error) { - if len(ss.Histograms) > 0 && len(ss.Values) > 0 { + switch { + case len(ss.Histograms) > 0 && len(ss.Values) > 0: v := struct { Metric Metric `json:"metric"` Values []SamplePair `json:"values"` @@ -202,7 +203,7 @@ func (ss SampleStream) MarshalJSON() ([]byte, error) { Histograms: ss.Histograms, } return json.Marshal(&v) - } else if len(ss.Histograms) > 0 { + case len(ss.Histograms) > 0: v := struct { Metric Metric `json:"metric"` Histograms []SampleHistogramPair `json:"histograms"` @@ -211,7 +212,7 @@ func (ss SampleStream) MarshalJSON() ([]byte, error) { Histograms: ss.Histograms, } return json.Marshal(&v) - } else { + default: v := struct { Metric Metric `json:"metric"` Values []SamplePair `json:"values"` @@ -258,7 +259,7 @@ func (s Scalar) String() string { // MarshalJSON implements json.Marshaler. func (s Scalar) MarshalJSON() ([]byte, error) { v := strconv.FormatFloat(float64(s.Value), 'f', -1, 64) - return json.Marshal([...]interface{}{s.Timestamp, string(v)}) + return json.Marshal([...]interface{}{s.Timestamp, v}) } // UnmarshalJSON implements json.Unmarshaler. @@ -349,9 +350,9 @@ func (m Matrix) Len() int { return len(m) } func (m Matrix) Less(i, j int) bool { return m[i].Metric.Before(m[j].Metric) } func (m Matrix) Swap(i, j int) { m[i], m[j] = m[j], m[i] } -func (mat Matrix) String() string { - matCp := make(Matrix, len(mat)) - copy(matCp, mat) +func (m Matrix) String() string { + matCp := make(Matrix, len(m)) + copy(matCp, m) sort.Sort(matCp) strs := make([]string, len(matCp)) diff --git a/model/value_histogram.go b/model/value_histogram.go index 895e6a3e8..91ce5b7a4 100644 --- a/model/value_histogram.go +++ b/model/value_histogram.go @@ -86,22 +86,22 @@ func (s *HistogramBucket) Equal(o *HistogramBucket) bool { return s == o || (s.Boundaries == o.Boundaries && s.Lower == o.Lower && s.Upper == o.Upper && s.Count == o.Count) } -func (b HistogramBucket) String() string { +func (s HistogramBucket) String() string { var sb strings.Builder - lowerInclusive := b.Boundaries == 1 || b.Boundaries == 3 - upperInclusive := b.Boundaries == 0 || b.Boundaries == 3 + lowerInclusive := s.Boundaries == 1 || s.Boundaries == 3 + upperInclusive := s.Boundaries == 0 || s.Boundaries == 3 if lowerInclusive { sb.WriteRune('[') } else { sb.WriteRune('(') } - fmt.Fprintf(&sb, "%g,%g", b.Lower, b.Upper) + fmt.Fprintf(&sb, "%g,%g", s.Lower, s.Upper) if upperInclusive { sb.WriteRune(']') } else { sb.WriteRune(')') } - fmt.Fprintf(&sb, ":%v", b.Count) + fmt.Fprintf(&sb, ":%v", s.Count) return sb.String() } diff --git a/model/value_type.go b/model/value_type.go index 726c50ee6..078910f46 100644 --- a/model/value_type.go +++ b/model/value_type.go @@ -66,8 +66,8 @@ func (et *ValueType) UnmarshalJSON(b []byte) error { return nil } -func (e ValueType) String() string { - switch e { +func (et ValueType) String() string { + switch et { case ValNone: return "" case ValScalar: diff --git a/promslog/flag/flag.go b/promslog/flag/flag.go index 0a164fcc1..40187cfcd 100644 --- a/promslog/flag/flag.go +++ b/promslog/flag/flag.go @@ -40,14 +40,14 @@ const FormatFlagName = "log.format" var FormatFlagHelp = "Output format of log messages. One of: [" + strings.Join(promslog.FormatFlagOptions, ", ") + "]" // AddFlags adds the flags used by this package to the Kingpin application. -// To use the default Kingpin application, call AddFlags(kingpin.CommandLine) +// To use the default Kingpin application, call AddFlags(kingpin.CommandLine). func AddFlags(a *kingpin.Application, config *promslog.Config) { - config.Level = &promslog.AllowedLevel{} + config.Level = promslog.NewLevel() a.Flag(LevelFlagName, LevelFlagHelp). Default("info").HintOptions(promslog.LevelFlagOptions...). SetValue(config.Level) - config.Format = &promslog.AllowedFormat{} + config.Format = promslog.NewFormat() a.Flag(FormatFlagName, FormatFlagHelp). Default("logfmt").HintOptions(promslog.FormatFlagOptions...). SetValue(config.Format) diff --git a/promslog/slog.go b/promslog/slog.go index 6e8fbabce..f5b9e98ba 100644 --- a/promslog/slog.go +++ b/promslog/slog.go @@ -25,73 +25,43 @@ import ( "path/filepath" "strconv" "strings" + "time" ) +// LogStyle represents the common logging formats in the Prometheus ecosystem. type LogStyle string const ( SlogStyle LogStyle = "slog" GoKitStyle LogStyle = "go-kit" + + reservedKeyPrefix = "logged_" ) var ( - LevelFlagOptions = []string{"debug", "info", "warn", "error"} + // LevelFlagOptions represents allowed logging levels. + LevelFlagOptions = []string{"debug", "info", "warn", "error"} + // FormatFlagOptions represents allowed formats. FormatFlagOptions = []string{"logfmt", "json"} - callerAddFunc = false - defaultWriter = os.Stderr - 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) - - 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)) - } - case slog.LevelKey: - a.Value = slog.StringValue(strings.ToLower(a.Value.String())) - default: - } - - return a - } - defaultReplaceAttrFunc = func(groups []string, a slog.Attr) slog.Attr { - key := a.Key - switch key { - case slog.TimeKey: - t := a.Value.Time() - a.Value = slog.TimeValue(t.UTC()) - case slog.SourceKey: - src, _ := a.Value.Any().(*slog.Source) - a.Value = slog.StringValue(filepath.Base(src.File) + ":" + strconv.Itoa(src.Line)) - default: - } - - return a - } + defaultWriter = os.Stderr ) -// AllowedLevel is a settable identifier for the minimum level a log entry -// must be have. -type AllowedLevel struct { - s string +// Level controls a logging level, with an info default. +// It wraps slog.LevelVar with string-based level control. +// Level is safe to be used concurrently. +type Level struct { lvl *slog.LevelVar } -func (l *AllowedLevel) UnmarshalYAML(unmarshal func(interface{}) error) error { +// NewLevel returns a new Level. +func NewLevel() *Level { + return &Level{ + lvl: &slog.LevelVar{}, + } +} + +func (l *Level) UnmarshalYAML(unmarshal func(interface{}) error) error { var s string type plain string if err := unmarshal((*plain)(&s)); err != nil { @@ -100,55 +70,65 @@ func (l *AllowedLevel) UnmarshalYAML(unmarshal func(interface{}) error) error { if s == "" { return nil } - lo := &AllowedLevel{} - if err := lo.Set(s); err != nil { + if err := l.Set(s); err != nil { return err } - *l = *lo return nil } -func (l *AllowedLevel) String() string { - return l.s +// Level returns the value of the logging level as an slog.Level. +func (l *Level) Level() slog.Level { + return l.lvl.Level() } -// Set updates the value of the allowed level. -func (l *AllowedLevel) Set(s string) error { - if l.lvl == nil { - l.lvl = &slog.LevelVar{} +// String returns the current level. +func (l *Level) String() string { + switch l.lvl.Level() { + case slog.LevelDebug: + return "debug" + case slog.LevelInfo: + return "info" + case slog.LevelWarn: + return "warn" + case slog.LevelError: + return "error" + default: + return "" } +} +// Set updates the logging level with the validation. +func (l *Level) Set(s string) error { switch strings.ToLower(s) { case "debug": l.lvl.Set(slog.LevelDebug) - callerAddFunc = true case "info": l.lvl.Set(slog.LevelInfo) - callerAddFunc = false case "warn": l.lvl.Set(slog.LevelWarn) - callerAddFunc = false case "error": l.lvl.Set(slog.LevelError) - callerAddFunc = false default: return fmt.Errorf("unrecognized log level %s", s) } - l.s = s return nil } -// AllowedFormat is a settable identifier for the output format that the logger can have. -type AllowedFormat struct { +// Format controls a logging output format. +// Not concurrency-safe. +type Format struct { s string } -func (f *AllowedFormat) String() string { +// NewFormat creates a new Format. +func NewFormat() *Format { return &Format{} } + +func (f *Format) String() string { return f.s } // Set updates the value of the allowed format. -func (f *AllowedFormat) Set(s string) error { +func (f *Format) Set(s string) error { switch s { case "logfmt", "json": f.s = s @@ -158,20 +138,128 @@ func (f *AllowedFormat) Set(s string) error { return nil } -// Config is a struct containing configurable settings for the logger +// Config is a struct containing configurable settings for the logger. type Config struct { - Level *AllowedLevel - Format *AllowedFormat + Level *Level + Format *Format Style LogStyle Writer io.Writer } +func newGoKitStyleReplaceAttrFunc(lvl *Level) func(groups []string, a slog.Attr) slog.Attr { + return func(_ []string, a slog.Attr) slog.Attr { + key := a.Key + switch key { + case slog.TimeKey, "ts": + if t, ok := a.Value.Any().(time.Time); ok { + 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). + 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 lvl.String() { + case "debug": + 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: + 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: + } + + // Ensure time.Duration values are _always_ formatted as a Go + // duration string (ie, "1d2h3m"). + if v, ok := a.Value.Any().(time.Duration); ok { + a.Value = slog.StringValue(v.String()) + } + + return a + } +} + +func defaultReplaceAttr(_ []string, a slog.Attr) slog.Attr { + key := a.Key + switch key { + case slog.TimeKey: + // Note that we do not change the timezone to UTC anymore. + if _, ok := a.Value.Any().(time.Time); !ok { + // 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: + 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: + } + + // Ensure time.Duration values are _always_ formatted as a Go duration + // string (ie, "1d2h3m"). + if v, ok := a.Value.Any().(time.Duration); ok { + a.Value = slog.StringValue(v.String()) + } + + return a +} + // New returns a new slog.Logger. Each logged line will be annotated // with a timestamp. The output always goes to stderr. func New(config *Config) *slog.Logger { if config.Level == nil { - config.Level = &AllowedLevel{} - _ = config.Level.Set("info") + config.Level = NewLevel() } if config.Writer == nil { @@ -181,11 +269,11 @@ func New(config *Config) *slog.Logger { logHandlerOpts := &slog.HandlerOptions{ Level: config.Level.lvl, AddSource: true, - ReplaceAttr: defaultReplaceAttrFunc, + ReplaceAttr: defaultReplaceAttr, } if config.Style == GoKitStyle { - logHandlerOpts.ReplaceAttr = goKitStyleReplaceAttrFunc + logHandlerOpts.ReplaceAttr = newGoKitStyleReplaceAttrFunc(config.Level) } if config.Format != nil && config.Format.s == "json" { @@ -197,5 +285,5 @@ func New(config *Config) *slog.Logger { // NewNopLogger is a convenience function to return an slog.Logger that writes // to io.Discard. func NewNopLogger() *slog.Logger { - return slog.New(slog.NewTextHandler(io.Discard, nil)) + return New(&Config{Writer: io.Discard}) } diff --git a/promslog/slog_test.go b/promslog/slog_test.go index fc824e04f..0ea1a31c2 100644 --- a/promslog/slog_test.go +++ b/promslog/slog_test.go @@ -21,9 +21,10 @@ import ( "regexp" "strings" "testing" + "time" "github.com/stretchr/testify/require" - "gopkg.in/yaml.v2" + "go.yaml.in/yaml/v2" ) var ( @@ -43,29 +44,29 @@ func TestDefaultConfig(t *testing.T) { } func TestUnmarshallLevel(t *testing.T) { - l := &AllowedLevel{} + l := NewLevel() err := yaml.Unmarshal([]byte(`debug`), l) if err != nil { t.Error(err) } - if l.s != "debug" { - t.Errorf("expected %s, got %s", "debug", l.s) + if got := l.String(); got != "debug" { + t.Errorf("expected %s, got %s", "debug", got) } } func TestUnmarshallEmptyLevel(t *testing.T) { - l := &AllowedLevel{} + l := NewLevel() err := yaml.Unmarshal([]byte(``), l) if err != nil { t.Error(err) } - if l.s != "" { - t.Errorf("expected empty level, got %s", l.s) + if got := l.String(); got != "info" { + t.Errorf("expected info (default) level, got %s", got) } } func TestUnmarshallBadLevel(t *testing.T) { - l := &AllowedLevel{} + l := NewLevel() err := yaml.Unmarshal([]byte(`debugg`), l) if err == nil { t.Error("expected error") @@ -74,8 +75,8 @@ func TestUnmarshallBadLevel(t *testing.T) { if err.Error() != expErr { t.Errorf("expected error %s, got %s", expErr, err.Error()) } - if l.s != "" { - t.Errorf("expected empty level, got %s", l.s) + if got := l.String(); got != "info" { + t.Errorf("expected info (default) level, got %s", got) } } @@ -94,6 +95,33 @@ func getLogEntryLevelCounts(s string, re *regexp.Regexp) map[string]int { return counters } +func TestDurationValues(t *testing.T) { + dur, err := time.ParseDuration("1m30s") + require.NoError(t, err) + + tests := map[string]struct { + logFormat string + want string + }{ + "logfmt_duration_testing": {want: "duration_raw=1m30s duration_string=1m30s", logFormat: "logfmt"}, + "json_duration_testing": {want: "\"duration_raw\":\"1m30s\",\"duration_string\":\"1m30s\"", logFormat: "json"}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + var buf bytes.Buffer + config := &Config{ + Writer: &buf, + Format: NewFormat(), + } + require.NoError(t, config.Format.Set(tc.logFormat)) + logger := New(config) + logger.Info("duration testing", "duration_raw", dur, "duration_string", dur.String()) + require.Contains(t, buf.String(), tc.want) + }) + } +} + func TestDynamicLevels(t *testing.T) { var buf bytes.Buffer wantedLevelCounts := map[string]int{"info": 1, "debug": 1} @@ -188,3 +216,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()) + }) + } +} diff --git a/route/route.go b/route/route.go index e89fe7ebc..18039fac0 100644 --- a/route/route.go +++ b/route/route.go @@ -78,7 +78,7 @@ func (r *Router) handle(handlerName string, h http.HandlerFunc) httprouter.Handl defer cancel() for _, p := range params { - ctx = context.WithValue(ctx, param(p.Key), p.Value) + ctx = context.WithValue(ctx, param(p.Key), p.Value) //nolint:fatcontext } h(w, req.WithContext(ctx)) } diff --git a/route/route_test.go b/route/route_test.go index 24977b32a..e20e1db17 100644 --- a/route/route_test.go +++ b/route/route_test.go @@ -24,7 +24,7 @@ import ( func TestRedirect(t *testing.T) { router := New().WithPrefix("/test/prefix") w := httptest.NewRecorder() - r, err := http.NewRequest("GET", "http://localhost:9090/foo", nil) + r, err := http.NewRequest(http.MethodGet, "http://localhost:9090/foo", nil) require.NoErrorf(t, err, "Error building test request: %s", err) router.Redirect(w, r, "/some/endpoint", http.StatusFound) @@ -37,20 +37,20 @@ func TestRedirect(t *testing.T) { func TestContext(t *testing.T) { router := New() - router.Get("/test/:foo/", func(w http.ResponseWriter, r *http.Request) { + router.Get("/test/:foo/", func(_ http.ResponseWriter, r *http.Request) { want := "bar" got := Param(r.Context(), "foo") require.Equalf(t, want, got, "Unexpected context value: want %q, got %q", want, got) }) - r, err := http.NewRequest("GET", "http://localhost:9090/test/bar/", nil) + r, err := http.NewRequest(http.MethodGet, "http://localhost:9090/test/bar/", nil) require.NoErrorf(t, err, "Error building test request: %s", err) router.ServeHTTP(nil, r) } func TestContextWithValue(t *testing.T) { router := New() - router.Get("/test/:foo/", func(w http.ResponseWriter, r *http.Request) { + router.Get("/test/:foo/", func(_ http.ResponseWriter, r *http.Request) { want := "bar" got := Param(r.Context(), "foo") require.Equalf(t, want, got, "Unexpected context value: want %q, got %q", want, got) @@ -62,7 +62,7 @@ func TestContextWithValue(t *testing.T) { require.Equalf(t, want, got, "Unexpected context value: want %q, got %q", want, got) }) - r, err := http.NewRequest("GET", "http://localhost:9090/test/bar/", nil) + r, err := http.NewRequest(http.MethodGet, "http://localhost:9090/test/bar/", nil) require.NoErrorf(t, err, "Error building test request: %s", err) params := map[string]string{ "lorem": "ipsum", @@ -71,7 +71,7 @@ func TestContextWithValue(t *testing.T) { ctx := r.Context() for p, v := range params { - ctx = WithParam(ctx, p, v) + ctx = WithParam(ctx, p, v) //nolint:fatcontext } r = r.WithContext(ctx) router.ServeHTTP(nil, r) @@ -79,13 +79,13 @@ func TestContextWithValue(t *testing.T) { func TestContextWithoutValue(t *testing.T) { router := New() - router.Get("/test", func(w http.ResponseWriter, r *http.Request) { + router.Get("/test", func(_ http.ResponseWriter, r *http.Request) { want := "" got := Param(r.Context(), "foo") require.Equalf(t, want, got, "Unexpected context value: want %q, got %q", want, got) }) - r, err := http.NewRequest("GET", "http://localhost:9090/test", nil) + r, err := http.NewRequest(http.MethodGet, "http://localhost:9090/test", nil) require.NoErrorf(t, err, "Error building test request: %s", err) router.ServeHTTP(nil, r) } @@ -109,9 +109,9 @@ func TestInstrumentation(t *testing.T) { } for _, c := range cases { - c.router.Get("/foo", func(w http.ResponseWriter, r *http.Request) {}) + c.router.Get("/foo", func(_ http.ResponseWriter, _ *http.Request) {}) - r, err := http.NewRequest("GET", "http://localhost:9090/foo", nil) + r, err := http.NewRequest(http.MethodGet, "http://localhost:9090/foo", nil) require.NoErrorf(t, err, "Error building test request: %s", err) c.router.ServeHTTP(nil, r) require.Equalf(t, c.want, got, "Unexpected value: want %q, got %q", c.want, got) @@ -149,12 +149,12 @@ func TestInstrumentations(t *testing.T) { } for _, c := range cases { - c.router.Get("/foo", func(w http.ResponseWriter, r *http.Request) {}) + c.router.Get("/foo", func(_ http.ResponseWriter, _ *http.Request) {}) - r, err := http.NewRequest("GET", "http://localhost:9090/foo", nil) + r, err := http.NewRequest(http.MethodGet, "http://localhost:9090/foo", nil) require.NoErrorf(t, err, "Error building test request: %s", err) c.router.ServeHTTP(nil, r) - require.Equalf(t, len(c.want), len(got), "Unexpected value: want %q, got %q", c.want, got) + require.Lenf(t, got, len(c.want), "Unexpected value: want %q, got %q", c.want, got) for i, v := range c.want { require.Equalf(t, v, got[i], "Unexpected value: want %q, got %q", c.want, got) } diff --git a/server/static_file_server_test.go b/server/static_file_server_test.go index 9aa8d76fb..ae8bf599d 100644 --- a/server/static_file_server_test.go +++ b/server/static_file_server_test.go @@ -23,7 +23,7 @@ import ( type dummyFileSystem struct{} -func (fs dummyFileSystem) Open(path string) (http.File, error) { +func (dummyFileSystem) Open(string) (http.File, error) { return http.Dir(".").Open(".") } @@ -68,7 +68,7 @@ func TestServeHttp(t *testing.T) { for _, c := range cases { t.Run(c.name, func(t *testing.T) { rr := httptest.NewRecorder() - req, err := http.NewRequest("GET", "http://localhost/"+c.path, nil) + req, err := http.NewRequest(http.MethodGet, "http://localhost/"+c.path, nil) require.NoError(t, err) s := StaticFileServer(dummyFileSystem{})