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

Closes #13471 - Change split behavior of gauges and metrics vis #13583

Merged
merged 3 commits into from
Aug 28, 2017

Conversation

simianhacker
Copy link
Member

This PR closes #13471 by changing the behavior of gauges and metrics visualizations. Before this PR when you split a series by a terms (or filters) aggregation they would just show up in the primary (and secondary) metric positions for the visualizations. With this PR it will create a gauge for each split.

image

@simianhacker simianhacker added Feature:TSVB TSVB (Time Series Visual Builder) Feature:Visualizations Generic visualization features (in case no more specific feature label is available) bug Fixes for quality problems that affect the customer experience review v6.0.0 v6.0.0-rc1 v7.0.0 labels Aug 18, 2017
@simianhacker
Copy link
Member Author

Jenkins test this

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.

nice! works really well.

This is not a bug fix IMHO, more of an enhancement, and I would remove 6.0 label, leaving this as a 6.1 enhancement.

@ppisljar do you want to take second look?

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

@simianhacker simianhacker merged commit 9257f21 into elastic:master Aug 28, 2017
simianhacker added a commit that referenced this pull request Aug 28, 2017
* Closes #13471 - Change split behavior of gauges and metrics vis

* removing old css classes
simianhacker added a commit that referenced this pull request Aug 28, 2017
* Closes #13471 - Change split behavior of gauges and metrics vis

* removing old css classes
@simianhacker
Copy link
Member Author

Back ported to 6.0 with 3cb25ad
Back ported to 6.x with 24c7fdf

export function visWithSplits(WrappedComponent) {
function SplitVisComponent(props) {
const { model, visData } = props;
if (visData[model.id].series.every(s => s.id.split(':').length === 1)) {
Copy link
Contributor

@shaharmor shaharmor Sep 12, 2017

Choose a reason for hiding this comment

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

@simianhacker I think that this change introduced a bug.
Before this PR, the .series property was being defaulted to [] here, and now its not.

Maybe do something like this:

import { get, last, findIndex, first } from 'lodash';

function SplitVisComponent(props) {
  const { model, visData } = props;
  const series = get(visData, `${model.id}.series`, []);
// ...

and erase all visData[model.id].

@simianhacker simianhacker deleted the tsvb-split-gauge-metric branch April 17, 2024 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience 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-rc1 v6.0.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Group-by in tsvb should show multiple gauges
4 participants