Skip to content

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

Merged
merged 1 commit into from
Feb 1, 2024

Conversation

spikecurtis
Copy link
Contributor

@spikecurtis spikecurtis commented Jan 30, 2024

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.

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @spikecurtis and the rest of your teammates on Graphite Graphite

@spikecurtis spikecurtis requested a review from mafredri January 30, 2024 13:08
@spikecurtis spikecurtis marked this pull request as ready for review January 30, 2024 13:08
req := testutil.RequireRecvCtx(ctx, t, fDest.reqs)
require.NotNil(t, req)
require.Nil(t, req.Stats)
interval := time.Second * 34
Copy link
Member

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?

Copy link
Contributor Author

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.

@spikecurtis spikecurtis force-pushed the spike/10534-stats-reporter branch from 11f3d11 to 8c43c94 Compare January 30, 2024 20:42
@spikecurtis spikecurtis requested a review from mafredri January 30, 2024 20:43
Copy link
Member

@mafredri mafredri left a 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)
Copy link
Member

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.

@spikecurtis spikecurtis merged commit eb03e44 into main Feb 1, 2024
@spikecurtis spikecurtis deleted the spike/10534-stats-reporter branch February 1, 2024 04:21
@github-actions github-actions bot locked and limited conversation to collaborators Feb 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants