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

Loki: Add backend healthcheck #74330

Merged
merged 5 commits into from
Sep 5, 2023
Merged

Loki: Add backend healthcheck #74330

merged 5 commits into from
Sep 5, 2023

Conversation

svennergr
Copy link
Contributor

What is this feature?

Added the backend healthcheck based on the Prom implementation and https://grafana.com/docs/grafana/latest/developers/http_api/data_source/#check-data-source-health

Which issue(s) does this PR fix?:

Fixes #63726

Special notes for your reviewer:

  1. E.g. call http://localhost:3000/api/datasources/uid/DS_UID/health and make sure to replace DS_UID.
  2. Or go to DS settings and hit the "save" button.

return getHealthCheckMessage("There was an error returned querying the Loki API.", errors.New("invalid response"))
}

return getHealthCheckMessage("Successfully queried the Loki API.", nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

imagen

This is the message we're going to show to the users, and I'm not sure about the concept of "querying the Loki API". How about:

  • The data source test was successful.
  • Successfully queried the Loki instance.

Or just the good old "Data source successfully connected.".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good suggestion! Changed it to Data source successfully connected..

Copy link
Contributor

Choose a reason for hiding this comment

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

Perfect, thanks!

Copy link
Contributor

@matyax matyax left a comment

Choose a reason for hiding this comment

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

Looks good! Left a suggestion about error/success messages.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 4, 2023

Backend code coverage report for PR #74330

Plugin Main PR Difference
loki 59.5% 67.6% 8.1%

@github-actions
Copy link
Contributor

github-actions bot commented Sep 4, 2023

Frontend code coverage report for PR #74330

Plugin Main PR Difference
loki 85.43% 85.4% -.03%

@svennergr svennergr requested a review from a team as a code owner September 4, 2023 18:34
@svennergr svennergr requested review from ashharrison90 and JoaoSilvaGrafana and removed request for a team September 4, 2023 18:34
@svennergr svennergr merged commit a403027 into main Sep 5, 2023
16 checks passed
@svennergr svennergr deleted the svennergr/loki-check-health branch September 5, 2023 06:59
chauchausoup pushed a commit to chauchausoup/grafana that referenced this pull request Sep 15, 2023
* add loki healthcheck

* remove `testDatasource` call

* remove unused error check

* change success message

* improve error messages
rwwiv pushed a commit that referenced this pull request Oct 2, 2023
* add loki healthcheck

* remove `testDatasource` call

* remove unused error check

* change success message

* improve error messages
@zerok zerok modified the milestones: 10.2.x, 10.2.0 Oct 23, 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.

Loki: Implement CheckHealth method in backend
3 participants