Skip to content

[RW2] Fix: Only update metadata to WAL when metadata-wal-records feature is enabled#17470

Merged
bwplotka merged 3 commits intoprometheus:mainfrom
pipiland2612:feature_check_metadata
Nov 4, 2025
Merged

[RW2] Fix: Only update metadata to WAL when metadata-wal-records feature is enabled#17470
bwplotka merged 3 commits intoprometheus:mainfrom
pipiland2612:feature_check_metadata

Conversation

@pipiland2612
Copy link
Contributor

@pipiland2612 pipiland2612 commented Nov 3, 2025

Which issue(s) does the PR fix:

Does this PR introduce a user-facing change?

[BUGFIX] Remote Write Receive: Only update metadata to WAL when metadata-wal-records feature is enabled.

Signed-off-by: pipiland2612 <nguyen.t.dang.minh@gmail.com>
@pipiland2612 pipiland2612 changed the title [RW2] Fix: Only append metadata to WAL when type-and-unit-labels feature is enabled [RW2] Fix: Only update metadata to WAL when type-and-unit-labels feature is enabled Nov 3, 2025
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

LGTM.

NOTE: This is to match

if sl.appendMetadataToWAL && lastMeta != nil {
logic.

I wonder if OTLP receiving does not have the same bug:

_, err := b.app.UpdateMetadata(ref, ls, meta)
cc @krajorama @dashpole

Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Actually we need a different flag (:

@bwplotka
Copy link
Member

bwplotka commented Nov 4, 2025

Please also update title and description - it's not type and unit flag

@pipiland2612 pipiland2612 changed the title [RW2] Fix: Only update metadata to WAL when type-and-unit-labels feature is enabled [RW2] Fix: Only update metadata to WAL when metadata-wal-records feature is enabled Nov 4, 2025
Signed-off-by: pipiland2612 <nguyen.t.dang.minh@gmail.com>
Signed-off-by: pipiland2612 <nguyen.t.dang.minh@gmail.com>
@pipiland2612
Copy link
Contributor Author

pipiland2612 commented Nov 4, 2025

Hi @bwplotka, thanks for reminding me. I was quite also questioning whether it is type-and-unit-labels or metadata-wal-records when I was first doing this PR, but ended up type-and-unit and was my faults. Anyways, please check 86a669c. I have also updated title and change log. PTAL Thanks!

Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Thanks!

Hi @bwplotka, thanks for reminding me. I was quite also questioning whether it is type-and-unit-labels or metadata-wal-records when I was first doing this PR

So what made you to eventually decide on type and unit? What made you to ignore your intuition here? (:

@bwplotka bwplotka merged commit 30992dd into prometheus:main Nov 4, 2025
29 checks passed
@pipiland2612
Copy link
Contributor Author

Thanks!

Hi @bwplotka, thanks for reminding me. I was quite also questioning whether it is type-and-unit-labels or metadata-wal-records when I was first doing this PR

So what made you to eventually decide on type and unit? What made you to ignore your intuition here? (:

Err, because I see the type-and-unit feature already there so may be it is what we're wanting :-)

@pipiland2612 pipiland2612 deleted the feature_check_metadata branch November 4, 2025 08:18
songy23 pushed a commit to open-telemetry/opentelemetry-collector-contrib that referenced this pull request Dec 12, 2025
## Summary

This PR upgrades the `github.com/prometheus/prometheus` dependency from
v0.307.3 to v0.308.0.

## Changes Made

#### Fixed `RemoteWriteProtoMsg` unrecognized

In prometheus/prometheus v0.308.0, the remote write protocol message
types were moved from `github.com/prometheus/prometheus/config` to
`github.com/prometheus/client_golang/exp/api/remote` as part of
[prometheus/prometheus#17197](prometheus/prometheus#17197).

#### Fixed `CreatedTimestamp` → `StartTimestamp`

In
[prometheus/prometheus#17411](prometheus/prometheus#17411),
the Remote Write 2.0 spec was updated to 2.0-rc.4. The
`CreatedTimestamp` field was renamed to `StartTimestamp` and moved from
`TimeSeries` to individual `Sample` and `Histogram` messages.

#### Fixed not enough arguments in`api_v1.NewAPI` 

The `api_v1.NewAPI` function signature changed due to
[prometheus/prometheus#17470](prometheus/prometheus#17470)
- Added `appendMetadata bool` parameter for remote write metadata
handling. Added the new parameter `appendMetadata = false` -
prometheusreceiver scrapes targets, doesn't receive remote write
requests

## Remaining Issues

### `EnableNativeHistogramsIngestion` removed from `scrape.Options`

There's one change in this upgrade that I'd love to get the community's
input on before finalizing.

In
[prometheus/prometheus#17315](prometheus/prometheus#17315),
Prometheus removed the `EnableNativeHistogramsIngestion` field from
`scrape.Options`. Native histogram ingestion is now controlled through
the Prometheus scrape config option `scrape_native_histograms` rather
than programmatically.

This creates an interesting situation for us. Currently, we have the
feature gate `receiver.prometheusreceiver.EnableNativeHistograms` that
controls whether native histograms are converted to OTel exponential
histograms. But with this Prometheus change, the *ingestion* of native
histograms during scraping is now a separate concern controlled by
Prometheus config.

I see two paths forward here. We could automatically inject
`scrape_native_histograms: true` into the scrape config whenever our
feature gate is enabled, preserving the current behavior where enabling
the feature gate is all users need to do. Alternatively, we could simply
remove the line and require users to configure
`scrape_native_histograms: true` in their Prometheus scrape config
themselves, which is more explicit but adds an extra configuration step.

What do you think? I'm also wondering if this constitutes a breaking
change that should be called out in the changelog. Would appreciate any
thoughts!

---------

Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[RW2] receive: Metadata should not be appended to WAL unless feature flag is used.

2 participants