Add monitoring.metrics-return-all-points flag

[nir-sagi]

What

Adds a --monitoring.metrics-return-all-points flag (default false). When enabled, the collector reports every data point in the requested interval, each stamped with its own end time, instead of only the most recent one. Default false → existing behavior is byte-for-byte unchanged.

Why

The collector keeps only the newest point per series in the request window. For a large window (e.g. a 60m interval), all intermediate points are discarded. This flag lets a caller backfill the full window.

Changes

• stackdriver_exporter.go — new flag, wired into MonitoringCollectorOptions.
• collectors/monitoring_collector.go — new ReturnAllPoints option; reportTimeSeriesMetrics loops over all points (each with its own parsed end time) when enabled, else the single newest point as before.
• collectors/deduplicator.go — in all-points mode a series' points share labels and differ only by timestamp, so the deduplicator mixes the point end time into its signature (gated on the flag) to avoid collapsing them into one. Default path untouched.
• Tests — added TestMetricDeduplicator_IncludeTimestamp; fixed the benchmark's direct hashLabels call.

Not a breaking change

The timestamp is not a new mechanism — the exporter already stamps each emitted metric with its point's end time via NewMetricWithTimestamp.

Note on DELTA + AggregateDeltas

For DELTA metrics with AggregateDeltas, points feed the delta counter store rather than being emitted directly, and the store only accepts strictly-newer samples. In all-points mode this is at best a no-op (GCP returns points newest-first) and can double-count across overlapping windows. The flag is intended for GAUGE/DISTRIBUTION snapshot metrics; deltas should stay on newest-only or be verified before enabling.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added an option to return all monitoring data points from a time window instead of only the latest point.
  • Added a new command-line flag to enable or disable this behavior.
  • Improved duplicate handling so points with the same series can be distinguished by timestamp when needed.

• Bug Fixes

  • Fixed point processing so timestamp-based reporting works consistently, including for distribution and unknown metric types.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 10 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 39da543d-b4dd-491d-ad95-38025878c72d

📥 Commits

Reviewing files that changed from the base of the PR and between f0e003f and 55d0d83.

📒 Files selected for processing (1)

• collectors/monitoring_collector.go

Walkthrough

This PR adds optional timestamp-based deduplication to MetricDeduplicator and introduces a ReturnAllPoints mode in MonitoringCollector that reports every point in an interval instead of only the newest, exposed via a new CLI flag.

Changes

Return-all-points feature

Layer / File(s)	Summary
Deduplicator timestamp signature support collectors/deduplicator.go, collectors/deduplicator_benchmark_test.go, collectors/deduplicator_test.go	MetricDeduplicator adds includeTimestamp; hashLabels mixes timestamp into signature when enabled; CheckAndMark/RevertMark pass timestamps; benchmark and new unit test updated.
Monitoring collector return-all-points mode collectors/monitoring_collector.go	MonitoringCollectorOptions/MonitoringCollector add ReturnAllPoints/returnAllPoints; constructor configures dedup for timestamps; reportTimeSeriesMetrics iterates all points per interval, dedup/revert keyed by per-point end time.
CLI flag wiring stackdriver_exporter.go	New --monitoring.metrics-return-all-points flag wired into NewMonitoringCollector options.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Exporter as stackdriver_exporter
  participant Collector as MonitoringCollector
  participant Dedup as MetricDeduplicator
  participant Prom as Prometheus Metrics

  Exporter->>Collector: NewMonitoringCollector(ReturnAllPoints)
  Collector->>Dedup: set includeTimestamp=true
  Collector->>Collector: reportTimeSeriesMetrics(timeSeries)
  loop for each point (all or newest)
    Collector->>Dedup: CheckAndMark(signature, pointEndTime)
    alt not duplicate
      Collector->>Prom: emit metric value
    else duplicate or error
      Collector->>Dedup: RevertMark(signature, pointEndTime)
    end
  end

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: adding the monitoring.metrics-return-all-points flag.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch nirsagi/be-2409-stackdriver-add-bool-and-return-all-data-points-also-change

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@collectors/monitoring_collector.go`:
- Around line 568-584: Sort the return-all-points path in
monitoring_collector.go before iterating: when c.returnAllPoints is true,
pointsToReport should be ordered by Interval.EndTime ascending so older samples
are processed first. Update the logic around timeSeries.Points and the loop in
the collection path to sort pointsToReport by parsed end time before computing
pointEndTime and collecting metrics, preserving the existing nil checks and
parsing behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b2200464-2ddb-4296-a4d7-096b4a403a04

📥 Commits

Reviewing files that changed from the base of the PR and between 6fac74e and f0e003f.

📒 Files selected for processing (5)

• collectors/deduplicator.go
• collectors/deduplicator_benchmark_test.go
• collectors/deduplicator_test.go
• collectors/monitoring_collector.go
• stackdriver_exporter.go

[yuvco]

oops