Skip to content
This repository has been archived by the owner on Apr 17, 2021. It is now read-only.

Add warning caching to datasource #117

Merged
merged 1 commit into from
Mar 11, 2020
Merged

Add warning caching to datasource #117

merged 1 commit into from
Mar 11, 2020

Conversation

ryanvazquez
Copy link
Contributor

This PR contains the following changes:

Currently, "inline" error warnings are lost between dashboard <--> panel navigations. Heroic (not Grafana) warnings are only generated after data is received in the query panel. When users navigate away from the editor to the dashboard and back to the editor - without requerying - these warnings are lost and can lead to confusion.

Warnings will now persist in the query editor across panel/dashboard navigations until they are manually dismissed or the user requeries Heroic. This PR does not address dashboard level warnings (aka "dogear" warnings) generated by Grafana and may be addressed in a separate PR.

@@ -0,0 +1,61 @@
export declare namespace IWarningsCache {
Copy link
Contributor

@dmichel1 dmichel1 Feb 20, 2020

Choose a reason for hiding this comment

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

Is the I for inline or interface? Can this be named more descriptive?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is the I for inline or interface?

Interface.

Can this be named more descriptive?

👍

Copy link
Contributor

@dmichel1 dmichel1 left a comment

Choose a reason for hiding this comment

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

I had one question about the naming otherwise LGTM.

@ryanvazquez
Copy link
Contributor Author

ryanvazquez commented Feb 26, 2020

I've been holding off on moving forward with this until I've had a clearer picture for how Heroic error handling could work and thus play with this cacheing implementation.

Since Heroic responses could contain time series data, even those containing errors/limits/etc - relying on Grafana's native alerting to display warnings to users is not possible. Datasource responses must be rejected and doing so will discard the data.

The datasource currently relies on the onDataReceived hook to generate warnings. However, this hook is only fired while the user is in editor mode and only after a response has been resolved by the datasource. This means the user must either navigate directly to the panel and manually refresh the query to view warnings. Users refreshing dashboards, for example, do not receive them.

The alternative approach would be to raise warnings manually at the time of the query, before the response is passed to Grafana. Since many different components within Grafana can trigger a datasource query, these warnings could be triggered excessively. The datasource receives limited context at the time of the query, so being able to conditionally render warnings whether in dashboard or panel mode is not possible.

Additionally, Grafana offers no way to map option.panelIds that datasource.query receives to the panel that generated the query see related. This means panel properties such as the panel's title, etc. are not accessible to the datasource and can't be used when generating warnings while in dashboard mode. Thus users in dashboard mode could see warnings for all of their panels but not be able to map those warnings to the panel that generated them.

I'm going to stick a pin in this until a cleaner solution becomes available.

testing - initialize variables before each test run

formatting + move error warnings from inline to multiple labels

add and clear warnings from cache to persist across navigations

return an array of warning messages instead of a string with line breaks

deprecate event names as strings

update tsconfig to allow typescript support of es7

add cache key to series metadata. cache errors before returning series to grafana for rendering

add cache key to series metadata

cleanup

Use app events for suggestion alerting; check if series exists before checking for suggestions; git cleanup

add warningsKey type

warnings -> suggestions; return array of strings for rendering

cache for heroic query error warnings
@ryanvazquez
Copy link
Contributor Author

ryanvazquez commented Mar 11, 2020

Several updates were made since this PR and makes changes to how errors are displayed in the datasource

warnings

  • Displays raw Heroic query errors above the query that generated them.
  • Uses Grafana alert-warning dissmissable pop up to display suggestions
  • Enables cacheing of query errors. Query errors are cached at the time of the request before time series data is returned to Grafana.
    • When refreshing dashboard, errors can be displayed immediately when navigating to panel
    • Errors will persist between panel and dashboard navigations until they are dismissed or the query is refreshed.
  • Bugfixes
    • Fixes an issue where error warnings appeared "squashed"
    • Fixes an issue where the "$key" warning is displayed before a query has been created

@ryanvazquez ryanvazquez merged commit 7e2ca08 into spotify:master Mar 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants