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

Create switch to render line charts using vega-lite #3106

Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import { i18n } from '@osd/i18n';

import { VisOptionsProps } from 'src/plugins/vis_default_editor/public';
import { getNotifications } from '../services';
import { VisParams } from '../vega_fn';
import { VisParams } from '../expressions/vega_fn';
import { VegaHelpMenu } from './vega_help_menu';
import { VegaActionsMenu } from './vega_actions_menu';

Expand Down
243 changes: 243 additions & 0 deletions src/plugins/vis_type_vega/public/expressions/line_vega_spec_fn.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,243 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import { cloneDeep } from 'lodash';
import { i18n } from '@osd/i18n';
import {
ExpressionFunctionDefinition,
OpenSearchDashboardsDatatable,
OpenSearchDashboardsDatatableColumn,
} from '../../../expressions/public';
import { VegaVisualizationDependencies } from '../plugin';
import { VislibDimensions, VisParams } from '../../../visualizations/public';

type Input = OpenSearchDashboardsDatatable;
type Output = Promise<string>;

interface Arguments {
visLayers: string | null;
visParams: string;
dimensions: string;
}

export type VegaSpecExpressionFunctionDefinition = ExpressionFunctionDefinition<
'line_vega_spec',
Input,
Arguments,
Output
>;

// Get the first xaxis field as only 1 setup of X Axis will be supported and
// there won't be support for split series and split chart
const getXAxisId = (dimensions: any, columns: OpenSearchDashboardsDatatableColumn[]): string => {
return columns.filter((column) => column.name === dimensions.x.label)[0].id;
};

const cleanString = (rawString: string): string => {
return rawString.replaceAll('"', '');
};

const formatDataTable = (
datatable: OpenSearchDashboardsDatatable
): OpenSearchDashboardsDatatable => {
datatable.columns.forEach((column) => {
// clean quotation marks from names in columns
column.name = cleanString(column.name);
});
return datatable;
};

