Skip to content
This repository has been archived by the owner on Jul 12, 2023. It is now read-only.

call metric registration in a go routine and don't block sever startup #1542

Merged
merged 2 commits into from
Aug 4, 2021

Conversation

mikehelmick
Copy link
Contributor

Release Note

Create metric descriptors in the background during startup.

@mikehelmick mikehelmick requested a review from a team as a code owner August 4, 2021 19:15
@google-cla google-cla bot added the cla: yes Auto: added by CLA bot when all committers have signed a CLA. label Aug 4, 2021
go func() {
logger.Infow("starting metric registration")
for _, cmrdesc := range descriptorCreateRequests {
_, err = mclient.CreateMetricDescriptor(ctx, cmrdesc)
Copy link
Member

Choose a reason for hiding this comment

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

I think this ctx has a timeout from the parent. I think we should create a new per-request context:

ctx, done := context.WithTimeout(context.Background(), 10*time.Second)
defer done()

If we want to limit the overall time the function can run:

go func() {
  ctx, done := context.WithTimeout(context.Background(), 30*time.Second)
  defer done()


  for _, cmrdesc := range descriptorCreateRequests {
    func() {
      subCtx, done := context.WithTimeout(ctx, 5*time.Second)
      defer done()

      if _, err := mclient.CreateMetricDescriptor(subCtx, cmrdesc); err != nil {
        logger.Errorw(...)
      }
    }()
  }
}()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch - updated

Copy link
Member

@sethvargo sethvargo left a comment

Choose a reason for hiding this comment

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

@mikehelmick approved. You didn't enable auto-approve though, so you'll need to click the button.

@mikehelmick mikehelmick merged commit 6fa52e6 into google:main Aug 4, 2021
@mikehelmick mikehelmick deleted the bgObs branch August 4, 2021 22:02
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes Auto: added by CLA bot when all committers have signed a CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants