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

Rendering partial results when visualizations are backed off the expression language #53336

Closed
stacey-gammon opened this issue Dec 17, 2019 · 8 comments
Labels
Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:medium Medium Level of Effort

Comments

@stacey-gammon
Copy link
Contributor

stacey-gammon commented Dec 17, 2019

Partial results are returned from our search services via an Observable but the interpreter and expression functions currently expect promises.

In order to show partial results rendering, we want to execute the remainder of an expression every time the expression fn fires.

One corner case that could cause issues is if a subsequent search fn relies on output from an earlier search fn. That could create a slew of search requests. e.g.:

essql "select max(val) as val from foo" | joinOn table="essql 'select * from bar where ${val} > 100'" | render debug

I know this string is wrong, but however you write it, the point is that the second query relies on the first. The request cache would miss every time that value changes. We'd want to abort those requests as soon as they aren't needed.

However for almost all intents and purposes there is only a single data fetching fn per expression. If we can solve some of the make it slow issues on that assumption (if it's true, @lukasolson did point out a real situation we do use two queries... so maybe this would be a problem, needs investigation), but if we could force only a single data fetching fn, or at least have decent fallback behavior, I think that would solve the majority of use cases.

cc @lukasolson @lukeelmers @alexh97

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@clintandrewhall
Copy link
Contributor

clintandrewhall commented Dec 17, 2019

From a Canvas perspective, we've always supported external plugins to write Expression Functions. This kind of change would need to add support for Observables, rather than replace Promises, to avoid breaking 3rd party functions. At the very least, I would propose adding support for Observables, optionally convert Canvas Functions, then deprecate in a future release.

It might be interesting to add Observables to the spec, rather than replace fn... it might be interesting to have fn and obFn, where fn would return a single batch of results, and obFn would return a stream. The render function would then be aware of which to use based on which the consuming user decided to select...? Or maybe not... thoughts welcome!

@lukeelmers
Copy link
Member

it might be interesting to have fn and obFn, where fn would return a single batch of results, and obFn would return a stream

If we end up needing to go the observables route, I like this idea because it feels less risky.

Fully replacing promises with observables is a pretty big paradigm shift in the way we've been thinking/talking about expressions to date. Though expressions are a simple concept at their core, we know people already struggle with understanding them because they can quickly get complex. Having to think about them as streams adds another layer of complexity.

An approach like this would help us avoid a hard breaking change for existing integrations, while still giving us the option to go full-observables down the road if it turns out to be the best way to handle expressions.

for almost all intents and purposes there is only a single data fetching fn per expression.

The more I think about this, the less confident I become 😄. There's also subexpressions to worry about here. What if somebody needs to query something in a function arg as a subexpression? Technically that's still multiple data fetching functions in an expression. In general, I think it'd be hard to enforce a single-data-fetching-fn rule; this is something we'd need to discuss in more depth.

@lukasolson
Copy link
Member

Another alternative that might not be everything we need (but would certainly be closer) (related to #51245) is to have a separate method for invoking the interpreter that returns an observable. When executing the expression, it would also call subscriber.next with the result of executing each function, and would also pass the subscriber to the individual functions so they could actually manually invoke subscriber.next for partial values related to their execution. We'd need to pass along some metadata so subscribers would know what function/where in the expression the values are being emitted from.

@streamich streamich added the Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) label Jan 7, 2020
@streamich streamich mentioned this issue Jan 7, 2020
20 tasks
@streamich
Copy link
Contributor

This seems like a big change for expressions, maybe we should schedule some technical meeting discussing pros/cons of various approaches. Also, I'm currently working on refactoring expressions plugin #44196, so if we decide to support observables in expressions it could be relevant for refactoring.

@stacey-gammon stacey-gammon changed the title Change expression fns to return observables instead of promises Rendering partial results when visualizations are backed off the expression language Jan 7, 2020
@stacey-gammon
Copy link
Contributor Author

We had a meeting today about running expressions server side for alerting purposes. I was not thinking about that aspect in this context. When we do start thinking more seriously about this, we should keep that in mind: #50270.

But, this is likely not something we will tackle until 8.x. We will probably have figured out server side expression execution first.

@ppisljar
Copy link
Member

i think we first need to better identify the problem. What exactly do we mean with partial results and where do they make sense ?

imo partial results don't make sense in most cases inside kibana. i think with partial results we want part of results not wrong results ... somebody making decision based on wrong results could have big consequences. With a lot of aggregations we would be showing wrong results, not part of results. I agree it makes sense with specific types of date histograms (but not all of them). I also think there is a huge case for partial results in apps like discover, which are showing documents and not aggregated values like most visualizations do.

Defining and scoping the problem better might help us to better think about the infrastructure needed to support it.

@ppisljar
Copy link
Member

our search infrastructure is close to ready so we are starting active work on adding partial support to expressions in the next minor.

closing in favor of #84051

@exalate-issue-sync exalate-issue-sync bot added impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort labels Apr 20, 2021
@exalate-issue-sync exalate-issue-sync bot added loe:medium Medium Level of Effort and removed loe:small Small Level of Effort labels Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:medium Medium Level of Effort
Projects
None yet
Development

No branches or pull requests

7 participants