api: Add remote API with write client; add remote handler. #1658

bwplotka · 2024-10-21T13:01:51Z

This proposes Remote Write API directly in client_golang main module. No dependency added (some new small packages are added under the internal vendored code).

Old attempt: #1656

Both API (client) and handler supports 1.0 and 2.0 Proto messages, but I intentionally only host writev2.Request generated Go code as we plan to deprecate 1.0 and users should use writev2 now. Both API and handler also will work against protobuf message types that use custom generators e.g. with gogoproto or anything else.

TODO:

Add examples, documentation
Open PR that uses it on Prometheus

Signed-off-by: bwplotka <[email protected]>

bwplotka · 2024-10-21T13:34:14Z

api/prometheus/v1/remote/remote_api.go

+//
+// It is not safe to use the returned API from multiple goroutines, create a
+// separate *API for each goroutine.
+func NewAPI(c api.Client, opts ...APIOption) (*API, error) {


We could consider removing error from here and from APIOption as we don't have any option with validation or so. However I added it for future compatibility (we won't be able to easily extend it without breaking compatibility). Note: Currently api/ is experimenta... 🙈 I think I will remove error here and hope for the best, YAGNI.

Suggested change

func NewAPI(c api.Client, opts ...APIOption) (*API, error) {

func NewAPI(c api.Client, opts ...APIOption) *API {

jmichalek132 · 2024-10-21T18:07:00Z

api/prometheus/v1/remote/genproto/v2/symbols.go

+}
+
+// DesymbolizeLabels decodes label references, with given symbols to labels.
+func DesymbolizeLabels(labelRefs []uint32, symbols, buf []string) []string {


yeah I just ran into DesymbolizeLabels not being public in prometheus codebase here open-telemetry/opentelemetry-collector-contrib#35751 (comment)

Do you use labels.Labels or some other struct (for labels in Otel)

In the translation layer we have createAttributes https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/pkg/translator/prometheusremotewrite/helper.go#L99 which returns []prompb.Label and here we store them https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/pkg/translator/prometheusremotewrite/metrics_to_prw_v2.go#L119

bwplotka · 2024-10-22T07:54:18Z

api/prometheus/v1/remote/remote_api.go

+
+// NewRemoteWriteHandler returns HTTP handler that receives Remote Write 2.0
+// protocol https://prometheus.io/docs/specs/remote_write_spec_2_0/.
+func NewRemoteWriteHandler(logger *slog.Logger, store writeStorage) http.Handler {


It's bit inconsistent with NewRemoteAPI in terms of options, let's unify one to one or another 🤔

bwplotka · 2024-10-22T10:23:41Z

api/prometheus/v1/remote/remote_headers.go

+// WriteProtoFullName represents the fully qualified name of the protobuf message
+// to use in Remote write 1.0 and 2.0 protocols.
+// See https://prometheus.io/docs/specs/remote_write_spec_2_0/#protocol.
+type WriteProtoFullName protoreflect.FullName


nit: Change to string, bit too hard to use

bwplotka · 2024-10-22T10:24:06Z

api/prometheus/v1/remote/remote_headers.go

+const (
+	// WriteProtoFullNameV1 represents the `prometheus.WriteRequest` protobuf
+	// message introduced in the https://prometheus.io/docs/specs/remote_write_spec/.
+	// DEPRECATED: Use WriteProtoFullNameV2 instead.


For receivers who wants to support it for longer, deprecation might cause some friction, but maybe worth it? 🤔

bwplotka · 2024-10-22T11:32:54Z

Example use of handler in sink project: https://github.com/bwplotka/sink/blob/main/go/sink/main.go#L51

cstyan

Both API (client) and handler supports 1.0 and 2.0 Proto messages, but I intentionally only host writev2.Request generated Go code as we plan to deprecate 1.0 and users should use writev2 now. Both API and handler also will work against protobuf message types that use custom generators e.g. with gogoproto or anything else.

Given that the spec is only just finalized recently and (afaik) still no receivers like Mimir support v2, we should include v1 request code imo.

cstyan · 2024-10-25T18:29:09Z

api/prometheus/v1/remote/remote_api.go

+}
+
+func (r *API) attemptWrite(ctx context.Context, compr Compression, proto WriteProtoFullName, payload []byte, attempt int) (WriteResponseStats, error) {
+	u := r.client.URL("api/v1/write", nil)


this should be configurable imo, not all remote write receivers use the same endpoint

cstyan · 2024-10-25T18:29:54Z

api/prometheus/v1/remote/remote_api.go

+	rs, err := parseWriteResponseStats(resp)
+	if err != nil {
+		r.opts.logger.Warn("parsing rw write statistics failed; partial or no stats", "err", err)
+	}


should we only try to get write response stats/log a warning if the request type was v2?

cstyan · 2024-10-25T18:32:03Z

api/prometheus/v1/remote/remote_api.go

+	return &handler{logger: logger, store: store}
+}
+
+func parseProtoMsg(contentType string) (WriteProtoFullName, error) {


a comment with an example of what the string might look like would be useful for people who haven't read or fully understood the v2 spec

cstyan · 2024-10-25T18:39:27Z

internal/github.com/efficientgo/core/backoff/backoff.go

+// Copyright (c) The EfficientGo Authors.
+// Licensed under the Apache License 2.0.
+
+// Initially copied from Cortex project.
+
+// Package backoff implements backoff timers which increases wait time on every retry, incredibly useful
+// in distributed system timeout functionalities.


should we be pulling this via go.mod as opposed to having copies of the files? or is this the way we include dependencies for the client library?

We try as hard as we can to avoid dependencies. I'm in favor of copying other libraries code (respecting copyright) if possible

yeya24 · 2024-11-17T19:55:46Z

api/prometheus/v1/remote/remote_api.go

+	}
+
+	// Read the request body.
+	body, err := io.ReadAll(r.Body)


It might be out of scope for Prometheus but it would be nice to limit the request body size to protect the handler. https://github.com/cortexproject/cortex/blob/master/pkg/util/http.go#L180

yeya24 · 2024-11-17T19:58:08Z

api/prometheus/v1/remote/remote_api.go

+		return
+	}
+
+	decompressed, err := snappy.Decode(nil, body)


Similar here. It would be nice if we can check snappy decoded length and see if it exceeds the max size https://github.com/cortexproject/cortex/blob/master/pkg/util/http.go#L247

yeya24 · 2024-11-17T20:05:00Z

api/prometheus/v1/remote/remote_api.go

+	return &handler{logger: logger, store: store}
+}
+
+func parseProtoMsg(contentType string) (WriteProtoFullName, error) {


Is this something we can expose?

yeya24 · 2024-11-17T20:16:15Z

api/prometheus/v1/remote/remote_api.go

+		h.logger.Error("Error decompressing remote write request", "err", err.Error())
+		http.Error(w, err.Error(), http.StatusBadRequest)
+		return
+	}


I wonder if we can expose a hook for the logic of decoding and decompressing part so that downstream project can just use their existing code without many adjustment.

func decodeRequestBody(r *http.Request) (body []byte, err error)

The main logic that can benefit us is handling v1 and v2 proto and response headers. It would be nice to still reuse the other code.

bwplotka requested review from cstyan and ArthurSens October 21, 2024 13:01

bwplotka mentioned this pull request Oct 21, 2024

api: Create a new Go module for API; with remote write client and handler. #1656

Closed

3 tasks

bwplotka force-pushed the prwclient-em branch 4 times, most recently from 17b12a6 to 1985d61 Compare October 21, 2024 13:14

api: Add remote API with write client; add remote handler.

bffa922

Signed-off-by: bwplotka <[email protected]>

bwplotka force-pushed the prwclient-em branch from 1985d61 to bffa922 Compare October 21, 2024 13:18

bwplotka commented Oct 21, 2024

View reviewed changes

jmichalek132 reviewed Oct 21, 2024

View reviewed changes

bwplotka commented Oct 22, 2024

View reviewed changes

cstyan reviewed Oct 25, 2024

View reviewed changes

jmichalek132 mentioned this pull request Nov 13, 2024

Make DesymbolizeLabels func public prometheus/prometheus#15395

Closed

yeya24 mentioned this pull request Nov 14, 2024

Support remote write v2 by converting request cortexproject/cortex#6330

Open

3 tasks

SungJin1212 mentioned this pull request Nov 15, 2024

Prometheus Remote Write v2 Implementation cortexproject/cortex#6324

Open

6 tasks

yeya24 reviewed Nov 17, 2024

View reviewed changes

bwplotka mentioned this pull request Nov 27, 2024

Promsafe: Strongly-typed safe labels #1598

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

api: Add remote API with write client; add remote handler. #1658

api: Add remote API with write client; add remote handler. #1658

bwplotka commented Oct 21, 2024 •

edited

Loading

bwplotka Oct 21, 2024

jmichalek132 Oct 21, 2024

bwplotka Oct 21, 2024

jmichalek132 Oct 25, 2024

bwplotka Oct 22, 2024

bwplotka Oct 22, 2024

bwplotka Oct 22, 2024

bwplotka commented Oct 22, 2024

cstyan left a comment

cstyan Oct 25, 2024

cstyan Oct 25, 2024

cstyan Oct 25, 2024

cstyan Oct 25, 2024

ArthurSens Oct 28, 2024

yeya24 Nov 17, 2024

yeya24 Nov 17, 2024

yeya24 Nov 17, 2024

yeya24 Nov 17, 2024

	func NewAPI(c api.Client, opts ...APIOption) (*API, error) {
	func NewAPI(c api.Client, opts ...APIOption) *API {

api: Add remote API with write client; add remote handler. #1658

Are you sure you want to change the base?

api: Add remote API with write client; add remote handler. #1658

Conversation

bwplotka commented Oct 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bwplotka commented Oct 22, 2024

cstyan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bwplotka commented Oct 21, 2024 •

edited

Loading