usagereporter: on-prem dial home#23916
Conversation
espadolini
force-pushed
the
espadolini/usagereporter-onprem
branch
2 times, most recently
from
b410a65 to
cad19c7
Compare
zmb3
left a comment
zmb3
left a comment
There was a problem hiding this comment.
This implementation seems a bit over complicated for the task at hand. I worry that we're somewhat mis-using a number of concurrency primitives (contexts, waitgroups, and muteness).
|
@zmb3 a913ac50a4c322742530c9062ef4b38342042854 reworks the |
There was a problem hiding this comment.
I don't understand the semantics here - what is the passed in context used for?
There was a problem hiding this comment.
Documented in the interface in 91b75dc:
// GracefulStop gracefully closes and runs any finalization needed by the
// UsageReporter; operations can run as long as the context is alive, but
// must terminate quickly (even losing data) if the context is closed.
// Returns nil if operations have completed cleanly.
GracefulStop(context.Context) errorThere was a problem hiding this comment.
In the case of the aggregating.Reporter, closing the context that's passed in will trigger the closing of the base context, and thus all closing operations ungracefully (and quickly) fail.
There was a problem hiding this comment.
| }).Errorf("Failed to send usage reports.") | |
| }).Error("Failed to send usage reports.") |
espadolini
force-pushed
the
espadolini/usagereporter-onprem
branch
from
199cc77 to
766011c
Compare
This includes a move to prehog.v1 for the new rpc and messages
This renames imports of ".../prehog/v1alpha" from prehogv1a to prehogv1, and imports of ".../prehog/v1alpha/prehogv1alphaconnect" from prehogv1c to prehogv1ac, to avoid confusion with imports of the newly added ".../prehog/v1" and ".../prehog/v1/prehogv1connect".
espadolini
requested review from
klizhentas,
r0mant and
russjones
as code owners
|
@espadolini - this PR will require admin approval to merge due to its size. Consider breaking it up into a series smaller changes. |
|
Sorry for the noise, the manual merge lost its correct parents |
public-teleport-github-review-bot
Bot
removed request for
atburke and
timothyb89
|
Waiting for gravitational/cloud#4018 to confirm the .proto changes |
3 participants
This PR implements the public changes to Teleport for the implementation of gravitational/cloud#3830. Proto changes depend on gravitational/cloud#4018.
Will be enabled by gravitational/teleport.e#1061.