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

Annotations: Support filtering the target panels #66325

Merged
merged 63 commits into from
Apr 18, 2023
Merged

Conversation

ryantxu
Copy link
Member

@ryantxu ryantxu commented Apr 11, 2023

Fixes #717

This PR updates the annotations system to support filtering which panels should get the data:

From the annotations editor you can now select where to show the annotations:
image

image

2023-04-13_14-45-44 (1)

Then annotations are only sent to the selected panels:
image

TODO:

  • this PR also includes changes to the testdata annotations... that could be moved to its own PR -> TestData Annotation changes PR (Testdata: Update testdata annotations editor #66620) ✅
  • this also tries to use the schema defined from cue -- also could be its own PR 🤷‍♂️
  • improve editor UI ✅

@ryantxu ryantxu added area/annotations no-backport Skip backport of PR no-changelog Skip including change in changelog/release notes labels Apr 11, 2023
@ryantxu ryantxu added this to the 10.0.0 milestone Apr 11, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Apr 12, 2023

⚠️   Possible breaking changes

(Open the links below in a new tab to go to the correct steps)

grafana-data has possible breaking changes (more info)
grafana-schema has possible breaking changes (more info)

Console output
Read our guideline

@grafanabot grafanabot added the levitate breaking change A label indicating a breaking change and assigned by Levitate. label Apr 12, 2023
@ryantxu ryantxu changed the title TestData: Support more annotation options Annotations: Support filtering the target panels Apr 12, 2023
annotations?: {

// TODO -- should not be a public interface on its own, but required for Veneer
#AnnotationContainer: {
Copy link
Member Author

Choose a reason for hiding this comment

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

@sdboyer -- thoughts on how to best handle this? a nested type AnnotationQuery.target needs a Veneer... but that sets off a cascade of all the parents needing one also

Copy link
Contributor

Choose a reason for hiding this comment

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

the cascade being this change, for example? https://github.com/grafana/grafana/pull/66325/files#diff-932ad9420b4f3774837637528ee5bfe0ac3581cca3c5fdad61beb944aedbc45aR33

nope, no current or planned way around that. Would require the sort of generics support in cuetsy that i haven't yet seen what i consider to be a feasible proposal

Copy link
Member Author

Choose a reason for hiding this comment

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

not a huge deal -- just checking that I am not missing anything.


// unless datasources have migrated to the target+mapping,
// thhey just spread their query into the base object :(
...
Copy link
Member Author

Choose a reason for hiding this comment

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

@sdboyer is this the accepted choice for "or anything else" here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, if we have to accept unknown fields being spilled in here as valid, this is how to mark it.

It may be possible to not do this if it's possible for us to know at schema-time whether the datasource plugin in question has done the "target+mapping migration" you mention in the comment (idk what that is). That would be analogous to how if we encounter a panel that's from a plugin for which we have no composable kind, we let the options field be anything/unrestricted

#AnnotationTarget: {
limit: int64
// Only required/valid for the grafana datasource...
Copy link
Contributor

Choose a reason for hiding this comment

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

we could potentially model this special case directly, here in schema - but it would be preferable to refactor the grafana datasource to avoid such hacks

@torkelo torkelo requested a review from Ijin08 April 14, 2023 05:23
Comment on lines 135 to 146
const panels: Array<SelectableValue<number>> = useMemo(
() =>
dashboard?.panels
.map((panel) => ({
value: panel.id,
label: panel.title ?? `Panel ${panel.id}`,
description: panel.description,
imgUrl: config.panels[panel.type].info.logos.small,
}))
.sort((a, b) => (a.label > b.label ? 1 : -1)) ?? [],
[dashboard]
);
Copy link
Member

Choose a reason for hiding this comment

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

a way to move this to DashboardModel and maybe we can reuse this in the Shared dashboard query editor?

Copy link
Member

Choose a reason for hiding this comment

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

It looks a bit different, hm..

ashharrison90 and others added 8 commits April 17, 2023 15:33
…#66671)

remove localStorage mock which doesn't work in node v18.16.0
Co-authored-by: Gilles De Mey <gilles.de.mey@gmail.com>
* clean up navmodel interface

* remove concept of sections from NavModel interface

* clean up applinks
…#66281)

* TimePicker: Fixes issues with "Recently used absolute ranges" section

Squashed commit of the following:

commit 99d5076
Author: Joao Silva <joao.silva@grafana.com>
Date:   Tue Apr 11 14:06:27 2023 +0100

    user essentials mob! :trident:

    lastFile:public/app/core/components/TimePicker/TimePickerWithHistory.tsx

commit cad0201
Author: eledobleefe <laura.fernandez@grafana.com>
Date:   Tue Apr 11 11:44:34 2023 +0200

    user essentials mob! :trident:

    lastFile:public/app/core/components/TimePicker/TimePickerWithHistory.tsx

Co-authored-by: eledobleefe <laura.fernandez@grafana.com>

* TimePicker: Add correct date format

* Add convertRawToRange tests

* Rename test variables

* RTL tests

* Proper RTL tests

* Apply suggestions from code review

Co-authored-by: Joao Silva <100691367+JoaoSilvaGrafana@users.noreply.github.com>

* Remove commented line

* Fix linting

---------

Co-authored-by: eledobleefe <laura.fernandez@grafana.com>
Co-authored-by: Tobias Skarhed <tobias.skarhed@gmail.com>
Co-authored-by: Tobias Skarhed <1438972+tskarhed@users.noreply.github.com>
Squashed commit of the following:

commit 421bd0b
Author: Joao Silva <joao.silva@grafana.com>
Date:   Tue Apr 4 10:48:04 2023 +0100

    user essentials mob! :trident:

    lastFile:public/app/plugins/panel/annolist/AnnoListPanel.tsx

commit 862846a
Author: joshhunt <josh@trtr.co>
Date:   Tue Apr 4 10:35:35 2023 +0100

    user essentials mob! :trident:

commit 4ddc7fe
Author: Tobias Skarhed <tobias.skarhed@gmail.com>
Date:   Tue Apr 4 11:20:08 2023 +0200

    mob next [ci-skip] [ci skip] [skip ci]

    lastFile:public/app/plugins/panel/annolist/AnnoListPanel.tsx

Co-authored-by: joshhunt <josh@trtr.co>
Co-authored-by: Tobias Skarhed <tobias.skarhed@gmail.com>
* try a different way to run integration tests

* fix formatting

* clean test cache again

* use previous command

* playing around with random commands

* dont run tests in parallel yet

* use parallel option instead of gomaxprocs

* use same package set for redis/memcached; use p=1
This commit adds support for limits and filters to the Prometheus Rules
API.

Limits:

It adds a number of limits to the Grafana flavour of the Prometheus Rules
API:

- `limit` limits the maximum number of Rule Groups returned
- `limit_rules` limits the maximum number of rules per Rule Group
- `limit_alerts` limits the maximum number of alerts per rule

It sorts Rule Groups and rules within Rule Groups such that data in the
response is stable across requests. It also returns summaries (totals)
for all Rule Groups, individual Rule Groups and rules.

Filters:

Alerts can be filtered by state with the `state` query string. An example
of an HTTP request asking for just firing alerts might be
`/api/prometheus/grafana/api/v1/rules?state=alerting`.

A request can filter by two or more states by adding additional `state`
query strings to the URL. For example `?state=alerting&state=normal`.

Like the alert list panel, the `firing`, `pending` and `normal` state are
first compared against the state of each alert rule. All other states are
ignored. If the alert rule matches then its alert instances are filtered
against states once more.

Alerts can also be filtered by labels using the `matcher` query string.
Like `state`, multiple matchers can be provided by adding additional
`matcher` query strings to the URL.

The match expression should be parsed using existing regular expression
and sent to the API as URL-encoded JSON in the format:

{
    "name": "test",
    "value": "value1",
    "isRegex": false,
    "isEqual": true
}

The `isRegex` and `isEqual` options work as follows:

| IsEqual | IsRegex  | Operator |
| ------- | -------- | -------- |
| true    | false    |    =     |
| true    | true     |    =~    |
| false   | true     |    !~    |
| false   | false    |    !=    |
Copy link
Contributor

@nmarrs nmarrs left a comment

Choose a reason for hiding this comment

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

LGTM; a few nits but overall this is looking great!

@adela-almasan adela-almasan merged commit 9452c0d into main Apr 18, 2023
15 of 16 checks passed
@adela-almasan adela-almasan deleted the filter-annotations branch April 18, 2023 20:39
@zerok zerok modified the milestones: 10.0.0, 10.0.0-preview May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Annotations on a single panel