app/vlselect: introduce -vmalert.proxyURL flag to proxy /select/vmalert requests to VMAlert #5
Conversation
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
AndrewChubatiuk
requested review from
Haleygo,
func25,
hagen1778,
vadimalekseev and
valyala
Member
vadimalekseev
left a comment
vadimalekseev
left a comment
There was a problem hiding this comment.
Looks good overall.
A small suggestion: It would be nice if we could access the vmalert UI through our start page: https://github.com/VictoriaMetrics/VictoriaLogs/blob/master/app/victoria-logs/main.go#L88-L91, when the vmalert proxy flag is set.
Contributor
Author
There's a separate issue for this, VMAlert UI will be a part of VMUI and these endpoints are needed to simplify debugging and to decouple review process of frontend and backend since different engineers are responsible for these parts |
AndrewChubatiuk
force-pushed
the
vmalert-proxy
branch
from
23e3ffa to
254bc7a
Compare
2 tasks
AndrewChubatiuk
force-pushed
the
vmalert-proxy
branch
from
950ff92 to
ecddfc7
Compare
AndrewChubatiuk
force-pushed
the
vmalert-proxy
branch
3 times, most recently
from
b45e562 to
0f8cb9a
Compare
AndrewChubatiuk
changed the title
AndrewChubatiuk
force-pushed
the
vmalert-proxy
branch
4 times, most recently
from
07ae6c9 to
3b1598e
Compare
hagen1778
reviewed
Aug 28, 2025
AndrewChubatiuk
force-pushed
the
vmalert-proxy
branch
from
bebea8c to
1a325fa
Compare
hagen1778
previously approved these changes
Sep 9, 2025
AndrewChubatiuk
force-pushed
the
vmalert-proxy
branch
from
9d2c211 to
e0d0fcc
Compare
hagen1778
approved these changes
Sep 10, 2025
AndrewChubatiuk
force-pushed
the
vmalert-proxy
branch
3 times, most recently
from
46dff9d to
098f850
Compare
AndrewChubatiuk
force-pushed
the
vmalert-proxy
branch
7 times, most recently
from
ee9d85b to
e2598e0
Compare
2 tasks
func25
reviewed
Sep 11, 2025
AndrewChubatiuk
force-pushed
the
vmalert-proxy
branch
2 times, most recently
from
a845dc3 to
db778ab
Compare
…g requests to /select/vmalert path to VMAlert
AndrewChubatiuk
force-pushed
the
vmalert-proxy
branch
from
db778ab to
2d9f998
Compare
valyala
added a commit
that referenced
this pull request
Sep 17, 2025
Reason for revert: this functionality is non-trivial, so it needs additional maintenance efforts in the future. This functionality is non-core for VictoriaLogs web UI (aka 'nice to have feature'). It is already provided by vmalert web UI ( https://docs.victoriametrics.com/victoriametrics/vmalert/#web ). I doubt we'll have enough capacity for providing high-quiality maintenance for this functionality. It is better to revert it instead of providing half-baked functionality for our users. Updates #522 Updates VictoriaMetrics/VictoriaMetrics#8989 Updates #5
valyala
added a commit
that referenced
this pull request
Sep 17, 2025
…D headers are set by proxy or remove it completely (#664)" This reverts commit 97ffbff. Reason for revert: this functionality depends on the commit 2d4b343 , which is going to be reverted. The reason for reverting the 2d4b343 is that it mixes multiple unrelated features in a single commit. This functionality must be implemented in a distinct pull request dedicated solely for this functionality at both VictoriaLogs server side and VictoriaLogs web UI. Updates #664 Updates #5 Updates #656
valyala
added a commit
that referenced
this pull request
Sep 17, 2025
…ct/vmalert requests to VMAlert (#5)" This reverts commit 2d4b343. Reason for revert: this commit mixes two unrelated features: - Proxying vmalert requests ( #90 ) - Introducing vmui config handler at /select/vmui/config.json ( there should be a separate feature request with the explanation why this handler is needed ) - Returning AccountID and ProjectID response headers ( #656 ) It is better from the maintenance and git history investigation PoV to split the implementation of these features into separate pull requests. This commit also has a few issues in the code, which must be addressed in the follow-up pull requests: - The description of the `-vmalert.proxyURL` command-line flag misses a whitespace between the `vmalert.` and `See`. - The description of the `-vmalert.proxyURL` command-line flag contains non-canonical link to docs - https://docs.victoriametrics.com/victorialogs#vmalert . It must be https://docs.victoriametrics.com/victorialogs/#vmalert - The `r.URL.Path = strings.TrimPrefix(path, "/select")` line isn't needed in the code block, which returns vmui config to the client. Updates #5
Contributor
|
@AndrewChubatiuk , @func25 , this pull request has been reverted in the commit ac32268 . See the commit description for the reason of the revert. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 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.
related issue #90
additionally PR introduces dynamic configuration for frontend, which depends on command line flags
Describe Your Changes
Please provide a brief description of the changes you made. Be as specific as possible to help others understand the purpose and impact of your modifications.
Checklist
The following checks are mandatory: