Explore: Use SeriesData format for loki/logs #16793

marefr · 2019-04-26T20:15:02Z

What this PR does / why we need it:
This is the first step moving towards Explore supporting logs for additional datasources than Loki. In the first step we move all the log processing from Loki into Explore.

Summary of changes:
Make explore convert logs result returned from datasources to SeriesData,
if needed, and for now convert SeriesData to LogsModel.
Loki datasource query now returns SeriesData and all
processing have been moved into explore instead.
Removed key from LogRowModel and use log row indexes as
the unique key instead.
Removed id from LogsModel since it looks like it's not in use.

Which issue(s) this PR fixes:
Closes #16287

Special notes for your reviewer:
I removed key from LogRowModel and used index of each log row as key for rendering unique keys for <LogRow /> component. To me this seems to work and makes the row model easier, but I may have missed some important aspect?

Make explore expect SeriesData to be returned as logs result from datasources and for now convert SeriesData to LogsModel. Loki datasource query now returns SeriesData and all processing have been moved into explore instead.

marefr · 2019-04-26T20:16:58Z

@ryantxu Please have a look at the changes if you're interested to see if this maps to your vision in referenced issue. This is the first step and there will probably be a lot of other changes needed, but now we make it easier for each datasource to return log data.

marefr

Some comments on some of the design decision I've made.

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

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

public/app/core/logs_model.ts

torkelo

Great work! So nice to see progress on this front! Just some minor comments about the overuse of callback loops (reduce/maps etc) for arrays that can have thousands of entries.

public/app/core/logs_model.ts

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

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

public/app/core/logs_model.ts

ryantxu · 2019-04-27T13:00:05Z

@marefr -- this looks really good. thank you.

LogsSeriesFieldProcessor extends SeriesFieldProcessor and can validate that series are valid for being visualized as logs. Valid log series is now defined as having a time and a string field. Also, there now support for checking if a series has a log level field. The processing and sorting of logs have been refactored to hopefully be more efficient than before.

marefr · 2019-04-28T14:46:55Z

@ryantxu @torkelo thanks for the feedback 👍 Have pushed some changes now. I guess the biggest change is the SeriesFieldsProcessor which I did find handy when processing a lot of rows without needing to loop thru all the fields every time to find the correct field indices.

Besides this have tried to make the log processing more efficient by using regular loops instead.

public/app/features/explore/LogsContainer.tsx

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

packages/grafana-ui/src/utils/processSeriesData.ts

ryantxu · 2019-04-29T00:54:13Z

@marefr this is looking really good -- thanks.

The name on SeriesFieldsProcessor feels a bit off, and potentially in conflict with some series processing/transforming work I hope to standardize in #16756

As implemented it is a way to find field indexes based on name/type -- maybe FieldIndexFinder or IndexFieldFinder?

hugohaggmark

Awesome work, found some very minor stuff.

public/app/core/logs_model.ts

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

marefr · 2019-04-29T07:36:46Z

The name on SeriesFieldsProcessor feels a bit off, and potentially in conflict with some series processing/transforming work I hope to standardize in #16756

As implemented it is a way to find field indexes based on name/type -- maybe FieldIndexFinder or IndexFieldFinder?

@ryantxu I agree, I'm not super happy with that name. But it's not only a way for finding field indexes based on name/type - it also guesses field type for undefined/other field types and "caches" it so you don't have to iterate over all fields over and over again. And for LogsSeriesFieldProcessor it adds some extra field type processing to find log level field => the log level finding seems kind of similar to your proposed matchers.

Would it be possible/a good thing to wrap the SeriesData in some sort of class/structure, similar to SeriesFieldsProcessor, mixed with your matchers, extensions and transforms and then use this class/structure internally in Grafana instead of SeriesData?

I guess it's easier to start with renaming SeriesFieldsProcessor and get this merged and later if we merge your proposed PR and potential additional matchers/transforms we may be able to easier spot what is missing/how to properly refactor things.

marefr · 2019-04-29T08:02:25Z

@ryantxu I think changes in https://github.com/grafana/grafana/pull/16793/files#diff-317237f74791e054ee61a4e01ed3cb3bR58 and https://github.com/grafana/grafana/pull/16793/files#diff-c10a219f8d98d5afcc02054eb27f23a3R36 highlights the need of using some abstraction on top of SeriesData. What do you think?

ryantxu · 2019-04-29T13:47:52Z

@marefr -- maybe, I don't think we need more now. regardless, the details of this class should not hold up merging the PR.

ryantxu · 2019-04-29T14:31:14Z

it also guesses field type for undefined/other field types and "caches" it so you don't have to iterate over all fields over and over again.

FYI - this step already happens if you query with the new PanelQueryRunner right now that is only react panels, but soon will be everything. Essentially it makes sure there is a type on everything before sending it to any panel.

marefr · 2019-04-29T22:35:04Z

it also guesses field type for undefined/other field types and "caches" it so you don't have to iterate over all fields over and over again.

FYI - this step already happens if you query with the new PanelQueryRunner right now that is only react panels, but soon will be everything. Essentially it makes sure there is a type on everything before sending it to any panel.

I totally see the win and simplicity of using this. Next step for me in explore is to refractor how queries are executed and then I think it will make sense to use the same, guessFieldTypes(toSeriesData(...)), and later on add some additional field processing specific for logs. Then the SeriesDataProcessor may not be needed or it will be just a FieldIndexFinder or whatever we decide to call it. I'll give this a try tomorrow before merging.

ryantxu · 2019-04-30T03:58:21Z

If you get ambitions and subscribe to the results, you may get streaming to work for "free" -- see:
#16815

Renames SeriesFieldsProcessor to FieldCache which makes it explicit to only be used for lookup of fields by type or name. Removes LogsSeriesFieldProcessor and postpone finding log level fields to later.

…ng logs

marefr · 2019-04-30T15:48:17Z

@ryantxu decided to rename SeriesFieldsProcessor to FieldCache and remove field type processing from that one. Also removed guessing of log level for now and postpone that one to later since Loki is the only (official) datasource supporting logs in Explore so far and I want to get this merged.

ryantxu · 2019-04-30T18:15:25Z

@marefr -- looks great. this is a huge improvement.

marefr · 2019-04-30T18:21:51Z

Thanks for great feedback @ryantxu 👍 Really love how the new SeriesData format simplifies a lot of stuff! Next up is #16823 and next after that simplifying query execution in explore so that SeriesData is used for all both metrics and logs.

davkal · 2019-04-30T19:29:15Z

@marefr using indexes as React keys is rarely advisable. The key tells react what to unmount. so now when you look at a loki stream (seeing all results) and then decide to add a search term that would match some of the later results (e.g., a term from row 10) and then run the query, the non-matched results still show. This happens because in the filtered set the first indexes are still present, so react thinks it won't have to unmount those rows.

Update: can't repro index-as-keys issues anymore.

