Add feature to build and enable UI V2#4974
Add feature to build and enable UI V2#4974sysadmind wants to merge 9 commits intoprometheus:mainfrom
Conversation
Adds the Makefile and UI embedding components to serve the new UI based on Mantine. This can be enabled when built with the tag `builtinassets`. This is a close match to what Prometheus does to build and embed the UI. The UI is still very early, so the utility is very limited, but this will allow testing the features more easily as they become available. Signed-off-by: Joe Adams <github@joeadams.io>
Signed-off-by: Joe Adams <github@joeadams.io>
Signed-off-by: Joe Adams <github@joeadams.io>
|
CI is failing even after a re-run. |
|
The problem is that this is now happening in the build process where it was not before and it's already inside docker so it doesn't have access to run docker. I think the logical option is to remove the elm build from the regular build process. The other option would be to enable the docker in docker build. |
Signed-off-by: Joe Adams <github@joeadams.io>
Signed-off-by: Joe Adams <github@joeadams.io>
| @@ -0,0 +1,46 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
We removed this file in #4617 do we need to add it back now again?
There was a problem hiding this comment.
It's used in prometheus for the bundling of static assets into a go file. I think we should keep it.
There was a problem hiding this comment.
I don't think we should add compress_assets.sh.
-
It is very complex, having to both generate .go files and running extra shell scripts as part of the build. It would be better to just directly use Vite for building the gzip files in a
distfolder. This would allow us to easily add brotli compression later. -
Everytime the ui is opened,
alertmanagerwill decompress the files before sending them over the network. This can be seen from thecurloutput:
curl -s -D - -H "Accept-Encoding: gzip" http://localhost:9093/assets/index-D3YozeYE.js -o /dev/null
HTTP/1.1 200 OK
Accept-Ranges: bytes
Content-Length: 530907
Content-Type: application/javascript
Date: Sat, 07 Mar 2026 19:22:22 GMT
So, we are missing out on the main benefit of compression: Reducing the number of TCP round trips.
- The benefits of compression are negligible:
❯ du -hs assets/index-D3YozeYE.js.gz assets/index-pOPVlMO9.css.gz index.html.gz
164K assets/index-D3YozeYE.js.gz
32K assets/index-pOPVlMO9.css.gz
4.0K index.html.gz
❯ du -hs assets/index-D3YozeYE.js assets/index-pOPVlMO9.css index.html
520K assets/index-D3YozeYE.js
216K assets/index-pOPVlMO9.css
4.0K index.htmlThus, your change will reduce the binary size by 500K .
TLDR; We should keep it simple and not do any compression. If we want to do compression, we should do it properly, and support streaming the files compressed over the network.
There was a problem hiding this comment.
The binary file saving are small right now because of tree shaking. There is very little in the UI if you compile it from this PR. As more features and functionality get added, this will grow. I don't have hard data around what that will be, but if there's consensus around embedding the uncompressed files, I'll pull this out.
| if enableUIV2 { | ||
| RegisterUIV2(r, logger) | ||
| } else { | ||
| RegisterUIV1(r, logger) | ||
| } |
There was a problem hiding this comment.
Should we use Elm and React for naming these to be more clear?
There was a problem hiding this comment.
I'm certainly open to renaming them. I don't know that I love the word "react" though because in some future, there may be a UI v3 that is also based on react.
|
|
||
| .PHONY: build | ||
| build: common-build | ||
| build: assets-compress common-build |
There was a problem hiding this comment.
Currently the project ships pre-compiled assets bundle so UI does not have to be built each time.
There was a problem hiding this comment.
I think we should re-evaluate that for the mantine based UI. The build output has unique filenames per build. This will cause a lot of churn in git.
|
|
||
| reactAssetsRoot := "/static/mantine-ui" | ||
|
|
||
| serveReactApp := func(w http.ResponseWriter, _ *http.Request) { |
There was a problem hiding this comment.
Missing cache-control headers?
There was a problem hiding this comment.
What outcome would you like to see with those?
There was a problem hiding this comment.
Just noticed they are missing and wondered if we need them.
There was a problem hiding this comment.
Personally, if we think they will provide value, I would want to see this happen towards the end of the project. That way we can test what benefits and what tradeoffs they cause.
There was a problem hiding this comment.
The caching headers are need such that users can seamlessly update their alertmanager binaries: If the browser caches the index.html, we will have situation that a browser will run an old index.html against a new alertmanager server. This will definitely cause failures, as far as I can see, because alertmanager will return 404s for these files:
<script type="module" crossorigin src="/assets/index-D3YozeYE.js"></script>
<link rel="stylesheet" crossorigin href="/assets/index-pOPVlMO9.css">We should add the cache-control headers now, since this cannot be fixed retroactively.
The other caches should be fine, since the index files contain the hashes.
Signed-off-by: Joe Adams <github@joeadams.io>
siavashs
left a comment
There was a problem hiding this comment.
LGTM at this stage since it is not the default UI.
Let's wait for other maintainers to also take a look.
SoloJacobs
left a comment
There was a problem hiding this comment.
Hey,
I finally had a bit of time to look over these changes. Thanks for gathering all this information, I tried it out locally and it works. I do have some feedback, though: You stuck very close to Prometheus, but that implementation has drawbacks.
But have a look at the comments. I can also help with concrete implementations in this change.
Kind regards
| @@ -0,0 +1,46 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
I don't think we should add compress_assets.sh.
-
It is very complex, having to both generate .go files and running extra shell scripts as part of the build. It would be better to just directly use Vite for building the gzip files in a
distfolder. This would allow us to easily add brotli compression later. -
Everytime the ui is opened,
alertmanagerwill decompress the files before sending them over the network. This can be seen from thecurloutput:
curl -s -D - -H "Accept-Encoding: gzip" http://localhost:9093/assets/index-D3YozeYE.js -o /dev/null
HTTP/1.1 200 OK
Accept-Ranges: bytes
Content-Length: 530907
Content-Type: application/javascript
Date: Sat, 07 Mar 2026 19:22:22 GMT
So, we are missing out on the main benefit of compression: Reducing the number of TCP round trips.
- The benefits of compression are negligible:
❯ du -hs assets/index-D3YozeYE.js.gz assets/index-pOPVlMO9.css.gz index.html.gz
164K assets/index-D3YozeYE.js.gz
32K assets/index-pOPVlMO9.css.gz
4.0K index.html.gz
❯ du -hs assets/index-D3YozeYE.js assets/index-pOPVlMO9.css index.html
520K assets/index-D3YozeYE.js
216K assets/index-pOPVlMO9.css
4.0K index.htmlThus, your change will reduce the binary size by 500K .
TLDR; We should keep it simple and not do any compression. If we want to do compression, we should do it properly, and support streaming the files compressed over the network.
| "path/filepath" | ||
| "strings" | ||
|
|
||
| "github.com/shurcooL/httpfs/filter" |
There was a problem hiding this comment.
We should not be using shurcool. The files can be easily be routed to the correct location with a few lines of go.
|
|
||
| reactAssetsRoot := "/static/mantine-ui" | ||
|
|
||
| serveReactApp := func(w http.ResponseWriter, _ *http.Request) { |
There was a problem hiding this comment.
The caching headers are need such that users can seamlessly update their alertmanager binaries: If the browser caches the index.html, we will have situation that a browser will run an old index.html against a new alertmanager server. This will definitely cause failures, as far as I can see, because alertmanager will return 404s for these files:
<script type="module" crossorigin src="/assets/index-D3YozeYE.js"></script>
<link rel="stylesheet" crossorigin href="/assets/index-pOPVlMO9.css">We should add the cache-control headers now, since this cannot be fixed retroactively.
The other caches should be fine, since the index files contain the hashes.
| return | ||
| } | ||
| defer func() { _ = f.Close() }() | ||
| idx, err := io.ReadAll(f) |
There was a problem hiding this comment.
Nit: Why not use io.Copy
| // Static files required by the React app. | ||
| router.Get(reactStaticAssetsDir+"/*filepath", func(w http.ResponseWriter, r *http.Request) { | ||
| r.URL.Path = path.Join(reactAssetsRoot+reactStaticAssetsDir, route.Param(r.Context(), "filepath")) | ||
| fs := server.StaticFileServer(Assets) |
There was a problem hiding this comment.
I don't think we should create a StaticFileServer on every request.
| return f.enableAutoGOMAXPROCS | ||
| } | ||
|
|
||
| func (f *Flags) EnableUIV2() bool { |
There was a problem hiding this comment.
I don't think UI is ready to be shipped with next release, so I would rather not include it in the release. Would it be possible for us to provide a separate build target instead?
| @@ -0,0 +1,31 @@ | |||
| import { FC } from 'react'; | |||
There was a problem hiding this comment.
Does this possibly belong in a different PR?
There was a problem hiding this comment.
Yes I will pull it out.
| router.Get(p, serveReactApp) | ||
| } | ||
|
|
||
| reactStaticAssetsDir := "/assets" |
| // Serve the React app. | ||
| reactRouterPaths := newUIReactRouterPaths | ||
|
|
||
| for _, p := range reactRouterPaths { |
There was a problem hiding this comment.
I'm wondering about this: Aren't we building a SPA? Typically, you would serve everything, not just a fixed set of subpaths, i.e., localhost:9093/alerts/special-name should also serve the react app?
For now, serving 404 is fine I suppose, but we will definitely have to touch this part, once we build deep links. I am wondering whether it is better just have a general /ui path.
There was a problem hiding this comment.
I think that /ui is a fair option if there is general agreement on that direction.
There was a problem hiding this comment.
Just a comment from the sidelines, since I don't have the capacity to get more deeply involved at the moment:
Since this routing code was taken from the Prometheus server: any UI URL parametrization in the Prometheus server happens only via HTTP parameters (since we encode a lot of different parameters in there on some of the pages, and then we just standardized on that single mechanism), so we only need to explicitly list a couple of fixed paths there. I wonder if for Alertmanager you really want to split this up and store some (prominent) parameters in the path and others (like detailed filters) in HTTP params, or whether it's nicer to just dump them all into HTTP params like in Prometheus and have one unified way of handling them all?
Aside from that: as a user I personally aesthetically prefer UI paths without a /ui/... prefix. Maybe the path routing can also be restructured to serve the UI for a set of patterns, or for everything that is not /api/..., /-/healthy, or one of the other special endpoints.
|
Hey, just because I saw you might be active here: I did make some progress on what I think the UI should look like. This change is not ready, yet: I put the first three commit into review, but the final commit contains the interesting changes:
The basic idea is that we have to move to vite sooner or later, so we might as well do it now and find out how it affects users. |
|
I think based on the feedback, I want to take a different approach. I have opened a PR for the small UI change - #5106 . After the changes from @SoloJacobs above from the |
Adds the Makefile and UI embedding components to serve the new UI based on Mantine. This can be enabled when built with the tag
builtinassets. This is a close match to what Prometheus does to build and embed the UI.The UI is still very early, so the utility is very limited, but this will allow testing the features more easily as they become available.