Skip to content

fix: decouple Beholder observation metrics from Prometheus scrape cycle#21720

Open
emate wants to merge 2 commits intodevelopfrom
emate/fix-ccip-ocr-to-beholder-metrics
Open

fix: decouple Beholder observation metrics from Prometheus scrape cycle#21720
emate wants to merge 2 commits intodevelopfrom
emate/fix-ccip-ocr-to-beholder-metrics

Conversation

@emate
Copy link
Contributor

@emate emate commented Mar 26, 2026

ocr3_sent_observations_total (and ocr3_included_observations_total) were only forwarded to Beholder when Prometheus scraped the /metrics endpoint. The wrappedCounter.Collect() method — the only place deltas are computed and PublishMetric is called — is driven entirely by the Prometheus scrape cycle.

On nodes where scrapes were slow, missing, or misaligned, the Beholder counter would appear stuck even though the underlying OCR3 protocol was functioning normally.

Added a background polling loop to ObservationMetricsCollector that ticks every 10 seconds (matching the OTel PeriodicReader default) and calls poll(), which invokes Collect() on each wrapped counter directly

@github-actions
Copy link
Contributor

👋 emate, thanks for creating this pull request!

To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team.

Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks!

mateusz-sekara
mateusz-sekara previously approved these changes Mar 26, 2026
@github-actions
Copy link
Contributor

I see you updated files related to core. Please run make gocs in the root directory to add a changeset as well as in the text include at least one of the following tags:

  • #added For any new functionality added.
  • #breaking_change For any functionality that requires manual action for the node to boot.
  • #bugfix For bug fixes.
  • #changed For any change to the existing functionality.
  • #db_update For any feature that introduces updates to database schema.
  • #deprecation_notice For any upcoming deprecation functionality.
  • #internal For changesets that need to be excluded from the final changelog.
  • #nops For any feature that is NOP facing and needs to be in the official Release Notes for the release.
  • #removed For any functionality/config that is removed.
  • #updated For any functionality that is updated.
  • #wip For any change that is not ready yet and external communication about it should be held off till it is feature complete.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 26, 2026

✅ No conflicts with other open PRs targeting develop

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Risk Rating: HIGH (adds a long-lived background goroutine that triggers metric collection concurrently with Prometheus scrapes; incorrect deltas or panics would directly impact telemetry reliability and could crash the process)

Decouples OCR3 observation metric forwarding to Beholder from the Prometheus scrape cycle by adding a periodic polling loop that calls Collect() on the wrapped counters on a fixed interval.

Changes:

  • Start a background polling loop (default 10s) after libocr3.NewOracle(...) so wrapped counters publish deltas even without /metrics scrapes.
  • Add Start(interval) + poll() to ObservationMetricsCollector, with cancellation via Close().
  • Add a unit test verifying polling publishes deltas without an external Prometheus scrape.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
core/capabilities/ccip/oraclecreator/plugin.go Starts metrics collector polling after oracle creation and ensures it’s closed with the oracle.
core/capabilities/ccip/oraclecreator/observation_metrics_collector.go Adds polling interval, lifecycle context/cancel, and the polling goroutine to drive delta publishing independently of scrapes.
core/capabilities/ccip/oraclecreator/observation_metrics_collector_test.go Adds a test asserting polling publishes and does not republish without new increments.

Scrupulous human review recommended (high-impact areas):

  • ObservationMetricsCollector.Start(...) / poll() interaction with Prometheus scrapes: this introduces concurrent Collect() calls and can cause incorrect delta publishing unless the wrapped-counter delta tracking is made concurrency-safe.
  • Lifecycle correctness: ensuring Start() cannot panic (invalid interval) and that Close() reliably stops any background goroutines in all runtime paths.

Reviewer recommendations (based on CODEOWNERS for /core/capabilities/ccip):

  • @smartcontractkit/ccip-offchain (primary owners for this directory)
  • @smartcontractkit/keystone and/or @smartcontractkit/capabilities-team (owners for /core/capabilities/ broadly)

@trunk-io
Copy link

trunk-io bot commented Mar 26, 2026

Static BadgeStatic BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

@cl-sonarqube-production
Copy link

@emate emate requested a review from mateusz-sekara March 26, 2026 15:30
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.

3 participants