Skip to content
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

Instrument async replication worker pool utilization #2059

Merged
merged 2 commits into from
Dec 5, 2019

Conversation

simonrobb
Copy link
Contributor

@simonrobb simonrobb commented Dec 4, 2019

PooledWorkerPool is already instrumented so this PR just passes in instrument options during instantiation.

@@ -312,8 +312,10 @@ func (c Configuration) NewAdminClient(
size = *c.AsyncWriteWorkerPoolSize
}

workerPoolInstrumentOpts := iopts.SetMetricsScope(writeRequestScope)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super nit: should this maybe be a sub-scope of writeRequestScope? maybe .Subscope("workerpool" or something

Copy link
Contributor

@richardartoul richardartoul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, take or leave the nit

@simonrobb simonrobb force-pushed the srobb/instrument-replication-worker-pool branch from 1b52061 to 1644a68 Compare December 5, 2019 17:20
@simonrobb simonrobb merged commit 54cdb87 into master Dec 5, 2019
@simonrobb simonrobb deleted the srobb/instrument-replication-worker-pool branch December 5, 2019 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants