Conversation
…ate edges and add logging for nil specs
确定无地方使用 Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…st/response models
|
@ikun-Lg, pls help to review this PR. |
There was a problem hiding this comment.
Pull request overview
This PR introduces new “Topology” tabs in the UI (Applications + Services) that render relationship graphs using AntV G6, along with routing/i18n wiring, iconfont assets, and dev mock endpoints to support the new views.
Changes:
- Add G6-based topology views for
/resources/applications/...and/resources/services/...with click-to-open detail drawers. - Register new routes and i18n labels for “Topology”; add graph API wrappers and Mock.js endpoints.
- Add iconfont assets + global CSS import; add G6 dependencies; minor UI/layout tweaks.
Reviewed changes
Copilot reviewed 19 out of 22 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| ui-vue3/src/views/resources/services/tabs/topology.vue | New services topology graph view with detail drawer and caching |
| ui-vue3/src/views/resources/applications/tabs/topology.vue | New applications topology graph view with detail drawer |
| ui-vue3/src/router/defaultRoutes.ts | Add topology routes under applications/services tabs |
| ui-vue3/src/main.ts | Import iconfont CSS; auto-load mock module in DEV |
| ui-vue3/src/base/i18n/zh.ts | Add “topology/拓扑” translations |
| ui-vue3/src/base/i18n/en.ts | Add “topology/Topology” translations |
| ui-vue3/src/base/http/request.ts | Comment-only baseURL tweak |
| ui-vue3/src/assets/iconfont/iconfont.css | Add iconfont @font-face + classes |
| ui-vue3/src/assets/iconfont/iconfont.js | Add iconfont symbol loader (demo-related) |
| ui-vue3/src/assets/iconfont/iconfont.json | Iconfont metadata |
| ui-vue3/src/assets/iconfont/iconfont.ttf | Iconfont binary |
| ui-vue3/src/assets/iconfont/iconfont.woff | Iconfont binary |
| ui-vue3/src/assets/iconfont/iconfont.woff2 | Iconfont binary |
| ui-vue3/src/assets/iconfont/demo_index.html | Iconfont demo HTML (includes external CDNs) |
| ui-vue3/src/assets/iconfont/demo.css | Iconfont demo CSS |
| ui-vue3/src/api/service/service.ts | Add getServiceGraph; formatting change in getServiceDetail |
| ui-vue3/src/api/service/app.ts | Add getInterfaceGraph |
| ui-vue3/src/api/mock/mockService.ts | Mock /service/graph |
| ui-vue3/src/api/mock/mockApp.ts | Mock /interface/graph |
| ui-vue3/src/Login.vue | Add min-width for login panel |
| ui-vue3/src/App.vue | Remove stray whitespace line |
| ui-vue3/package.json | Add @antv/g6 and g6-extension-vue dependencies |
Comments suppressed due to low confidence (1)
ui-vue3/src/api/service/service.ts:34
- Indentation regression:
return request({ ... })is over-indented here, which is inconsistent with the surrounding file and may fail formatting/lint checks. Align it with the other exported functions in this module.
export const getServiceDetail = (params: any): Promise<any> => {
return request({
url: '/service/detail',
method: 'get',
params
})
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| if (import.meta.env.DEV) { | ||
| import('./api/mock/index') | ||
| } |
| <link rel="stylesheet" href="https://g.alicdn.com/thx/cube/1.3.2/cube.min.css"> | ||
| <link rel="stylesheet" href="demo.css"> | ||
| <link rel="stylesheet" href="iconfont.css"> | ||
| <script src="iconfont.js"></script> | ||
| <!-- jQuery --> | ||
| <script src="https://a1.alicdn.com/oss/uploads/2018/12/26/7bfddb60-08e8-11e9-9b04-53e73bb6408b.js"></script> | ||
| <!-- 代码高亮 --> | ||
| <script src="https://a1.alicdn.com/oss/uploads/2018/12/26/a3f714d0-08e6-11e9-8a15-ebf944d7534c.js"></script> |
| <template> | ||
| <div class="__container_app_topology"> | ||
| <a-flex> | ||
| <a-card class="topology-warpper"> <div id="topology"></div> </a-card> |
| <template> | ||
| <div class="__container_app_topology"> | ||
| <a-flex> | ||
| <a-card class="topology-warpper"> <div id="topology"></div> </a-card> |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
feat: Add service graph visualization and enhance SearchService
…ameter transmission
…ce service details parameter transmission
sohandsomejie
force-pushed
the
feat/topology
branch
from
484b3d4 to
1efdb31
Compare
…il endpoints - Updated service.proto to include methods in the Service message. - Implemented GetServiceDetail and GetServiceGraph handlers for detailed service information and service relationships. - Introduced GraphNode, GraphEdge, and GraphData structures to represent service graphs. - Added ServiceDetailReq and ServiceDetailResp models for service detail requests and responses. - Modified service search logic to accommodate new service structure and indexing by service name and key. - Enhanced service provider metadata handling to sync service resources based on provider updates and deletions. - Added unit tests for service provider metadata event subscriber to ensure correct service synchronization. - Introduced new indexing strategies for service consumer and provider metadata.
… remove deprecated code; handling merge conflicts
…f node and refining edge connections. remove the node and edge associated with the service.
…service identity end-to-end
…o-admin into feat/topology
|
There was a problem hiding this comment.
Pull request overview
This PR adds topology (graph) capabilities for applications and services across the console backend and the Vue3 UI, alongside some related API/model reshaping (notably removing providerAppName usage and introducing service identity keys).
Changes:
- Add new Service Topology and Application Topology UI tabs powered by AntV G6.
- Add backend /application/graph, /service/graph, and /service/detail endpoints plus supporting models/indexers/resources.
- Remove
providerAppNamefrom Grafana/service dashboards and related UI navigation/search fields.
Reviewed changes
Copilot reviewed 41 out of 44 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| ui-vue3/src/views/resources/services/tabs/tracing.vue | Drops providerAppName from Grafana params. |
| ui-vue3/src/views/resources/services/tabs/topology.vue | New service topology view (G6 graph + detail drawer). |
| ui-vue3/src/views/resources/services/tabs/monitor.vue | Drops providerAppName from Grafana params. |
| ui-vue3/src/views/resources/services/tabs/distribution.vue | Stops sending providerAppName in distribution query params. |
| ui-vue3/src/views/resources/services/search.vue | Removes provider column & query propagation; adjusts placeholder. |
| ui-vue3/src/views/resources/applications/tabs/topology.vue | New application topology view (G6 graph + detail drawer). |
| ui-vue3/src/types/grafana.ts | Removes providerAppName from dashboard param typing. |
| ui-vue3/src/router/defaultRoutes.ts | Adds new topology routes for services and applications. |
| ui-vue3/src/main.ts | Imports iconfont CSS; enables mock loading in DEV. |
| ui-vue3/src/Login.vue | Adds min-width styling tweak. |
| ui-vue3/src/base/i18n/zh.ts | Adds i18n keys for “topology”. |
| ui-vue3/src/base/i18n/en.ts | Adds i18n keys for “topology”. |
| ui-vue3/src/base/http/request.ts | Minor baseURL comment tweak. |
| ui-vue3/src/assets/iconfont/iconfont.woff2 | Adds iconfont asset for topology icons. |
| ui-vue3/src/assets/iconfont/iconfont.woff | Adds iconfont asset for topology icons. |
| ui-vue3/src/assets/iconfont/iconfont.ttf | Adds iconfont asset for topology icons. |
| ui-vue3/src/assets/iconfont/iconfont.json | Adds iconfont metadata. |
| ui-vue3/src/assets/iconfont/iconfont.js | Adds iconfont SVG symbol injector. |
| ui-vue3/src/assets/iconfont/iconfont.css | Adds iconfont CSS classes used by topology nodes. |
| ui-vue3/src/assets/iconfont/demo.css | Adds iconfont demo stylesheet. |
| ui-vue3/src/assets/iconfont/demo_index.html | Adds iconfont demo HTML. |
| ui-vue3/src/App.vue | Removes stray whitespace line. |
| ui-vue3/src/api/service/service.ts | Stronger typing for service detail; adds service graph API. |
| ui-vue3/src/api/service/app.ts | Adjusts application detail API; adds application graph API. |
| ui-vue3/src/api/mock/mockService.ts | Updates mock responses for graph/detail/search endpoints. |
| ui-vue3/src/api/mock/mockApp.ts | Adds application graph mock; updates application detail mock route/shape. |
| ui-vue3/package.json | Adds @antv/g6 and g6-extension-vue deps. |
| pkg/core/store/index/service.go | Introduces Service indexer (ByServiceName). |
| pkg/core/store/index/service_provider_metadata.go | Adds provider metadata indexer by service identity key. |
| pkg/core/store/index/service_consumer_metadata.go | Adds consumer metadata indexer by service identity key. |
| pkg/core/resource/apis/mesh/v1alpha1/service_helper.go | Adds BuildServiceIdentityKey helper. |
| pkg/core/discovery/subscriber/service_provider_metadata.go | Builds/updates Service resources from provider metadata events. |
| pkg/core/discovery/component.go | Wires new stores into provider metadata subscriber. |
| pkg/console/service/service.go | Service search now based on Service resources; adds service graph/detail APIs. |
| pkg/console/service/application.go | Adds application graph assembly API. |
| pkg/console/router/router.go | Registers new /application/graph, /service/graph, /service/detail, /service/interfaces routes. |
| pkg/console/model/service.go | Removes providerAppName; adds service detail request/response types. |
| pkg/console/model/observability.go | Removes providerAppName from ServiceDashboardReq. |
| pkg/console/model/graph.go | Adds graph node/edge/data models + service graph request. |
| pkg/console/model/application.go | Adds application graph request model. |
| pkg/console/handler/service.go | Adds handlers for graph/detail; leaves interfaces handler stubbed. |
| pkg/console/handler/application.go | Adds handler for application graph endpoint. |
| api/mesh/v1alpha1/service.proto | Simplifies Service proto fields (adds methods, removes providers/consumers/features). |
| api/mesh/v1alpha1/service.pb.go | Regenerated Go bindings for updated Service proto. |
Comments suppressed due to low confidence (1)
pkg/core/store/index/service.go:46
byServiceNameindexes onservice.Spec.Namewithout checking for empty string. Returning[""]will create an index entry for empty names and can pollute lookups. Consider returning an empty slice whenSpec.Nameis blank (similar to the nil-spec case).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const getApplicationGraph = (serviceName: string): Promise<any> => { | ||
| return request({ | ||
| url: '/application/graph', | ||
| method: 'get', | ||
| params: { serviceName } |
There was a problem hiding this comment.
getApplicationGraph is calling /application/graph with query param serviceName, but the backend request model binds appName (form:"appName"). This mismatch will result in empty/incorrect graphs. Align the param name (and function argument naming) to appName.
| export const getApplicationGraph = (serviceName: string): Promise<any> => { | |
| return request({ | |
| url: '/application/graph', | |
| method: 'get', | |
| params: { serviceName } | |
| export const getApplicationGraph = (appName: string): Promise<any> => { | |
| return request({ | |
| url: '/application/graph', | |
| method: 'get', | |
| params: { appName } |
| export const getApplicationDetail = (appName: string): Promise<any> => { | ||
| return request({ | ||
| url: '/application/detail', | ||
| method: 'get', | ||
| params | ||
| params: { | ||
| appName | ||
| } | ||
| }) |
There was a problem hiding this comment.
getApplicationDetail now requires a plain appName: string, but there are existing call sites passing an object (e.g. ui-vue3/src/views/resources/applications/tabs/detail.vue calls getApplicationDetail({ appName })). Either update those call sites in this PR or keep backward-compatible parameter handling here to avoid runtime/type errors.
| export const getServiceGraph = (serviceName: string): Promise<any> => { | ||
| return request({ | ||
| url: '/service/graph', | ||
| method: 'get', | ||
| params: { | ||
| serviceName | ||
| } |
There was a problem hiding this comment.
getServiceGraph only sends serviceName, but the backend service graph is keyed by {serviceName}:{version}:{group}. With non-empty version/group, this will return no data. Consider accepting version/group (or a serviceKey) here and passing them through.
| export const getServiceGraph = (serviceName: string): Promise<any> => { | |
| return request({ | |
| url: '/service/graph', | |
| method: 'get', | |
| params: { | |
| serviceName | |
| } | |
| export const getServiceGraph = ( | |
| service: | |
| | string | |
| | { | |
| serviceName: string | |
| version?: string | RouteParamValue[] | |
| group?: string | RouteParamValue[] | |
| } | |
| ): Promise<any> => { | |
| const params = | |
| typeof service === 'string' | |
| ? { serviceName: service } | |
| : { | |
| serviceName: service.serviceName, | |
| ...(service.version !== undefined ? { version: service.version } : {}), | |
| ...(service.group !== undefined ? { group: service.group } : {}) | |
| } | |
| return request({ | |
| url: '/service/graph', | |
| method: 'get', | |
| params |
| export const getServiceDetail = ({ | ||
| serviceName, | ||
| version, | ||
| group | ||
| }: { | ||
| serviceName: string | ||
| version?: string | ||
| group?: string | ||
| }): Promise<any> => { | ||
| return request({ | ||
| url: '/service/detail', | ||
| method: 'get', | ||
| params | ||
| params: { serviceName, version, group } | ||
| }) |
There was a problem hiding this comment.
getServiceDetail signature changed to require { serviceName, version?, group? }, but there are existing call sites passing {} (e.g. ui-vue3/src/views/resources/services/tabs/detail.vue). Update those call sites in this PR or provide a backward-compatible overload/defaulting to avoid crashes.
| if (import.meta.env.DEV) { | ||
| import('./api/mock/index') | ||
| } | ||
|
|
||
| app.use(Antd).use(Vue3ColorPicker).use(pinia).use(i18n).use(router).mount('#app') |
There was a problem hiding this comment.
In dev mode the mock module is imported asynchronously but not awaited. If the app makes requests immediately on mount, they can race before the mocks register. Consider using top-level await import(...) (supported by Vite) or an async bootstrap before mount() to ensure mocks are active first.
| for _, item := range consumerAppServiceList { | ||
| if item.Spec == nil { | ||
| continue | ||
| } | ||
| if _, ok := consumerAppSet[item.Spec.ConsumerAppName]; !ok { | ||
| consumerAppSet[item.Spec.ConsumerAppName] = struct{}{} | ||
| nodes = append(nodes, model.GraphNode{ | ||
| ID: item.Spec.ConsumerAppName, | ||
| Label: item.Spec.ConsumerAppName, | ||
| Type: "application", | ||
| Rule: constants.ConsumerSide, | ||
| Data: nil, | ||
| }) | ||
| edges = append(edges, model.GraphEdge{ | ||
| Source: item.Spec.ConsumerAppName, | ||
| Target: provider.Spec.ProviderAppName, | ||
| Data: nil, | ||
| }) | ||
| } |
There was a problem hiding this comment.
Edges are only appended inside the if _, ok := consumerAppSet[...] block. This means if a consumer app node already exists (seen via another provided service), additional edges to other providers won’t be created, resulting in an incomplete graph. Deduplicate nodes separately from edges (e.g., always append edge, but guard node insertion).
| for _, item := range providerAppList { | ||
| if item.Spec == nil { | ||
| continue | ||
| } | ||
| if _, ok := providerAppSet[item.Spec.ProviderAppName]; !ok { | ||
| providerAppSet[item.Spec.ProviderAppName] = struct{}{} | ||
| nodes = append(nodes, model.GraphNode{ | ||
| ID: item.Spec.ProviderAppName, | ||
| Label: item.Spec.ProviderAppName, | ||
| Type: "application", | ||
| Rule: constants.ProviderSide, | ||
| Data: nil, | ||
| }) | ||
| edges = append(edges, model.GraphEdge{ | ||
| Source: consumer.Spec.ConsumerAppName, | ||
| Target: item.Spec.ProviderAppName, | ||
| Data: nil, | ||
| }) | ||
| } |
There was a problem hiding this comment.
Similarly, edges to provider apps are only added when the provider node is first seen (if _, ok := providerAppSet[...]). This drops edges for subsequent consumed services that map to an already-added provider app node. Consider tracking edges with a separate set or always appending edges while only deduping node creation.
| if metadata.Spec == nil { | ||
| return []string{}, nil | ||
| } | ||
| serviceKey := metadata.Spec.ServiceName + ":" + metadata.Spec.Version + ":" + metadata.Spec.Group |
There was a problem hiding this comment.
byServiceProviderServiceKey manually concatenates serviceName + ":" + version + ":" + group. Prefer using the shared helper (BuildServiceIdentityKey) to avoid separator drift and keep key construction consistent across the codebase.
| serviceKey := metadata.Spec.ServiceName + ":" + metadata.Spec.Version + ":" + metadata.Spec.Group | |
| serviceKey := BuildServiceIdentityKey(metadata.Spec.ServiceName, metadata.Spec.Version, metadata.Spec.Group) |
| if metadata.Spec == nil { | ||
| return []string{}, nil | ||
| } | ||
| serviceKey := metadata.Spec.ServiceName + ":" + metadata.Spec.Version + ":" + metadata.Spec.Group |
There was a problem hiding this comment.
byServiceConsumerServiceKey manually concatenates serviceName + ":" + version + ":" + group. Prefer the shared helper (BuildServiceIdentityKey) to ensure consistency with other service-key users/indexers.
| serviceKey := metadata.Spec.ServiceName + ":" + metadata.Spec.Version + ":" + metadata.Spec.Group | |
| serviceKey := BuildServiceIdentityKey(metadata.Spec.ServiceName, metadata.Spec.Version, metadata.Spec.Group) |
| func (s *ServiceProviderMetadataEventSubscriber) syncService(mesh, serviceName, version, group string) error { | ||
| serviceKey := meshresource.BuildServiceIdentityKey(serviceName, version, group) | ||
| resources, err := s.providerStore.ListByIndexes(map[string]string{ | ||
| index.ByMeshIndex: mesh, | ||
| index.ByServiceProviderServiceName: serviceName, | ||
| }) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| providers := make([]*meshresource.ServiceProviderMetadataResource, 0, len(resources)) | ||
| for _, item := range resources { | ||
| res, ok := item.(*meshresource.ServiceProviderMetadataResource) | ||
| if !ok { | ||
| return bizerror.NewAssertionError(meshresource.ServiceProviderMetadataKind, reflect.TypeOf(item).Name()) | ||
| } | ||
| if res.Spec == nil { | ||
| continue | ||
| } | ||
| if res.Spec.Version == version && res.Spec.Group == group { | ||
| providers = append(providers, res) | ||
| } | ||
| } |
There was a problem hiding this comment.
syncService lists provider metadata by serviceName and then filters by version/group in-memory. Now that ByServiceProviderServiceKey index exists, you can query directly by the full identity key ({service}:{version}:{group}) to avoid scanning and reduce store load on large meshes.
Please provide a description of this PR:
To help us figure out who should review this PR, please put an X in all the areas that this PR affects.
Please check any characteristics that apply to this pull request.