Metric update for the Upstairs#362
Merged
Merged
Conversation
This changes the way the upstairs handles metrics. The new way is to provide up_main() with a ProducerRegister handle that it can use to register whatever stats it so desires. The caller/user of up_main() is responsible for creating a dropshot endpoint to serve the collection requests coming from Oximeter. Updated the client test to include a dropshot server and register with Nexus if requested. Removed the metric_collect and metric_register fields from CrucibleOpts as they are now obsolete. This required updating a few other files as well. The Volume::construct() now also takes a ProducerRegister that it will eventually pass to up_main().
added 13 commits
June 28, 2022 00:32
jmpesp
approved these changes
Jul 1, 2022
| pub fn construct(request: VolumeConstructionRequest) -> Result<Volume> { | ||
| pub fn construct( | ||
| request: VolumeConstructionRequest, | ||
| producer_registry: Arc<Mutex<Option<ProducerRegistry>>>, |
Contributor
There was a problem hiding this comment.
this should be Option<Arc<Mutex<ProducerRegistry>>> I think - it someone wants to pass None, it doesn't have to be wrapped in an arc and mutex.
Contributor
Author
There was a problem hiding this comment.
I tried to have it just an Option, but I'm passing the ProducerRegistry around on the propolis side and I was not able to get it to work without Arc Mutex. @luqmana had the same comment on the propolis side PR, and I'm going to try from the other side to see if I can detangle this and if so, I'll come back here and change it as well.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This changes the way the upstairs handles metrics. The new way is to
provide up_main() with a ProducerRegister handle that it can use to
register whatever stats it so desires. The caller/user of up_main() is
responsible for creating a dropshot endpoint to serve the collection
requests coming from Oximeter.
Updated the client test to include a dropshot server and register with
Nexus if requested.
Removed the metric_collect and metric_register fields from CrucibleOpts
as they are now obsolete. This required updating a few other files as well.
The Volume::construct() now also takes a ProducerRegister that it will
eventually pass to up_main().
This is needed before changes in oxidecomputer/propolis#147 can go in.