Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

contrib/go-redis/redis.v8: fix broken parsing of redis commands #1783

Merged
merged 6 commits into from
May 12, 2023

Conversation

knusbaum
Copy link
Contributor

@knusbaum knusbaum commented Mar 6, 2023

What does this PR do?

The previous code made some assumptions about the format of the commands that were not correct, namely that they would always contain at least 1 space. This seems to be true for commands generated by the library, but not necessarily for commands given as text to the Do function, e.g. client.Do(ctx, "PING").

This results in a panic due to out-of-bounds indexing.

This commit makes the parsing code not assume this format and adds a test case to ensure it works correctly for commands with zero spaces.

Describe how to test/QA your changes

Reviewer's Checklist

  • If known, an appropriate milestone has been selected; otherwise the Triage milestone is set.
  • Changed code has unit tests for its functionality.
  • If this interacts with the agent in a new way, a system test has been added.

The previous code made some assumptions about the format of the commands
that were not correct, namely that they would always contain at least 1
space. This seems to be true for commands generated by the library, but
not necessarily for commands given as text to the Do function, e.g.
client.Do(ctx, "PING").

This results in a panic due to out-of-bounds indexing.

This commit makes the parsing code not assume this format and adds a test
case to ensure it works correctly for commands with zero spaces.
@knusbaum knusbaum added this to the v1.49.0 milestone Mar 6, 2023
@knusbaum knusbaum requested a review from a team March 6, 2023 19:19
@pr-commenter
Copy link

pr-commenter bot commented Mar 6, 2023

Benchmarks

Comparing candidate commit c785de8 in PR branch knusbaum/fix-go-redis.v8 with baseline commit aff2b5a in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 18 metrics, 0 unstable metrics.

@ajgajg1134 ajgajg1134 merged commit 3b1462c into main May 12, 2023
46 checks passed
@ajgajg1134 ajgajg1134 deleted the knusbaum/fix-go-redis.v8 branch May 12, 2023 15:35
katiehockman pushed a commit that referenced this pull request Jun 6, 2023
Co-authored-by: Diana Shevchenko <40775148+dianashevchenko@users.noreply.github.com>
Co-authored-by: Andrew Glaude <andrew.glaude@datadoghq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants