Repository: hetznercloud/hcloud-go
Pull request #635 (2025-03-20 09:29:27)

fix: panic when a request did not set the OpPath for instrumentation
When an instrumented client (API or Metadata) made a request that did not explicitly specify the OpPath through `ctxutil.SetOpPath()` that instrumentation code caused a panic.

This is now handled by adding back a fallback if the explicit OpPath is not set. The fallback uses the same patterns to create the OpPath as our manual paths.

This only happened for the Metadata client and any custom clients/requests that users added besides the official endpoints implemented in hcloud-go.

Bug was introduced in #626, thanks to @lukasmetzner for finding and reporting.

Co-authored-by: Jonas Lammler <jonas.lammler@hetzner-cloud.de>

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/hetznercloud/hcloud-go/pull/635?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hetznercloud) Report
Attention: Patch coverage is `51.85185%` with `13 lines` in your changes missing coverage. Please review.
> Project coverage is 79.15%. Comparing base [(`69152df`)](https://app.codecov.io/gh/hetznercloud/hcloud-go/commit/69152dfa333ab1a7114d68f1d250d4019b9feff8?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hetznercloud) to head [(`e8a79a5`)](https://app.codecov.io/gh/hetznercloud/hcloud-go/commit/e8a79a51d4e46a652d9ed9b62a4737015d9e61f1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hetznercloud).

| [Files with missing lines](https://app.codecov.io/gh/hetznercloud/hcloud-go/pull/635?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hetznercloud) | Patch % | Lines |
|---|---|---|
| [hcloud/internal/instrumentation/metrics.go](https://app.codecov.io/gh/hetznercloud/hcloud-go/pull/635?src=pr&el=tree&filepath=hcloud%2Finternal%2Finstrumentation%2Fmetrics.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hetznercloud#diff-aGNsb3VkL2ludGVybmFsL2luc3RydW1lbnRhdGlvbi9tZXRyaWNzLmdv) | 58.82% | [7 Missing :warning: ](https://app.codecov.io/gh/hetznercloud/hcloud-go/pull/635?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hetznercloud) |
| [hcloud/exp/ctxutil/ctxutil.go](https://app.codecov.io/gh/hetznercloud/hcloud-go/pull/635?src=pr&el=tree&filepath=hcloud%2Fexp%2Fctxutil%2Fctxutil.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hetznercloud#diff-aGNsb3VkL2V4cC9jdHh1dGlsL2N0eHV0aWwuZ28=) | 25.00% | [2 Missing and 1 partial :warning: ](https://app.codecov.io/gh/hetznercloud/hcloud-go/pull/635?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hetznercloud) |
| [hcloud/metadata/client.go](https://app.codecov.io/gh/hetznercloud/hcloud-go/pull/635?src=pr&el=tree&filepath=hcloud%2Fmetadata%2Fclient.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hetznercloud#diff-aGNsb3VkL21ldGFkYXRhL2NsaWVudC5nbw==) | 50.00% | [2 Missing and 1 partial :warning: ](https://app.codecov.io/gh/hetznercloud/hcloud-go/pull/635?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hetznercloud) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main     #635      +/-   ##
==========================================
+ Coverage   78.93%   79.15%   +0.21%     
==========================================
  Files          50       50              
  Lines        4373     4395      +22     
==========================================
+ Hits         3452     3479      +27     
+ Misses        706      701       -5     
  Partials      215      215              
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/hetznercloud/hcloud-go/pull/635?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hetznercloud).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=hetznercloud).

<details><summary>🚀 New features to boost your workflow: </summary>

- ❄ [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>

Patch:
diff --git a/hcloud/exp/ctxutil/ctxutil.go b/hcloud/exp/ctxutil/ctxutil.go
index 18bbe3a..de0ce95 100644
--- a/hcloud/exp/ctxutil/ctxutil.go
+++ b/hcloud/exp/ctxutil/ctxutil.go
@@ -11,6 +11,7 @@ type key struct{}
 // opPathKey is the key for operation path in Contexts.
 var opPathKey = key{}
 
+// SetOpPath processes the operation path and save it in the context before returning it.
 func SetOpPath(ctx context.Context, path string) context.Context {
 	path, _, _ = strings.Cut(path, "?")
 	path = strings.ReplaceAll(path, "%d", "-")
@@ -19,7 +20,11 @@ func SetOpPath(ctx context.Context, path string) context.Context {
 	return context.WithValue(ctx, opPathKey, path)
 }
 
+// OpPath returns the operation path from the context.
 func OpPath(ctx context.Context) string {
-	result := ctx.Value(opPathKey).(string)
+	result, ok := ctx.Value(opPathKey).(string)
+	if !ok {
+		return ""
+	}
 	return result
 }
diff --git a/hcloud/internal/instrumentation/metrics.go b/hcloud/internal/instrumentation/metrics.go
index c89d2e6..01bc60b 100644
--- a/hcloud/internal/instrumentation/metrics.go
+++ b/hcloud/internal/instrumentation/metrics.go
@@ -3,6 +3,7 @@ package instrumentation
 import (
 	"fmt"
 	"net/http"
+	"regexp"
 	"strconv"
 	"strings"
 
@@ -74,10 +75,16 @@ func (i *Instrumenter) instrumentRoundTripperEndpoint(counter *prometheus.Counte
 	return func(r *http.Request) (*http.Response, error) {
 		resp, err := next.RoundTrip(r)
 		if err == nil {
+			apiEndpoint := ctxutil.OpPath(r.Context())
+			// If the request does not set the operation path, we must construct it. Happens e.g. for
+			// user crafted requests.
+			if apiEndpoint == "" {
+				apiEndpoint = preparePathForLabel(resp.Request.URL.Path)
+			}
 			counter.WithLabelValues(
 				strconv.Itoa(resp.StatusCode),
 				strings.ToLower(resp.Request.Method),
-				ctxutil.OpPath(r.Context()),
+				apiEndpoint,
 			).Inc()
 		}
 
@@ -105,3 +112,17 @@ func registerOrReuse[C prometheus.Collector](registry prometheus.Registerer, col
 
 	return collector
 }
+
+var pathLabelRegexp = regexp.MustCompile("[^a-z/_]+")
+
+func preparePathForLabel(path string) string {
+	path = strings.ToLower(path)
+
+	// replace the /v1/ that indicated the API version
+	path, _ = strings.CutPrefix(path, "/v1")
+
+	// replace all numbers and chars that are not a-z, / or _
+	path = pathLabelRegexp.ReplaceAllString(path, "-")
+
+	return path
+}
diff --git a/hcloud/metadata/client.go b/hcloud/metadata/client.go
index b873cb2..45704ca 100644
--- a/hcloud/metadata/client.go
+++ b/hcloud/metadata/client.go
@@ -2,6 +2,7 @@ package metadata
 
 import (
 	"bytes"
+	"context"
 	"fmt"
 	"io"
 	"net"
@@ -12,6 +13,7 @@ import (
 
 	"github.com/prometheus/client_golang/prometheus"
 
+	"github.com/hetznercloud/hcloud-go/v2/hcloud/exp/ctxutil"
 	"github.com/hetznercloud/hcloud-go/v2/hcloud/internal/instrumentation"
 )
 
@@ -80,9 +82,14 @@ func NewClient(options ...ClientOption) *Client {
 
 // get executes an HTTP request against the API.
 func (c *Client) get(path string) (string, error) {
-	url := c.endpoint + path
+	ctx := ctxutil.SetOpPath(context.Background(), path)
 
-	resp, err := c.httpClient.Get(url)
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.endpoint+path, http.NoBody)
+	if err != nil {
+		return "", err
+	}
+
+	resp, err := c.httpClient.Do(req)
 	if err != nil {
 		return "", err
 	}


Test patch:
diff --git a/hcloud/client_generic_test.go b/hcloud/client_generic_test.go
index e9e0ebe..17698ce 100644
--- a/hcloud/client_generic_test.go
+++ b/hcloud/client_generic_test.go
@@ -7,6 +7,7 @@ import (
 
 	"github.com/stretchr/testify/require"
 
+	"github.com/hetznercloud/hcloud-go/v2/hcloud/exp/ctxutil"
 	"github.com/hetznercloud/hcloud-go/v2/hcloud/exp/mockutil"
 	"github.com/hetznercloud/hcloud-go/v2/hcloud/schema"
 )
@@ -23,7 +24,10 @@ func TestGenericRequest(t *testing.T) {
 			},
 		})
 
-		respBody, resp, err := getRequest[schema.ActionGetResponse](ctx, client, "/resource")
+		const opPath = "/resource"
+		ctx = ctxutil.SetOpPath(ctx, opPath)
+
+		respBody, resp, err := getRequest[schema.ActionGetResponse](ctx, client, opPath)
 		require.NoError(t, err)
 		require.NotNil(t, resp)
 		require.NotNil(t, respBody)
@@ -47,7 +51,10 @@ func TestGenericRequest(t *testing.T) {
 			},
 		})
 
-		respBody, resp, err := postRequest[schema.ActionGetResponse](ctx, client, "/resource", map[string]string{"hello": "world"})
+		const opPath = "/resource"
+		ctx = ctxutil.SetOpPath(ctx, opPath)
+
+		respBody, resp, err := postRequest[schema.ActionGetResponse](ctx, client, opPath, map[string]string{"hello": "world"})
 		require.NoError(t, err)
 		require.NotNil(t, resp)
 		require.NotNil(t, respBody)
@@ -71,7 +78,10 @@ func TestGenericRequest(t *testing.T) {
 			},
 		})
 
-		respBody, resp, err := putRequest[schema.ActionGetResponse](ctx, client, "/resource", map[string]string{"hello": "world"})
+		const opPath = "/resource"
+		ctx = ctxutil.SetOpPath(ctx, opPath)
+
+		respBody, resp, err := putRequest[schema.ActionGetResponse](ctx, client, opPath, map[string]string{"hello": "world"})
 		require.NoError(t, err)
 		require.NotNil(t, resp)
 		require.NotNil(t, respBody)
@@ -90,7 +100,10 @@ func TestGenericRequest(t *testing.T) {
 			},
 		})
 
-		respBody, resp, err := deleteRequest[schema.ActionGetResponse](ctx, client, "/resource")
+		const opPath = "/resource"
+		ctx = ctxutil.SetOpPath(ctx, opPath)
+
+		respBody, resp, err := deleteRequest[schema.ActionGetResponse](ctx, client, opPath)
 		require.NoError(t, err)
 		require.NotNil(t, resp)
 		require.NotNil(t, respBody)
@@ -108,7 +121,10 @@ func TestGenericRequest(t *testing.T) {
 			},
 		})
 
