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

Simplify Bundling of UI assets #5917

Merged
merged 19 commits into from
Sep 2, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,6 @@ the source code for the UI assets (requires Node.js 6+).
The assets must be compiled first with `make build-ui`, which runs Node.js build and then
packages the assets into a Go file that is `.gitignore`-ed.

The packaged assets can be enabled by providing a build tag `ui`, for example:

```
$ go run -tags ui ./cmd/all-in-one/main.go
```

`make run-all-in-one` essentially runs Jaeger all-in-one by combining both of the above
steps into a single `make` command.

Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ lint-go: $(LINT)

.PHONY: run-all-in-one
run-all-in-one: build-ui
go run -tags ui ./cmd/all-in-one --log-level debug
go run ./cmd/all-in-one --log-level debug

.PHONY: changelog
changelog:
Expand Down
3 changes: 0 additions & 3 deletions Makefile.BuildBinaries.mk
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,11 @@ _build-a-binary-%:

.PHONY: build-jaeger
build-jaeger: BIN_NAME = jaeger
build-jaeger: GO_TAGS = -tags ui
build-jaeger: BUILD_INFO = $(BUILD_INFO_V2)
build-jaeger: build-ui _build-a-binary-jaeger$(SUFFIX)-$(GOOS)-$(GOARCH)

.PHONY: build-all-in-one
build-all-in-one: BIN_NAME = all-in-one
build-all-in-one: GO_TAGS = -tags ui
build-all-in-one: build-ui _build-a-binary-all-in-one$(SUFFIX)-$(GOOS)-$(GOARCH)

.PHONY: build-agent
Expand All @@ -89,7 +87,6 @@ build-agent: _build-a-binary-agent$(SUFFIX)-$(GOOS)-$(GOARCH)

.PHONY: build-query
build-query: BIN_NAME = query
build-query: GO_TAGS = -tags ui
build-query: build-ui _build-a-binary-query$(SUFFIX)-$(GOOS)-$(GOARCH)