…MetricPanelCtrl * grafana/master: Explore: Use SeriesData format for loki/logs (grafana#16793) Refactor: move NavModel to @grafana/ui (grafana#16813) Auth: Enable retries and transaction for some db calls for auth tokens (grafana#16785) Provisioning: Show file path of provisioning file in save/delete dialogs (grafana#16706) Add tracing headers for prometheus datasource (grafana#16724) Config: Fixes bug where timeouts for alerting was not parsed correctly (grafana#16784) build: removes gopkg files from dev docker file (grafana#16817) Provisioning: Trying to fix failing test (grafana#16800) Table: React table fix rotate support in storybook (grafana#16816) TestData: add log level in dummy message (grafana#16815) removes gopkg.lock from root folder Explore: Support user timezone (grafana#16469) Plugins: rename vizPlugin to panelPlugin (grafana#16802) Plugins: move app/feature/plugin properties into PluginMeta (grafana#16809) Plugins: move PanelPluginMeta to grafana/ui (grafana#16804) Plugins: move datasource specific meta out of the main meta type (grafana#16803) updates changelog for 6.1.6 Fix: Fetch histogram series from other api route (grafana#16768) phantomjs: set web-security to true Chore: Lowered implicit anys limit to 5668

* grafana/master: Explore: Use SeriesData format for loki/logs (grafana#16793) Refactor: move NavModel to @grafana/ui (grafana#16813) Auth: Enable retries and transaction for some db calls for auth tokens (grafana#16785) Provisioning: Show file path of provisioning file in save/delete dialogs (grafana#16706) Add tracing headers for prometheus datasource (grafana#16724) Config: Fixes bug where timeouts for alerting was not parsed correctly (grafana#16784) build: removes gopkg files from dev docker file (grafana#16817) Provisioning: Trying to fix failing test (grafana#16800) Table: React table fix rotate support in storybook (grafana#16816) TestData: add log level in dummy message (grafana#16815) removes gopkg.lock from root folder Explore: Support user timezone (grafana#16469) Plugins: rename vizPlugin to panelPlugin (grafana#16802) Plugins: move app/feature/plugin properties into PluginMeta (grafana#16809) Plugins: move PanelPluginMeta to grafana/ui (grafana#16804) Plugins: move datasource specific meta out of the main meta type (grafana#16803) updates changelog for 6.1.6 Fix: Fetch histogram series from other api route (grafana#16768) phantomjs: set web-security to true Chore: Lowered implicit anys limit to 5668

* grafana/master: FormLabel: allow any rather than just a string for tooltip (grafana#16841) prometheus: fix regression after adding support for tracing headers (grafana#16829) area/circleci: Speed up circleci build process for branches and pr (grafana#16778) DataProxy: Restore Set-Cookie header after proxy request (grafana#16838) docs: clarify page parameter version support for folder/dashboard search (grafana#16836) Chore: revise some of the gosec rules (grafana#16713) Refactor: consistant plugin/meta usage (grafana#16834) Explore: Use SeriesData format for loki/logs (grafana#16793) Refactor: move NavModel to @grafana/ui (grafana#16813)

* grafana/master: (27 commits) FormLabel: allow any rather than just a string for tooltip (grafana#16841) prometheus: fix regression after adding support for tracing headers (grafana#16829) area/circleci: Speed up circleci build process for branches and pr (grafana#16778) DataProxy: Restore Set-Cookie header after proxy request (grafana#16838) docs: clarify page parameter version support for folder/dashboard search (grafana#16836) Chore: revise some of the gosec rules (grafana#16713) Refactor: consistant plugin/meta usage (grafana#16834) Explore: Use SeriesData format for loki/logs (grafana#16793) Refactor: move NavModel to @grafana/ui (grafana#16813) Auth: Enable retries and transaction for some db calls for auth tokens (grafana#16785) Provisioning: Show file path of provisioning file in save/delete dialogs (grafana#16706) Add tracing headers for prometheus datasource (grafana#16724) Config: Fixes bug where timeouts for alerting was not parsed correctly (grafana#16784) build: removes gopkg files from dev docker file (grafana#16817) Provisioning: Trying to fix failing test (grafana#16800) Table: React table fix rotate support in storybook (grafana#16816) TestData: add log level in dummy message (grafana#16815) removes gopkg.lock from root folder Explore: Support user timezone (grafana#16469) Plugins: rename vizPlugin to panelPlugin (grafana#16802) ...

explore: use SeriesData format for loki/logs

6fb16e1

Make explore expect SeriesData to be returned as logs result from datasources and for now convert SeriesData to LogsModel. Loki datasource query now returns SeriesData and all processing have been moved into explore instead.

marefr requested review from davkal, torkelo and hugohaggmark April 26, 2019 20:17

marefr commented Apr 26, 2019

View reviewed changes

public/app/plugins/datasource/loki/datasource.ts Show resolved Hide resolved

public/app/plugins/datasource/loki/result_transformer.ts Outdated Show resolved Hide resolved

public/app/core/logs_model.ts Outdated Show resolved Hide resolved

torkelo reviewed Apr 27, 2019

View reviewed changes

ryantxu reviewed Apr 27, 2019

View reviewed changes

public/app/core/logs_model.ts Show resolved Hide resolved

ryantxu reviewed Apr 27, 2019

View reviewed changes

public/app/core/logs_model.ts Show resolved Hide resolved

marefr added 6 commits April 28, 2019 12:48

Merge remote-tracking branch 'origin/master' into 16287_loki_series_data

19dfc7d

new SeriesFieldProcessor that encapsulates fields processing

b06529c

loki: remove usage of specific series and field names

6f5ea2e

add optional fields to QueryResultMeta

654d9f9

fix tests

81420f6

marefr commented Apr 28, 2019

View reviewed changes

public/app/features/explore/LogsContainer.tsx Show resolved Hide resolved

add tests and fix bugs

3727f3e

ryantxu reviewed Apr 29, 2019

View reviewed changes

packages/grafana-ui/src/types/data.ts Outdated Show resolved Hide resolved

ryantxu reviewed Apr 29, 2019

View reviewed changes

packages/grafana-ui/src/utils/processSeriesData.ts Outdated Show resolved Hide resolved

hugohaggmark approved these changes Apr 29, 2019

View reviewed changes

public/app/core/logs_model.ts Outdated Show resolved Hide resolved

public/app/plugins/datasource/loki/datasource.ts Show resolved Hide resolved

move IndexedField to processSeriesData

44cec44

cleanup SeriesFieldProcessor

baba406

Merge remote-tracking branch 'origin/master' into 16287_loki_series_data

c46ba82

marefr added 3 commits April 30, 2019 15:24

Merge remote-tracking branch 'origin/master' into 16287_loki_series_data

bb161fb

refactor SeriesFieldsProcessor to FieldCache

ecfa3f8

Renames SeriesFieldsProcessor to FieldCache which makes it explicit to only be used for lookup of fields by type or name. Removes LogsSeriesFieldProcessor and postpone finding log level fields to later.

explore: guess field types and convert to series data before processi…

f4a1849

…ng logs

marefr merged commit fe20dde into master Apr 30, 2019

marefr deleted the 16287_loki_series_data branch April 30, 2019 16:21

marefr added this to the 6.2-beta1 milestone Apr 30, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Explore: Use SeriesData format for loki/logs #16793

Explore: Use SeriesData format for loki/logs #16793

marefr commented Apr 26, 2019 •

edited

Loading

marefr commented Apr 26, 2019

marefr left a comment

torkelo left a comment

ryantxu commented Apr 27, 2019

marefr commented Apr 28, 2019

ryantxu commented Apr 29, 2019 •

edited

Loading

hugohaggmark left a comment

marefr commented Apr 29, 2019

marefr commented Apr 29, 2019

ryantxu commented Apr 29, 2019 •

edited

Loading

ryantxu commented Apr 29, 2019

marefr commented Apr 29, 2019

ryantxu commented Apr 30, 2019

marefr commented Apr 30, 2019

ryantxu commented Apr 30, 2019

marefr commented Apr 30, 2019

davkal commented Apr 30, 2019 •

edited

Loading

Explore: Use SeriesData format for loki/logs #16793

Explore: Use SeriesData format for loki/logs #16793

Conversation

marefr commented Apr 26, 2019 • edited Loading

marefr commented Apr 26, 2019

marefr left a comment

Choose a reason for hiding this comment

torkelo left a comment

Choose a reason for hiding this comment

ryantxu commented Apr 27, 2019

marefr commented Apr 28, 2019

ryantxu commented Apr 29, 2019 • edited Loading

hugohaggmark left a comment

Choose a reason for hiding this comment

marefr commented Apr 29, 2019

marefr commented Apr 29, 2019

ryantxu commented Apr 29, 2019 • edited Loading

ryantxu commented Apr 29, 2019

marefr commented Apr 29, 2019

ryantxu commented Apr 30, 2019

marefr commented Apr 30, 2019

ryantxu commented Apr 30, 2019

marefr commented Apr 30, 2019

davkal commented Apr 30, 2019 • edited Loading

marefr commented Apr 26, 2019 •

edited

Loading

ryantxu commented Apr 29, 2019 •

edited

Loading

ryantxu commented Apr 29, 2019 •

edited

Loading

davkal commented Apr 30, 2019 •

edited

Loading