Skip to content

Commit

Permalink
feat(compute/metadata): add context for all functions/methods
Browse files Browse the repository at this point in the history
Hi!

The [GMP product got hit by the case](GoogleCloudPlatform/prometheus-engine#1021) where calls to metadata server
were hanging "forever" (GKE sandbox with disabled metadata server). Thanks to
recent work in this library, the clients support retry with backoff with timeouts.

However, still a single call was taking ~14s and since we made multiple of those, it took down
the service (readiness probe of 30s).

We can hackly workaround things (as we did in [here](GoogleCloudPlatform/prometheus-engine@e02408c)), by copying some code around, but I think the
above issue proves it would be helpful to actually add context to all metadata functions and methods.

I saw googleapis#9733 we started small, but
in this PR I actually spent 1h to add context to all things for consistency.

I self-reviewed carefuly, and tried to be consistent and extra safe, but careful
review is needed, given little testing.

Some minor extra changes (we can split to new commit/PR):
* Added comment around 14s worst case
* Added comment about OnGCE semantics which was a bit surprising for us.
* When reviewing tests, I updated one test for best practices.

Alternative would be something like

```
// WithContext sets the context that will be used for all client methods that
// does not support context. This means that e.g. the context passed in
// GetWithContext will override the context in WithContext
func (c *Client) WithContext(ctx context.Context) {
	c.ctx = ctx
}

```

.. but the logic is not trivial and we introdue some state to the global variable (defaultClient), which
can be source of problems if someone sets context for them, but another package in same binary resues it etc.

Given it's trivial to add (and use) new methods, I just did it.

Signed-off-by: bwplotka <bwplotka@gmail.com>
  • Loading branch information
bwplotka committed Jun 12, 2024
1 parent 203fc92 commit bc2404c
Show file tree
Hide file tree
Showing 2 changed files with 331 additions and 57 deletions.
Loading

0 comments on commit bc2404c

Please sign in to comment.