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

Support in-page-search in trace view on key=value pairs #1391

Merged
merged 5 commits into from Jun 4, 2023

Conversation

bobrik
Copy link
Contributor

@bobrik bobrik commented May 3, 2023

Previously one could only filter on key or value. This wasn't useful when one needed to find spans with result=false in a trace view.

With this change one can use key=value as a search term, for example:

  • http.method=GET

Before (no matches):

image

After (a successful match):

image

@bobrik
Copy link
Contributor Author

bobrik commented May 3, 2023

I'm happy to add tests if this addition makes sense.

@yurishkuro
Copy link
Member

a bit dirty (i.e. would also match on value="a=b"), but seems useful regardless. +1

@yurishkuro
Copy link
Member

Overall the in-page search functionality is not very discoverable. When it was just a direct string match it was less of an issue, but if supporting a=b format it would be good to have some help tip somewhere.

Q: in the backend search form we support multiple space-separated tag queries, would be nice to allow the same syntax for in-page search. I believe we even have a parser for the syntax in the UI.

@bobrik bobrik force-pushed the ivan/key=value-filter branch 2 times, most recently from aefe32f to f785fec Compare May 5, 2023 04:22
bobrik added 2 commits May 4, 2023 21:28
Previously one could only filter on `key` or `value`. This wasn't useful
when one needed to find spans with `result=false` in a trace view.

With this change one can use `key=value` as a search term, for example:

* `http.method=GET`

Signed-off-by: Ivan Babrou <github@ivan.computer>
Signed-off-by: Ivan Babrou <github@ivan.computer>
@codecov
Copy link

codecov bot commented May 5, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.03 ⚠️

Comparison is base (9924af6) 95.60% compared to head (2ddc2b3) 95.58%.

❗ Current head 2ddc2b3 differs from pull request most recent head df89c77. Consider uploading reports for the commit df89c77 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1391      +/-   ##
==========================================
- Coverage   95.60%   95.58%   -0.03%     
==========================================
  Files         244      244              
  Lines        7653     7657       +4     
  Branches     2006     2008       +2     
==========================================
+ Hits         7317     7319       +2     
- Misses        336      338       +2     
Impacted Files Coverage Δ
...s/TracePage/TracePageHeader/TracePageSearchBar.tsx 100.00% <100.00%> (ø)
packages/jaeger-ui/src/utils/filter-spans.tsx 96.77% <100.00%> (+0.22%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@bobrik
Copy link
Contributor Author

bobrik commented May 5, 2023

I added some tests for the key=value syntax.

Here's the tooltip I added in the second commit:

image

Using the same syntax as the tag filter on the search page would mean that we lose -field syntax, so it might not be a desired change. Trace view search also looks at more things than just tags, which might not be compatible with what we have in the regular search. I'd also say that it's outside of my expertise, since I'm not a frontend person.

@bobrik
Copy link
Contributor Author

bobrik commented May 15, 2023

@yurishkuro, any comments on the latest update?

Signed-off-by: Yuri Shkuro <github@ysh.us>
To preclude certain key-value pairs from participating in the matching, prefix the key
with the minus <code>'-'</code> sign, e.g., <code>-http.status_code</code>.
</p>
</div>
Copy link
Member

Choose a reason for hiding this comment

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

I rewrote the tooltip to make the explanation a bit more clear and to remove the styling applied to some key words which was difficult to read on the dark background.

Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
@yurishkuro yurishkuro changed the title Allow filtering on key=value pairs in trace view Support in-page-search in trace view on key=value pairs Jun 4, 2023
@yurishkuro yurishkuro merged commit 77203ce into jaegertracing:main Jun 4, 2023
6 checks passed
@yurishkuro
Copy link
Member

Sorry it took a while to review, a busy month.

ramumanam pushed a commit to ramumanam/jaeger-ui that referenced this pull request Jun 6, 2023
…g#1391)

Previously one could only filter on `key` or `value`. This wasn't useful
when one needed to find spans with `result=false` in a trace view.

With this change one can use `key=value` as a search term, for example:

* `http.method=GET`

Before (no matches):

<img width="1231" alt="image"
src="https://user-images.githubusercontent.com/89186/236012512-f97a67e8-8bf8-4372-b698-d0f34383b441.png">

After (a successful match):

<img width="1231" alt="image"
src="https://user-images.githubusercontent.com/89186/236012540-7c011a81-ca63-41d2-9051-cad4c5eab02e.png">

---------

Signed-off-by: Ivan Babrou <github@ivan.computer>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Co-authored-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: RAMU MANAM <manam.ramu@uber.com>
@bobrik bobrik deleted the ivan/key=value-filter branch June 10, 2023 18:34
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.

None yet

2 participants