This repository was archived by the owner on Sep 30, 2024. It is now read-only.
prom-wrapper: remove direct Prometheus access, remove alerts endpoints#46299
Merged
bobheadxi merged 6 commits intoJan 11, 2023
Conversation
Member
Author
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
b57a979 to
0560e17
Compare
bobheadxi
commented
Jan 10, 2023
Comment on lines
-82
to
-90
| // prometheus client | ||
| promClient, err := prometheusAPI.NewClient(prometheusAPI.Config{ | ||
| Address: fmt.Sprintf("http://127.0.0.1:%s", prometheusPort), | ||
| }) | ||
| if err != nil { | ||
| logger.Fatal("failed to initialize prometheus client", | ||
| log.Error(err)) | ||
| } | ||
|
|
Member
Author
There was a problem hiding this comment.
The key result - we no longer need to talk to Prometheus directly.
michaellzc
approved these changes
Jan 10, 2023
Bundle size report 📦
Look at the Statoscope report for a full comparison between the commits a16abf7 and 18911c0 or learn more. Open explanation
|
bobheadxi
referenced
this pull request
in sourcegraph/src-cli
Jan 12, 2023
Removes usage of the endpoint deprecated in https://github.com/sourcegraph/sourcegraph/pull/46299, related to sourcegraph/customer#1525
scjohns
referenced
this pull request
in sourcegraph/src-cli
Apr 24, 2023
Removes usage of the endpoint deprecated in https://github.com/sourcegraph/sourcegraph/pull/46299, related to sourcegraph/customer#1525
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This removes the one instance of Sourcegraph being able to directly query Prometheus. Likely not something we should have done in the first place, and will make it easier for Cloud the potentially run a completely-managed Prometheus. We retain some APIs for the embedded Alertmanager instance for configuration validation and the like - Alertmanager has a smaller surface area, and future alert summary queries should use the Alertmanager API instead.
The GraphQL endpoints have been marked as deprecated and no longer return any data. In the future, we could replace with this the endpoints that talk to Alertmanager to get alerts summaries - the Alertmanager API is a smaller surface area and we likely want to continue to self-host Alertmanager in the future for the ability to better configure alerting.
This endpoint is rarely used - the only two usages I could find are:
instancehealthinsrc-cli- this is a non-critical, experimental integration and we can replace it with manual checks and/or a new API as described aboveTest plan
Test pass,
sg startworks