From a6e776b923bf4ae9549ac6bbb8d36510f0387fed Mon Sep 17 00:00:00 2001 From: gagliardetto Date: Fri, 4 Oct 2024 14:45:17 +0200 Subject: [PATCH 1/6] Cleanup metrics --- http-range.go | 48 ++++++++++++++++++++++++++-------------------- metrics/metrics.go | 4 ++-- 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/http-range.go b/http-range.go index 593bf793..d03346fb 100644 --- a/http-range.go +++ b/http-range.go @@ -38,6 +38,21 @@ func (r *readCloserWrapper) ReadAt(p []byte, off int64) (n int, err error) { // if has suffix .index, then it's an index file if strings.HasSuffix(r.name, ".index") { isIndex = true + } + // if has suffix .car, then it's a car file + if strings.HasSuffix(r.name, ".car") || r.isSplitCar { + isCar = true + } + + if isCar { + carName := filepath.Base(r.name) + metrics.CarLookupHistogram.WithLabelValues( + carName, + boolToString(r.isRemote), + boolToString(r.isSplitCar), + ).Observe(float64(took.Seconds())) + } + if isIndex { // get the index name, which is the part before the .index suffix, after the last . indexName := strings.TrimSuffix(r.name, ".index") // split the index name by . and get the last part @@ -45,18 +60,14 @@ func (r *readCloserWrapper) ReadAt(p []byte, off int64) (n int, err error) { if len(byDot) > 0 { indexName = byDot[len(byDot)-1] } - // TODO: distinguish between remote and local index reads - metrics.IndexLookupHistogram.WithLabelValues(indexName).Observe(float64(took.Seconds())) - } - // if has suffix .car, then it's a car file - if strings.HasSuffix(r.name, ".car") || r.isSplitCar { - isCar = true - carName := filepath.Base(r.name) - // TODO: distinguish between remote and local index reads - metrics.CarLookupHistogram.WithLabelValues(carName).Observe(float64(took.Seconds())) + metrics.IndexLookupHistogram.WithLabelValues( + indexName, + boolToString(r.isRemote), + ).Observe(float64(took.Seconds())) } if klog.V(5).Enabled() { + // Very verbose logging: var icon string if r.isRemote { // add internet icon @@ -69,15 +80,6 @@ func (r *readCloserWrapper) ReadAt(p []byte, off int64) (n int, err error) { prefix := icon + "[READ-UNKNOWN]" if isIndex { prefix = icon + azureBG("[READ-INDEX]") - // get the index name, which is the part before the .index suffix, after the last . - indexName := strings.TrimSuffix(r.name, ".index") - // split the index name by . and get the last part - byDot := strings.Split(indexName, ".") - if len(byDot) > 0 { - indexName = byDot[len(byDot)-1] - } - // TODO: distinguish between remote and local index reads - metrics.IndexLookupHistogram.WithLabelValues(indexName).Observe(float64(took.Seconds())) } // if has suffix .car, then it's a car file if isCar { @@ -86,9 +88,6 @@ func (r *readCloserWrapper) ReadAt(p []byte, off int64) (n int, err error) { } else { prefix = icon + purpleBG("[READ-CAR]") } - carName := filepath.Base(r.name) - // TODO: distinguish between remote and local index reads - metrics.CarLookupHistogram.WithLabelValues(carName).Observe(float64(took.Seconds())) } klog.V(5).Infof(prefix+" %s:%d+%d (%s)\n", (r.name), off, len(p), took) @@ -111,3 +110,10 @@ func azureBG(s string) string { func (r *readCloserWrapper) Close() error { return r.rac.Close() } + +func boolToString(b bool) string { + if b { + return "true" + } + return "false" +} diff --git a/metrics/metrics.go b/metrics/metrics.go index c0e6a293..004d68b7 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -68,7 +68,7 @@ var IndexLookupHistogram = promauto.NewHistogramVec( Help: "Index lookup latency", Buckets: prometheus.ExponentialBuckets(0.000001, 10, 10), }, - []string{"index_type"}, + []string{"index_type", "is_remote", "is_split_car"}, ) var CarLookupHistogram = promauto.NewHistogramVec( @@ -77,7 +77,7 @@ var CarLookupHistogram = promauto.NewHistogramVec( Help: "Car lookup latency", Buckets: prometheus.ExponentialBuckets(0.000001, 10, 10), }, - []string{"car"}, + []string{"car", "is_remote"}, ) var RpcResponseLatencyHistogram = promauto.NewHistogramVec( From ccfa37e4701b6c1e91bcc4c8a2312ef08770b66b Mon Sep 17 00:00:00 2001 From: gagliardetto Date: Fri, 4 Oct 2024 14:49:55 +0200 Subject: [PATCH 2/6] Fix latency buckets --- metrics/metrics.go | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/metrics/metrics.go b/metrics/metrics.go index 004d68b7..ec593ea1 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -62,11 +62,20 @@ var Version = promauto.NewGaugeVec( []string{"started_at", "tag", "commit", "compiler", "goarch", "goos", "goamd64", "vcs", "vcs_revision", "vcs_time", "vcs_modified"}, ) +var RpcResponseLatencyHistogram = promauto.NewHistogramVec( + prometheus.HistogramOpts{ + Name: "rpc_response_latency_histogram", + Help: "RPC response latency histogram", + Buckets: latencyBuckets, + }, + []string{"rpc_method"}, +) + var IndexLookupHistogram = promauto.NewHistogramVec( prometheus.HistogramOpts{ Name: "index_lookup_latency_histogram", Help: "Index lookup latency", - Buckets: prometheus.ExponentialBuckets(0.000001, 10, 10), + Buckets: latencyBuckets, }, []string{"index_type", "is_remote", "is_split_car"}, ) @@ -75,16 +84,18 @@ var CarLookupHistogram = promauto.NewHistogramVec( prometheus.HistogramOpts{ Name: "car_lookup_latency_histogram", Help: "Car lookup latency", - Buckets: prometheus.ExponentialBuckets(0.000001, 10, 10), + Buckets: latencyBuckets, }, []string{"car", "is_remote"}, ) -var RpcResponseLatencyHistogram = promauto.NewHistogramVec( - prometheus.HistogramOpts{ - Name: "rpc_response_latency_histogram", - Help: "RPC response latency histogram", - Buckets: prometheus.ExponentialBuckets(0.000001, 10, 10), - }, - []string{"rpc_method"}, -) +var latencyBuckets = []float64{ + // fractional seconds from 0 to 1, with increments of 0.05 (= 50 ms) + 0, 0.05, 0.1, 0.15, 0.2, 0.25, 0.3, 0.35, 0.4, 0.45, + 0.5, 0.55, 0.6, 0.65, 0.7, 0.75, 0.8, 0.85, 0.9, 0.95, + 1, 1.5, 2, 2.5, 3, 3.5, 4, 4.5, 5, + // then 5-10 with increments of 1 + 6, 7, 8, 9, 10, + // then 10-60 with increments of 5 + 15, 20, 25, 30, 35, 40, 45, 50, 55, 60, +} From 277ff019d3930514a8d90c63418f6f7fb64b9782 Mon Sep 17 00:00:00 2001 From: gagliardetto Date: Fri, 4 Oct 2024 14:58:08 +0200 Subject: [PATCH 3/6] Limit rusts tests for when rust is changed --- .github/workflows/tests.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index bd49ba8a..854d7685 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -29,6 +29,17 @@ jobs: - name: Test run: go test ./... test_rust: + on: + push: + paths: + - 'Cargo.toml' + - 'Cargo.lock' + - 'rust-toolchain.toml' + - '.github/workflows/tests.yml' + - 'txstatus/**' + - 'old-faithful-proto/**' + - 'geyser-plugin-runner/**' + - 'tests/rust/**' strategy: matrix: os: [ubuntu-20.04, ubuntu-22.04] From fbb04fda9303a7ec76fe7dbc20c498566d3f6745 Mon Sep 17 00:00:00 2001 From: gagliardetto Date: Fri, 4 Oct 2024 14:59:40 +0200 Subject: [PATCH 4/6] Fix? --- .github/workflows/tests.yml | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 854d7685..dc0c8130 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -29,17 +29,15 @@ jobs: - name: Test run: go test ./... test_rust: - on: - push: - paths: - - 'Cargo.toml' - - 'Cargo.lock' - - 'rust-toolchain.toml' - - '.github/workflows/tests.yml' - - 'txstatus/**' - - 'old-faithful-proto/**' - - 'geyser-plugin-runner/**' - - 'tests/rust/**' + paths: + - 'Cargo.toml' + - 'Cargo.lock' + - 'rust-toolchain.toml' + - '.github/workflows/tests.yml' + - 'txstatus/**' + - 'old-faithful-proto/**' + - 'geyser-plugin-runner/**' + - 'tests/rust/**' strategy: matrix: os: [ubuntu-20.04, ubuntu-22.04] From 9e85bab5f75936f8b512bf25b18a64488b828f0d Mon Sep 17 00:00:00 2001 From: gagliardetto Date: Fri, 4 Oct 2024 15:05:22 +0200 Subject: [PATCH 5/6] Split between test go and test rust --- .github/workflows/tests-go.yml | 30 ++++++++++++++ .../workflows/{tests.yml => tests-rust.yml} | 40 ++++++++----------- 2 files changed, 46 insertions(+), 24 deletions(-) create mode 100644 .github/workflows/tests-go.yml rename .github/workflows/{tests.yml => tests-rust.yml} (82%) diff --git a/.github/workflows/tests-go.yml b/.github/workflows/tests-go.yml new file mode 100644 index 00000000..7fa4ee7e --- /dev/null +++ b/.github/workflows/tests-go.yml @@ -0,0 +1,30 @@ +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +on: + pull_request: + workflow_dispatch: + +env: + CARGO_TERM_COLOR: always + +name: tests_go +jobs: + test: + strategy: + matrix: + go-version: [1.21.x] + os: [ubuntu-latest] + runs-on: ${{ matrix.os }} + steps: + - name: Install Go + uses: actions/setup-go@v5 + with: + go-version: ${{ matrix.go-version }} + - name: Checkout code + uses: actions/checkout@v4 + with: + fetch-depth: 0 + - name: Test + run: go test ./... diff --git a/.github/workflows/tests.yml b/.github/workflows/tests-rust.yml similarity index 82% rename from .github/workflows/tests.yml rename to .github/workflows/tests-rust.yml index dc0c8130..4ceb6852 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests-rust.yml @@ -4,31 +4,16 @@ concurrency: on: pull_request: + paths: + - 'Cargo.toml' + - 'Cargo.lock' + - 'rust-toolchain.toml' + - '.github/workflows/tests.yml' + - 'txstatus/**' + - 'old-faithful-proto/**' + - 'geyser-plugin-runner/**' + - 'tests/rust/**' workflow_dispatch: - -env: - CARGO_TERM_COLOR: always - -name: tests -jobs: - test: - strategy: - matrix: - go-version: [1.21.x] - os: [ubuntu-latest] - runs-on: ${{ matrix.os }} - steps: - - name: Install Go - uses: actions/setup-go@v5 - with: - go-version: ${{ matrix.go-version }} - - name: Checkout code - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - name: Test - run: go test ./... - test_rust: paths: - 'Cargo.toml' - 'Cargo.lock' @@ -38,6 +23,13 @@ jobs: - 'old-faithful-proto/**' - 'geyser-plugin-runner/**' - 'tests/rust/**' + +env: + CARGO_TERM_COLOR: always + +name: tests_rust +jobs: + test_rust: strategy: matrix: os: [ubuntu-20.04, ubuntu-22.04] From a7d5c2daf599c7b995967cbcde66567d50cc658e Mon Sep 17 00:00:00 2001 From: gagliardetto Date: Fri, 4 Oct 2024 15:06:08 +0200 Subject: [PATCH 6/6] Fix --- .github/workflows/tests-rust.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests-rust.yml b/.github/workflows/tests-rust.yml index 4ceb6852..833e7b34 100644 --- a/.github/workflows/tests-rust.yml +++ b/.github/workflows/tests-rust.yml @@ -8,7 +8,7 @@ on: - 'Cargo.toml' - 'Cargo.lock' - 'rust-toolchain.toml' - - '.github/workflows/tests.yml' + - '.github/workflows/tests-rust.yml' - 'txstatus/**' - 'old-faithful-proto/**' - 'geyser-plugin-runner/**' @@ -18,7 +18,7 @@ on: - 'Cargo.toml' - 'Cargo.lock' - 'rust-toolchain.toml' - - '.github/workflows/tests.yml' + - '.github/workflows/tests-rust.yml' - 'txstatus/**' - 'old-faithful-proto/**' - 'geyser-plugin-runner/**'