Skip to content

poc: Filter bot instances using predicate language#58496

Closed
nicholasmarais1158 wants to merge 1 commit intomasterfrom
nicholasmarais1158/poc/bot-instance-filter-expression
Closed

poc: Filter bot instances using predicate language#58496
nicholasmarais1158 wants to merge 1 commit intomasterfrom
nicholasmarais1158/poc/bot-instance-filter-expression

Conversation

@nicholasmarais1158
Copy link
Copy Markdown
Contributor

Summary

This poc seeks to determine if using the Teleport predicate language is a viable mechanism for filtering the list of bot instances in the web app.

Tested using 100k instance records. Using a local cluster, results are generally returned within 120ms. This is no slower that the existing search text filter. Filtering of any sort is heavily dependent on how many records need to be scanned before enough items are matched to fill the page size.

Given the user experience and latency observed, I feel confident that this is a viable approach to advanced filtering.

Demo

Screen.Recording.2025-08-29.at.14.04.33.mov

@nicholasmarais1158 nicholasmarais1158 self-assigned this Aug 29, 2025
return nil, trace.CompareFailed("failed to update bot instance within %v iterations", iterLimit)
}

func MatchBotInstance(b *machineidv1.BotInstance, botName string, search string, exp typical.Expression[*Environment, bool]) bool {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should probably be in lib/services/ and not lib/services/local.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed. I just threw it somewhere to get stuff working.


// ListBotInstances returns a page of BotInstance resources.
ListBotInstances(ctx context.Context, botName string, pageSize int, lastToken string, search string, sort *types.SortBy) ([]*machineidv1.BotInstance, string, error)
ListBotInstances(ctx context.Context, botName string, pageSize int, lastToken string, search string, sort *types.SortBy, query string) ([]*machineidv1.BotInstance, string, error)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@okraport has been working on standardizing our List APIs and might have some opinions on this

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The general idea is to bundle filter related fields into a proto message (example RoleFilter) so that it can be extended without the function signature growing like this.
Since sort and filter are optional we could then use a ...opts functional pattern for all List calls.

It's tricky to do this retroactively, I think in this instance it's understandable to extend the existing API. I will work on a design to standardise these and propose a path forward once I have something concrete. Until then, if we are adding a filter to a List api without preexisting filter terms, let's use a proto message. If we are extending (as is the case here) then it may be necessary evil until we have a backwards compatible migration path.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We could always freeze the current ListBotInstances API and add ListBotInstancesV2 which followed that pattern.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was thinking the same thing when I added query. I'm more than happy to create a new call that uses a message for non-paging related params (filter and sort).

Is the idea to create a V2 call to cater for the possibility that an older proxy version talks to a newer auth service? Are there any other steps that need to be taken to ensure backwards compatibility?

// func (env *Environment) message() *pb.BotInstance { return env.instance }

// TODO Docs for NewBotInstanceExpressionParser
func NewBotInstanceExpressionParser() (*typical.Parser[*Environment, bool], error) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't know that lib/services/local is the right home for this either. The local package is meant to house the storage layer for resources and I don't think there is anything particularly tied to the storage layer about this expression parser.

@nicholasmarais1158
Copy link
Copy Markdown
Contributor Author

Implemented for real in #59374.

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.

3 participants