remote_write: return 422 when native histograms are sent but disabled; add tests by shibo911 · Pull Request #17196 · prometheus/prometheus

shibo911 · 2025-09-15T23:04:56Z

Which issue(s) does the PR fix:

Fixes Remote Write 2.0 returns 500 when Native Histograms are sent but the feature is disabled #17181

Does this PR introduce a user-facing change?

[BUGFIX] Remote Write: Return HTTP 422 Unprocessable Entity when native histograms are sent to a receiver with native histograms disabled. Applies to:

RW v1 (all-or-nothing).
RW v2 (partial write): samples/exemplars are accepted, histograms rejected; X-Prometheus-Remote-Write-* headers reflect zero histograms written.
OTLP write handler.
Previously these requests returned HTTP 500.

[BUGFIX] Remote Write: Return HTTP 422 (Unprocessable Entity) instead of 500 when native histograms are sent to a receiver with native histograms disabled. For RW v2, valid samples/exemplars are still accepted (partial write) and X-Prometheus-Remote-Write-* headers reflect 0 histograms written. Also applied to the OTLP write endpoint.

shibo911 · 2025-09-15T23:42:00Z

Using HTTP 422 Unprocessable Content (RFC 9110 §15.5.16) for native histograms when the feature is disabled. The request is syntactically valid but semantically unacceptable.
Behavior:
- RW v1: 422, entire request rejected.
- RW v2: partial write; samples/exemplars accepted, native histograms rejected; X-Prometheus-Remote-Write-* headers report 0 histograms.
- OTLP: 422 mapped similarly.
This makes these client errors non-retriable. No change when NH is enabled.
Tests cover v1 all-or-nothing and v2 partial write, with headers. Follow-ups noted below.

CC @roidelapluie

roidelapluie · 2025-09-16T11:55:19Z

Is this compatible with #17197 ?

shibo911 · 2025-09-16T14:51:53Z

Is this compatible with #17197 ?

Yes. The 422 mapping is orthogonal to the handler abstraction used in [RW]: Adopt client_golang/exp/api/remote types for receiving RW1 and RW2 #17197. RW v2 remains compatible because appendV2 still returns 422 for ErrNativeHistogramsDisabled and the stats are propagated via the response object.
One small follow-up needed if [RW]: Adopt client_golang/exp/api/remote types for receiving RW1 and RW2 #17197 lands: in the v1 path within storage/remote/write_handler.go after the refactor (Store -> msgType == remoteapi.WriteV1MessageType), include a case to map storage.ErrNativeHistogramsDisabled to HTTP 422, same as this PR does today. Concretely, in the error switch after h.write(...), add: case errors.Is(err, storage.ErrNativeHistogramsDisabled): wr.SetStatusCode(http.StatusUnprocessableEntity) return wr, err This keeps v1 behavior aligned with this PR and ensures our new v1 test (asserting 422) continues to pass.

Notes:

RW v2: already returns 422 for NH disabled and sets X-Prometheus-Remote-Write-* headers; [RW]: Adopt client_golang/exp/api/remote types for receiving RW1 and RW2 #17197’s wrapper forwards these via wr.Add(respStats).
OTLP: unaffected by [RW]: Adopt client_golang/exp/api/remote types for receiving RW1 and RW2 #17197; the 422 mapping added here stays valid.

So, aside from adding that single v1 case when #17197 merges, the behavior introduced in this PR is fully compatible.

PTAL @roidelapluie

krajorama · 2025-09-17T11:45:28Z

storage/remote/write_handler.go

