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

Add support for range specifications in trace resolver #752

Closed
yunqu opened this issue Mar 30, 2024 · 1 comment
Closed

Add support for range specifications in trace resolver #752

yunqu opened this issue Mar 30, 2024 · 1 comment

Comments

@yunqu
Copy link

yunqu commented Mar 30, 2024

https://github.com/google/perfetto/blob/main/python/perfetto/trace_uri_resolver/resolver.py#L29

The current constraintClass only supports a couple of compactors. The API will parse an input URI into dict, if we specific a URI like "custom:time>123;time<456", from my understanding, the trace resolver won't work properly, since the key "time" in the dict will get overwritten. For example, if you see how the result is parsed: https://github.com/google/perfetto/blob/main/python/perfetto/trace_uri_resolver/resolver.py#L127, my guess is that only 'time<456' will take effect.

I am proposing that we add an additional OP to support ranges. e.g. the op "<>" can be added, so we can specify the above URI like "custom:time<>123,456". Also, the parsed results need to be specified in a list, something like

'key': [ConstraintClass(123, Op.GE), ConstraintClass(456, Op.LE)]

Then our customized trace resolver can take it over from there.

Let me know if this makes sense. I can help contribute the code.

@LalitMaganti
Copy link
Collaborator

Yeah all seems reasonable. Contributions welcome.

primiano pushed a commit that referenced this issue Apr 17, 2024
Feature request: We'd like the URI provided to the trace resolver to support multiple constraints. For example, the current trace URI will not be able to support URIs like 'a>1;a<3', since keys are stored in a dict, and later values will overwrite the previous one. With this change, we can support such cases, because for constraint classes, we are storing their values as an array, where each element will be a constraint object.

The original discussion in #752

Change-Id: I189af4333409e2dba4b891d85d1d83170ff1135f
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

No branches or pull requests

2 participants