-		resp, err := deleteRequestNoResult(ctx, client, "/resource")
+		const opPath = "/resource"
+		ctx = ctxutil.SetOpPath(ctx, opPath)
+
+		resp, err := deleteRequestNoResult(ctx, client, opPath)
 		require.NoError(t, err)
 		require.NotNil(t, resp)
 	})
diff --git a/hcloud/client_test.go b/hcloud/client_test.go
index e2253d0..b9926c6 100644
--- a/hcloud/client_test.go
+++ b/hcloud/client_test.go
@@ -12,6 +12,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/prometheus/client_golang/prometheus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -28,6 +29,8 @@ func makeTestUtils(t *testing.T) (context.Context, *mockutil.Server, *Client) {
 		WithEndpoint(server.URL),
 		WithRetryOpts(RetryOpts{BackoffFunc: ConstantBackoff(0), MaxRetries: 5}),
 		WithPollOpts(PollOpts{BackoffFunc: ConstantBackoff(0)}),
+		// This makes sure that our instrumentation does not cause any panics/errors
+		WithInstrumentation(prometheus.DefaultRegisterer),
 	)
 
 	return ctx, server, client
@@ -63,6 +66,8 @@ func newTestEnvWithServer(server *httptest.Server, mux *http.ServeMux) testEnv {
 		WithPollOpts(PollOpts{
 			BackoffFunc: ConstantBackoff(0),
 		}),
+		// This makes sure that our instrumentation does not cause any panics/errors
+		WithInstrumentation(prometheus.DefaultRegisterer),
 	)
 	return testEnv{
 		Server: server,
diff --git a/hcloud/internal/instrumentation/metrics_test.go b/hcloud/internal/instrumentation/metrics_test.go
index 9fba845..a8405d5 100644
--- a/hcloud/internal/instrumentation/metrics_test.go
+++ b/hcloud/internal/instrumentation/metrics_test.go
@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/prometheus/client_golang/prometheus"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestMultipleInstrumentedClients(t *testing.T) {
@@ -15,3 +16,24 @@ func TestMultipleInstrumentedClients(t *testing.T) {
 		New("test", reg).InstrumentedRoundTripper()
 	})
 }
+
+func TestPreparePathForLabel(t *testing.T) {
+	tests := []struct {
+		path string
+		want string
+	}{
+		{
+			"/v1/volumes/123456",
+			"/volumes/-",
+		},
+		{
+			"/v1/volumes/123456/actions/attach",
+			"/volumes/-/actions/attach",
+		},
+	}
+	for _, tt := range tests {
+		t.Run("", func(t *testing.T) {
+			assert.Equal(t, tt.want, preparePathForLabel(tt.path))
+		})
+	}
+}
diff --git a/hcloud/metadata/client_test.go b/hcloud/metadata/client_test.go
index ab50499..5f655e3 100644
--- a/hcloud/metadata/client_test.go
+++ b/hcloud/metadata/client_test.go
@@ -5,6 +5,7 @@ import (
 	"net/http/httptest"
 	"testing"
 
+	"github.com/prometheus/client_golang/prometheus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -27,6 +28,8 @@ func newTestEnv() testEnv {
 	server := httptest.NewServer(mux)
 	client := NewClient(
 		WithEndpoint(server.URL),
+		// This makes sure that our instrumentation does not cause any panics/errors
+		WithInstrumentation(prometheus.DefaultRegisterer),
 	)
 	return testEnv{
 		Server: server,