Skip to content

Commit

Permalink
Merge pull request #13987 from prometheus/nativeHis-flag-ingestion
Browse files Browse the repository at this point in the history 
bugfix: Decouple native histogram ingestions and protobuf parsing
  • Loading branch information
@ArthurSens
ArthurSens committed Apr 25, 2024
2 parents dde2e5e + 7aacef9 commit 0305490
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 38 deletions.
1 change: 1 addition & 0 deletions cmd/prometheus/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ func (c *flagConfig) setFeatureListOptions(logger log.Logger) error {
level.Info(logger).Log("msg", "Experimental PromQL functions enabled.")
case "native-histograms":
c.tsdb.EnableNativeHistograms = true
c.scrape.EnableNativeHistogramsIngestion = true
// Change relevant global variables. Hacky, but it's hard to pass a new option or default to unmarshallers.
config.DefaultConfig.GlobalConfig.ScrapeProtocols = config.DefaultProtoFirstScrapeProtocols
config.DefaultGlobalConfig.ScrapeProtocols = config.DefaultProtoFirstScrapeProtocols
Expand Down
2 changes: 2 additions & 0 deletions scrape/manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ type Options struct {
// Option to enable the ingestion of the created timestamp as a synthetic zero sample.
// See: https://github.com/prometheus/proposals/blob/main/proposals/2023-06-13_created-timestamp.md
EnableCreatedTimestampZeroIngestion bool
// Option to enable the ingestion of native histograms.
EnableNativeHistogramsIngestion bool

// Optional HTTP client options to use when scraping.
HTTPClientOptions []config_util.HTTPClientOption
Expand Down
64 changes: 35 additions & 29 deletions scrape/scrape.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ func newScrapePool(cfg *config.ScrapeConfig, app storage.Appendable, offsetSeed
opts.interval,
opts.timeout,
opts.scrapeClassicHistograms,
options.EnableNativeHistogramsIngestion,
options.EnableCreatedTimestampZeroIngestion,
options.ExtraMetrics,
options.EnableMetadataStorage,
Expand Down Expand Up @@ -827,7 +828,10 @@ type scrapeLoop struct {
interval time.Duration
timeout time.Duration
scrapeClassicHistograms bool
enableCTZeroIngestion bool

// Feature flagged options.
enableNativeHistogramIngestion bool
enableCTZeroIngestion bool

appender func(ctx context.Context) storage.Appender
symbolTable *labels.SymbolTable
Expand Down Expand Up @@ -1123,6 +1127,7 @@ func newScrapeLoop(ctx context.Context,
interval time.Duration,
timeout time.Duration,
scrapeClassicHistograms bool,
enableNativeHistogramIngestion bool,
enableCTZeroIngestion bool,
reportExtraMetrics bool,
appendMetadataToWAL bool,
Expand Down Expand Up @@ -1153,33 +1158,34 @@ func newScrapeLoop(ctx context.Context,
}

sl := &scrapeLoop{
scraper: sc,
buffers: buffers,
cache: cache,
appender: appender,
symbolTable: symbolTable,
sampleMutator: sampleMutator,
reportSampleMutator: reportSampleMutator,
stopped: make(chan struct{}),
offsetSeed: offsetSeed,
l: l,
parentCtx: ctx,
appenderCtx: appenderCtx,
honorTimestamps: honorTimestamps,
trackTimestampsStaleness: trackTimestampsStaleness,
enableCompression: enableCompression,
sampleLimit: sampleLimit,
bucketLimit: bucketLimit,
maxSchema: maxSchema,
labelLimits: labelLimits,
interval: interval,
timeout: timeout,
scrapeClassicHistograms: scrapeClassicHistograms,
enableCTZeroIngestion: enableCTZeroIngestion,
reportExtraMetrics: reportExtraMetrics,
appendMetadataToWAL: appendMetadataToWAL,
metrics: metrics,
skipOffsetting: skipOffsetting,
scraper: sc,
buffers: buffers,
cache: cache,
appender: appender,
symbolTable: symbolTable,
sampleMutator: sampleMutator,
reportSampleMutator: reportSampleMutator,
stopped: make(chan struct{}),
offsetSeed: offsetSeed,
l: l,
parentCtx: ctx,
appenderCtx: appenderCtx,
honorTimestamps: honorTimestamps,
trackTimestampsStaleness: trackTimestampsStaleness,
enableCompression: enableCompression,
sampleLimit: sampleLimit,
bucketLimit: bucketLimit,
maxSchema: maxSchema,
labelLimits: labelLimits,
interval: interval,
timeout: timeout,
scrapeClassicHistograms: scrapeClassicHistograms,
enableNativeHistogramIngestion: enableNativeHistogramIngestion,
enableCTZeroIngestion: enableCTZeroIngestion,
reportExtraMetrics: reportExtraMetrics,
appendMetadataToWAL: appendMetadataToWAL,
metrics: metrics,
skipOffsetting: skipOffsetting,
}
sl.ctx, sl.cancel = context.WithCancel(ctx)

Expand Down Expand Up @@ -1627,7 +1633,7 @@ loop:
}
}

if isHistogram {
if isHistogram && sl.enableNativeHistogramIngestion {
if h != nil {
ref, err = app.AppendHistogram(ref, lset, t, h, nil)
} else {
Expand Down
28 changes: 19 additions & 9 deletions scrape/scrape_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,7 @@ func newBasicScrapeLoop(t testing.TB, ctx context.Context, scraper scraper, app
false,
false,
false,
false,
nil,
false,
newTestScrapeMetrics(t),
Expand Down Expand Up @@ -819,6 +820,7 @@ func TestScrapeLoopRun(t *testing.T) {
false,
false,
false,
false,
nil,
false,
scrapeMetrics,
Expand Down Expand Up @@ -962,6 +964,7 @@ func TestScrapeLoopMetadata(t *testing.T) {
false,
false,
false,
false,
nil,
false,
scrapeMetrics,
Expand Down Expand Up @@ -1571,6 +1574,7 @@ func TestScrapeLoop_HistogramBucketLimit(t *testing.T) {
app := &bucketLimitAppender{Appender: resApp, limit: 2}

sl := newBasicScrapeLoop(t, context.Background(), nil, func(ctx context.Context) storage.Appender { return app }, 0)
sl.enableNativeHistogramIngestion = true
sl.sampleMutator = func(l labels.Labels) labels.Labels {
if l.Has("deleteme") {
return labels.EmptyLabels()
Expand Down Expand Up @@ -1797,14 +1801,15 @@ func TestScrapeLoopAppendStalenessIfTrackTimestampStaleness(t *testing.T) {

func TestScrapeLoopAppendExemplar(t *testing.T) {
tests := []struct {
title string
scrapeClassicHistograms bool
scrapeText string
contentType string
discoveryLabels []string
floats []floatSample
histograms []histogramSample
exemplars []exemplar.Exemplar
title string
scrapeClassicHistograms bool
enableNativeHistogramsIngestion bool
scrapeText string
contentType string
discoveryLabels []string
floats []floatSample
histograms []histogramSample
exemplars []exemplar.Exemplar
}{
{
title: "Metric without exemplars",
Expand Down Expand Up @@ -1862,6 +1867,8 @@ metric_total{n="2"} 2 # {t="2"} 2.0 20000
},
{
title: "Native histogram with three exemplars",

enableNativeHistogramsIngestion: true,
scrapeText: `name: "test_histogram"
help: "Test histogram with many buckets removed to keep it manageable in size."
type: HISTOGRAM
Expand Down Expand Up @@ -1976,6 +1983,8 @@ metric: <
},
{
title: "Native histogram with three exemplars scraped as classic histogram",

enableNativeHistogramsIngestion: true,
scrapeText: `name: "test_histogram"
help: "Test histogram with many buckets removed to keep it manageable in size."
type: HISTOGRAM
Expand Down Expand Up @@ -2115,6 +2124,7 @@ metric: <
}

sl := newBasicScrapeLoop(t, context.Background(), nil, func(ctx context.Context) storage.Appender { return app }, 0)
sl.enableNativeHistogramIngestion = test.enableNativeHistogramsIngestion
sl.sampleMutator = func(l labels.Labels) labels.Labels {
return mutateSampleLabels(l, discoveryLabels, false, nil)
}
Expand Down Expand Up @@ -3710,7 +3720,7 @@ scrape_configs:
s.DB.EnableNativeHistograms()
reg := prometheus.NewRegistry()

mng, err := NewManager(nil, nil, s, reg)
mng, err := NewManager(&Options{EnableNativeHistogramsIngestion: true}, nil, s, reg)
require.NoError(t, err)
cfg, err := config.Load(configStr, false, log.NewNopLogger())
require.NoError(t, err)
Expand Down

0 comments on commit 0305490

Please sign in to comment.