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 log min limit option to value axis options #93352

Closed
wants to merge 1 commit into from

Conversation

nickofthyme
Copy link
Contributor

@nickofthyme nickofthyme commented Mar 2, 2021

Summary

fixes #83917

Add option to Y-Axes config in visualize xy charts to set a log min limit to contain log data using an explicit limit and not the kibana implicit limit of 1.

This option is only available when an axes config is linked to one or more non-stacked line series. This is to discourage bad practices for area charts and stacked lines.

Note: When Set log min limit is disabled, the implied value of 1 will be used.

Screen.Recording.2021-04-06.at.06.12.40.PM.mp4

Checklist

For maintainers

@nickofthyme nickofthyme added enhancement New value added to drive a business result Feature:XYAxis XY-Axis charts (bar, area, line) release_note:enhancement labels Mar 3, 2021
@nickofthyme
Copy link
Contributor Author

Waiting on elastic/elastic-charts#1057

@nickofthyme nickofthyme requested a review from markov00 April 6, 2021 23:17
@elastic elastic deleted a comment from kibanamachine Apr 6, 2021
@nickofthyme nickofthyme marked this pull request as ready for review April 6, 2021 23:18
@nickofthyme nickofthyme requested a review from a team April 6, 2021 23:18
@kibanamachine
Copy link
Contributor

kibanamachine commented Apr 6, 2021

💔 Build Failed

Failed CI Steps


Test Failures

Kibana Pipeline / jest / Jest Tests.src/plugins/vis_type_xy/public/editor/components/options/metrics_axes.ValueAxesPanel component should init with the default set of props

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has not failed recently on tracked branches


Stack Trace

Error: expect(received).toMatchSnapshot()

Snapshot name: `ValueAxesPanel component should init with the default set of props 1`

- Snapshot  - 0
+ Received  + 2

@@ -118,10 +118,11 @@
        }
        index={0}
        onValueAxisPositionChanged={[MockFunction]}
        setMultipleValidity={[MockFunction]}
        setParamByIndex={[MockFunction]}
+       showLogOptions={false}
        valueAxis={
          Object {
            "id": "ValueAxis-1",
            "labels": Object {
              "color": "black",
@@ -226,10 +227,11 @@
        }
        index={1}
        onValueAxisPositionChanged={[MockFunction]}
        setMultipleValidity={[MockFunction]}
        setParamByIndex={[MockFunction]}
+       showLogOptions={false}
        valueAxis={
          Object {
            "id": "ValueAxis-2",
            "labels": Object {
              "color": "black",
    at Object.<anonymous> (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/src/plugins/vis_type_xy/public/editor/components/options/metrics_axes/value_axes_panel.test.tsx:67:18)
    at Promise.then.completed (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/utils.js:276:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/utils.js:216:10)
    at _callCircusTest (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:212:40)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at _runTest (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:149:3)
    at _runTestsForDescribeBlock (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:63:9)
    at _runTestsForDescribeBlock (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:57:9)
    at run (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/run.js:25:3)
    at runAndTransformResultsToJestFormat (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:176:21)
    at jestAdapter (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapter.js:109:19)
    at runTestInternal (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-runner/build/runTest.js:380:16)
    at runTest (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-runner/build/runTest.js:472:34)
    at Object.worker (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/node_modules/jest-runner/build/testWorker.js:133:12)

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
visTypeXy 110.9KB 113.2KB +2.3KB

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Member

@markov00 markov00 left a comment

Choose a reason for hiding this comment

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

I've tested it locally and I've to admit that this is a bit too complicated to understand for me.
In particular, I can't understand exactly what is the difference between setting the domain extent to 0.01 or the log min limit to 0.01 what is the actual difference?

@nickofthyme
Copy link
Contributor Author

nickofthyme commented Apr 7, 2021

I've tested it locally and I've to admit that this is a bit too complicated to understand for me.
In particular, I can't understand exactly what is the difference between setting the domain extent to 0.01 or the log min limit to 0.01 what is the actual difference?

The difference is that the log limit is more of a soft limit and the extent is a hard limit. The log limit excludes values that are smaller than the given value. I agree it is not the most obvious but I don't know how else to explain it.

@nickofthyme
Copy link
Contributor Author

Apologies for the delay on this @th0ger. We couldn't find a good way to implement this with adding a new parameter to the vis editor. I opened a new issue in charts repo to set this min log limit automatically instead of hardcoding it to 1 see elastic/elastic-charts#1257.

@nickofthyme nickofthyme deleted the log-options branch July 16, 2021 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Feature:XYAxis XY-Axis charts (bar, area, line) release_note:enhancement v7.15.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Y-axis automatic scaling not working on Log for values <1
4 participants