PublicDashboards: Add annotations support #56413
Conversation
…able to read the dashboard and dashboard annotations.
|
Drone build failed: https://drone.grafana.net/grafana/grafana-enterprise/36394 |
|
Drone build failed: https://drone.grafana.net/grafana/grafana-enterprise/36414 |
|
It is worth checking if we can do this though the builtin grafana datasource -- that already has annotation support (but may require some munging on the frontend) |
pkg/api/http_server.go
Outdated
| @@ -199,7 +199,7 @@ type HTTPServer struct { | |||
| orgService org.Service | |||
| teamService team.Service | |||
| accesscontrolService accesscontrol.Service | |||
| annotationsRepo annotations.Repository | |||
| AnnotationsRepo annotations.Repository | |||
There was a problem hiding this comment.
why does it need to be public? better if we we can avoid that
There was a problem hiding this comment.
I remember needing it public for some reason when I started tinkering around with this card, but looks like I don't need it anymore. Changed it back to private!
|
Drone build failed: https://drone.grafana.net/grafana/grafana-enterprise/36563 |
|
@ryantxu We could use the Grafana datasource for annotations, but I'm hesitant about sprinkling pubdash code around the various datasources we already have. Most have enough complexity as it is, and it's nice to have all the pubdash stuff in one datasource right now. Its been helpful to use the |
|
Drone build failed: https://drone.grafana.net/grafana/grafana-enterprise/36598 |
…wensmallwood/pubdash-add-annotations-support
| type annotation struct { | ||
| Datasource struct { | ||
| DType string `json:"type"` | ||
| Uid string `json:"uid"` | ||
| } | ||
| Enable bool `json:"enable"` | ||
| Type string `json:"type"` | ||
| Target struct { | ||
| Limit int64 `json:"limit"` | ||
| MatchAny bool `json:"matchAny"` | ||
| Tags []string `json:"tags"` | ||
| Type string `json:"type"` | ||
| } | ||
| } | ||
|
|
||
| type annotationsDto struct { | ||
| Annotations struct { | ||
| List []annotation `json:"list"` | ||
| } | ||
| } | ||
|
|
||
| dto := annotationsDto{} | ||
| bytes, err := dash.Data.MarshalJSON() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| err = json.Unmarshal(bytes, &dto) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| anonymousUser, err := pd.BuildAnonymousUser(ctx, dash) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| var results []*annotations.ItemDTO |
There was a problem hiding this comment.
@sdboyer how close are we to use the cue generated go types for dashboard to loop through in this case Annotation queries. We loop and manipulate the dashboard model in many places in the core code would be a good place to start using the generated types.
There was a problem hiding this comment.
For this purpose, we're close, plausibly close enough that this PR should be refactored to use generated types.
Looks like the main issue is that there's no specification for Target written down yet. But if the fields given here are indeed what's expected to be there (even if it's not ALL of them), then there's no reason this PR couldn't just add them to the schema then re-run make gen-cue.
There was a problem hiding this comment.
Once this PR is functional, I'll come back and tackle this.
| for _, anno := range dto.Annotations.List { | ||
| if anno.Enable { |
There was a problem hiding this comment.
Because this is only dealing with Grafana's built-in annotation data source here (and many use Prometheus, loki or Elasticsearch instead to query for annotations) you need to check the annotation query datasource prop.
Check if the uid is grafanads.DatasourceUID
There was a problem hiding this comment.
Thanks! This is still a work in progress. Just put up the draft PR for visibility
pkg/services/publicdashboards/service/testData/dashboardWithAnnotations.json
Outdated
Show resolved
Hide resolved
public/app/features/query/state/DashboardQueryRunner/AnnotationsWorker.ts
Show resolved
Hide resolved
| let datasourceObservable; | ||
| if (dashboard.meta.publicDashboardAccessToken !== '') { | ||
| const pubdashDatasource = new PublicDashboardDataSource(PUBLIC_DATASOURCE); | ||
| datasourceObservable = of(pubdashDatasource).pipe(catchError(handleDatasourceSrvError)); |
There was a problem hiding this comment.
Nice work with RXJS, would love a rundown of your experience!
| }; | ||
| const annotations = await getBackendSrv().get(`/api/public/dashboards/${accessToken}/annotations`, params); | ||
|
|
||
| return { data: [toDataFrame(annotations)] }; |
There was a problem hiding this comment.
Question: would it be a smart idea to return it as data frames directly from the backend instead of needing to transform the response in the frontend? Or do we have any other consumers of this API a part of this frontend?
There was a problem hiding this comment.
This is the only consumer of this endpoint right now. Since we can already transform it to dataframes on the frontend, I did it there. I didn't see a benefit to reimplementing it on the backend at this point.
… themselves. Adds another test around generating permissions for annotations.
|
Drone build failed: https://drone.grafana.net/grafana/grafana-enterprise/38174 |
|
Drone build failed: https://drone.grafana.net/grafana/grafana-enterprise/38186 |
|
Drone build failed: https://drone.grafana.net/grafana/grafana-enterprise/38189 |
owensmallwood
deleted the
owensmallwood/pubdash-add-annotations-support
branch
daniellee
changed the title
daniellee
changed the title
Changes
This adds support for Grafana annotations. Annotations that query an external datasource are not supported yet. Any Grafana annotations that query by dashboard or by tag should work with public dashboards.
Implementation Notes
fixes https://github.com/grafana/grafana-enterprise/issues/4475