+			if errors.Is(err, storage.ErrNativeHistogramsDisabled) {
+				// Receiver supports protocol, but cannot process native histograms.
+				nhDisabled = true
+				h.logger.Error("Out of order histogram from remote write", "err", err.Error(), "series", ls.String(), "timestamp", hp.Timestamp)


Suggested change

h.logger.Error("Out of order histogram from remote write", "err", err.Error(), "series", ls.String(), "timestamp", hp.Timestamp)

h.logger.Error("Native histograms not enabled", "err", err.Error(), "series", ls.String(), "timestamp", hp.Timestamp)

I'm not sure we want to log this as error. The feature may be disabled on purpose, so maybe debug level is better.

Done. Updated the log to:

message: “Native histograms not enabled”

level: debug

Location: storage/remote/write_handler.go, in appendV2() under the ErrNativeHistogramsDisabled branch.

Behavior unchanged: we still mark nhDisabled = true, include the error in badRequestErrs, and return HTTP 422 for v2 with partial-write headers (0 histograms written).

CC @krajorama

krajorama · 2025-09-17T11:49:36Z

storage/remote/write_handler.go


 	switch {
 	case err == nil:
+	case errors.Is(err, storage.ErrNativeHistogramsDisabled):


for completeness, please add test

Added a test covering the OTLP path:

Test name: TestOTLPWriteHandler_NativeHistogramsDisabled

File: storage/remote/write_test.go

It builds an OTLP request with histogram data, injects ErrNativeHistogramsDisabled via the mock appendable, and asserts HTTP 422.

We already had coverage for RW v1 and v2:

TestNativeHistogramsDisabled_V1Message (v1 returns 422)

TestNativeHistogramsDisabled_V2Message (v2 returns 422, with samples/exemplars accepted and headers reflecting 0 histograms)

CC @krajorama

…; add tests (prometheus#17181) Signed-off-by: shibo911 <[email protected]>

… 422 test Signed-off-by: shibo911 <[email protected]>

pipiland2612

LGTM ! Thanks

bwplotka

Thanks!

I'd argue 422 for this case is an overkill, why not 400? Otherwise great!

bwplotka · 2025-11-25T21:54:03Z

storage/remote/write_handler.go

 			switch {
+			case errors.Is(err, storage.ErrNativeHistogramsDisabled):
+				// Receiver supports the protocol, but cannot process native histograms.
+				http.Error(w, err.Error(), http.StatusUnprocessableEntity)


422 is weird, quite rare. You could argue 422 for other cases too (e.g. too old samples, someone didn't configure out of order etc).

Why not invalid request?

Logically, 422 makes more sense. The request is correct but can not be processed.

let's align on 400

bwplotka · 2025-11-25T21:54:53Z

storage/remote/write_handler.go

 		return samplesWithoutMetadata, 0, nil
 	}
 	// TODO(bwplotka): Better concat formatting? Perhaps add size limit?
+	if nhDisabled {


Again, an odd snowflake IMO, just StatusBadRequest would be fine (simpler code)

roidelapluie · 2025-12-01T13:08:30Z

I am fine with both 400 and 422. Semantically 422 is better but we can be consistent and just reply 400.

cstyan · 2025-12-01T18:43:48Z

I am fine with both 400 and 422. Semantically 422 is better but we can be consistent and just reply 400.

I'm fine with either, but this points out that we should probably update the v2 spec to include details about what error code the receiver should return when it wants to reject data like this.

We already have conditions for "receiver may reject", for example if the receiver requires that metadata be present. This is another "may reject" case IMO, and the decision about which code to return in such cases should be documented in the spec.

beorn7 · 2026-02-10T11:43:38Z

Hello from the bug scrub!

@cstyan @bwplotka @roidelapluie has there been any decision so that we can get this PR unblocked?

roidelapluie · 2026-03-24T12:48:56Z

Let's align on 400 cc @shibo911

bwplotka · 2026-03-24T14:26:12Z

We have other cases when something is not supported, so we would need to return 422 for those too. Then if we use 422 and make that a pattern, ideally we change the spec. So... I'd vote for 400 for now.

shibo911 requested review from bwplotka, cstyan and tomwilkie as code owners September 15, 2025 23:04

shibo911 force-pushed the shibo911/rw-17181-422-nh-disabled branch from cc2a9a5 to 64db6ad Compare September 15, 2025 23:06

bboreham requested review from krajorama and roidelapluie and removed request for tomwilkie September 16, 2025 11:09

krajorama reviewed Sep 17, 2025

View reviewed changes

shibo911 force-pushed the shibo911/rw-17181-422-nh-disabled branch from 0b684ac to 47302d5 Compare September 18, 2025 07:12

shibo911 added 2 commits September 18, 2025 12:59

remote_write: return 422 when native histograms are sent but disabled…

fc3003f

…; add tests (prometheus#17181) Signed-off-by: shibo911 <[email protected]>

remote_write: debug log when native histograms are disabled; add OTLP…

4a55466

… 422 test Signed-off-by: shibo911 <[email protected]>

shibo911 force-pushed the shibo911/rw-17181-422-nh-disabled branch from 47302d5 to 4a55466 Compare September 18, 2025 07:30

github-actions bot added the stale label Nov 20, 2025

pipiland2612 approved these changes Nov 25, 2025

View reviewed changes

bwplotka reviewed Nov 25, 2025

View reviewed changes

github-actions bot removed the stale label Nov 25, 2025

github-actions bot added the stale label Feb 9, 2026

beorn7 mentioned this pull request Feb 10, 2026

Remote Write 2.0 returns 500 when Native Histograms are sent but the feature is disabled #17181

Open

github-actions bot removed the stale label Feb 15, 2026

austindyoung mentioned this pull request Feb 25, 2026

[mirror] prometheus/prometheus#17196 remote_write: return 422 when native histograms are sent but disabled; add tests austindyoung/prometheus-driftfence-mirror-fork#84

Open

	h.logger.Error("Out of order histogram from remote write", "err", err.Error(), "series", ls.String(), "timestamp", hp.Timestamp)
	h.logger.Error("Native histograms not enabled", "err", err.Error(), "series", ls.String(), "timestamp", hp.Timestamp)

Conversation

shibo911 commented Sep 15, 2025

Which issue(s) does the PR fix:

Does this PR introduce a user-facing change?

Uh oh!

shibo911 commented Sep 15, 2025

Uh oh!

roidelapluie commented Sep 16, 2025

Uh oh!

shibo911 commented Sep 16, 2025

Uh oh!

krajorama Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

krajorama Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

shibo911 Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

krajorama Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

shibo911 Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pipiland2612 left a comment

Choose a reason for hiding this comment

Uh oh!

bwplotka left a comment

Choose a reason for hiding this comment

Uh oh!

bwplotka Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

roidelapluie Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

roidelapluie Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

bwplotka Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roidelapluie commented Dec 1, 2025

Uh oh!

cstyan commented Dec 1, 2025

Uh oh!

beorn7 commented Feb 10, 2026

Uh oh!

roidelapluie commented Mar 24, 2026

Uh oh!

bwplotka commented Mar 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

shibo911 Sep 18, 2025 •

edited

Loading

bwplotka Nov 25, 2025 •

edited

Loading