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

[TSVB] Greater Than or Equal to Interval Pattern #13872

Merged
merged 7 commits into from
Sep 26, 2017

Conversation

simianhacker
Copy link
Member

This PR adds a new interval pattern to TSVB allowing the user to specify an index pattern that is greater than or equal to a specified bucket. For example if the user enters >=1m, TSVB will calculate the auto bucket size for the time period. If that bucket is less than 1m then it will create 1m buckets. If it's greater then it will use the auto generated bucket. This is really useful when collecting metrics at 1m intervals and you don't want to user to zoom in beyond the resolution of the data.

Before:

image

After:

image

This PR also removes the interval validation on the string only. It still checks to make sure you're not specifying more buckets then the system can handle.

@simianhacker simianhacker added Feature:TSVB TSVB (Time Series Visual Builder) Feature:Visualizations Generic visualization features (in case no more specific feature label is available) review v6.0.0 v6.0.0-rc1 v7.0.0 labels Sep 6, 2017
@alexfrancoeur
Copy link

We're eventually going to need to put together documentation for this new functionality. No one is going to know what this option is and that it's available (#13433).

To start we could at least modify the interval label to include a >= example

Copy link
Contributor

@thomasneirynck thomasneirynck left a comment

Choose a reason for hiding this comment

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

Not sure if this >= is the way to go to satisfy this use-case. This approach is not very discoverable and is going to require additional documentation. It is also different from all the other time-formatting in e.g. timelion and Visualize.

imo, the behavior is basically a modifier for the auto duration setting. It ensures you cannot exceed the minimum threshold.

If the selection is auto, could we not have an input for a modifier that says something like: never smaller than: 1m/1w/.... I think it would better communicate this is actually the auto-interval setting.

This is more of a usability aspect, so @alexfrancoeur, feel free to call one way or the other.

Could you also add some tests for this?

@alexfrancoeur
Copy link

++ I agree with @thomasneirynck it is certainly easier to understand with an additional input box if auto is selected. Minimum interval maybe? If we went down this road, this input box would need to be enabled or disabled if auto is selected and would need to be disabled by default. We're running out of real estate here 😄 If we expect this feature to be heavily adopted, additional visibility can only help.

@simianhacker
Copy link
Member Author

The problem with adding additional UI elements is that this component is reused in the series options as well. If you think real estate in the "Panel Config" is an issue, the real estate in the"Series Options" is even more limited. I don't think this use case is going to be widely adopted, most people will just use auto. Having this will be an escape hatch for users who's resolution is >10s. I also think it's OK if this was just a documented interval pattern and we add it to the examples in the label. People who need it will find it and be grateful it's there.

Copy link
Member

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

LGTM, useful functionality, i agree with thomas regarding UI

@thomasneirynck thomasneirynck self-requested a review September 18, 2017 14:04
@thomasneirynck thomasneirynck removed their request for review September 26, 2017 02:40
@simianhacker simianhacker merged commit 893b399 into elastic:master Sep 26, 2017
simianhacker added a commit that referenced this pull request Sep 26, 2017
* Create new interval pattern for specifying greater than or equal to a bucket size

* Updating label to add >=1m example

* Adding test case for gteAutoMatch
simianhacker added a commit to simianhacker/kibana that referenced this pull request Sep 26, 2017
* Create new interval pattern for specifying greater than or equal to a bucket size

* Updating label to add >=1m example

* Adding test case for gteAutoMatch
@simianhacker
Copy link
Member Author

Back ported to 6.0 with dcba340
Back ported to 6.x with 7178b38

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:TSVB TSVB (Time Series Visual Builder) Feature:Visualizations Generic visualization features (in case no more specific feature label is available) review v6.0.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants