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.

Sign up for GitHub

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

Jump to bottom

Alerting: Show annotations markers in TimeSeries panel when using Loki as … #72084

Merged
merged 18 commits into from
Aug 1, 2023
Merged

Alerting: Show annotations markers in TimeSeries panel when using Loki as … #72084

merged 18 commits into from
Aug 1, 2023

Conversation

soniaAguilarPeiron
Copy link
Member

@soniaAguilarPeiron soniaAguilarPeiron commented Jul 21, 2023 •
edited
Loading

What is this feature?

This PR renders Annotations markers for alerts in the TimeSeries panel, when using Loki for the alert state history.

So far, the TimeSeries panel still queries the old alert state history.
For this reason, these markers don't appear when using Loki as ASH, as it still only queries the /annotations endpoint directly.

Why do we need this feature?

TimeSeries panel should query the ASH endpoint and show the makers, even when in Loki alert state history mode.

Who is this feature for?

All users.

Special notes for your reviewer:

ash-final.mp4

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@soniaAguilarPeiron soniaAguilarPeiron added this to the 10.1.x milestone Jul 21, 2023
@soniaAguilarPeiron soniaAguilarPeiron self-assigned this Jul 21, 2023
@soniaAguilarPeiron soniaAguilarPeiron requested review from a team as code owners July 21, 2023 08:24
@soniaAguilarPeiron soniaAguilarPeiron requested review from gillesdemey, VikaCep and konrad147 and removed request for a team July 21, 2023 08:24
@soniaAguilarPeiron soniaAguilarPeiron marked this pull request as draft July 21, 2023 08:24
@grafanabot grafanabot added the area/panel/timeseries The main time series Graph panel label Jul 21, 2023
@soniaAguilarPeiron soniaAguilarPeiron force-pushed the alerting/show-annotation-markers-loki-history branch from ab6350b to f5ca316 Compare July 24, 2023 16:09
@soniaAguilarPeiron soniaAguilarPeiron force-pushed the alerting/show-annotation-markers-loki-history branch 2 times, most recently from 8784041 to edfa992 Compare July 24, 2023 16:46
@soniaAguilarPeiron soniaAguilarPeiron force-pushed the alerting/show-annotation-markers-loki-history branch from edfa992 to b0d5626 Compare July 25, 2023 07:01
@soniaAguilarPeiron soniaAguilarPeiron force-pushed the alerting/show-annotation-markers-loki-history branch from b0d5626 to e34fb29 Compare July 25, 2023 07:07
@soniaAguilarPeiron soniaAguilarPeiron force-pushed the alerting/show-annotation-markers-loki-history branch from ea59ab4 to 814935a Compare July 25, 2023 07:17
@soniaAguilarPeiron soniaAguilarPeiron marked this pull request as ready for review July 25, 2023 07:49
@ephemeral-instances-bot
Copy link

  • Preparing your instance. A comment containing your instance's url will be added to this PR when the instance is ready.
  • Your instance will be ready in ~10 minutes.
  • Check the GitHub actions tab to follow the workflow progress- Slack channel: #proj-ephemeral-hg-instances

@soniaAguilarPeiron
Copy link
Member Author

/deploy-to-hg

@ephemeral-instances-bot
Copy link

  • Preparing your instance. A comment containing your instance's url will be added to this PR when the instance is ready.
  • Your instance will be ready in ~10 minutes.
  • Check the GitHub actions tab to follow the workflow progress- Slack channel: #proj-ephemeral-hg-instances

@soniaAguilarPeiron
Copy link
Member Author

/deploy-to-hg

@ephemeral-instances-bot
Copy link

  • Preparing your instance. A comment containing your instance's url will be added to this PR when the instance is ready.
  • Your instance will be ready in ~10 minutes.
  • Check the GitHub actions tab to follow the workflow progress- Slack channel: #proj-ephemeral-hg-instances

@ephemeral-instances-bot
Copy link

@soniaAguilarPeiron
Copy link
Member Author

/deploy-to-hg

@ephemeral-instances-bot
Copy link

  • Preparing your instance. A comment containing your instance's url will be added to this PR when the instance is ready.
  • Your instance will be ready in ~10 minutes.
  • Check the GitHub actions tab to follow the workflow progress- Slack channel: #proj-ephemeral-hg-instances

@ephemeral-instances-bot
Copy link

@soniaAguilarPeiron soniaAguilarPeiron merged commit de6ef53 into main Aug 1, 2023
12 checks passed
@soniaAguilarPeiron soniaAguilarPeiron deleted the alerting/show-annotation-markers-loki-history branch August 1, 2023 10:00
@grafana-delivery-bot grafana-delivery-bot bot modified the milestones: 10.1.x, 10.2.x Aug 1, 2023
sarahzinger pushed a commit that referenced this pull request Aug 1, 2023
@soniaAguilarPeiron @sarahzinger
Alerting: Show annotations markers in TimeSeries panel when using Lok…
ca38547 
…i as … (#72084)

* WIP: Show annotations markers in TimeSeries panel when using Loki as alert state history

* WIP changes

* Fix converting log records to data frame for panel

* Move fetching alert state history with Loki to the PannelQueryRunner to keep the panel flow

* use dasboardUID and panelUID for requesting Loki ash

* fix wrong prettier change

* Only request loki ash when having alertstate

* Use panelID as param in history query

* Refactor: move getRuleHistoryRecordsForPanel and remove filtering code as is not used

* Adress PR review comments

* Add try catch for ash request

* Add tests for updatePanelDataWithASHFromLoki method

* Address PR review suggestions

* review suggestion

* Add test for logRecordsToDataFrameForPanel method

* pr Review nit suggestion

* Dont show toast messages from Loki request
@torkelo
Copy link
Member

torkelo commented Sep 7, 2023

@soniaAguilarPeiron this could have needed a design doc and review from dashboard squad, adding a query like this directly to PanelQueryRunner feels a bit hacky.,

and a loki store for annotations should go through the normal annotations API endpoint, so it can be used to store user created annotations events

these annotations now sidestep Grafana's annotation system and cannot be disabled

soniaAguilarPeiron added a commit that referenced this pull request Sep 7, 2023
@soniaAguilarPeiron
Revert "Alerting: Show annotations markers in TimeSeries panel when u…
b543e81 
…sing Loki as … (#72084)"

This reverts commit de6ef53.
@soniaAguilarPeiron soniaAguilarPeiron mentioned this pull request Sep 7, 2023
Merged
3 tasks
@soniaAguilarPeiron
Copy link
Member Author

@torkelo I've just created a PR to revert this commit. We should have get the dashboard squad involved to prevent a wrong approach.
The alerting team had some DD regarding the new alert state endpoint FOR Loki. Maybe @alexweav can give here some more context.

@soniaAguilarPeiron
Copy link
Member Author

After an internal conversation in slack, we are going to rethink how this new alert state history with Loki is implemented so we can get it directly from the current annotation system in Grafana.

soniaAguilarPeiron added a commit that referenced this pull request Sep 8, 2023
@soniaAguilarPeiron
Alerting: Revert "Alerting: Show annotations markers in TimeSeries pa…
cad39b7 
…nel when u… (#74576)

Revert "Alerting: Show annotations markers in TimeSeries panel when using Loki as … (#72084)"

This reverts commit de6ef53.
chauchausoup pushed a commit to chauchausoup/grafana that referenced this pull request Sep 15, 2023
@soniaAguilarPeiron @chauchausoup
Alerting: Show annotations markers in TimeSeries panel when using Lok…
dfd08c8 
…i as … (grafana#72084)

* WIP: Show annotations markers in TimeSeries panel when using Loki as alert state history

* WIP changes

* Fix converting log records to data frame for panel

* Move fetching alert state history with Loki to the PannelQueryRunner to keep the panel flow

* use dasboardUID and panelUID for requesting Loki ash

* fix wrong prettier change

* Only request loki ash when having alertstate

* Use panelID as param in history query

* Refactor: move getRuleHistoryRecordsForPanel and remove filtering code as is not used

* Adress PR review comments

* Add try catch for ash request

* Add tests for updatePanelDataWithASHFromLoki method

* Address PR review suggestions

* review suggestion

* Add test for logRecordsToDataFrameForPanel method

* pr Review nit suggestion

* Dont show toast messages from Loki request
chauchausoup pushed a commit to chauchausoup/grafana that referenced this pull request Sep 15, 2023
@soniaAguilarPeiron @chauchausoup
Alerting: Revert "Alerting: Show annotations markers in TimeSeries pa…
4cc8fde 
…nel when u… (grafana#74576)

Revert "Alerting: Show annotations markers in TimeSeries panel when using Loki as … (grafana#72084)"

This reverts commit de6ef53.
rwwiv pushed a commit that referenced this pull request Oct 2, 2023
@soniaAguilarPeiron @rwwiv
Alerting: Revert "Alerting: Show annotations markers in TimeSeries pa…
9c48a31 
…nel when u… (#74576)

Revert "Alerting: Show annotations markers in TimeSeries panel when using Loki as … (#72084)"

This reverts commit de6ef53.
@rwwiv rwwiv mentioned this pull request Oct 19, 2023
Open
@zerok zerok modified the milestones: 10.2.x, 10.2.0 Oct 23, 2023
@dnhn dnhn mentioned this pull request Oct 24, 2023
Closed
@BrewTestBot BrewTestBot mentioned this pull request Oct 25, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matyax matyax matyax left review comments

@gillesdemey gillesdemey gillesdemey approved these changes

@VikaCep VikaCep VikaCep approved these changes

@dprokop dprokop Awaiting requested review from dprokop dprokop is a code owner automatically assigned from grafana/dashboards-squad

@polibb polibb Awaiting requested review from polibb polibb is a code owner automatically assigned from grafana/dashboards-squad

@konrad147 konrad147 Awaiting requested review from konrad147

Assignees

@soniaAguilarPeiron soniaAguilarPeiron

Labels
add to changelog area/alerting Grafana Alerting area/frontend area/panel/timeseries The main time series Graph panel no-backport Skip backport of PR
Projects
Alerting
Archived in project
Milestone
10.2.0
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@soniaAguilarPeiron @torkelo @gillesdemey @matyax @VikaCep @konrad147 @zerok @grafanabot