-
Notifications
You must be signed in to change notification settings - Fork 209
Introducing ReportStat service to pipedservice #2181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a1cf903
3c2a96e
67f6614
0eb829f
2bb3bcb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,7 +34,14 @@ import "pkg/model/event.proto"; | |
| service PipedService { | ||
| // Ping is periodically sent to report its realtime status/stats to control-plane. | ||
| // The received stats will be pushed to the metrics collector. | ||
| rpc Ping(PingRequest) returns (PingResponse) {} | ||
| // Note: This rpc is deprecated, use ReportStat instead. | ||
| rpc Ping(PingRequest) returns (PingResponse) { | ||
| option deprecated = true; | ||
| } | ||
|
|
||
| // ReportStat is periodically sent to report its realtime status/stats to control-plane. | ||
| // The received stats will be pushed to the metrics collector. | ||
| rpc ReportStat(ReportStatRequest) returns (ReportStatResponse) {} | ||
|
|
||
| // ReportPipedMeta is sent while starting up to report its metadata | ||
| // such as configured cloud providers. | ||
|
|
@@ -149,11 +156,20 @@ enum ListOrder { | |
| } | ||
|
|
||
| message PingRequest { | ||
| pipe.model.PipedStats piped_stats = 1 [(validate.rules).message.required = true]; | ||
| pipe.model.PipedStats piped_stats = 1 [(validate.rules).message.required = true, deprecated = true]; | ||
| } | ||
|
|
||
| message PingResponse { | ||
| int64 ping_interval = 1; | ||
| int64 ping_interval = 1 [deprecated = true]; | ||
| } | ||
|
|
||
| message ReportStatRequest { | ||
| // Metrics byte sequence in OpenMetrics format. | ||
| bytes piped_stats = 1 [(validate.rules).bytes.min_len = 1]; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice. For those who look at this first will be more likely to wonder why it's represented as a byte sequence. I feel like it should be commented like: "Metrics byte sequence in OpenMetrics format"
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 Lets me adopt your idea, thx 🙆♂️
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. addressed by 2bb3bcb 🙏 |
||
| } | ||
|
|
||
| message ReportStatResponse { | ||
| int64 report_interval = 1; | ||
| } | ||
|
|
||
| message ReportPipedMetaRequest { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -119,9 +119,6 @@ func NewCommand() *cobra.Command { | |
| } | ||
|
|
||
| func (p *piped) run(ctx context.Context, t cli.Telemetry) (runErr error) { | ||
| // Register all metrics. | ||
| registerMetrics() | ||
|
|
||
| group, ctx := errgroup.WithContext(ctx) | ||
| if p.addLoginUserToPasswd { | ||
| if err := p.insertLoginUserToPasswd(ctx); err != nil { | ||
|
|
@@ -136,6 +133,9 @@ func (p *piped) run(ctx context.Context, t cli.Telemetry) (runErr error) { | |
| return err | ||
| } | ||
|
|
||
| // Register all metrics. | ||
| registerMetrics(cfg.PipedID) | ||
|
|
||
| // Initialize notifier and add piped events. | ||
| notifier, err := notifier.NewNotifier(cfg, t.Logger) | ||
| if err != nil { | ||
|
|
@@ -616,8 +616,14 @@ func (p *piped) insertLoginUserToPasswd(ctx context.Context) error { | |
| return nil | ||
| } | ||
|
|
||
| func registerMetrics() { | ||
| func registerMetrics(pipedID string) { | ||
| r := prometheus.DefaultRegisterer | ||
| k8scloudprovidermetrics.Register(r) | ||
| k8slivestatestoremetrics.Register(r) | ||
| // TODO: Add piped version as label. | ||
| wrapped := prometheus.WrapRegistererWith( | ||
| prometheus.Labels{"piped": pipedID}, | ||
| r, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice. One thing we have to update is fixing But that can be done in another PR.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice catch, lets me address another missing in this PR 🙏 |
||
| ) | ||
|
|
||
| k8scloudprovidermetrics.Register(wrapped) | ||
| k8slivestatestoremetrics.Register(wrapped) | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.