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 jaeger-ui search configuration jaegertracing/jaeger-ui#511 #348

Merged
merged 4 commits into from
Feb 16, 2020

Conversation

GabrielDyck
Copy link
Contributor

Which problem is this PR solving?

Related to jaeger-ui PR jaegertracing/jaeger-ui#511

Short description of the changes

I've added documentation for search.maxLimit input. I've noticed that maxLookback didn't have any documentation so I added.

Let me know if it's ok

Thank you so much!
Gabriel Fernando Dyck

Signed-off-by: Gabriel Fernando Dyck <[email protected]>
@GabrielDyck GabrielDyck changed the title Add jaeger-ui search configuration https://github.com/jaegertracing/jaeger-ui/pull/511 Add jaeger-ui search configuration jaegertracing/jaeger-ui#511 Jan 16, 2020
Signed-off-by: Gabriel Fernando Dyck <[email protected]>
@@ -12,7 +12,8 @@ Several aspects of the UI can be configured:
* The Dependencies section can be enabled / configured
* Google Analytics tracking can be enabled / configured
* Additional menu options can be added to the global nav

* Search input can be configured
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Search input can be configured
* Search defaults can be configured

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case, it isn't a default value, it's the max value that the input(maxLimit and maxLookback) can take.

Copy link
Member

Choose a reason for hiding this comment

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

then let's change to "search input limits", because we're not configuring "inputs"

@@ -92,6 +100,11 @@ Links can either be members of the `menu` Array, directly, or they can be groupe

The `items` Array should contain one or more link configurations.

### Search Input

`search.maxLookback` allows to define the max look back interval that the ui will show in the select input.
Copy link
Member

Choose a reason for hiding this comment

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

What are the valid values for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mmm, I'm not sure about that. I'll check

Choose a reason for hiding this comment

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

search.maxLookback configures the maximum time before the present users can query for traces.

Field Description
label The text displayed in the search form dropdown
value The limit used to truncate the lookbackOptions in SearchForm.js, and the value submitted in the search query if the label is selected

Raw for above markdown:

Field | Description
------|------------
label | The text displayed in the search form dropdown
value | The limit used to truncate the `lookbackOptions` in `SearchForm.js`, and the value submitted in the search query if the label is selected

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've just added this. Let me know for any changes.
Thanks a lot @everett980!

@jpkrohling
Copy link
Contributor

jpkrohling commented Jan 24, 2020

@GabrielDyck, do you need some help with moving forward with this one?

@GabrielDyck
Copy link
Contributor Author

@GabrielDyck, do you need some help with moving forward with this one?

Hello @jpkrohling . Yes, I do. I'm not sure about the valid values maxLookback can take. Do you know the valid values?
I appreciate your help.
Thank you!

@jpkrohling
Copy link
Contributor

@GabrielDyck looks like @everett980 added this as a comment a few hours ago: #348 (comment)

Let me know if need any further help!

@GabrielDyck GabrielDyck force-pushed the master branch 2 times, most recently from 9f9bbf6 to 4d7b8e2 Compare February 15, 2020 20:44
@@ -92,6 +100,17 @@ Links can either be members of the `menu` Array, directly, or they can be groupe

The `items` Array should contain one or more link configurations.

### Search Input
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
### Search Input
### Search Input Limits


The `search.maxLimit` configures the maximum results that the input let you search.

The `search.maxLookback` configures the maximum time before the present users can query for traces.
Copy link
Member

@yurishkuro yurishkuro Feb 15, 2020

Choose a reason for hiding this comment

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

Suggested change
The `search.maxLookback` configures the maximum time before the present users can query for traces.
The `search.maxLookback` configures the maximum time before the present users can query for traces. The options in the Lookback dropdown greater than this value will not be shown.

Field | Description
------|------------
label | The text displayed in the search form dropdown
value | The limit used to truncate the `lookbackOptions` in `SearchForm.js`, and the value submitted in the search query if the label is selected
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
value | The limit used to truncate the `lookbackOptions` in `SearchForm.js`, and the value submitted in the search query if the label is selected
value | The value submitted in the search query if the label is selected

Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

Thanks!

@yurishkuro yurishkuro merged commit d6ba55e into jaegertracing:master Feb 16, 2020
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.

4 participants