-
Notifications
You must be signed in to change notification settings - Fork 889
feat: add statsReporter for reporting stats on agent v2 API #11920
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
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @spikecurtis and the rest of your teammates on |
req := testutil.RequireRecvCtx(ctx, t, fDest.reqs) | ||
require.NotNil(t, req) | ||
require.Nil(t, req.Stats) | ||
interval := time.Second * 34 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a special meaning to this magic value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just wanted something that's not likely to be some internal constant and proves that we actually use the interval in the response.
11f3d11
to
8c43c94
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had one more question but looks good, thanks for added commentary 👍🏻
return xerrors.Errorf("initial update: %w", err) | ||
} | ||
s.lastInterval = resp.ReportInterval.AsDuration() | ||
s.source.SetConnStatsCallback(s.lastInterval, maxConns, s.callback) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to also unset the callback on exit? Mostly a safety precaution in case callback
ever has code that may block without a consumer.
Adds a new statsReporter subcomponent of the agent, which in a later PR will be used to report stats over the v2 API.
Refactors the logic a bit so that we can handle starting and stopping stats reporting if the agent API connection drops and reconnects.