Repository navigation
Conversation
Contributor
✅ API Diff Results -
|
…P env chainlink-common#2373 made the chip ingress client's gRPC retry policy configurable (WithRetryPolicy), but no caller can reach it yet: beholder builds the node's chip ingress client internally, and LOOP plugins build their own, with no retry field on either config surface. - beholder.Config gains ChipIngressRetryPolicy (*chipingress.RetryPolicy, nil keeps retries off), applied via chipingress.WithRetryPolicy when the chip ingress emitter is enabled. - beholder.ChipIngressRetryConfig is the string-config-friendly form for hosts: Enabled plus optional overrides, where zero fields fall back to chipingress.DefaultRetryPolicy so a config layer that only sets Enabled still gets a valid policy instead of a zero-valued one gRPC would silently discard. - loop.EnvConfig gains ChipIngressRetry* fields carried over the existing CL_CHIP_INGRESS_* env channel (AsCmdEnv/parse), and the LOOP server resolves them into its own beholder client's retry policy. Retries stay opt-in everywhere; this only adds the pass-through. Wiring into the chainlink node's [Telemetry] TOML config follows in the chainlink repo once this lands. Jira: INFOPLAT-19422
pkcll
force-pushed
the
incident-2673-pmai-chipingress-retry-config
branch
from
October 6, 2026 03:33
91c6604 to
84edf08
Compare
pkcll
marked this pull request as ready for review
October 6, 2026 03:44
product-security-plaid-production
Bot
requested review from
4of9,
bolekk,
engnke,
fouadkada,
kirqz23,
skippaDaBitFlippa and
thomaska
October 6, 2026 03:44
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
A configured maximum of one attempt silently disables retries, and two comments reference a nonexistent configuration type.
Review effort: Balanced
Findings: 1
Open (3)
What changed in this PR
Adds opt-in chip ingress retry configuration to beholder and LOOP clients.
Changes:
- Adds retry-policy configuration and default resolution.
- Passes retry settings through LOOP environment variables.
- Adds configuration and round-trip tests.
| File | Description |
|---|---|
pkg/beholder/chip_ingress_retry.go |
Resolves retry configuration. |
pkg/beholder/chip_ingress_retry_test.go |
Tests policy resolution. |
pkg/beholder/client.go |
Applies retry policies. |
pkg/beholder/client_test.go |
Tests client construction. |
pkg/beholder/config.go |
Exposes the retry policy. |
pkg/beholder/testdata/config-example.json |
Updates configuration fixture. |
pkg/loop/config.go |
Adds retry environment settings. |
pkg/loop/config_test.go |
Tests environment round trips. |
pkg/loop/server.go |
Wires settings into beholder. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+43
to
+45
| if c.MaxAttempts > 0 { | ||
| p.MaxAttempts = c.MaxAttempts | ||
| } |
| ChipIngressInsecureConnection bool // Disables TLS for Chip Ingress Emitter | ||
| // ChipIngressRetryPolicy enables gRPC-level retries on the chip ingress client when | ||
| // non-nil (see chipingress.WithRetryPolicy). Nil (the default) keeps retries off. | ||
| // Build it from string-based config with chipingress.RetryPolicyConfig. |
Comment on lines
+143
to
+146
| // ChipIngressRetry* configures gRPC-level retries on the LOOP's own beholder | ||
| // chip ingress client. The host sends fully resolved values; the server resolves | ||
| // them into a policy via chipingress.RetryPolicyConfig, where zero fields fall | ||
| // back to chipingress.DefaultRetryPolicy and Enabled=false keeps retries off. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
#2373 made the chip ingress client's gRPC retry policy configurable (
WithRetryPolicy), but no caller can reach it yet: beholder builds the node's chip ingress client internally, and LOOP plugins build their own, with no retry field on either config surface. This adds the pass-through only — retries stay opt-in everywhere.beholder.ConfiggainsChipIngressRetryPolicy(*chipingress.RetryPolicy, nil keeps retries off), applied viachipingress.WithRetryPolicywhen the chip ingress emitter is enabled.beholder.ChipIngressRetryConfigis the string-config-friendly form for hosts:Enabledplus optional overrides, where zero fields fall back tochipingress.DefaultRetryPolicyso a config layer that only setsEnabledstill gets a valid policy instead of a zero-valued one gRPC would silently discard.loop.EnvConfiggainsChipIngressRetry*fields carried over the existingCL_CHIP_INGRESS_*env channel (AsCmdEnv/parse), and the LOOP server resolves them into its own beholder client's retry policy.Kept in the root module on purpose: the pass-through only uses API already shipped by #2373 (
WithRetryPolicy,DefaultRetryPolicy,ParseStatusCodes), so nopkg/chipingressnested-module pin bump is needed.Wiring into the chainlink node's
[Telemetry]TOML config follows in the chainlink repo once this lands.Test plan
pkg/beholderfull suite green (incl. newchip_ingress_retry_test.goand a retry-policy construction subtest inTestNewGRPCClient_ChipIngressEmitter)pkg/loopfull suite green (env round-trip incl. policy resolution, parse error case forCL_CHIP_INGRESS_RETRY_BACKOFF_MULTIPLIER)go build ./...,go veton touched packages,gofmtcleanJira: INFOPLAT-19422