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

toxiproxy-cli - sortedAttributes sort by attribute.key instead attribute.value #543

Merged
merged 1 commit into from
Feb 26, 2024

Conversation

jesseward
Copy link
Contributor

@jesseward jesseward commented Nov 13, 2023

Summary

When "inspecting" a toxic via toxiproxy-cli (eg: toxiproxy-cli inspect mysql), sortedAttributes fails to handle non float types.

To work around this issue, I chose sort via the attribute.key instead of the attribute.value.

The function being modified in this PR is only used when rendering to the console. Other uses of the attribute fields are cast or serialized into their correct type elsewhere in the codebase.

 » toxiproxy-cli inspect mysql
Name: mysql	Listen: [::]:4306	Upstream: yahoo.ca:80
======================================================================
Upstream toxics:
debug_upstream:	type=debug	stream=upstream	toxicity=1.00	attributes=[panic: interface conversion: interface {} is string, not float64

goroutine 1 [running]:
main.sortedAttributes(0x140000204b0)
	/Users/jesseward/code/toxiproxy/cmd/cli/cli.go:629 +0x1e4
main.listToxics({0x14000082180?, 0x1, 0x10243d486?}, {0x10241e65b?, 0x8?})
	/Users/jesseward/code/toxiproxy/cmd/cli/cli.go:653 +0x440
main.inspectProxy(0x140001ce300, 0x18?)
	/Users/jesseward/code/toxiproxy/cmd/cli/cli.go:364 +0x728
main.cliCommands.withToxi.func2(0x140001ce300)
	/Users/jesseward/code/toxiproxy/cmd/cli/cli.go:272 +0xec
github.com/urfave/cli/v2.(*Command).Run(0x140001cc580, 0x140001ce300, {0x140001a86e0, 0x2, 0x2})
	/Users/jesseward/go/pkg/mod/github.com/urfave/cli/v2@v2.25.7/command.go:274 +0x730
github.com/urfave/cli/v2.(*Command).Run(0x140001ccdc0, 0x140001ce140, {0x1400012c180, 0x3, 0x3})
	/Users/jesseward/go/pkg/mod/github.com/urfave/cli/v2@v2.25.7/command.go:267 +0x940
github.com/urfave/cli/v2.(*App).RunContext(0x140001c6000, {0x1025c8e98?, 0x102923580}, {0x1400012c180, 0x3, 0x3})
	/Users/jesseward/go/pkg/mod/github.com/urfave/cli/v2@v2.25.7/app.go:332 +0x518
github.com/urfave/cli/v2.(*App).Run(...)
	/Users/jesseward/go/pkg/mod/github.com/urfave/cli/v2@v2.25.7/app.go:309
main.main()
	/Users/jesseward/code/toxiproxy/cmd/cli/cli.go:104 +0x358

Testing

Create a toxic with an int-ish type and a string

dist/toxiproxy-server & ; dist/toxiproxy-cli create -l :4306 -u yahoo.ca:80 mysql && \
dist/toxiproxy-cli toxic add -u -a number=1 -a a_string=yahoo -t debug mysql

update _examples/toxics/debug_toxic.go to include

// DebugToxic prints bytes processed through pipe.
type DebugToxic struct {
	Number  int    `json:"number"`
	AString string `json:"a_string"`
}
» go run cmd/cli/cli.go inspect mysql                                                                                                       jesseward@Jesses-MacBook-Pro
Name: mysql     Listen: [::]:4306       Upstream: yahoo.ca:80
======================================================================
Upstream toxics:
debug_upstream: type=debug      stream=upstream toxicity=1.00   attributes=[    a_string=yahoo  number=1        ]

Downstream toxics:
Proxy has no Downstream toxics enabled.

Hint: add a toxic with `toxiproxy-cli toxic add`

@jesseward jesseward changed the title sortedAttributes sort by Key instead of value toxiproxy-cli - sortedAttributes sort by attribute.key instead attribute.value Nov 13, 2023
@jesseward jesseward changed the title toxiproxy-cli - sortedAttributes sort by attribute.key instead attribute.value toxiproxy-cli - sortedAttributes sort by attribute.key instead attribute.value Nov 13, 2023
@jesseward
Copy link
Contributor Author

I have signed the CLA!

@casperisfine casperisfine requested a review from a team February 14, 2024 12:33
@casperisfine
Copy link

@Shopify/resiliency-acceleration could you have a look at this PR please?

Copy link
Member

@abecevello abecevello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thanks for the contribution and sorry about the delay reviewing. I'll try to make a release with this change tomorrow.

@abecevello abecevello merged commit 7f56e3d into Shopify:main Feb 26, 2024
1 check passed
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.

4 participants