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

Early cancelation of streaming responses based on predicate #184

Closed
Sushisource opened this issue May 4, 2020 · 5 comments
Closed

Early cancelation of streaming responses based on predicate #184

Sushisource opened this issue May 4, 2020 · 5 comments
Labels
CLI Issues for ghz CLI enhancement New feature or request

Comments

@Sushisource
Copy link

Is your feature request related to a problem? Please describe.
Hi there, really love this tool. Appreciate your work on it. I have a need to instruct ghz to drop/cancel the stream once it sees a message matching some predicate (even though the server may have more info to send) as this best matches the behavior of my real clients in some situations.

Describe the solution you'd like
Some way to match against incoming messages from the server in a streaming response, and have the client cancel that particular stream when a match is detected. The simplest version could just be to match some regex against the deserialized/jsonified content, a more elaborate implementation might allow some form of pattern matching.

Describe alternatives you've considered
Right now I just have a special flag that tells the server to terminate the stream earlier than it usually would during my testing, but ideally that is not necessary as generally the client will decide when it's done.

I would be happy to try to make a PR for this. Don't have a ton of Go experience but I can probably find some time to put it in if you're open.

@bojand
Copy link
Owner

bojand commented May 6, 2020

Hello thanks for reporting this! I have an idea of the problem, but it may help to provide more detail like an example of type of conditions and predicates desired and perhaps some ideas on what the CLI input could look like (json path perhaps?).

@bojand bojand added CLI Issues for ghz CLI enhancement New feature or request labels May 6, 2020
@Sushisource
Copy link
Author

Happy to provide some:

So in my particular case my server is returning a stream of updates to the state of some record. So the response type looks something like:

service MyApi {
    rpc Foo (Req) returns (stream Response) {
        option (google.api.http) = {
            post: "/v1/foo"
            body: "*"
        };
    }
}

message Req {}

message Response {
  string id = 1;
  oneof state {
    A a = 10;
    B b = 11;
    C c = 12;
  }
}

Normally, the server will terminate the stream once it has emitted a response with state = c. The client though might only need to see up to b.

My imagined way you could handle this in ghz might be:
ghz --call=pkg.MyApi.Foo -d "{}" --cancel-stream-when ".state='b'"

Where the syntax for --cancel-stream-when is obviously the part that bears some thought. Here I've done a jq-style json path, and then the right hand side could be a string/regex. Ghz would then match this selector to each incoming reponse, and in the case it matches, cancel that stream.

How does that sound?

@bojand
Copy link
Owner

bojand commented May 16, 2020

Yup this is something like what I was envisioning as well. I think from package API perspective this can be relatively simpler to implement; but will involve some creativity to get the CLI interface correct. For example, if we check for JSON presence, or regex, or if we embed and support some for of simple expression evaluation. Just some things to think about but I get the feature. Hopefully I can work on this soon.

@bojand
Copy link
Owner

bojand commented Jan 2, 2021

I think most of this should be addressed with the options and API introduced in 0.80.0.

A new API WithStreamRecvMsgIntercept can be used to add an interceptor function to streaming call that is invoked every time we receive a streaming message. Example usage:

WithStreamRecvMsgIntercept(func(msg *dynamic.Message, err error) error {
	if err == nil && msg != nil {
		reply := &helloworld.HelloReply{}
		convertErr := msg.ConvertTo(reply)
		if convertErr == nil {
			if reply.GetMessage() == "Hello bar" {
				return ErrEndStream
			}
		}
	}
	return nil
})

Not that in a future release we hope to remove reliance on the github.com/jhump/protoreflect/dynamic package for this API, and just use the standard proto interfaces.

I think a CLI option involving some kind of language interpretation and execution is trickier to include, and might result in significant runtime performance penalty (thus effecting accuracy of results). So I am still on the fence about it and need to research that some more. But the API like above I believe should be sufficient. Please feel free to comment some more or close the issue if it addresses the specific scenario.

@Sushisource
Copy link
Author

@bojand Awesome! Love it. This'll do what I needed. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI Issues for ghz CLI enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants