Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
## unreleased

* [BUGFIX] Remote write: the path of `--remote-url` is now respected instead of always appending `api/v1/write`. URLs that relied on the previous prefix behavior (own path with `api/v1/write` appended) must now include the full write path. #196

## 0.7.0 / 2025-01-14

* [CHANGE] (breaking) Removed the deprecated `--metric-count` flag (use `--gauge-metric-count` instead). #119
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
Avalanche is a load-testing binary capable of generating metrics that can be either:

* scraped via [Prometheus scrape formats](https://prometheus.io/docs/instrumenting/exposition_formats/) (including [OpenMetrics](https://github.com/OpenObservability/OpenMetrics)) endpoint.
* written via Prometheus Remote Write (v1 only for now) to a target endpoint.
* written via Prometheus Remote Write (v1, or experimental v2 with `--remote-write-v2`) to a target endpoint.

This allows load testing services that can scrape (e.g. Prometheus, OpenTelemetry Collector and so), as well as, services accepting data via Prometheus remote_write API such as [Thanos](https://github.com/thanos-io/thanos), [Cortex](https://github.com/cortexproject/cortex), [M3DB](https://m3db.github.io/m3/integrations/prometheus/), [VictoriaMetrics](https://github.com/VictoriaMetrics/VictoriaMetrics/) and other services [listed here](https://prometheus.io/docs/operating/integrations/#remote-endpoints-and-storage).

Expand Down
27 changes: 21 additions & 6 deletions metricsgen/write.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ type ConfigWrite struct {

func NewWriteConfigFromFlags(flagReg func(name, help string) *kingpin.FlagClause) *ConfigWrite {
cfg := &ConfigWrite{}
flagReg("remote-url", "URL to send samples via remote_write API. By default, path is set to api/v1/write").
flagReg("remote-url", "URL to send samples via remote_write API. A URL path (other than a bare '/') is used after the client's path cleaning (e.g. trailing slashes are dropped); host-only URLs get the default path api/v1/write appended.").
URLVar(&cfg.URL)
flagReg("remote-concurrency-limit", "how many concurrent writes can happen at any given time").Default("20").
IntVar(&cfg.Concurrency)
Expand Down Expand Up @@ -165,11 +165,7 @@ func RunRemoteWriting(ctx context.Context, logger *slog.Logger, cfg *ConfigWrite
rt = &userAgentRoundTripper{userAgent: "avalanche", rt: rt}
httpClient := &http.Client{Transport: rt}

remoteAPI, err := remote.NewAPI(
cfg.URL.String(),
remote.WithAPIHTTPClient(httpClient),
remote.WithAPILogger(logger.With("component", "remote_write_api")),
)
remoteAPI, err := newRemoteAPI(cfg, logger, httpClient)
if err != nil {
return err
}
Expand All @@ -189,6 +185,25 @@ func RunRemoteWriting(ctx context.Context, logger *slog.Logger, cfg *ConfigWrite
return writer.write(ctx)
}

// newRemoteAPI builds the remote write client. If the user-supplied URL has
// a path (other than a bare "/"), it is kept, subject to the client's path
// cleaning (trailing and duplicate slashes are dropped). Host-only URLs and
// a bare "/" get the client's default path, api/v1/write; posting samples
// to "/" is almost never intended.
// See https://github.com/prometheus-community/avalanche/issues/173.
func newRemoteAPI(cfg *ConfigWrite, logger *slog.Logger, httpClient *http.Client) (*remote.API, error) {
opts := []remote.APIOption{
remote.WithAPIHTTPClient(httpClient),
remote.WithAPILogger(logger.With("component", "remote_write_api")),
}
if cfg.URL.Path != "" && cfg.URL.Path != "/" {
// remote.NewAPI path.Join-s its path option onto the URL's path;
// an empty path option keeps the URL's path as-is.
opts = append(opts, remote.WithAPIPath(""))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this option is sort of tricky to use.

I think originally we thought clients would dedicatedly build paths this way so, you could pass in URL with path of metrics/v1/tenant and call WithAPIPath("api/v1/receive") or so.

I think a cleaner option here, might be to parse the URL from the flag, and split url and path when calling remote.NewAPI and WithAPIPath?

Or we can think about changing behaviour on the exp module, so that if it detects path, it won't append default

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, took your first suggestion: 3baa985 splits the flag's URL, base to remote.NewAPI and path to WithAPIPath. Host-only URLs and a bare / still get the api/v1/write default.

One wrinkle it introduces: NewAPI joins into url.URL.Path and leaves RawPath alone, so the endpoint sees the decoded path. Harmless for unreserved characters (/ten%61nt posts to /tenant), but %2F exists to stop a slash being a segment boundary, so /tenant%2Freceive would quietly post to /tenant/receive. 5099ae5 rejects that instead of rewriting it.

If you'd rather not carry that validation, your second idea solves it properly: an exp option meaning "this URL is the exact endpoint", skipping the default and leaving the parsed URL untouched, would preserve RawPath. Happy to open that PR.

}
return remote.NewAPI(cfg.URL.String(), opts...)
}

// Add the tenant ID header
func (rt *tenantRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
req = cloneRequest(req)
Expand Down
78 changes: 78 additions & 0 deletions metricsgen/write_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,20 @@
package metricsgen

import (
"context"
"io"
"log/slog"
"net/http"
"net/http/httptest"
"net/url"
"sync"
"testing"
"time"

"github.com/prometheus/client_golang/exp/api/remote"
writev2 "github.com/prometheus/client_golang/exp/api/remote/genproto/v2"
"github.com/prometheus/prometheus/prompb"
"github.com/stretchr/testify/require"
)

func TestShuffleTimestamps(t *testing.T) {
Expand Down Expand Up @@ -58,3 +68,71 @@ func TestShuffleTimestamps(t *testing.T) {
t.Error("Timestamps are not out of order")
}
}

func TestNewRemoteAPIPath(t *testing.T) {
for _, tc := range []struct {
name string
urlPath string
wantPath string
wantQuery string
}{
{name: "no path appends default", urlPath: "", wantPath: "/api/v1/write"},
{name: "root path appends default", urlPath: "/", wantPath: "/api/v1/write"},
{name: "custom path is respected", urlPath: "/api/v1/receive", wantPath: "/api/v1/receive"},
{name: "trailing slash is cleaned", urlPath: "/api/v1/receive/", wantPath: "/api/v1/receive"},
{name: "prefix path is respected", urlPath: "/prometheus", wantPath: "/prometheus"},
{name: "query string preserved", urlPath: "/api/v1/receive?tenant=a", wantPath: "/api/v1/receive", wantQuery: "tenant=a"},
{name: "escaped path segment preserved", urlPath: "/tenant%2Freceive", wantPath: "/tenant%2Freceive"},
{name: "double slash is cleaned", urlPath: "//foo", wantPath: "/foo"},
} {
t.Run(tc.name, func(t *testing.T) {
var (
mu sync.Mutex
gotPath string
gotQuery string
)
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
mu.Lock()
gotPath = r.URL.EscapedPath()
gotQuery = r.URL.RawQuery
mu.Unlock()
w.Header().Set("X-Prometheus-Remote-Write-Samples-Written", "0")
w.Header().Set("X-Prometheus-Remote-Write-Histograms-Written", "0")
w.Header().Set("X-Prometheus-Remote-Write-Exemplars-Written", "0")
w.WriteHeader(http.StatusNoContent)
}))
t.Cleanup(srv.Close)

u, err := url.Parse(srv.URL + tc.urlPath)
require.NoError(t, err)

api, err := newRemoteAPI(
&ConfigWrite{URL: u},
slog.New(slog.NewTextHandler(io.Discard, nil)),
srv.Client(),
)
require.NoError(t, err)

for _, msg := range []struct {
typ remote.WriteMessageType
req any
}{
{typ: remote.WriteV1MessageType, req: &prompb.WriteRequest{}},
{typ: remote.WriteV2MessageType, req: &writev2.Request{Symbols: []string{""}}},
} {
mu.Lock()
gotPath = ""
gotQuery = ""
mu.Unlock()

_, err = api.Write(context.Background(), msg.typ, msg.req)
require.NoError(t, err)

mu.Lock()
require.Equal(t, tc.wantPath, gotPath)
require.Equal(t, tc.wantQuery, gotQuery)
mu.Unlock()
}
})
}
}
Loading