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

Mismatched type for fortio response #932

Open
Revolyssup opened this issue Oct 27, 2022 · 5 comments
Open

Mismatched type for fortio response #932

Revolyssup opened this issue Oct 27, 2022 · 5 comments
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed

Comments

@Revolyssup
Copy link

Revolyssup commented Oct 27, 2022

Title: Mismatched type for fortio response causes failure in interconversion

Description:
The message format in protobuff here

uint32 RequestedQPS = 3;
mismatches the expected datatype by fortio here https://github.com/fortio/fortio/blob/d2a2d42f4f17df4c09e608353546ebd0d443747d/periodic/periodic.go#L221 on the field RequestedQPS

This is causing Unmarshalling to fail while conversion from one to another.
Reproduction steps:

Include sample requests, environment, etc. All data and inputs
required to reproduce the bug.

Note: If there are privacy concerns, sanitize the data prior to
sharing.

Logs:

Include the Nighthawk logs.

Note: If there are privacy concerns, sanitize the data prior to
sharing.

Call Stack:

If the Envoy binary is crashing, a call stack is required.
Please refer to the Bazel Stack trace documentation.

@Revolyssup Revolyssup added the bug Something isn't working label Oct 27, 2022
@leecalcote
Copy link

Thank you for filing this, @Revolyssup

@mum4k
Copy link
Collaborator

mum4k commented Oct 28, 2022

Thank you for the report @Revolyssup.

Can you help me understand - are you saying that the correct type is int as exported by Nighthawk, but fortio incorrectly takes that in as a string? Or are we saying that fortio is correct and we should adjust the Nighthawk's fortio transform to export QPS as string instead of int?

@leecalcote
Copy link

@MUzairS15 will you offer these details? Let's work through this item.

@MUzairS15
Copy link

MUzairS15 commented Jun 15, 2023

yes, @mum4k
fortio exports QPS as string https://pkg.go.dev/fortio.org/fortio/periodic#RunnerResults.
Another example is RetCodes https://github.com/fortio/fortio/blob/v1.55.0/fhttp/httprunner.go#L37
Fortio expects map[int]int were as nighthawk transforms it as map'[string]int

map<string, uint64> RetCodes = 11;

The Transformation needs to be updated.

@mum4k
Copy link
Collaborator

mum4k commented Jun 20, 2023

Thank you for explaining that @MUzairS15, given this just needs a modification in the interface between Nighthawk and Fortio, we should be ok to make this change without breaking anyone. I will go ahead and mark this issue as "help wanted", that way anyone with the cycles can pick it up and contribute the fix.

@mum4k mum4k added help wanted Extra attention is needed good first issue Good for newcomers labels Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants