Skip to content

Commit

Permalink
Adding support of default_comparison (#3346)
Browse files Browse the repository at this point in the history
* Adding support of default_comparison

* Adding E2E tests

* PR comments

* Adding unit tests

* Handling empty dimension string

* PR comments

* Fix non timestamp case
  • Loading branch information
AdityaHegde authored Nov 2, 2023
1 parent 56b87f7 commit c7aef1d
Show file tree
Hide file tree
Showing 13 changed files with 788 additions and 388 deletions.
833 changes: 462 additions & 371 deletions proto/gen/rill/runtime/v1/resources.pb.go

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions proto/gen/rill/runtime/v1/resources.pb.validate.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions proto/gen/rill/runtime/v1/runtime.swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2359,6 +2359,14 @@ definitions:
type: object
$ref: '#/definitions/SecurityFieldCondition'
title: Security for the metrics view
MetricsViewSpecComparisonMode:
type: string
enum:
- COMPARISON_MODE_UNSPECIFIED
- COMPARISON_MODE_NONE
- COMPARISON_MODE_TIME
- COMPARISON_MODE_DIMENSION
default: COMPARISON_MODE_UNSPECIFIED
MetricsViewSpecDimensionV2:
type: object
properties:
Expand Down Expand Up @@ -3589,6 +3597,12 @@ definitions:
type: integer
format: int64
description: Month number to use as the base for time aggregations by year. Defaults to 1 (January).
defaultComparisonMode:
$ref: '#/definitions/MetricsViewSpecComparisonMode'
description: Selected default comparison mode.
defaultComparisonDimension:
type: string
title: If comparison mode is dimension then this determines which is the default dimension
v1MetricsViewState:
type: object
properties:
Expand Down
10 changes: 10 additions & 0 deletions proto/rill/runtime/v1/resources.proto
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,12 @@ message MetricsViewSpec {
repeated FieldConditionV2 include = 3;
repeated FieldConditionV2 exclude = 4;
}
enum ComparisonMode {
COMPARISON_MODE_UNSPECIFIED = 0;
COMPARISON_MODE_NONE = 1;
COMPARISON_MODE_TIME = 2;
COMPARISON_MODE_DIMENSION = 3;
}
// Connector containing the table
string connector = 1;
// Name of the table the metrics view is based on
Expand All @@ -180,6 +186,10 @@ message MetricsViewSpec {
uint32 first_day_of_week = 12;
// Month number to use as the base for time aggregations by year. Defaults to 1 (January).
uint32 first_month_of_year = 13;
// Selected default comparison mode.
ComparisonMode default_comparison_mode = 14;
// If comparison mode is dimension then this determines which is the default dimension
string default_comparison_dimension = 15;
}

message MetricsViewState {
Expand Down
27 changes: 27 additions & 0 deletions runtime/compilers/rillv1/parse_metrics_view.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,20 @@ type MetricsViewYAML struct {
Condition string `yaml:"if"`
}
}
DefaultComparison struct {
Mode string `yaml:"mode"`
Dimension string `yaml:"dimension"`
} `yaml:"default_comparison"`
}

var comparisonModesMap = map[string]runtimev1.MetricsViewSpec_ComparisonMode{
"": runtimev1.MetricsViewSpec_COMPARISON_MODE_UNSPECIFIED,
"none": runtimev1.MetricsViewSpec_COMPARISON_MODE_NONE,
"time": runtimev1.MetricsViewSpec_COMPARISON_MODE_TIME,
"dimension": runtimev1.MetricsViewSpec_COMPARISON_MODE_DIMENSION,
}
var validComparisonModes = []string{"none", "time", "dimension"}

