Package static assets inside the query-service binary #918
Conversation
yurishkuro
requested review from
black-adder,
jpkrohling,
pavolloffay and
vprithvi
as code owners
ghost
assigned 
Codecov Report
@@ Coverage Diff @@
## master #918 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 126 126
Lines 6073 6070 -3
=====================================
- Hits 6073 6070 -3
Continue to review full report at Codecov.
|
yurishkuro
force-pushed
the
embed-ui-assets
branch
from
024f99a
to
d86f741
Compare
| ADD jaeger-ui-build /go/jaeger-ui/ | ||
| CMD ["/go/bin/query-linux", "--query.static-files=/go/jaeger-ui/"] | ||
|
|
||
| # TODO replace this with ENTRYPOINT |
There was a problem hiding this comment.
why cant this be done now?
There was a problem hiding this comment.
I did at first, but it would be a breaking change (existing docker-compose, helm/kubernetes configs won't work), so I think it's better to do for all binaries at once (#815).
| if err != nil { | ||
| return nil, errors.Wrap(err, "Cannot read UI static assets") | ||
| return nil, errors.Wrap(err, "Cannot load index.html") |
There was a problem hiding this comment.
loadIndexHTML seems to return errors that are sufficient, don't think the wrap is necessary.
There was a problem hiding this comment.
If this proves to be an issue, a possible workaround is to have the real archive generated into a different file or directory that is included in .gitignore, to prevent accidental check-ins, and use build tags to switch between fake and real archive.
I'd rather have it now and not have any auto-generated code checked in. It will be a PITA to git checkout -- cmd/query/app/statik/statik.go every time we send a PR and then make build_ui whenever we need to browse the UI.
Another question: would this change affect the possibility (or inability) to have custom paths to the UI? People have expressed in the past that they need/want to access the UI under a specific path.
cmd/query/app/statik/doc.go
Outdated
| @@ -0,0 +1,20 @@ | |||
| // Copyright (c) 2018 Uber Technologies, Inc. | |||
There was a problem hiding this comment.
Copyright headers on new files should be "The Jaeger Authors", no?
| COPY ./cmd/standalone/standalone-linux /go/bin/ | ||
| COPY ./cmd/standalone/sampling_strategies.json /go/src/config/ | ||
| COPY ./cmd/standalone/sampling_strategies.json /etc/jaeger/ |
Signed-off-by: Yuri Shkuro <ys@uber.com>
Signed-off-by: Yuri Shkuro <ys@uber.com>
Signed-off-by: Yuri Shkuro <ys@uber.com>
Signed-off-by: Yuri Shkuro <ys@uber.com>
Signed-off-by: Yuri Shkuro <ys@uber.com>
Signed-off-by: Yuri Shkuro <ys@uber.com>
Signed-off-by: Yuri Shkuro <ys@uber.com>
Signed-off-by: Yuri Shkuro <ys@uber.com>
Signed-off-by: Yuri Shkuro <ys@uber.com>
yurishkuro
force-pushed
the
embed-ui-assets
branch
from
123ac0a
to
8965b25
Compare
ok, done and updated the description. |
Signed-off-by: Yuri Shkuro <ys@uber.com>
Signed-off-by: Yuri Shkuro <ys@uber.com>
Signed-off-by: Yuri Shkuro <ys@uber.com>
| echo "Cannot find UI assets, e.g. $INDEX_HTML. Aborting." | ||
| exit 1 | ||
| fi | ||
|
|
There was a problem hiding this comment.
no longer needed since compile will fail if the ui packaged file is missing
Signed-off-by: Yuri Shkuro <ys@uber.com>
The Makefile target
build_uiruns an additional command to package NPM-generated assets into a single Go file. The resulting file is huge (13Mb), so it is.gitignore-ed to prevent accidental check-ins. Instead, another placeholder file is generated and checked-in that packages a dummy index.html. When the build taguiis provided, the real assets are used, cf.Another way to run with the real UI is by passing the assets path via
--query.static-filesflag: