Explore: Unification of logs/metrics/traces user interface #25890

aocenas · 2020-06-29T08:53:25Z

Design doc: https://docs.google.com/document/d/1hZURlhvU5LVejMWXeQTaTMOLAC73FQjQR_EjgYtOV9c/edit#heading=h.sp7xjuf6unar

aocenas · 2020-06-29T09:00:07Z

packages/grafana-data/src/types/data.ts

@@ -15,7 +15,7 @@ export enum LoadingState {
  Error = 'Error',
 }

-export type PreferredVisualisationType = 'graph' | 'table';
+export type PreferredVisualisationType = 'graph' | 'table' | 'logs' | 'trace';


I think we can use this to figure out where to show the dataFrames. For data sources which won't use that we should just make sure the output is sensible (ie fallback to table table/graph)

aocenas · 2020-06-29T09:01:14Z

public/app/features/explore/Explore.tsx

@@ -332,7 +338,8 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
              <SecondaryActions
                addQueryRowButtonDisabled={isLive}
                // We cannot show multiple traces at the same time right now so we do not show add query button.
-                addQueryRowButtonHidden={mode === ExploreMode.Tracing}
+                //TODO:unification
+                // addQueryRowButtonHidden={mode === ExploreMode.Tracing}


I think we can just show one trace view per frame and deal with multiple traces that way.

aocenas · 2020-06-29T11:12:42Z

We also need to check code like this one

grafana/public/app/plugins/datasource/prometheus/datasource.ts

Line 221 in 72b8300

