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

Separate explore and metrics_view resources #5659

Merged
merged 23 commits into from
Sep 18, 2024

Conversation

begelundmuller
Copy link
Contributor

@begelundmuller begelundmuller commented Sep 10, 2024

Overview

This PR introduces an Explore resource to represent explore dashboards. The goal of this work is a) to support multiple Explore resources deriving from one MetricsView, and b) to avoid automatically creating dashboards when creating a MetricsView.

Detailed changes:

  • Adds an Explore resource type containing config for explore dashboards based on an underlying metrics view.
  • An Explore resource inherits the access and field access security rules of its parent metrics view. This means its dimensions and measures fields will reflect the current caller's attributes.
  • Adds a GetExplore RPC that returns an explore resource and its underlying metrics view in a single request.
  • Uses a YAML syntax supporting '*' and exclude: for selecting dimensions and measures from the parent metrics view (see example).
  • Deprecates the following properties on metrics views: default_dimensions, default_measures, default_time_range, default_comparison_mode, default_comparison_dimension, default_theme, available_time_zones.
  • Adds a webOpenPath option in the alert and report creation APIs for persisting the explore resource to open. This should be used in addition to web_open_state to identify the UI page to open.
  • For backwards compatibility, metrics views without version: 1 in the YAML will automatically emit an Explore resource with the same name as the metrics view.

Frontend integration notes

After this PR, the UI should always find and use an Explore resource before consuming a MetricsView resource. The Explore resource will contain contextual information about how to query and render metrics. When creating alerts/reports/public URLs/bookmarks/etc., the frontend should make sure it captures the relevant Explore resource that was used so that context can be restored later. We will need to pay attention to backwards compatibility for legacy resources that do not contain this contextual info.

Here's an initial outline for integrating these changes in the UI:

  • Add a new URL route /org/project/-/explore/<name> that calls GetExplore to resolve the explore and metrics view resources to render.
  • Only use the state.validSpec of the Explore and MetricsView resources, never consume the spec directly. This ensures that the dimensions and measures are valid and reflect the security policies.
  • Change the /org/project/<name> route to redirect to /org/project/-/explore/<name>.
  • When creating or editing alerts and reports, populate web_open_path to the relevant Explore resource's name or path (up to you).
  • When opening alerts and reports, use the web_open_path to redirect to the correct Explore resource. It will not be set for legacy alerts/reports – here, you can fall back to searching for an Explore resource with the same name as the alert's/report's metrics view.
  • If state.validSpec.presets is not empty, the first entry will contain preset defaults to render. This replaces the current default_dimensions, default_measures, default_time_range, etc.
  • Generally audit for use of ResourceKind.MetricsView and consider if an explore resource should be used instead (or as well).
  • Adapt the embeds to accept explore resource names.
  • Adapt for the breaking changes described in this separate PR: Migrate metrics_view to explore resources (breaking) #5722

Syntax

Full syntax:

type: explore
title: string (no default)
description: string (no default)
metrics_view: name of a metrics view (no default)
dimensions: '*', list of names, or nested `exclude:` (default: '*')
measures: '*', list of names, or nested `exclude:` (default: '*')
theme: name of a theme (no default)
time_ranges: equivalent to current `available_time_ranges` on metrics views (default: none)
time_zones: list of IANA time zone identifiers
presets:
  - label: <string> (no default)
    dimensions: '*', list of names, or nested `exclude:` (default: '*')
    measures: '*', list of names, or nested `exclude:` (default: '*')
    time_range: value matching an entry in time_ranges
    comparison_mode: 'none', 'time', or 'dimension'
    comparison_dimension: dimension name if comparison_mode=='dimension'

Example:

# metrics/my_sales.yaml
version: 1
type: metrics_view
dimensions:
...
# etc. as usual

# explores/my_sales.yaml
type: explore
title: My sales
metrics_view: sales_metrics
dimensions: '*'
measures:
  - count
  - revenue
  - profit
  - costs
  - margin
theme: main_theme
time_ranges:
  - PT1H
  - PT6H
  - range: P5D
    comparison_offsets:
      - rill-PP
      - rill-PW
time_zones:
  - America/Los_Angeles
  - America/New_York
  - Europe/London
  - Europe/Paris
presets:
  - label: Default view
    dimensions:
      except:
        - order_id
        - payment_method
    comparison_mode: time

@begelundmuller begelundmuller self-assigned this Sep 10, 2024
@begelundmuller begelundmuller changed the title Separate explore and metrics_view resources Separate explore and metrics_view resources Sep 10, 2024
@begelundmuller begelundmuller marked this pull request as ready for review September 11, 2024 17:55
@begelundmuller begelundmuller merged commit a95afc7 into main Sep 18, 2024
10 checks passed
@begelundmuller begelundmuller deleted the begelundmuller/explore-resource branch September 18, 2024 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants