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

monitoring: use instrumented backoff as default for waiting #71

Conversation

MichaHoffmann
Copy link
Contributor

  • use the instrumented wait backoff of the instrumented runnable as default for waiting for metrics
  • composite instrumented options didnt allow overwriting the backoff
  • fixing the tests

This makes it possible to set the wait backoff once when creating the runnable. It is not concurrency save but i think it was not to begin with. At least WaitRemovedMetric also reused the instrumented runnable wait backoff already.

Signed-off-by: Michael Hoffmann <mhoffm@posteo.de>
@MichaHoffmann MichaHoffmann force-pushed the mhoffm-use-default-backoff-for-waiting branch from 119b874 to 21ca61c Compare October 28, 2023 09:45
}

// WithWaitBackoff is an option to configure a backoff when waiting on a metric value.
func WithWaitBackoff(backoffConfig *backoff.Config) MetricsOption {
return func(o *metricsOptions) {
o.waitBackoff = backoffConfig
o.waitBackoff = backoff.New(context.Background(), *backoffConfig)
Copy link
Collaborator

@yeya24 yeya24 Oct 28, 2023

Choose a reason for hiding this comment

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

Can we allow passing ctx? Maybe we can add another function WithWaitBackoffCtx to avoid too many changes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lets do this in a followup PR!

Copy link
Collaborator

@saswatamcode saswatamcode left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM!

@saswatamcode saswatamcode merged commit 6d823e2 into efficientgo:main Oct 31, 2023
5 checks passed
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.

4 participants