Add WithClientName#2246
Conversation
|
👋 thomaska, 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! |
✅ API Diff Results -
|
There was a problem hiding this comment.
Pull request overview
This PR adds an optional chip_client metric label to the Chip Ingress batch client, enabling downstream systems to attribute batch client metrics to a specific producer/client identity (e.g., beholder, durable_emitter).
Changes:
- Introduces
WithChipClient(name string)to set an optionalchip_clientlabel on all batch client metrics. - Propagates
chip_clientinto batch client metric attribute sets (status counters, config gauge, split/mismatch counters, and size/latency histograms). - Adds tests asserting
chip_clientis present when configured and omitted otherwise, plus introduces well-knownchip_clientconstant values.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/chipingress/batch/client.go | Adds chipClient option plumbing and ensures all batch metrics include/omit the chip_client attribute consistently. |
| pkg/chipingress/batch/client_test.go | Adds coverage verifying chip_client labeling behavior across all batch metrics. |
| pkg/chipingress/batch/chip_client.go | Defines well-known chip_client label values for consistent usage across callers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var chipClientAttr otelmetric.MeasurementOption | ||
| if chipClient != "" { | ||
| chipClientAttr = otelmetric.WithAttributeSet(attribute.NewSet( | ||
| attribute.String("chip_client", chipClient), |
There was a problem hiding this comment.
| attribute.String("chip_client", chipClient), | |
| attribute.String("client_name", chipClient), |
| // Well-known chip_client metric label values for batch client metrics. | ||
| // Pass to WithChipClient when wiring batch.NewBatchClient. | ||
| const ( | ||
| ChipClientBeholder = "beholder" |
There was a problem hiding this comment.
Move these to pkg/chipingress/batch/client.go please
| } | ||
|
|
||
| // WithChipClient sets chip_client on batch client metrics. Omitted when unset. | ||
| func WithChipClient(name string) Opt { |
There was a problem hiding this comment.
| func WithChipClient(name string) Opt { | |
| func WithClientName(name string) Opt { |
| // WithChipClient sets chip_client on batch client metrics. Omitted when unset. | ||
| func WithChipClient(name string) Opt { | ||
| return func(c *Client) { | ||
| c.chipClient = name |
There was a problem hiding this comment.
| c.chipClient = name | |
| c.clientName = name |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.
| Benchmark suite | Current: d11931d | Previous: c9ce07a | Ratio |
|---|---|---|---|
BenchmarkKeystore_Sign/nop/out-of-process |
94030 ns/op |
43993 ns/op |
2.14 |
This comment was automatically generated by workflow using github-action-benchmark.
https://smartcontract-it.atlassian.net/browse/INFOPLAT-14703
Add chip-client in chip ingress
Supports
#2228