Append Created Timestamps #12733

ArthurSens · 2023-08-21T21:03:10Z

Part of #6541

Implements ingestion of Created Timestamps(prometheus/proposals#29) in the head_chunk. Metadata ingestion will be handled separately.

bwplotka

Nice, good start!

scrape/manager_test.go

TheSpiritXIII

Great start!

scrape/manager_test.go

scrape/helpers_test.go

scrape/manager_test.go

tsdb/head_append.go

bboreham · 2023-10-20T09:55:08Z

Part of #6541

bwplotka · 2023-10-23T14:42:10Z

Nice! We discussed in our meeting to add feature flag and perhaps split this per metric types (:

ArthurSens · 2023-10-23T22:56:28Z

Nice! We discussed in our meeting to add feature flag and perhaps split this per metric types (:

Feature flag added! I've also run manual tests(example app) and verified that created timestamps are ingested for counters, summaries, and histograms. I don't think we need to split this into one PR for each metric type.

Removing draft mode since we should be close to something mergeable, just need to figure out what tests we still lack (feedback/suggestions on what tests to add are welcome)

bwplotka

Epic work! Some quick comments, we can talk on our 1:1 later more 💪🏽

Close to be LGTM!

config/config.go

scrape/manager_test.go

storage/interface.go

tsdb/head_append.go

bwplotka · 2023-12-06T09:42:44Z

Sorry for lag, looking on this now

bwplotka · 2023-12-06T11:14:52Z

I plan to submit small PR to this PR with the following high-lvl changes:

I really think we should rename feature flag to created-timestamp-zero-ingestion and treat future metadata as a separate feature flag. Derisks everything.
Test cleanup
Mentioning feature flag in our new interface

.. going to gym now though 🙈

ArthurSens · 2023-12-06T12:10:34Z

I really think we should rename feature flag to created-timestamp-zero-ingestion and treat future metadata as a separate feature flag. Derisks everything.

I understand the reliability concerns, but sometimes I few like we add too many feature flags for very similar things 😅

Should I send a PR updating the proposal?

Test cleanup

Awesome, thank you ❤️

Mentioning feature flag in our new interface

What do you mean here? What interface?

.. going to gym now though 🙈

💪 we should train together next time we see each other 😛

bwplotka · 2023-12-06T13:09:43Z

Should I send a PR updating the proposal?

No, it's a small tech detail at the end (the name and separation of feature flags).

What do you mean here? What interface?

I mean CreatedTimestampAppender. I think I miss the flexibility here. It's up to implementation what will be done (zero injection or metadata or both at some point). Tiny nit.

bwplotka · 2023-12-07T10:35:30Z

PR @ArthurSens: ArthurSens#1

I challenged some decisions e.g. where to put validation of CT. I would keep scrape loop as simple as possible. We also need to test appender properly (so TSDB).

bwplotka

Blocking as some work is still neded.

scrape/scrape.go

tsdb/agent/db.go

Signed-off-by: Arthur Silva Sens <arthur.sens@coralogix.com>

Changes: * Changed textparse Parser interface for consistency and robustness. * Changed CT interface to be more explicit and handle validation. * Simplified test, change scrapeManager to allow testability. * Added TODOs. Signed-off-by: bwplotka <bwplotka@gmail.com>

Signed-off-by: bwplotka <bwplotka@gmail.com>

Signed-off-by: Arthur Silva Sens <arthur.sens@coralogix.com>

bwplotka

Epic, one nit, otherwise LGTM (:

tsdb/head_test.go

Signed-off-by: Arthur Silva Sens <arthur.sens@coralogix.com>

bwplotka

We need to update documentation to mention "created-timestamp-zero-ingestion", but we can do that in the next PR.

Thanks @ArthurSens and reviewers 💪🏽

* Append created timestamps. Signed-off-by: Arthur Silva Sens <arthur.sens@coralogix.com> * Log when created timestamps are ignored Signed-off-by: Arthur Silva Sens <arthur.sens@coralogix.com> * Proposed changes to Append CT PR. Changes: * Changed textparse Parser interface for consistency and robustness. * Changed CT interface to be more explicit and handle validation. * Simplified test, change scrapeManager to allow testability. * Added TODOs. Signed-off-by: bwplotka <bwplotka@gmail.com> * Updates. Signed-off-by: bwplotka <bwplotka@gmail.com> * Addressed comments. Signed-off-by: bwplotka <bwplotka@gmail.com> * Refactor head_appender test Signed-off-by: Arthur Silva Sens <arthur.sens@coralogix.com> * Fix linter issues Signed-off-by: Arthur Silva Sens <arthur.sens@coralogix.com> * Use model.Sample in head appender test Signed-off-by: Arthur Silva Sens <arthur.sens@coralogix.com> --------- Signed-off-by: Arthur Silva Sens <arthur.sens@coralogix.com> Signed-off-by: bwplotka <bwplotka@gmail.com> Co-authored-by: bwplotka <bwplotka@gmail.com>

bwplotka reviewed Aug 22, 2023

View reviewed changes

scrape/manager_test.go Outdated Show resolved Hide resolved

scrape/manager_test.go Outdated Show resolved Hide resolved

scrape/manager_test.go Outdated Show resolved Hide resolved

TheSpiritXIII suggested changes Aug 22, 2023

View reviewed changes

scrape/manager_test.go Show resolved Hide resolved

scrape/manager_test.go Outdated Show resolved Hide resolved

scrape/manager_test.go Outdated Show resolved Hide resolved

ArthurSens force-pushed the scrape-synthetic-zero branch 3 times, most recently from b89912e to 2b10fc9 Compare September 1, 2023 21:45

TheSpiritXIII reviewed Sep 5, 2023

View reviewed changes

scrape/helpers_test.go Outdated Show resolved Hide resolved

scrape/manager_test.go Outdated Show resolved Hide resolved

ArthurSens force-pushed the scrape-synthetic-zero branch from 2f4ea3d to 67045d2 Compare September 8, 2023 19:16

ArthurSens force-pushed the scrape-synthetic-zero branch from 67045d2 to 35ee022 Compare September 15, 2023 21:22

ArthurSens force-pushed the scrape-synthetic-zero branch from 35ee022 to 4699008 Compare October 4, 2023 20:10

ArthurSens commented Oct 4, 2023

View reviewed changes

tsdb/head_append.go Outdated Show resolved Hide resolved

ArthurSens changed the title ~~Test scraping target with created timestamp~~ Parse and ingest Created Timestamps Oct 4, 2023

ArthurSens mentioned this pull request Oct 4, 2023

Add created timestamps to prompb #12936

Merged

ArthurSens force-pushed the scrape-synthetic-zero branch from 4699008 to c37030d Compare October 6, 2023 16:42

ArthurSens mentioned this pull request Oct 12, 2023

Parse created timestamps from Prometheus Protobuf #12973

Merged

ArthurSens force-pushed the scrape-synthetic-zero branch 3 times, most recently from a7b7165 to e0d224c Compare October 20, 2023 18:32

ArthurSens changed the title ~~Parse and ingest Created Timestamps~~ Append Created Timestamps Oct 20, 2023

ArthurSens force-pushed the scrape-synthetic-zero branch from e0d224c to cc111c9 Compare October 20, 2023 18:41

ArthurSens force-pushed the scrape-synthetic-zero branch 2 times, most recently from 7dc6a95 to 81217ad Compare October 23, 2023 22:41

ArthurSens marked this pull request as ready for review October 23, 2023 22:48

ArthurSens requested review from jesusvazquez, csmarchbanks, cstyan and tomwilkie as code owners October 23, 2023 22:48

ArthurSens force-pushed the scrape-synthetic-zero branch from 81217ad to 86127f7 Compare October 24, 2023 17:01

bwplotka reviewed Nov 28, 2023

View reviewed changes

ArthurSens force-pushed the scrape-synthetic-zero branch 3 times, most recently from 897e963 to 5cefd91 Compare November 29, 2023 15:57

SuperQ requested a review from bwplotka November 30, 2023 14:22

bwplotka mentioned this pull request Dec 6, 2023

Handle OpenMetrics ..._created lines #6541

Open

bwplotka requested changes Dec 7, 2023

View reviewed changes

scrape/scrape.go Outdated Show resolved Hide resolved

tsdb/agent/db.go Outdated Show resolved Hide resolved

Arthur Silva Sens and others added 6 commits December 8, 2023 16:51

Append created timestamps.

453706a

Signed-off-by: Arthur Silva Sens <arthur.sens@coralogix.com>

Log when created timestamps are ignored

eeab031

Signed-off-by: Arthur Silva Sens <arthur.sens@coralogix.com>

Updates.

a683131

Signed-off-by: bwplotka <bwplotka@gmail.com>

Addressed comments.

f558dd3

Signed-off-by: bwplotka <bwplotka@gmail.com>

Refactor head_appender test

a1aabd0

Signed-off-by: Arthur Silva Sens <arthur.sens@coralogix.com>

ArthurSens force-pushed the scrape-synthetic-zero branch from 385c860 to a1aabd0 Compare December 8, 2023 20:29

Fix linter issues

668f0e2

Signed-off-by: Arthur Silva Sens <arthur.sens@coralogix.com>

bwplotka approved these changes Dec 9, 2023

View reviewed changes

tsdb/head_test.go Outdated Show resolved Hide resolved

tsdb/head_test.go Show resolved Hide resolved

Use model.Sample in head appender test

b473436

Signed-off-by: Arthur Silva Sens <arthur.sens@coralogix.com>

bwplotka approved these changes Dec 11, 2023

View reviewed changes

bwplotka merged commit 5082655 into prometheus:main Dec 11, 2023

ArthurSens deleted the scrape-synthetic-zero branch December 11, 2023 11:54

ArthurSens mentioned this pull request Dec 11, 2023

Document feature flag 'created-timestamp-zero-ingestion' #13279

Merged

chenrui333 mentioned this pull request Feb 22, 2024

prometheus 2.50.0 Homebrew/homebrew-core#163774

Merged

ArthurSens mentioned this pull request Apr 1, 2024

feat: Implement Created Timestamp ingestion for agent mode #13865

Closed

machine424 mentioned this pull request Apr 15, 2024

Feature Flag "Created Timestamps Zero Injection" requires enabled "Native Histograms" #13920

Closed

Append Created Timestamps #12733

Append Created Timestamps #12733

Uh oh!

Conversation

ArthurSens commented Aug 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bwplotka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TheSpiritXIII left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bboreham commented Oct 20, 2023

Uh oh!

bwplotka commented Oct 23, 2023

Uh oh!

ArthurSens commented Oct 23, 2023

Uh oh!

bwplotka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bwplotka commented Dec 6, 2023

Uh oh!

bwplotka commented Dec 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ArthurSens commented Dec 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bwplotka commented Dec 6, 2023

Uh oh!

bwplotka commented Dec 7, 2023

Uh oh!

bwplotka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bwplotka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bwplotka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ArthurSens commented Aug 21, 2023 •

edited

Loading

bwplotka commented Dec 6, 2023 •

edited

Loading

ArthurSens commented Dec 6, 2023 •

edited

Loading