const createSpecFromDatatable = (
datatable: OpenSearchDashboardsDatatable,
visParams: VisParams,
dimensions: VislibDimensions
): object => {
// TODO: we can try to use VegaSpec type but it is currently very outdated, where many
// of the fields and sub-fields don't have other optional params that we want for customizing.
// For now, we make this more loosely-typed by just specifying it as a generic object.
const spec = {} as any;

const legendPosition = visParams.legendPosition;

spec.$schema = 'https://vega.github.io/schema/vega-lite/v5.json';
spec.data = {
values: datatable.rows,
};
spec.config = {
view: {
stroke: null,
},
concat: {
spacing: 0,
},
// the circle timeline representing annotations
circle: {
color: 'blue',
},
// the vertical line when user hovers over an annotation circle
rule: {
color: 'red',
},
lezzago marked this conversation as resolved.
Show resolved Hide resolved
legend: {
orient: legendPosition,
},
};

// Get the valueAxes data and generate a map to easily fetch the different valueAxes data
const valueAxis = {};
visParams?.valueAxes.forEach((yAxis: { id: { toString: () => string | number } }) => {
lezzago marked this conversation as resolved.
Show resolved Hide resolved
// @ts-ignore
valueAxis[yAxis.id.toString()] = yAxis;
});

spec.layer = [] as any[];

if (datatable.rows.length > 0 && dimensions.x != null) {
Copy link
Member

Choose a reason for hiding this comment

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

do we have error handling for the else condition?

Copy link
Member Author

Choose a reason for hiding this comment

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

Essentially this is an empty chart which would be expected when they didn't define the x axis yet. There is a unit test for this

Copy link
Member

Choose a reason for hiding this comment

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

I think there's a more standard error-rendering embeddable pattern to follow - but not a blocker, I can make a follow-up issue.

const xAxisId = getXAxisId(dimensions, datatable.columns);
const xAxisTitle = cleanString(dimensions.x.label);
const startTime = new Date(dimensions.x.params.bounds.min).valueOf();
const endTime = new Date(dimensions.x.params.bounds.max).valueOf();
lezzago marked this conversation as resolved.
Show resolved Hide resolved
let skip = 0;
datatable.columns.forEach((column, index) => {
// Check if its not xAxis column data
lezzago marked this conversation as resolved.
Show resolved Hide resolved
if (column.meta?.aggConfigParams?.interval != null) {
lezzago marked this conversation as resolved.
Show resolved Hide resolved
skip++;
} else {
const currentSeriesParams = visParams.seriesParams[index - skip];
const currentValueAxis =
// @ts-ignore
valueAxis[currentSeriesParams.valueAxis.toString()];
let tooltip: Array<{ field: string; type: string; title: string }> = [];
lezzago marked this conversation as resolved.
Show resolved Hide resolved
if (visParams.addTooltip) {
tooltip = [
{ field: xAxisId, type: 'temporal', title: xAxisTitle },
{ field: column.id, type: 'quantitative', title: column.name },
];
}
spec.layer.push({
mark: {
// Possible types are: line, area, histogram. The eligibility checker will
// prevent area and histogram (though area works in vega-lite)
type: currentSeriesParams.type,
// Possible types: linear, cardinal, step-after. All of these types work in vega-lite
interpolate: currentSeriesParams.interpolate,
// The possible values is any number, which matches what vega-lite supports
strokeWidth: currentSeriesParams.lineWidth,
// this corresponds to showing the dots in the visbuilder for each data point
point: currentSeriesParams.showCircles,
},
lezzago marked this conversation as resolved.
Show resolved Hide resolved
encoding: {
x: {
axis: {
title: xAxisTitle,
grid: visParams.grid.categoryLines,
},
field: xAxisId,
type: 'temporal',
lezzago marked this conversation as resolved.
Show resolved Hide resolved
scale: {
domain: [startTime, endTime],
Copy link
Member

Choose a reason for hiding this comment

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

just to confirm - this won't shrink the chart bounds to fit such that there is no empty positions at the beginning and end of the chart, right? I see you had a comment referencing a different approach here #3106 (comment)

Also, I think we still need time buckets even if there is empty data from the charts (empty rows). This is so the events can still fit into such empty buckets if that is the closest bucket based on their timestamps. Typically this wouldn't be the case with associated plugin resources, but because we are allowing unrelated resources to be added to the charts, it's possible that we have events even with empty data shown for a particular vis

Copy link
Member

Choose a reason for hiding this comment

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

If you weren't able to find a good solution via agg params, I think we will need a helper fn and artificially add rows to fit the full bounds.

Copy link
Member

Choose a reason for hiding this comment

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

Discussed offline - this won't shrink the chart bounds. We can keep the domain as temporal.

As a follow up I'll need to create a helper fn to add empty rows for potential event datapoints

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought that the previous approach was needed since there were some local issues with using scale and domain. The line charts actually do something very similar and dont create extra buckets. We will need to create a helper fn to add rows as changing agg params can cause a lot of unknown side effects.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, just adding rows after-the-fact is what we need, we should not touch the agg params

},
},
y: {
axis: {
title: cleanString(currentValueAxis.title.text) || column.name,
lezzago marked this conversation as resolved.
Show resolved Hide resolved
grid: visParams.grid.valueAxis !== '',
Copy link
Member

Choose a reason for hiding this comment

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

why an empty string instead of undefined?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for pointing this out. There was a bug here and changed this to visParams.grid.valueAxis as it would be true if its there and false if its not there.

orient: currentValueAxis.position,
labels: currentValueAxis.labels.show,
labelAngle: currentValueAxis.labels.rotate,
},
field: column.id,
type: 'quantitative',
},
tooltip,
color: {
datum: column.name,
},
lezzago marked this conversation as resolved.
Show resolved Hide resolved
},
});
}
});
}

if (visParams.addTimeMarker) {
spec.transform = [
{
calculate: 'now()',
as: 'now_field',
},
];

spec.layer.push({
mark: 'rule',
encoding: {
x: {
type: 'temporal',
field: 'now_field',
},
color: {
value: 'red',
},
lezzago marked this conversation as resolved.
Show resolved Hide resolved
size: {
value: 1,
},
},
});
}

if (visParams.thresholdLine.show as boolean) {
spec.layer.push({
mark: {
type: 'rule',
color: visParams.thresholdLine.color,
},
encoding: {
y: {
datum: visParams.thresholdLine.value,
},
},
});
}

return spec;
};