// parseMetricsView parses a metrics view (dashboard) definition and adds the resulting resource to p.Resources.
func (p *Parser) parseMetricsView(ctx context.Context, node *Node) error {
// Parse YAML
Expand Down Expand Up @@ -181,6 +193,16 @@ func (p *Parser) parseMetricsView(ctx context.Context, node *Node) error {
return fmt.Errorf("must define at least one measure")
}

tmp.DefaultComparison.Mode = strings.ToLower(tmp.DefaultComparison.Mode)
if _, ok := comparisonModesMap[tmp.DefaultComparison.Mode]; !ok {
return fmt.Errorf("invalid mode: %q. allowed values: %s", tmp.DefaultComparison.Mode, strings.Join(validComparisonModes, ","))
}
if tmp.DefaultComparison.Dimension != "" {
if ok := names[tmp.DefaultComparison.Dimension]; !ok {
return fmt.Errorf("default comparison dimension %q doesn't exist", tmp.DefaultComparison.Dimension)
}
}

if tmp.Security != nil {
templateData := TemplateData{User: map[string]interface{}{
"name": "dummy",
Expand Down Expand Up @@ -313,6 +335,11 @@ func (p *Parser) parseMetricsView(ctx context.Context, node *Node) error {
})
}

spec.DefaultComparisonMode = comparisonModesMap[tmp.DefaultComparison.Mode]
if tmp.DefaultComparison.Dimension != "" {
spec.DefaultComparisonDimension = tmp.DefaultComparison.Dimension
}

if tmp.Security != nil {
if spec.Security == nil {
spec.Security = &runtimev1.MetricsViewSpec_SecurityV2{}
Expand Down
3 changes: 2 additions & 1 deletion web-common/src/features/dashboards/proto-state/toProto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ export function getProtoFromDashboardState(
if (metrics.expandedMeasureName) {
state.expandedMeasure = metrics.expandedMeasureName;
}
if (metrics.selectedDimensionName) {
if (metrics.selectedDimensionName && !state.showTimeComparison) {
// TODO: we need an enum to decide what is being selected for comparison. This should be reflected in proto as well.
state.selectedDimension = metrics.selectedDimensionName;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@ import { isoDurationToFullTimeRange } from "@rilldata/web-common/lib/time/ranges
import type { TimeComparisonOption } from "@rilldata/web-common/lib/time/types";
import type {
V1ColumnTimeRangeResponse,
V1MetricsView,
V1MetricsViewSpec,
} from "@rilldata/web-common/runtime-client";
import { MetricsViewSpecComparisonMode } from "@rilldata/web-common/runtime-client";
import { get } from "svelte/store";

export function setDefaultTimeRange(
metricsView: V1MetricsView,
metricsView: V1MetricsViewSpec,
metricsExplorer: MetricsExplorerEntity,
fullTimeRange: V1ColumnTimeRangeResponse | undefined
) {
Expand All @@ -42,14 +43,39 @@ export function setDefaultTimeRange(
// TODO: refactor all sub methods and call setSelectedScrubRange here
metricsExplorer.selectedScrubRange = undefined;
metricsExplorer.lastDefinedScrubRange = undefined;
setDefaultComparisonTimeRange(metricsView, metricsExplorer, fullTimeRange);
}

function setDefaultComparison(
metricsView: V1MetricsViewSpec,
metricsExplorer: MetricsExplorerEntity,
fullTimeRange: V1ColumnTimeRangeResponse | undefined
) {
switch (metricsView.defaultComparisonMode) {
case MetricsViewSpecComparisonMode.COMPARISON_MODE_DIMENSION:
metricsExplorer.selectedComparisonDimension =
metricsView.defaultComparisonDimension ||
metricsView.dimensions?.[0]?.name;
break;

// if default_comparison is not specified it defaults to time comparison
case MetricsViewSpecComparisonMode.COMPARISON_MODE_UNSPECIFIED:
case MetricsViewSpecComparisonMode.COMPARISON_MODE_TIME:
setDefaultComparisonTimeRange(
metricsView,
metricsExplorer,
fullTimeRange
);
break;
}
}

function setDefaultComparisonTimeRange(
metricsView: V1MetricsView,
metricsView: V1MetricsViewSpec,
metricsExplorer: MetricsExplorerEntity,
fullTimeRange: V1ColumnTimeRangeResponse | undefined
) {
if (!fullTimeRange) return;

const preset = ISODurationToTimePreset(metricsView.defaultTimeRange, true);
const comparisonOption = DEFAULT_TIME_RANGES[preset]
?.defaultComparison as TimeComparisonOption;
Expand Down Expand Up @@ -78,7 +104,7 @@ function setDefaultComparisonTimeRange(

export function getDefaultMetricsExplorerEntity(
name: string,
metricsView: V1MetricsView,
metricsView: V1MetricsViewSpec,
fullTimeRange: V1ColumnTimeRangeResponse | undefined
) {
const metricsExplorer: MetricsExplorerEntity = {
Expand Down Expand Up @@ -108,5 +134,6 @@ export function getDefaultMetricsExplorerEntity(
};
// set time range related stuff
setDefaultTimeRange(metricsView, metricsExplorer, fullTimeRange);
setDefaultComparison(metricsView, metricsExplorer, fullTimeRange);
return metricsExplorer;
}
63 changes: 54 additions & 9 deletions web-common/src/features/dashboards/stores/dashboard-stores.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ import {
TestTimeOffsetConstants,
} from "@rilldata/web-common/features/dashboards/stores/dashboard-stores-test-data";
import { initLocalUserPreferenceStore } from "@rilldata/web-common/features/dashboards/user-preferences";
import { TimeRangePreset } from "@rilldata/web-common/lib/time/types";
import { V1TimeGrain } from "@rilldata/web-common/runtime-client";
import {
MetricsViewSpecComparisonMode,
V1TimeGrain,
} from "@rilldata/web-common/runtime-client";
import { get } from "svelte/store";
import { beforeAll, beforeEach, describe, expect, it } from "vitest";

Expand Down Expand Up @@ -217,6 +219,8 @@ describe("dashboard-stores", () => {
{
...AD_BIDS_INIT,
defaultTimeRange: "PT6H",
defaultComparisonMode:
MetricsViewSpecComparisonMode.COMPARISON_MODE_UNSPECIFIED,
},
{
timeRangeSummary: {
Expand All @@ -227,13 +231,8 @@ describe("dashboard-stores", () => {
}
);

assertMetricsView(AD_BIDS_NAME, undefined, {
name: TimeRangePreset.LAST_SIX_HOURS,
interval: V1TimeGrain.TIME_GRAIN_HOUR,
start: TestTimeOffsetConstants.LAST_6_HOURS,
end: TestTimeOffsetConstants.NOW,
});
const metrics = get(metricsExplorerStore).entities[AD_BIDS_NAME];
let metrics = get(metricsExplorerStore).entities[AD_BIDS_NAME];
// unspecified mode will default to time comparison
expect(metrics.showTimeComparison).toBeTruthy();
expect(metrics.selectedComparisonTimeRange.name).toBe("CONTIGUOUS");
expect(metrics.selectedComparisonTimeRange.start).toEqual(
Expand All @@ -242,6 +241,52 @@ describe("dashboard-stores", () => {
expect(metrics.selectedComparisonTimeRange.end).toEqual(
TestTimeOffsetConstants.LAST_6_HOURS
);
expect(metrics.selectedComparisonDimension).toBeUndefined();

metricsExplorerStore.remove(AD_BIDS_NAME);
metricsExplorerStore.init(
AD_BIDS_NAME,
{
...AD_BIDS_INIT,
defaultTimeRange: "PT6H",
defaultComparisonMode:
MetricsViewSpecComparisonMode.COMPARISON_MODE_DIMENSION,
},
{
timeRangeSummary: {
min: TestTimeConstants.LAST_DAY.toISOString(),
max: TestTimeConstants.NOW.toISOString(),
interval: V1TimeGrain.TIME_GRAIN_MINUTE as any,
},
}
);
metrics = get(metricsExplorerStore).entities[AD_BIDS_NAME];
expect(metrics.showTimeComparison).toBeFalsy();
// defaults to 1st dimension
expect(metrics.selectedComparisonDimension).toBe(
AD_BIDS_PUBLISHER_DIMENSION
);

metricsExplorerStore.remove(AD_BIDS_NAME);
metricsExplorerStore.init(
AD_BIDS_NAME,
{
...AD_BIDS_INIT,
defaultTimeRange: "PT6H",
defaultComparisonMode:
MetricsViewSpecComparisonMode.COMPARISON_MODE_DIMENSION,
defaultComparisonDimension: AD_BIDS_DOMAIN_DIMENSION,
},
{
timeRangeSummary: {
min: TestTimeConstants.LAST_DAY.toISOString(),
max: TestTimeConstants.NOW.toISOString(),
interval: V1TimeGrain.TIME_GRAIN_MINUTE as any,
},
}
);
metrics = get(metricsExplorerStore).entities[AD_BIDS_NAME];
expect(metrics.selectedComparisonDimension).toBe(AD_BIDS_DOMAIN_DIMENSION);
});

describe("Restore invalid state", () => {
Expand Down
11 changes: 10 additions & 1 deletion web-common/src/features/dashboards/stores/dashboard-stores.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import type {
V1ColumnTimeRangeResponse,
V1MetricsView,
V1MetricsViewFilter,
V1MetricsViewSpec,
V1TimeGrain,
} from "@rilldata/web-common/runtime-client";
import { derived, Readable, writable } from "svelte/store";
Expand Down Expand Up @@ -163,7 +164,7 @@ function syncDimensions(
const metricViewReducers = {
init(
name: string,
metricsView: V1MetricsView,
metricsView: V1MetricsViewSpec,
fullTimeRange: V1ColumnTimeRangeResponse | undefined
) {
update((state) => {
Expand Down Expand Up @@ -191,6 +192,14 @@ const metricViewReducers = {
for (const key in partial) {
metricsExplorer[key] = partial[key];
}
// this hack is needed since what is shown for comparison is not a single source
// TODO: use an enum and get rid of this
if (!partial.showTimeComparison) {
metricsExplorer.showTimeComparison = false;
}
if (!partial.selectedComparisonDimension) {
metricsExplorer.selectedComparisonDimension = undefined;
}
metricsExplorer.dimensionFilterExcludeMode =
includeExcludeModeFromFilters(partial.filters);
});
Expand Down
48 changes: 48 additions & 0 deletions web-common/src/proto/gen/rill/runtime/v1/resources_pb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -942,6 +942,20 @@ export class MetricsViewSpec extends Message<MetricsViewSpec> {
*/
firstMonthOfYear = 0;

/**
* Selected default comparison mode.
*
* @generated from field: rill.runtime.v1.MetricsViewSpec.ComparisonMode default_comparison_mode = 14;
*/
defaultComparisonMode = MetricsViewSpec_ComparisonMode.UNSPECIFIED;

/**
* If comparison mode is dimension then this determines which is the default dimension
*
* @generated from field: string default_comparison_dimension = 15;
*/
defaultComparisonDimension = "";

constructor(data?: PartialMessage<MetricsViewSpec>) {
super();
proto3.util.initPartial(data, this);
Expand All @@ -963,6 +977,8 @@ export class MetricsViewSpec extends Message<MetricsViewSpec> {
{ no: 11, name: "security", kind: "message", T: MetricsViewSpec_SecurityV2 },
{ no: 12, name: "first_day_of_week", kind: "scalar", T: 13 /* ScalarType.UINT32 */ },
{ no: 13, name: "first_month_of_year", kind: "scalar", T: 13 /* ScalarType.UINT32 */ },
{ no: 14, name: "default_comparison_mode", kind: "enum", T: proto3.getEnumType(MetricsViewSpec_ComparisonMode) },
{ no: 15, name: "default_comparison_dimension", kind: "scalar", T: 9 /* ScalarType.STRING */ },
]);

static fromBinary(bytes: Uint8Array, options?: Partial<BinaryReadOptions>): MetricsViewSpec {
Expand All @@ -982,6 +998,38 @@ export class MetricsViewSpec extends Message<MetricsViewSpec> {
}
}

/**
* @generated from enum rill.runtime.v1.MetricsViewSpec.ComparisonMode
*/
export enum MetricsViewSpec_ComparisonMode {
/**
* @generated from enum value: COMPARISON_MODE_UNSPECIFIED = 0;
*/
UNSPECIFIED = 0,

/**
* @generated from enum value: COMPARISON_MODE_NONE = 1;
*/
NONE = 1,

/**
* @generated from enum value: COMPARISON_MODE_TIME = 2;
*/
TIME = 2,

/**
* @generated from enum value: COMPARISON_MODE_DIMENSION = 3;
*/
DIMENSION = 3,
}
// Retrieve enum metadata with: proto3.getEnumType(MetricsViewSpec_ComparisonMode)
proto3.util.setEnumType(MetricsViewSpec_ComparisonMode, "rill.runtime.v1.MetricsViewSpec.ComparisonMode", [
{ no: 0, name: "COMPARISON_MODE_UNSPECIFIED" },
{ no: 1, name: "COMPARISON_MODE_NONE" },
{ no: 2, name: "COMPARISON_MODE_TIME" },
{ no: 3, name: "COMPARISON_MODE_DIMENSION" },
]);

/**
* Dimensions are columns to filter and group by
*
Expand Down
2 changes: 1 addition & 1 deletion web-common/src/proto/gen/rill/ui/v1/dashboard_pb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ proto3.util.setEnumType(DashboardState_LeaderboardSortDirection, "rill.ui.v1.Das
]);

/**
* *
*
* SortType is used to determine how to sort the leaderboard
* and dimension detail table, as well as where to place the
* sort arrow.
Expand Down
Loading

1 comment on commit c7aef1d

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

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

Please sign in to comment.