Update static UI assets path in contrib doc

[albertteoh]

Signed-off-by: albertteoh [email protected]

Which problem is this PR solving?

Following the instruction from the CONTRIBUTING.md file results in a "no such file or directory" error when trying to fetch static UI assets:

$ go run ./cmd/all-in-one/main.go --query.static-files jaeger-ui/packages

"Could not create static assets handler","error":"cannot load index.html: cannot open index.html: open jaeger-ui/build/index.html: no such file or directory"

Short description of the changes

The Makefile appears to copy the static UI assets into jaeger-ui/packages/jaeger-ui/build instead. See: https://github.com/jaegertracing/jaeger/blob/master/Makefile#L234

Changing the command to the correct path as specified in the build-ui Makefile target results in the all-in-one running successfully:

$ go run ./cmd/all-in-one/main.go --query.static-files jaeger-ui/packages/jaeger-ui/build 

[jpkrohling]

We actually have a make target now that can be used instead: make run-all-in-one

[albertteoh]

Ah okay, glad you pointed this out. Do you think we should delete this section of the contrib file then (i.e. "Alternatively, ..." + code block)?

Moreover, given we already have the make build-ui instruction in the previous section, could we just have a one-liner for this "Running local build with the UI" section of:

make run-all-in-one

Unless if we feel the first two sentences (i.e. "The jaeger-ui... .gitignored-ed.") are useful information.

[jpkrohling]

I think so, the whole section here could be replaced with this one make target.

[codecov]

Codecov Report

Merging #2548 into master will increase coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2548      +/-   ##
==========================================
+ Coverage   95.32%   95.36%   +0.03%     
==========================================
  Files         208      208              
  Lines        9248     9248              
==========================================
+ Hits         8816     8819       +3     
+ Misses        355      352       -3     
  Partials       77       77              

Impacted Files	Coverage Δ
plugin/storage/integration/integration.go	77.90% <0.00%> (+0.55%)	⬆️
...lugin/sampling/strategystore/adaptive/processor.go	100.00% <0.00%> (+0.92%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 600d023...c1281ca. Read the comment docs.

[yurishkuro]

The make target is great, but why remove the explanation? It's valuable information that now someone needs to dig out by reading source code.

[yurishkuro]

Not to mention that this section is about the UI, not all-in-one, i.e the same approach is used with query-service.

[albertteoh]

This is a valid point, @yurishkuro; how the UI assets are created and used deserves a more detailed explanation as it's not straightforward (at least to me) to understand from just looking at code and Makefiles.

I think though, that this instruction should belong in the Pre-requisites section as an explanation to what make build-ui does. So my proposal is (what do you think?):

Pre-requisites

Then install dependencies and run the tests:

git submodule update --init --recursive
make install-tools
make test

The jaeger-ui submodule contains 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:

# if you wish to build platform binaries locally - the step below is needed.
make build-ui

Running local build with the UI

The packaged assets (created by the make build-ui step above) can be enabled by providing a build tag ui. The following command will start Jaeger all-in-one with this ui tag.

make run-all-in-one

[yurishkuro]

I prefer UI-related explanations to be together under one section.