export const createVegaSpecFn = (
lezzago marked this conversation as resolved.
Show resolved Hide resolved
dependencies: VegaVisualizationDependencies
): VegaSpecExpressionFunctionDefinition => ({
name: 'line_vega_spec',
type: 'string',
inputTypes: ['opensearch_dashboards_datatable'],
help: i18n.translate('visTypeVega.function.help', {
defaultMessage: 'Construct vega spec',
lezzago marked this conversation as resolved.
Show resolved Hide resolved
}),
args: {
visLayers: {
types: ['string', 'null'],
default: '',
help: '',
},
visParams: {
types: ['string'],
default: '""',
help: '',
},
dimensions: {
types: ['string'],
default: '""',
help: '',
},
},
async fn(input, args, context) {
const table = cloneDeep(input);

// creating initial vega spec from table
const spec = createSpecFromDatatable(
formatDataTable(table),
JSON.parse(args.visParams),
JSON.parse(args.dimensions)
);
return JSON.stringify(spec);
},
});
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@ import {
ExpressionFunctionDefinition,
OpenSearchDashboardsContext,
Render,
} from '../../expressions/public';
import { VegaVisualizationDependencies } from './plugin';
import { createVegaRequestHandler } from './vega_request_handler';
import { VegaInspectorAdapters } from './vega_inspector/index';
import { TimeRange, Query } from '../../data/public';
import { VisRenderValue } from '../../visualizations/public';
import { VegaParser } from './data_model/vega_parser';
} from '../../../expressions/public';
import { VegaVisualizationDependencies } from '../plugin';
import { createVegaRequestHandler } from '../vega_request_handler';
import { VegaInspectorAdapters } from '../vega_inspector';
import { TimeRange, Query } from '../../../data/public';
import { VisRenderValue } from '../../../visualizations/public';
import { VegaParser } from '../data_model/vega_parser';

type Input = OpenSearchDashboardsContext | null;
type Output = Promise<Render<RenderValue>>;
Expand All @@ -52,6 +52,14 @@ interface Arguments {

export type VisParams = Required<Arguments>;

export type VegaExpressionFunctionDefinition = ExpressionFunctionDefinition<
'vega',
Input,
Arguments,
Output,
ExecutionContext<unknown, VegaInspectorAdapters>
>;

interface RenderValue extends VisRenderValue {
visData: VegaParser;
visType: 'vega';
Expand All @@ -60,13 +68,7 @@ interface RenderValue extends VisRenderValue {

export const createVegaFn = (
dependencies: VegaVisualizationDependencies
): ExpressionFunctionDefinition<
'vega',
Input,
Arguments,
Output,
ExecutionContext<unknown, VegaInspectorAdapters>
> => ({
): VegaExpressionFunctionDefinition => ({
name: 'vega',
type: 'render',
inputTypes: ['opensearch_dashboards_context', 'null'],
Expand Down
3 changes: 3 additions & 0 deletions src/plugins/vis_type_vega/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,6 @@ import { VegaPlugin as Plugin } from './plugin';
export function plugin(initializerContext: PluginInitializerContext<ConfigSchema>) {
return new Plugin(initializerContext);
}

export { VegaExpressionFunctionDefinition } from './expressions/vega_fn';
export { VegaSpecExpressionFunctionDefinition } from './expressions/line_vega_spec_fn';
lezzago marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 3 additions & 1 deletion src/plugins/vis_type_vega/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,14 @@ import {
setInjectedMetadata,
} from './services';

import { createVegaFn } from './vega_fn';
import { createVegaFn } from './expressions/vega_fn';
import { createVegaTypeDefinition } from './vega_type';
import { IServiceSettings } from '../../maps_legacy/public';
import './index.scss';
import { ConfigSchema } from '../config';

import { getVegaInspectorView } from './vega_inspector';
import { createVegaSpecFn } from './expressions/line_vega_spec_fn';
lezzago marked this conversation as resolved.
Show resolved Hide resolved

/** @internal */
export interface VegaVisualizationDependencies {
Expand Down Expand Up @@ -104,6 +105,7 @@ export class VegaPlugin implements Plugin<Promise<void>, void> {
inspector.registerView(getVegaInspectorView({ uiSettings: core.uiSettings }));

expressions.registerFunction(() => createVegaFn(visualizationDependencies));
expressions.registerFunction(() => createVegaSpecFn(visualizationDependencies));

visualizations.createBaseVisualization(createVegaTypeDefinition(visualizationDependencies));
}
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/vis_type_vega/public/vega_request_handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import { SearchAPI } from './data_model/search_api';
import { TimeCache } from './data_model/time_cache';

import { VegaVisualizationDependencies } from './plugin';
import { VisParams } from './vega_fn';
import { VisParams } from './expressions/vega_fn';
import { getData, getInjectedMetadata } from './services';
import { VegaInspectorAdapters } from './vega_inspector';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,8 @@ export class VegaBaseView {
* Set global debug variable to simplify vega debugging in console. Show info message first time
*/
setDebugValues(view, spec, vlspec) {
this._parser.searchAPI.inspectorAdapters?.vega.bindInspectValues({
// The vega inspector can now be null when rendering line charts using vega for the overlay visualization feature
this._parser.searchAPI.inspectorAdapters?.vega?.bindInspectValues({
lezzago marked this conversation as resolved.
Show resolved Hide resolved
view,
spec: vlspec || spec,
});
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/vis_type_vislib/opensearch_dashboards.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"version": "opensearchDashboards",
"server": true,
"ui": true,
"requiredPlugins": ["charts", "data", "expressions", "visualizations", "opensearchDashboardsLegacy"],
"requiredPlugins": ["charts", "data", "expressions", "visualizations", "opensearchDashboardsLegacy", "visTypeVega"],
lezzago marked this conversation as resolved.
Show resolved Hide resolved
"optionalPlugins": ["visTypeXy"],
"requiredBundles": ["opensearchDashboardsUtils", "visDefaultEditor"]
}
2 changes: 2 additions & 0 deletions src/plugins/vis_type_vislib/public/line.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import { createVislibVisController } from './vis_controller';
import { VisTypeVislibDependencies } from './plugin';
import { Rotates } from '../../charts/public';
import { VIS_EVENT_TO_TRIGGER } from '../../visualizations/public';
import { toExpressionAst } from './line_to_expression';

export const createLineVisTypeDefinition = (deps: VisTypeVislibDependencies) => ({
name: 'line',
Expand All @@ -58,6 +59,7 @@ export const createLineVisTypeDefinition = (deps: VisTypeVislibDependencies) =>
description: i18n.translate('visTypeVislib.line.lineDescription', {
defaultMessage: 'Emphasize trends',
}),
toExpressionAst,
lezzago marked this conversation as resolved.
Show resolved Hide resolved
visualization: createVislibVisController(deps),
getSupportedTriggers: () => {
return [VIS_EVENT_TO_TRIGGER.filter, VIS_EVENT_TO_TRIGGER.brush];
Expand Down
Loading