Feat: Add Deep Dependencies endpoint#7687
Feat: Add Deep Dependencies endpoint#7687AritraDey-Dev wants to merge 1 commit intojaegertracing:mainfrom
Conversation
AritraDey-Dev
force-pushed
the
deep-dependencies-support
branch
from
a9d8312 to
a5a3430
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds backend support for the Deep Dependencies graph feature in the Jaeger UI by exposing a new HTTP endpoint that the frontend can consume.
- Adds
/analytics/v1/dependenciesendpoint that routes to the existingdeepDependencieshandler - Applies proper middleware (OpenTelemetry route tagging and span naming) to the new endpoint
- Includes an integration test to verify the endpoint is accessible and returns the expected response structure
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| cmd/query/app/http_handler.go | Registers new /analytics/v1/dependencies route with proper middleware, reusing the existing deepDependencies handler |
| cmd/all-in-one/all_in_one_test.go | Adds integration test checkDeepDependencies to verify the new endpoint is accessible and returns valid JSON structure |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
please rebase, the query service logic moved into v2 extension |
AritraDey-Dev
force-pushed
the
deep-dependencies-support
branch
from
a5a3430 to
90fd9d4
Compare
|
Hey, this PR has been pending review. Not tagging anyone, but PTAL, |
| @@ -123,6 +123,13 @@ func (aH *APIHandler) RegisterRoutes(router *mux.Router) { | |||
| aH.handleFunc(router, aH.transformOTLP, "/transform").Methods(http.MethodPost) | |||
| aH.handleFunc(router, aH.dependencies, "/dependencies").Methods(http.MethodGet) | |||
| aH.handleFunc(router, aH.deepDependencies, "/deep-dependencies").Methods(http.MethodGet) | |||
There was a problem hiding this comment.
we already have this, why add another one?
There was a problem hiding this comment.
The UI's fetchDeepDependencyGraph func calls /analytics/v1/dependencies (see jaeger.ts L100-102), not /api/deep-dependencies. Both routes use the same handler, but/analytics/v1/dependenciesis required for the Deep dependencies UI to work.
There was a problem hiding this comment.
That's not the reason to duplicate endpoints in the service, with implementation that doesn't do anything. We can make UI path configurable.
There was a problem hiding this comment.
Oh thanks for pointing out.will update it to use /api/deep-dependencies .
There was a problem hiding this comment.
Should i create another pr in jaeger-ui repo to do that?
Fixes jaegertracing#6606 Signed-off-by: Aritra Dey <adey01027@gmail.com>
AritraDey-Dev
force-pushed
the
deep-dependencies-support
branch
from
90fd9d4 to
6ea6c16
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. You may re-open it if you need more time. |
github-actions
Bot
added
the
stale
|
This pull request has been automatically closed due to inactivity. You may re-open it if you need more time. We really appreciate your contribution and we are sorry that this has not been completed. |
Which problem is this PR solving?
Description of the changes
How was this change tested?
go run ./cmd/jaegernpm starthttp://localhost:5173/deep-dependencies?service=testand confirmed the graph renders.Checklist
jaeger:make lint testjaeger-ui:npm run lintandnpm run test