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

Query service: /api/config returning index.html #489

Closed
tiffon opened this issue Oct 23, 2017 · 9 comments
Closed

Query service: /api/config returning index.html #489

tiffon opened this issue Oct 23, 2017 · 9 comments

Comments

@tiffon
Copy link
Member

tiffon commented Oct 23, 2017

Description

The query-service is returning the body of index.html when the UI requests /api/config.

This is causing errors in the browser, but fortunately they are not interrupting the UI experience.

The ideal scenario is the config is dumped into the body of index.html when it is served. This will allow us to remove the HTTP call to /api/config entirely.

Per a previous discussion, the index.html can contain the following, which I believe can be replaced with a JSON serialization of the config:

JAEGER_CONFIG = DEFAULT_CONFIG;

Where DEFAULT_CONFIG is replaced with the UI config contents.

The UI config contents are {} by default, and can be overridden via a envar or cmd line parameter (maybe query.ui-config?) that conveys a filepath with a YAML or JSON file.

Steps to reproduce the issue:

  1. Follow the steps here
  2. Create a trace via the HotROD app
  3. View a trace

Describe the results you received:

/api/config returns the index.html content.

Describe the results you expected:

Either /api/config returns the index.html content or we switch to dumping the config into the body of the HTML.

@yurishkuro
Copy link
Member

Let's make a change in the UI project first that ensures that the generated index.html contains

DEFAULT_CONFIG = {};
JAEGER_CONFIG = DEFAULT_CONFIG;

Then we can add code to the query service to assign a different value to JAEGER_CONFIG when serving index.html

@tiffon
Copy link
Member Author

tiffon commented Oct 23, 2017

Sounds good, will post back when it's merged.

@yurishkuro
Copy link
Member

UI change: jaegertracing/jaeger-ui#107

@yurishkuro
Copy link
Member

All done & tested, keeping this open until we add docs on how to use it.

@pavolloffay
Copy link
Member

https://github.com/jaegertracing/jaeger-ui could list all configurable options.

@yurishkuro
Copy link
Member

@pavolloffay elaborate? Like a help page?

@yurishkuro
Copy link
Member

or you mean the README? +1 then

@pavolloffay
Copy link
Member

just the readme for now

@yurishkuro
Copy link
Member

I think we have one outstanding issue (jaegertracing/jaeger-ui#123) before we can document (although the current solution already works, so we could just document it as is - jaegertracing/jaeger-ui#124).

Since there are tickets in the UI repo now, I am going to close this one.

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

No branches or pull requests

3 participants