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 interfaces for layering data on Visualizations #3108

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import { render } from './render';
import { shape } from './shape';
import { string } from './string';
import { style } from './style';
import { visLayers } from './vis_layers';
import { AnyExpressionTypeDefinition } from '../types';

export const typeSpecs: AnyExpressionTypeDefinition[] = [
Expand All @@ -63,6 +64,7 @@ export const typeSpecs: AnyExpressionTypeDefinition[] = [
shape,
string,
style,
visLayers,
];

export * from './boolean';
Expand All @@ -81,3 +83,4 @@ export * from './render';
export * from './shape';
export * from './string';
export * from './style';
export * from './vis_layers';
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
ohltyler marked this conversation as resolved.
Show resolved Hide resolved
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import { ExpressionTypeDefinition } from '../types';
import { VisLayers } from '../../../../vis_augmenter/common';

const name = 'vis_layers';

export interface ExprVisLayers {
type: typeof name;
layers: VisLayers;
}

// Setting default empty arrays for null & undefined edge cases
export const visLayers: ExpressionTypeDefinition<typeof name, ExprVisLayers> = {
name,
from: {
null: () => {
return {
type: name,
layers: [] as VisLayers,
} as ExprVisLayers;
},
undefined: () => {
return {
type: name,
layers: [] as VisLayers,
} as ExprVisLayers;
},
},
};
1 change: 1 addition & 0 deletions src/plugins/expressions/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,5 +132,6 @@ export {
TypeString,
TypeToString,
UnmappedTypeStrings,
ExprVisLayers,
ExpressionValueRender as Render,
} from '../common';
6 changes: 6 additions & 0 deletions src/plugins/vis_augmenter/common/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

export * from './types';
33 changes: 33 additions & 0 deletions src/plugins/vis_augmenter/common/types.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import { VisLayer, isPointInTimeEventsVisLayer } from './types';

describe('isPointInTimeEventsVisLayer()', function () {
it('should return false if no events field', function () {
const visLayer = {
id: 'visLayerId',
name: 'visLayerName',
field1: 'value1',
field2: 'value2',
} as VisLayer;
expect(isPointInTimeEventsVisLayer(visLayer)).toBe(false);
});

it('should return true if events field exists', function () {
const visLayer = {
id: 'testId',
name: 'testName',
events: [
{
timestamp: 123,
resourceId: 'testId',
resourceName: 'testName',
},
],
} as VisLayer;
expect(isPointInTimeEventsVisLayer(visLayer)).toBe(true);
});
});
50 changes: 50 additions & 0 deletions src/plugins/vis_augmenter/common/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import { ExpressionFunctionDefinition } from '../../expressions';

export interface VisLayer {
// will be used as the column ID
id: string;
// will be used as the name when hovering over the tooltip
name: string;
}

export type VisLayers = VisLayer[];
joshuarrrr marked this conversation as resolved.
Show resolved Hide resolved

// resourceId & resourceName are required so that the
// events flyout can partition data based on these attributes
// (e.g., partitioning anomalies based on the detector they came from)
export interface PointInTimeEventMetadata {
ohltyler marked this conversation as resolved.
Show resolved Hide resolved
resourceId: string;
resourceName: string;
tooltip?: string;
joshuarrrr marked this conversation as resolved.
Show resolved Hide resolved
}

export interface PointInTimeEvent {
timestamp: number;
metadata: PointInTimeEventMetadata;
}

export interface PointInTimeEventsVisLayer extends VisLayer {
events: PointInTimeEvent[];
}

// used to determine what vis layer's interface is being implemented.
// currently PointInTimeEventsLayer is the only interface extending VisLayer
export const isPointInTimeEventsVisLayer = (obj: any) => {
return 'events' in obj;
};
Copy link
Member

Choose a reason for hiding this comment

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

How are you imagining this to be used? It seems like it would be simpler to have a static type string on the VisLayer interface that you can specify when defining a particular type of VisLayer, and then just access as a value. This type of convenient method may still be helpful, but it's actually implemented as more a capability check.

Copy link
Member Author

@ohltyler ohltyler Dec 24, 2022

Choose a reason for hiding this comment

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

Agreed, existing soln seemed hacky. Updated to use enums in latest commits. Also added a general isValidVisLayer fn that can be ran when verifying the new saved object types are valid or not, based on expression function type


export interface VisLayerResponseValue {
visLayers: object;
}

export type VisLayerFunctionDefinition = ExpressionFunctionDefinition<
string,
VisLayerResponseValue,
any,
Promise<VisLayerResponseValue>
>;