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

Allow embedding custom UI config in index.html #490

Merged
merged 11 commits into from
Oct 27, 2017
Merged

Allow embedding custom UI config in index.html #490

merged 11 commits into from
Oct 27, 2017

Conversation

yurishkuro
Copy link
Member

@yurishkuro yurishkuro commented Oct 24, 2017

For #489

$ cat yy.yaml
x: abcd
z:
  - a
  - b

$ go run ./cmd/standalone/main.go --span-storage.type=memory --query.ui-config yy.yaml

replaces

<!-- JAEGER_CONFIG = DEFAULT_CONFIG; -->

in index.html with

<!-- JAEGER_CONFIG = {"x":"abcd","z":["a","b"]}; -->

TODO:

  • tests
  • UI version that includes the config var in index.html

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 99.644% when pulling e241533 on ui-config into a7f203b on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling b026119 on ui-config into a7f203b on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.05%) to 99.954% when pulling 4b05d88 on ui-config into a7f203b on master.

@@ -2,4 +2,5 @@
<html lang="en">
<meta charset="UTF-8">
<title>Test Page</title>
<!-- JAEGER_CONFIG=DEFAULT_CONFIG; -->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change this to: "JAEGER_CONFIG = DEFAULT_CONFIG;" to be a more accurate fixture?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how is it more accurate? The regexp ignores the whitespace.

}

var c map[string]interface{}
var unmarshall func([]byte, interface{}) error
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/unmarshall/unmarshal

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 45f377d on ui-config into a7f203b on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling cdce79b on ui-config into a7f203b on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling e9f47fa on ui-config into 7f28c77 on master.

Yuri Shkuro added 10 commits October 27, 2017 00:50
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
…keys, instead of strings

Signed-off-by: Yuri Shkuro <[email protected]>
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling b557609 on ui-config into 256d7d2 on master.

Signed-off-by: Yuri Shkuro <[email protected]>
@coveralls
Copy link

coveralls commented Oct 27, 2017

Coverage Status

Coverage decreased (-81.3%) to 18.667% when pulling 72c09d1 on ui-config into 256d7d2 on master.

@yurishkuro yurishkuro merged commit 44de283 into master Oct 27, 2017
@ghost ghost removed the review label Oct 27, 2017
return &StaticAssetsHandler{staticAssetsRoot: staticAssetsRoot}
indexBytes, err := ioutil.ReadFile(staticAssetsRoot + "index.html")
if err != nil {
return nil, errors.Wrap(err, "Cannot read UI static assets")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it fails without UI. Before it was working. Workaround is to create empty index.html or do not include static handler if flag is empty

#493 uses query as a docker container, when xdock is executed it builds all images, Now we have to build UI which takes significant time.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's discuss on #497

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants