Fixing cluster metrics attributes slice corruption#3699
Conversation
|
Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset. In case there are security findings, they will be communicated to you as a comment inside the PR. Hope you’ll enjoy using Jit. Questions? Comments? Want to learn more? Get in touch with us. |
|
Hello @Jesse-Bonfire, thank you for opening this PR, will review it soon, we are preparing a new version, if possible - will include it. |
|
Not sure why that test has failed. It appears to be unrelated. Does that test sometimes have issues? |
|
@Jesse-Bonfire there are some flaky tests, we are resolving them in our PRs when we observe them, will just restart the build for you. |
|
Let me know if there is anything else I need to do to get this merged in :) |
|
Looks good, thank you @Jesse-Bonfire @ofekshenawa feel free to review and merge. |
🛡️ Jit Security Scan Results
✅ No security findings were detected in this PR
Security scan by Jit
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
This issue is a bit complicated but during initialization of redisotel we are finding that our custom attributes are being removed from some of the metricsHooks after getting registered.
During initialization of the redisotel on a Redis Cluster the "pool.name" was being appended to the attrs slice for each node in the cluster. This slice is then copied to the metricsHook but as the slice grows the data order rearranges so when processHook is finally called the slice is looking at 4 "pool.name" attrs and our custom attr is removed.
I also added support to be able to manually set the poolName during redisotel initialization which skips the getting of first address in the options for each client type.
Note
Medium Risk
Moderate risk because it changes how metrics attributes (including
pool.name) are derived and appended for all Redis client types, which can affect metric cardinality/labels but not Redis command execution.Overview
Ensures
pool.nameis added to the metrics attribute set once duringInstrumentMetrics(instead of per-node registration), preventing attribute slice growth/corruption that could drop user-provided attributes on Redis Cluster/Ring.Adds
WithPoolName(...)to allow callers to explicitly set the pool name and skip auto-detection from client addresses.Written by Cursor Bugbot for commit eae6053. This will update automatically on new commits. Configure here.