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.

Sign up for GitHub

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

Jump to bottom

[Feature]: Support external URL prefix #5157

Open
Sidhartha-Biswal opened this issue Jan 31, 2024 · 4 comments · May be fixed by #5219
Open

[Feature]: Support external URL prefix #5157

Sidhartha-Biswal opened this issue Jan 31, 2024 · 4 comments · May be fixed by #5219
Labels
enhancement good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement

Comments

@Sidhartha-Biswal
Copy link

Requirement

There are 2 prefixes, local (current implementation) and external (to be used from the UI). Jaeger supports the local prefix and does not support external prefix currently.
Please check the below discussion for more details.
https://github.com/orgs/jaegertracing/discussions/5042#discussioncomment-8236479

Problem

The traffic flow is gateway>virtual service>gateway>jaeger query.

We are setting the query_base_path as "/jaeger/viewer" and then doing the rewrite using the virtual service configuration mentioned below.

match:
    uri:
    prefix: /jaeger/viewer
    rewrite:
    uri: /hub/jaeger-query/v1/jaeger/viewer
    retries:
    attempts: 3
    perTryTimeout: 15s
    retryOn: connect-failure,refused-stream,reset,unavailable,cancelled,resource-exhausted
    route:
    destination:
    host: api-gateway
    port:
    number: 8080
    timeout: 15s

We are getting issue in the Jaeger UI.The index.html is getting loaded but the static files like index.js, index.css are showing loading error(404 not found) in the console tab of the browser.

Proposal

No response

Open questions

No response
@yurishkuro
Copy link
Member

yurishkuro commented Feb 1, 2024 •
edited

We have an example examples/reverse-proxy/docker-compose.yml that uses Jaeger UI with a base-path. However, it only works because both the base-path arg to Jaeger and the configuration in httpd use the same prefix /jaeger/prefix. If they are made different the example does not work, which is similar to this ticket.

Implementation-wise, Jaeger-UI uses base-path string to remove a prefix from the URLs (as seen by the query service) when loading the corresponding embedded static assets and when mapping the API routes.

jaeger/cmd/query/app/static_handler.go

Line 90 in e08f576

assetsFS = http.Dir(staticAssetsRoot)

It also injects that path into the main index.html as base, and the UI uses it when constructing URLs (both assets and API paths).

jaeger/cmd/query/app/static_handler.go

Lines 142 to 147 in e08f576

if sH.options.BasePath != "/" {
if !strings.HasPrefix(sH.options.BasePath, "/") || strings.HasSuffix(sH.options.BasePath, "/") {
return nil, fmt.Errorf("invalid base path '%s'. Must start but not end with a slash '/', e.g. '/jaeger/ui'", sH.options.BasePath)
}
indexBytes = basePathPattern.ReplaceAll(indexBytes, []byte(fmt.Sprintf(`<base href="%s/"`, sH.options.BasePath)))
}

Basically we combine the internal view of the URLs as seen by the query server and the external URLs used by the browser, and by using a single --query.base-path arg for them we artificially force them to be the same. If we introduce another arg, e.g. --query.ui-base-path, just for injecting into the UI, it would allow the external prefix to be different. The above-mentioned docker-compose can be used to validate that it's working.

@yurishkuro yurishkuro added help wanted Features that maintainers are willing to accept but do not have cycles to implement good first issue Good for beginners labels Feb 1, 2024
@yurishkuro yurishkuro changed the title [Feature]: Support external prefix [Feature]: Support external URL prefix Feb 1, 2024
@akagami-harsh
Copy link
Member

I'll work on it.

@varshith257 varshith257 mentioned this issue Feb 7, 2024
Closed
4 tasks
@Sidhartha-Biswal Sidhartha-Biswal linked a pull request Feb 21, 2024 that will close this issue
Open
@joeyyy09
Copy link

@yurishkuro Is this issue resolved or can i take it up?

@yurishkuro
Copy link
Member

It's not resolved afaik, you can see discussion in the linked PR #5219

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement
Projects
None yet
Milestone
No milestone
4 participants
@yurishkuro @joeyyy09 @Sidhartha-Biswal @akagami-harsh