Skip to content

Conversation

apricote
Copy link
Member

@apricote apricote commented Mar 20, 2025

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

apricote and others added 2 commits March 20, 2025 10:25
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.

Co-authored-by: Jonas Lammler <jonas.lammler@hetzner-cloud.de>
@apricote apricote added the Bug label Mar 20, 2025
@apricote apricote self-assigned this Mar 20, 2025
@apricote apricote requested a review from a team as a code owner March 20, 2025 09:29
Copy link

codecov bot commented Mar 20, 2025

Codecov 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) to head (e8a79a5).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
hcloud/internal/instrumentation/metrics.go 58.82% 7 Missing ⚠️
hcloud/exp/ctxutil/ctxutil.go 25.00% 2 Missing and 1 partial ⚠️
hcloud/metadata/client.go 50.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            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              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@apricote apricote merged commit 624a446 into main Mar 20, 2025
8 checks passed
@apricote apricote deleted the fix-metadata branch March 20, 2025 09:32
apricote pushed a commit that referenced this pull request Mar 20, 2025
### Bug Fixes

- panic when a request did not set the OpPath for instrumentation (#635)
apricote pushed a commit to hetznercloud/fleeting-plugin-hetzner that referenced this pull request Mar 20, 2025
…0.1 (hetznercloud/fleeting-plugin-hetzner!230)

This MR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [github.com/hetznercloud/hcloud-go/v2](https://github.com/hetznercloud/hcloud-go) | require | minor | `v2.19.1` -> `v2.20.1` |

---

### Release Notes

<details>
<summary>hetznercloud/hcloud-go (github.com/hetznercloud/hcloud-go/v2)</summary>

### [`v2.20.1`](https://github.com/hetznercloud/hcloud-go/blob/HEAD/CHANGELOG.md#v2201)

[Compare Source](hetznercloud/hcloud-go@v2.20.0...v2.20.1)

##### Bug Fixes

-   panic when a request did not set the OpPath for instrumentation ([#&#8203;635](hetznercloud/hcloud-go#635))

### [`v2.20.0`](https://github.com/hetznercloud/hcloud-go/blob/HEAD/CHANGELOG.md#v2200)

[Compare Source](hetznercloud/hcloud-go@v2.19.1...v2.20.0)

In this release, the `api_endpoint` metric labels changed for all our API requests. Please make sure to update your setup if you were relying on them. The new labels are now easier to understand, see the example below:

-   the path `/volumes/64314930` now has the label `/volumes/-` instead of `/volumes/`
-   the path `/volumes/64314930/actions/resize` now has the label `/volumes/-/actions/resize` instead of `/volumes/actions/resize`

##### Features

-   support go v1.24 ([#&#8203;604](hetznercloud/hcloud-go#604))
-   drop go v1.21 ([#&#8203;604](hetznercloud/hcloud-go#604))
-   **exp**: remove sliceutil package ([#&#8203;610](hetznercloud/hcloud-go#610))
-   drop go v1.22 ([#&#8203;602](hetznercloud/hcloud-go#602)) ([#&#8203;621](hetznercloud/hcloud-go#621))
-   redefine `api_endpoint` metric labels ([#&#8203;626](hetznercloud/hcloud-go#626))

##### Bug Fixes

-   request in batches of 25 when waiting for actions ([#&#8203;611](hetznercloud/hcloud-go#611))
-   missing response from requests return values ([#&#8203;613](hetznercloud/hcloud-go#613))
-   move primary ip client request/response structs to schema package ([#&#8203;633](hetznercloud/hcloud-go#633))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS4yMDguMCIsInVwZGF0ZWRJblZlciI6IjM5LjIxMC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants