Skip to content

Activate lazy common labels in collectors - #1261

Open
Mike Keesey (mkeesey) wants to merge 2 commits into
mainfrom
perf/common-label-collectors
Open

Mike Keesey (mkeesey) wants to merge 2 commits into
mainfrom
perf/common-label-collectors

Conversation

@mkeesey

@mkeesey Mike Keesey (mkeesey) commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Activate request-level common-label transformation for Prometheus scraper batches.
  • Activate request-level common-label transformation for OTLP metric batches.
  • Preserve canonical ordering for synthetic OTLP labels and safely return filtered and retained series to pools.
  • Add coverage for request-level labels, filtering, pooled cleanup, full batches, and remainder batches.

Context

This PR builds on the common-label foundations and consumer support to remove per-series label materialization from the collector paths. Effective labels remain available to downstream remote-write, storage, CSV, and OTLP consumers through the existing merged-label APIs.

Testing

I exercised this against my local adx-mon deployment and checked the various metric tables before and after to ensure stability of labels, seriesIds, etc.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The changes appear correct and well-covered by tests, with only a minor typo in an OTLP flush error message noted.

Pull request overview

This PR switches collector-side metric batching to apply label transformation at the request level (via CommonLabels + LabelFilter) rather than per-series materialization, aligning the scraper and OTLP metric writer paths with the “lazy common labels” approach and adding regression coverage.

Changes:

  • Prometheus scraper batches now apply TransformWriteRequestWithCommonLabels during flush, and ensure pooled series are returned/cleared after filtering.
  • OTLP metric batching now sorts synthetic labels before enqueueing and flushes via the common-label transformation path.
  • Added tests to validate common-label propagation, filtering behavior, and pool cleanup for both “full batch” and “remainder” flush scenarios.
File summaries
File Description
collector/scraper.go Moves drop/label transformation to request-level on flush and centralizes batch flushing + pool cleanup.
collector/scraper_test.go Adds coverage ensuring scraper flush uses CommonLabels and drops filtered series while returning pooled objects.
collector/otlp/metrics.go Switches OTLP batching to sort series labels early and flush via request-level common labels.
collector/otlp/metrics_transfer_test.go Adds coverage ensuring OTLP writer flushes with common labels for both full and remainder batches.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread collector/otlp/metrics.go
Transform scraper and OTLP metric requests at batch flush time so
configured and dynamic labels remain shared at the request level instead
of being materialized into every series. This migrates the entire flow
into common labels being stored at the request level, not at the
individual timeseries level to improve performance.

One change to behavior is to the metric export path. Labels are now
filtered _prior_ to any filters used by `keep-metrics-with-label-value`.

Preserve canonical OTLP label ordering after synthetic labels are
appended, and return all retained and filtered time series to their
pools while clearing request-level label state between batches.

Add scraper and OTLP coverage for request-level labels, filtered series
cleanup, full batches, and final batch remainders.
@mkeesey
Mike Keesey (mkeesey) enabled auto-merge (rebase) September 18, 2026 15:53
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.

2 participants