if (target.showingTable) {

. It seems to be working but data sources should probably not be getting explore internal state and depend on that.

kaydelaney · 2020-06-29T12:50:40Z

public/app/features/explore/utils/ResultProcessor.ts

+          this.tableFrames.push(frame);
+        }
+      } else {
+        if (shouldShowInVisualisationType(frame, 'logs')) {


Do we need to nest these if/else-if statements inside the else statement starting at line 42 or can we just use all else-if statements instead?

the thing is time series by default are shown in both graph and table. So they need to be separate, other types are sort of priority based in the sense that when we think something should be logs we won't try to match this to anything else.

Also this assumes we will start using the meta attributes so may not work at the moment for all cases, but as I mentioned if we can do some sensible default that should be fine.

kylebrandt · 2020-06-30T11:13:09Z

For me this is a lot of JS :-) If I am to review (some concepts maybe?) I think I would need some context or a summary.

aocenas · 2020-06-30T15:23:33Z

@kylebrandt seems like you ware added because the codeowners config and this touches some backend files. I think the automatic code owners assignment isn't that great in some cases,

davkal

Awesome 🚀

No blockers from my side.

Minor:

Loki is missing the table result. Either add in this PR if simple, or add a comment TODO
Elastic agg histogram from the datasource was nice. Is there no other way to reintroduce this?

davkal · 2020-07-02T17:49:13Z

packages/grafana-data/src/types/data.ts

@@ -47,6 +47,7 @@ export interface QueryResultMeta {
  searchWords?: string[]; // used by log models and loki
  limit?: number; // used by log models and loki
  json?: boolean; // used to keep track of old json doc values
+  instant?: boolean;


This is a prometheus/loki thing, definitely needs a comment. Ideally it should be sub-field of custom.

Do we still need this? I thought that was solved by the preferredVis type?

Yeah seems like it's not used anymore

public/app/core/logs_model.ts

davkal · 2020-07-02T17:56:14Z

public/app/features/explore/Explore.tsx

@@ -58,6 +58,7 @@ import { getTimeZone } from '../profile/state/selectors';
 import { ErrorContainer } from './ErrorContainer';
 import { scanStopAction } from './state/actionTypes';
 import { ExploreGraphPanel } from './ExploreGraphPanel';
+//TODO:unification


What's left todo here?

davkal · 2020-07-02T17:58:19Z

public/app/features/explore/Explore.tsx

@@ -334,7 +340,8 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
              <SecondaryActions
                addQueryRowButtonDisabled={isLive}
                // We cannot show multiple traces at the same time right now so we do not show add query button.
-                addQueryRowButtonHidden={mode === ExploreMode.Tracing}
+                //TODO:unification


I guess this condition might need to be changed to depend on datasource types.

My idea for simple solution would be to just show 1 trace view per each query. Would not be very practical from user perspective but would be easy to implement without any special casing here.

davkal · 2020-07-02T18:04:31Z

public/app/features/explore/QueryRow.tsx

    let QueryEditor;

-    if (mode === ExploreMode.Metrics && datasourceInstance.components?.ExploreMetricsQueryField) {
+    // TODO:unification


Looks like there is no other way than removing this field specialisation hand add a switcher to the field itself.

davkal · 2020-07-02T18:16:03Z

public/app/plugins/datasource/elasticsearch/module.ts

@@ -11,5 +10,4 @@ class ElasticAnnotationsQueryCtrl {
 export const plugin = new DataSourcePlugin(ElasticDatasource)
  .setQueryCtrl(ElasticQueryCtrl)
  .setConfigEditor(ConfigEditor)
-  .setExploreLogsQueryField(ElasticsearchQueryField)


The component file could be removed?

davkal · 2020-07-02T18:16:52Z

public/app/plugins/datasource/influxdb/module.ts

@@ -16,5 +15,4 @@ export const plugin = new DataSourcePlugin(InfluxDatasource)
  .setConfigEditor(ConfigEditor)
  .setQueryCtrl(InfluxQueryCtrl)
  .setAnnotationQueryCtrl(InfluxAnnotationsQueryCtrl)
-  .setExploreLogsQueryField(InfluxLogsQueryField)


Remove component?

davkal · 2020-07-02T18:17:36Z

public/app/plugins/datasource/jaeger/datasource.ts

@@ -49,6 +49,9 @@ export class JaegerDatasource extends DataSourceApi<JaegerQuery> {
                    values: response?.data?.data || [],
                  },
                ],
+                meta: {
+                  preferredVisualisationType: 'trace',


I feel like these values should be constants from something in core grafana.

davkal · 2020-07-02T18:21:01Z

public/app/plugins/datasource/loki/datasource.ts

-        )
-      );
-    }
+    filteredTargets.forEach(target => subQueries.push(this.runRangeQuery(target, options, filteredTargets.length)));


We're dropping the instant query here. Any way we can have it back for metric queries (or we just sent it anyway for logs and see what happens/deal with it in the response).

davkal · 2020-07-02T18:21:56Z

public/app/plugins/datasource/loki/datasource.ts

-          throw new Error('Metrics mode does not support logs. Use an aggregation or switch to Logs mode.');
+          return {
+            data: [],
+            key: `${target.refId}_instant`,


Unclear what's happening here. Also, the _instant suffix suggests this has something to do with instant queries.

kaydelaney · 2020-07-06T11:58:48Z

Failing right now as it depends on grafana/grafana-plugin-sdk-go#191 which needs to get merged first

…ailing

… Logs

Removes "Metrics"/"Logs" mode switcher from Explore, allowing for both metrics and logs queries at the same time. Co-authored-by: kay delaney <kay@grafana.com> (cherry picked from commit 64bc859)

NotAFile · 2020-07-13T16:20:49Z

Is it intended that this design doc is private?

aocenas requested a review from a team as a code owner June 29, 2020 08:53

aocenas requested review from a team, kylebrandt, mckn, dprokop and davkal and removed request for a team June 29, 2020 08:53

aocenas marked this pull request as draft June 29, 2020 08:53

aocenas assigned kaydelaney Jun 29, 2020

aocenas commented Jun 29, 2020

View reviewed changes

kaydelaney reviewed Jun 29, 2020

View reviewed changes

kaydelaney force-pushed the unification-wip branch from 4b32042 to ed8e95e Compare June 29, 2020 13:02

aocenas removed request for kylebrandt and mckn June 30, 2020 15:22

aocenas changed the title ~~Explore: Unification WIP~~ Explore: Unification of logs/metrics/traces user interface Jun 30, 2020

kaydelaney force-pushed the unification-wip branch 4 times, most recently from 634baa1 to b60e090 Compare July 1, 2020 14:12

davkal approved these changes Jul 2, 2020

View reviewed changes

kaydelaney force-pushed the unification-wip branch 2 times, most recently from 9eeabd6 to 863e763 Compare July 7, 2020 13:36

aocenas and others added 12 commits July 9, 2020 14:30

Group data frames

9437026

Update the data frame classification code, fix fallback to table

362eb0b

Remove responseType from QueryResultMeta

18e1a1f

Add visualisation hints to elasticsearch and cloudwatch

2a0d019

Add preferred vis type hints to test data source

396a3e0

Fix some tests

a653fc7

Fix some strict null errors

c062039

Add more descriptive error when metrics query used during loki live t…

72d1260

…ailing

Fixes InfluxDB for logs, changes frame classification logic a bit

7cd3b69

Fixes an issue with setting preferredVisualisationType for CloudWatch…

7f9875d

… Logs

Update names of frameMeta properties used

5cb2f00

fix failing datasource test

0f09190

kaydelaney force-pushed the unification-wip branch from d8ff252 to 0f09190 Compare July 9, 2020 13:38

kaydelaney merged commit 64bc859 into master Jul 9, 2020

kaydelaney deleted the unification-wip branch July 9, 2020 14:14

davkal mentioned this pull request Jul 9, 2020

Documentation for implementing required explore ui methods in custom datasource #15697

Closed

kaydelaney mentioned this pull request Jul 10, 2020

Loki: Automatically switch between metrics or logs based on the query #21451

Closed

aocenas added the add to changelog label Jul 10, 2020

aocenas mentioned this pull request Jul 10, 2020

7.1.0-beta3 cherrypicks #26238

Merged

aocenas removed the cherry-pick needed label Jul 13, 2020

aocenas modified the milestones: 7.1, 7.1-beta3 Jul 13, 2020

melchiormoulin mentioned this pull request Jul 22, 2020

Explore: Transform prometheus query to elasticsearch query #23670

Merged

aocenas mentioned this pull request Sep 11, 2020

POC/Exemplars: possible query runner #27437

Closed

ivanahuckova mentioned this pull request Oct 1, 2020

Grafana 7.2 + Loki 1.6.1 + pie-chart: query then query_range #27860

Closed

ifrost mentioned this pull request Jan 9, 2024

Logs: Initial setup to add infinite scrolling to Explore #76348

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Explore: Unification of logs/metrics/traces user interface #25890

Explore: Unification of logs/metrics/traces user interface #25890

aocenas commented Jun 29, 2020 •

edited

Loading

aocenas Jun 29, 2020

torkelo Jun 29, 2020

aocenas Jun 29, 2020

aocenas commented Jun 29, 2020

kaydelaney Jun 29, 2020

aocenas Jun 29, 2020

aocenas Jun 29, 2020

kylebrandt commented Jun 30, 2020

aocenas commented Jun 30, 2020

davkal left a comment

davkal Jul 2, 2020

aocenas Jul 6, 2020

kaydelaney Jul 6, 2020

davkal Jul 2, 2020

davkal Jul 2, 2020

aocenas Jul 6, 2020

davkal Jul 2, 2020

davkal Jul 2, 2020

davkal Jul 2, 2020

davkal Jul 2, 2020

davkal Jul 2, 2020

davkal Jul 2, 2020

kaydelaney commented Jul 6, 2020

NotAFile commented Jul 13, 2020

Explore: Unification of logs/metrics/traces user interface #25890

Explore: Unification of logs/metrics/traces user interface #25890

Conversation

aocenas commented Jun 29, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aocenas commented Jun 29, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kylebrandt commented Jun 30, 2020

aocenas commented Jun 30, 2020

davkal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaydelaney commented Jul 6, 2020

NotAFile commented Jul 13, 2020

aocenas commented Jun 29, 2020 •

edited

Loading