.PHONY: build-collector
Expand Down
6 changes: 1 addition & 5 deletions cmd/query/app/static_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,11 @@ type loadedConfig struct {

// NewStaticAssetsHandler returns a StaticAssetsHandler
func NewStaticAssetsHandler(staticAssetsRoot string, options StaticAssetsHandlerOptions) (*StaticAssetsHandler, error) {
assetsFS := ui.StaticFiles
assetsFS := ui.GetStaticFiles(options.Logger)
if staticAssetsRoot != "" {
assetsFS = http.Dir(staticAssetsRoot)
}

if options.Logger == nil {
options.Logger = zap.NewNop()
}
mahadzaryab1 marked this conversation as resolved.
Show resolved Hide resolved

h := &StaticAssetsHandler{
options: options,
assetsFS: assetsFS,
Expand Down
15 changes: 12 additions & 3 deletions cmd/query/app/static_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ import (
//go:generate mockery -all -dir ../../../pkg/fswatcher

func TestNotExistingUiConfig(t *testing.T) {
handler, err := NewStaticAssetsHandler("/foo/bar", StaticAssetsHandlerOptions{})
handler, err := NewStaticAssetsHandler("/foo/bar", StaticAssetsHandlerOptions{
Logger: zap.NewNop(),
})
require.Error(t, err)
assert.Contains(t, err.Error(), "no such file or directory")
assert.Nil(t, handler)
Expand Down Expand Up @@ -155,11 +157,18 @@ func TestRegisterStaticHandler(t *testing.T) {
}

func TestNewStaticAssetsHandlerErrors(t *testing.T) {
_, err := NewStaticAssetsHandler("fixture", StaticAssetsHandlerOptions{UIConfigPath: "fixture/invalid-config"})
_, err := NewStaticAssetsHandler("fixture", StaticAssetsHandlerOptions{
UIConfigPath: "fixture/invalid-config",
Logger: zap.NewNop(),
})
require.Error(t, err)

for _, base := range []string{"x", "x/", "/x/"} {
_, err := NewStaticAssetsHandler("fixture", StaticAssetsHandlerOptions{UIConfigPath: "fixture/ui-config.json", BasePath: base})
_, err := NewStaticAssetsHandler("fixture", StaticAssetsHandlerOptions{
UIConfigPath: "fixture/ui-config.json",
BasePath: base,
Logger: zap.NewNop(),
})
require.Errorf(t, err, "basePath=%s", base)
assert.Contains(t, err.Error(), "invalid base path")
}
Expand Down
21 changes: 0 additions & 21 deletions cmd/query/app/ui/actual.go

This file was deleted.

32 changes: 32 additions & 0 deletions cmd/query/app/ui/assets.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright (c) 2024 The Jaeger Authors.
// SPDX-License-Identifier: Apache-2.0

package ui

import (
"embed"
"net/http"

"go.uber.org/zap"

"github.com/jaegertracing/jaeger/pkg/gzipfs"
"github.com/jaegertracing/jaeger/pkg/httpfs"
)

//go:embed actual/*
var actualAssetsFS embed.FS

//go:embed placeholder/index.html
var placeholderAssetsFS embed.FS

// GetStaticFiles gets the static assets that the Jaeger UI will serve. If the actual
// assets are available, then this function will return them. Otherwise, a
// non-functional index.html is returned to be used as a placeholder.
func GetStaticFiles(logger *zap.Logger) http.FileSystem {
if _, err := actualAssetsFS.ReadFile("actual/index.html.gz"); err != nil {
logger.Warn("ui assets not embedded in the binary, using a placeholder", zap.Error(err))
return httpfs.PrefixedFS("placeholder", http.FS(placeholderAssetsFS))
yurishkuro marked this conversation as resolved.
Show resolved Hide resolved
}

return httpfs.PrefixedFS("actual", http.FS(gzipfs.New(actualAssetsFS)))

Check warning on line 31 in cmd/query/app/ui/assets.go

View check run for this annotation

Codecov / codecov/patch

cmd/query/app/ui/assets.go#L31

Added line #L31 was not covered by tests
}
31 changes: 31 additions & 0 deletions cmd/query/app/ui/assets_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright (c) 2024 The Jaeger Authors.
// SPDX-License-Identifier: Apache-2.0

package ui

import (
"embed"
"io"
"testing"

"github.com/stretchr/testify/require"

"github.com/jaegertracing/jaeger/pkg/testutils"
)

func TestGetStaticFiles_ReturnsPlaceholderWhenActualNotPresent(t *testing.T) {
// swap out the assets FS for an empty one and then replace it back when the
// test completes
currentActualAssets := actualAssetsFS
actualAssetsFS = embed.FS{}
t.Cleanup(func() { actualAssetsFS = currentActualAssets })

logger, logBuf := testutils.NewLogger()
fs := GetStaticFiles(logger)
file, err := fs.Open("/index.html")
require.NoError(t, err)
bytes, err := io.ReadAll(file)
require.NoError(t, err)
require.Contains(t, string(bytes), "This is a placeholder for the Jaeger UI home page")
require.Contains(t, logBuf.String(), "ui assets not embedded in the binary, using a placeholder")
}
5 changes: 2 additions & 3 deletions cmd/query/app/ui/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// SPDX-License-Identifier: Apache-2.0

// Package ui bundles UI assets packaged with stdlib/embed.
// By default it imports the placeholder, non-functional index.html.
// When building with "-tags ui", it imports the real UI assets
// generated in build-ui Makefile target.
// If the real UI assets are available, they are embedded and
// used at runtime. Otherwise, the non-functional index.html is used.
package ui
20 changes: 0 additions & 20 deletions cmd/query/app/ui/placeholder.go

This file was deleted.

2 changes: 1 addition & 1 deletion cmd/query/app/ui/placeholder/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<h1>🥋: This is not the Jaeger UI you are looking for!</h1>
<p>
This is a placeholder for the Jaeger UI home page.
If you are seeing this, you are running a binary that was not compiled with the UI assets (<code>-tags ui</code>).
If you are seeing this, you are running a binary that was not compiled with the UI assets.
See <a href="https://github.com/jaegertracing/jaeger/blob/main/CONTRIBUTING.md#running-local-build-with-the-ui">these instructions</a>.
</p>
<h1>UI configuration</h1>
Expand Down
Loading