From 7932917f5feaf89cf4540ececba01631bd3551e9 Mon Sep 17 00:00:00 2001 From: swapnil-signoz Date: Thu, 4 Jun 2026 23:02:56 +0530 Subject: [PATCH 1/4] refactor: service response changes and dashboardID handling (#11485) * refactor: service response changes and dashboardID handling * refactor: removing unused enrichDashboardIDs func * revert: restore frontend files to main state * refactor: updating response * refactor: updating nullable tag * chore: adding required tags * chore: adding required tags for ServiceDashboard struct * refactor(integrations): align service-details UI with new cloud-integration API + disabled-dashboard state (#11547) * refactor: cloud integration service * fix: update schemas * fix: update schema * fix: minor fixes * refactor: review comments --------- Co-authored-by: Gaurav Tewari --------- Co-authored-by: Gaurav Tewari Co-authored-by: Gaurav Tewari --- docs/api/openapi.yml | 76 +++++++++------- .../implcloudintegration/module.go | 55 +++++------- .../api/generated/services/sigNoz.schemas.ts | 87 +++++++++++-------- .../AmazonWebServices/__tests__/mockData.ts | 1 - .../ServiceDashboards/DashboardCard.tsx | 68 +++++++++++++++ .../ServiceDashboards.styles.scss | 5 ++ .../ServiceDashboards/ServiceDashboards.tsx | 83 ++++-------------- .../implcloudintegration/handler.go | 1 - .../integration_dashboard.go | 14 +-- pkg/types/cloudintegrationtypes/service.go | 54 ++++++++++-- 10 files changed, 267 insertions(+), 177 deletions(-) create mode 100644 frontend/src/container/Integrations/CloudIntegration/ServiceDashboards/DashboardCard.tsx diff --git a/docs/api/openapi.yml b/docs/api/openapi.yml index 495f9360588..6481ebeac13 100644 --- a/docs/api/openapi.yml +++ b/docs/api/openapi.yml @@ -870,14 +870,6 @@ components: - timestampMillis - data type: object - CloudintegrationtypesAssets: - properties: - dashboards: - items: - $ref: '#/components/schemas/CloudintegrationtypesDashboard' - nullable: true - type: array - type: object CloudintegrationtypesAzureAccountConfig: properties: deploymentRegion: @@ -1025,17 +1017,6 @@ components: - ingestionUrl - ingestionKey type: object - CloudintegrationtypesDashboard: - properties: - definition: - $ref: '#/components/schemas/DashboardtypesStorableDashboardData' - description: - type: string - id: - type: string - title: - type: string - type: object CloudintegrationtypesDataCollected: properties: logs: @@ -1209,7 +1190,7 @@ components: CloudintegrationtypesService: properties: assets: - $ref: '#/components/schemas/CloudintegrationtypesAssets' + $ref: '#/components/schemas/CloudintegrationtypesServiceAssets' cloudIntegrationService: $ref: '#/components/schemas/CloudintegrationtypesCloudIntegrationService' dataCollected: @@ -1222,8 +1203,6 @@ components: type: string supportedSignals: $ref: '#/components/schemas/CloudintegrationtypesSupportedSignals' - telemetryCollectionStrategy: - $ref: '#/components/schemas/CloudintegrationtypesTelemetryCollectionStrategy' title: type: string required: @@ -1234,9 +1213,17 @@ components: - assets - supportedSignals - dataCollected - - telemetryCollectionStrategy - cloudIntegrationService type: object + CloudintegrationtypesServiceAssets: + properties: + dashboards: + items: + $ref: '#/components/schemas/CloudintegrationtypesServiceDashboard' + type: array + required: + - dashboards + type: object CloudintegrationtypesServiceConfig: properties: aws: @@ -1244,6 +1231,18 @@ components: azure: $ref: '#/components/schemas/CloudintegrationtypesAzureServiceConfig' type: object + CloudintegrationtypesServiceDashboard: + properties: + description: + type: string + integrationDashboard: + $ref: '#/components/schemas/CloudintegrationtypesStorableIntegrationDashboard' + title: + type: string + required: + - title + - description + type: object CloudintegrationtypesServiceID: enum: - alb @@ -1278,6 +1277,30 @@ components: - icon - enabled type: object + CloudintegrationtypesStorableIntegrationDashboard: + properties: + createdAt: + format: date-time + type: string + dashboardId: + type: string + id: + type: string + provider: + type: string + slug: + type: string + updatedAt: + format: date-time + type: string + required: + - id + - dashboardId + - provider + - slug + - createdAt + - updatedAt + type: object CloudintegrationtypesSupportedSignals: properties: logs: @@ -1285,13 +1308,6 @@ components: metrics: type: boolean type: object - CloudintegrationtypesTelemetryCollectionStrategy: - properties: - aws: - $ref: '#/components/schemas/CloudintegrationtypesAWSTelemetryCollectionStrategy' - azure: - $ref: '#/components/schemas/CloudintegrationtypesAzureTelemetryCollectionStrategy' - type: object CloudintegrationtypesUpdatableAccount: properties: config: diff --git a/ee/modules/cloudintegration/implcloudintegration/module.go b/ee/modules/cloudintegration/implcloudintegration/module.go index a72165a8637..2ac5a98040b 100644 --- a/ee/modules/cloudintegration/implcloudintegration/module.go +++ b/ee/modules/cloudintegration/implcloudintegration/module.go @@ -355,26 +355,32 @@ func (module *module) GetService(ctx context.Context, orgID valuer.UUID, service var integrationService *cloudintegrationtypes.CloudIntegrationService - if !cloudIntegrationID.IsZero() { - storedService, err := module.store.GetServiceByServiceID(ctx, cloudIntegrationID, serviceID) - if err != nil && !errors.Ast(err, errors.TypeNotFound) { - return nil, err - } - if storedService != nil { - serviceConfig, err := cloudintegrationtypes.NewServiceConfigFromJSON(provider, storedService.Config) - if err != nil { - return nil, err - } + if cloudIntegrationID.IsZero() { + return cloudintegrationtypes.NewService(provider, serviceDefinition, nil, nil), nil + } - integrationService = cloudintegrationtypes.NewCloudIntegrationServiceFromStorable(storedService, serviceConfig) - } + storedService, err := module.store.GetServiceByServiceID(ctx, cloudIntegrationID, serviceID) + if err != nil && !errors.Ast(err, errors.TypeNotFound) { + return nil, err + } + if storedService == nil { + return cloudintegrationtypes.NewService(provider, serviceDefinition, nil, nil), nil + } - if err := module.enrichDashboardIDs(ctx, orgID, provider, serviceID, serviceDefinition); err != nil { - return nil, err - } + serviceConfig, err := cloudintegrationtypes.NewServiceConfigFromJSON(provider, storedService.Config) + if err != nil { + return nil, err } - return cloudintegrationtypes.NewService(*serviceDefinition, integrationService), nil + integrationService = cloudintegrationtypes.NewCloudIntegrationServiceFromStorable(storedService, serviceConfig) + + slugPrefix := cloudintegrationtypes.CloudIntegrationDashboardSlugPrefix(provider, serviceID) + integrationDashboards, err := module.store.ListIntegrationDashboardsBySlugPrefix(ctx, orgID, cloudintegrationtypes.IntegrationDashboardProviderCloudIntegration, slugPrefix) + if err != nil { + return nil, err + } + + return cloudintegrationtypes.NewService(provider, serviceDefinition, integrationService, integrationDashboards), nil } func (module *module) CreateService(ctx context.Context, orgID valuer.UUID, createdBy string, creator valuer.UUID, service *cloudintegrationtypes.CloudIntegrationService, provider cloudintegrationtypes.CloudProviderType) error { @@ -583,20 +589,3 @@ func (module *module) deprovisionDashboards(ctx context.Context, orgID valuer.UU } return nil } - -// enrichDashboardIDs replaces the raw dashboard name in each Dashboard.ID with the provisioned UUID. -// TODO: remove this hack and send idiomatic response to client. -func (module *module) enrichDashboardIDs(ctx context.Context, orgID valuer.UUID, provider cloudintegrationtypes.CloudProviderType, serviceID cloudintegrationtypes.ServiceID, serviceDefinition *cloudintegrationtypes.ServiceDefinition) error { - for i, d := range serviceDefinition.Assets.Dashboards { - slug := cloudintegrationtypes.CloudIntegrationDashboardSlug(provider, serviceID, d.ID) - row, err := module.store.GetIntegrationDashboardBySlug(ctx, orgID, cloudintegrationtypes.IntegrationDashboardProviderCloudIntegration, slug) - if err != nil { - if errors.Ast(err, errors.TypeNotFound) { - continue - } - return err - } - serviceDefinition.Assets.Dashboards[i].ID = row.DashboardID - } - return nil -} diff --git a/frontend/src/api/generated/services/sigNoz.schemas.ts b/frontend/src/api/generated/services/sigNoz.schemas.ts index 7fed15fe0d6..142d57e2c5a 100644 --- a/frontend/src/api/generated/services/sigNoz.schemas.ts +++ b/frontend/src/api/generated/services/sigNoz.schemas.ts @@ -2457,33 +2457,6 @@ export interface CloudintegrationtypesAccountDTO { updatedAt?: string; } -export interface DashboardtypesStorableDashboardDataDTO { - [key: string]: unknown; -} - -export interface CloudintegrationtypesDashboardDTO { - definition?: DashboardtypesStorableDashboardDataDTO; - /** - * @type string - */ - description?: string; - /** - * @type string - */ - id?: string; - /** - * @type string - */ - title?: string; -} - -export interface CloudintegrationtypesAssetsDTO { - /** - * @type array,null - */ - dashboards?: CloudintegrationtypesDashboardDTO[] | null; -} - export interface CloudintegrationtypesAzureConnectionArtifactDTO { /** * @type string @@ -2866,6 +2839,54 @@ export interface CloudintegrationtypesPostableAgentCheckInDTO { providerAccountId?: string; } +export interface CloudintegrationtypesStorableIntegrationDashboardDTO { + /** + * @type string + * @format date-time + */ + createdAt: string; + /** + * @type string + */ + dashboardId: string; + /** + * @type string + */ + id: string; + /** + * @type string + */ + provider: string; + /** + * @type string + */ + slug: string; + /** + * @type string + * @format date-time + */ + updatedAt: string; +} + +export interface CloudintegrationtypesServiceDashboardDTO { + /** + * @type string + */ + description: string; + integrationDashboard?: CloudintegrationtypesStorableIntegrationDashboardDTO; + /** + * @type string + */ + title: string; +} + +export interface CloudintegrationtypesServiceAssetsDTO { + /** + * @type array + */ + dashboards: CloudintegrationtypesServiceDashboardDTO[]; +} + export interface CloudintegrationtypesSupportedSignalsDTO { /** * @type boolean @@ -2877,13 +2898,8 @@ export interface CloudintegrationtypesSupportedSignalsDTO { metrics?: boolean; } -export interface CloudintegrationtypesTelemetryCollectionStrategyDTO { - aws?: CloudintegrationtypesAWSTelemetryCollectionStrategyDTO; - azure?: CloudintegrationtypesAzureTelemetryCollectionStrategyDTO; -} - export interface CloudintegrationtypesServiceDTO { - assets: CloudintegrationtypesAssetsDTO; + assets: CloudintegrationtypesServiceAssetsDTO; cloudIntegrationService: CloudintegrationtypesCloudIntegrationServiceDTO | null; dataCollected: CloudintegrationtypesDataCollectedDTO; /** @@ -2899,7 +2915,6 @@ export interface CloudintegrationtypesServiceDTO { */ overview: string; supportedSignals: CloudintegrationtypesSupportedSignalsDTO; - telemetryCollectionStrategy: CloudintegrationtypesTelemetryCollectionStrategyDTO; /** * @type string */ @@ -3705,6 +3720,10 @@ export interface DashboardtypesCustomVariableSpecDTO { customValue: string; } +export interface DashboardtypesStorableDashboardDataDTO { + [key: string]: unknown; +} + export enum DashboardtypesSourceDTO { user = 'user', system = 'system', diff --git a/frontend/src/container/Integrations/CloudIntegration/AmazonWebServices/__tests__/mockData.ts b/frontend/src/container/Integrations/CloudIntegration/AmazonWebServices/__tests__/mockData.ts index 7f099becb78..3e0cf3ed4f4 100644 --- a/frontend/src/container/Integrations/CloudIntegration/AmazonWebServices/__tests__/mockData.ts +++ b/frontend/src/container/Integrations/CloudIntegration/AmazonWebServices/__tests__/mockData.ts @@ -55,7 +55,6 @@ const buildServiceDetailsResponse = ( }, }, }, - telemetryCollectionStrategy: { aws: {} }, }, }); diff --git a/frontend/src/container/Integrations/CloudIntegration/ServiceDashboards/DashboardCard.tsx b/frontend/src/container/Integrations/CloudIntegration/ServiceDashboards/DashboardCard.tsx new file mode 100644 index 00000000000..970ab58d3d7 --- /dev/null +++ b/frontend/src/container/Integrations/CloudIntegration/ServiceDashboards/DashboardCard.tsx @@ -0,0 +1,68 @@ +import type { KeyboardEvent, MouseEvent } from 'react'; +import { TooltipSimple } from '@signozhq/ui/tooltip'; +import { CloudintegrationtypesServiceDashboardDTO } from 'api/generated/services/sigNoz.schemas'; +import { useSafeNavigate } from 'hooks/useSafeNavigate'; +import { openInNewTab } from 'utils/navigation'; + +const DISABLED_TOOLTIP = + 'Enable metrics collection for this service to view this dashboard.'; + +function DashboardCard({ + dashboard, + isInteractive, +}: { + dashboard: CloudintegrationtypesServiceDashboardDTO; + isInteractive: boolean; +}): JSX.Element { + const dashboardId = dashboard.integrationDashboard?.dashboardId; + const isClickable = Boolean(dashboardId) && isInteractive; + const dashboardUrl = dashboardId ? `/dashboard/${dashboardId}` : ''; + + const { safeNavigate } = useSafeNavigate(); + + const interactiveProps = isClickable + ? { + role: 'button', + tabIndex: 0, + onClick: (event: MouseEvent): void => { + if (event.metaKey || event.ctrlKey) { + openInNewTab(dashboardUrl); + return; + } + safeNavigate(dashboardUrl); + }, + onKeyDown: (event: KeyboardEvent): void => { + if (event.key === 'Enter' || event.key === ' ') { + event.preventDefault(); + safeNavigate(dashboardUrl); + } + }, + } + : {}; + + const card = ( +
+
+
{dashboard.title}
+
+ {dashboard.description} +
+
+
+ ); + + if (!dashboardId) { + return {card}; + } + + return card; +} + +export default DashboardCard; diff --git a/frontend/src/container/Integrations/CloudIntegration/ServiceDashboards/ServiceDashboards.styles.scss b/frontend/src/container/Integrations/CloudIntegration/ServiceDashboards/ServiceDashboards.styles.scss index 6874153f812..cd2c3cfca55 100644 --- a/frontend/src/container/Integrations/CloudIntegration/ServiceDashboards/ServiceDashboards.styles.scss +++ b/frontend/src/container/Integrations/CloudIntegration/ServiceDashboards/ServiceDashboards.styles.scss @@ -53,6 +53,11 @@ } } + &.aws-service-dashboard-item-disabled { + cursor: not-allowed; + opacity: 0.6; + } + .aws-service-dashboard-item-content { display: flex; flex-direction: column; diff --git a/frontend/src/container/Integrations/CloudIntegration/ServiceDashboards/ServiceDashboards.tsx b/frontend/src/container/Integrations/CloudIntegration/ServiceDashboards/ServiceDashboards.tsx index abfeb1b0522..8248b0f714d 100644 --- a/frontend/src/container/Integrations/CloudIntegration/ServiceDashboards/ServiceDashboards.tsx +++ b/frontend/src/container/Integrations/CloudIntegration/ServiceDashboards/ServiceDashboards.tsx @@ -1,11 +1,9 @@ -/* eslint-disable sonarjs/cognitive-complexity */ import { - CloudintegrationtypesDashboardDTO, + CloudintegrationtypesServiceDashboardDTO, CloudintegrationtypesServiceDTO, } from 'api/generated/services/sigNoz.schemas'; -import { useSafeNavigate } from 'hooks/useSafeNavigate'; -import { withBasePath } from 'utils/basePath'; +import DashboardCard from './DashboardCard'; import './ServiceDashboards.styles.scss'; function ServiceDashboards({ @@ -16,7 +14,6 @@ function ServiceDashboards({ isInteractive?: boolean; }): JSX.Element { const dashboards = service?.assets?.dashboards || []; - const { safeNavigate } = useSafeNavigate(); if (!dashboards.length) { return <>; } @@ -25,68 +22,20 @@ function ServiceDashboards({
Dashboards
- {dashboards.map((dashboard: CloudintegrationtypesDashboardDTO) => { - if (!dashboard.id) { - return null; - } - - const dashboardUrl = `/dashboard/${dashboard.id}`; - - return ( -
{ - if (!isInteractive) { - return; - } - if (event.metaKey || event.ctrlKey) { - window.open( - withBasePath(dashboardUrl), - '_blank', - 'noopener,noreferrer', - ); - return; - } - safeNavigate(dashboardUrl); - }} - onAuxClick={(event): void => { - if (!isInteractive) { - return; - } - if (event.button === 1) { - window.open( - withBasePath(dashboardUrl), - '_blank', - 'noopener,noreferrer', - ); - } - }} - onKeyDown={(event): void => { - if (!isInteractive) { - return; - } - if (event.key === 'Enter' || event.key === ' ') { - event.preventDefault(); - safeNavigate(dashboardUrl); - } - }} - > -
-
- {dashboard.title} -
-
- {dashboard.description} -
-
-
- ); - })} + {dashboards.map( + (dashboard: CloudintegrationtypesServiceDashboardDTO, index: number) => { + const key = + dashboard.integrationDashboard?.dashboardId || + `${dashboard.title}-${index}`; + return ( + + ); + }, + )}
); diff --git a/pkg/modules/cloudintegration/implcloudintegration/handler.go b/pkg/modules/cloudintegration/implcloudintegration/handler.go index f0167db35b0..7a9764cb604 100644 --- a/pkg/modules/cloudintegration/implcloudintegration/handler.go +++ b/pkg/modules/cloudintegration/implcloudintegration/handler.go @@ -440,4 +440,3 @@ func (handler *handler) AgentCheckIn(rw http.ResponseWriter, r *http.Request) { render.Success(rw, http.StatusOK, cloudintegrationtypes.NewGettableAgentCheckIn(provider, resp)) } - diff --git a/pkg/types/cloudintegrationtypes/integration_dashboard.go b/pkg/types/cloudintegrationtypes/integration_dashboard.go index 81d3768c88b..f6510d78a2a 100644 --- a/pkg/types/cloudintegrationtypes/integration_dashboard.go +++ b/pkg/types/cloudintegrationtypes/integration_dashboard.go @@ -18,14 +18,16 @@ var ( type StorableIntegrationDashboard struct { bun.BaseModel `bun:"table:integration_dashboard"` - ID string `bun:"id,pk,type:text"` - DashboardID string `bun:"dashboard_id,type:text"` - Provider IntegrationDashboardProviderType `bun:"provider,type:text"` - Slug string `bun:"slug,type:text"` - CreatedAt time.Time `bun:"created_at"` - UpdatedAt time.Time `bun:"updated_at"` + ID string `json:"id" bun:"id,pk,type:text" required:"true"` + DashboardID string `json:"dashboardId" bun:"dashboard_id,type:text" required:"true"` + Provider IntegrationDashboardProviderType `json:"provider" bun:"provider,type:text" required:"true"` + Slug string `json:"slug" bun:"slug,type:text" required:"true"` + CreatedAt time.Time `json:"createdAt" bun:"created_at" required:"true"` + UpdatedAt time.Time `json:"updatedAt" bun:"updated_at" required:"true"` } +type IntegrationDashboard = StorableIntegrationDashboard + func NewStorableIntegrationDashboard(dashboardID string, provider IntegrationDashboardProviderType, slug string) *StorableIntegrationDashboard { now := time.Now() return &StorableIntegrationDashboard{ diff --git a/pkg/types/cloudintegrationtypes/service.go b/pkg/types/cloudintegrationtypes/service.go index a7799a9f13c..f947a5fdd43 100644 --- a/pkg/types/cloudintegrationtypes/service.go +++ b/pkg/types/cloudintegrationtypes/service.go @@ -50,10 +50,18 @@ type ListServicesMetadataParams struct { // Service represents a cloud integration service with its definition, // cloud integration service is non nil only when the service entry exists in DB with ANY config (enabled or disabled). type Service struct { - ServiceDefinition + ServiceDefinitionMetadata + Overview string `json:"overview" required:"true"` // markdown + ServiceAssets ServiceAssets `json:"assets" required:"true"` + SupportedSignals SupportedSignals `json:"supportedSignals" required:"true"` + DataCollected DataCollected `json:"dataCollected" required:"true"` CloudIntegrationService *CloudIntegrationService `json:"cloudIntegrationService" required:"true" nullable:"true"` } +type ServiceAssets struct { + Dashboards []*ServiceDashboard `json:"dashboards" required:"true" nullable:"false"` +} + type GetServiceParams struct { CloudIntegrationID valuer.UUID `query:"cloud_integration_id" required:"false"` } @@ -121,6 +129,12 @@ type Dashboard struct { Definition dashboardtypes.StorableDashboardData `json:"definition,omitempty"` } +type ServiceDashboard struct { + Title string `json:"title" required:"true"` + Description string `json:"description" required:"true"` + IntegrationDashboard *IntegrationDashboard `json:"integrationDashboard,omitempty" required:"false"` +} + func NewCloudIntegrationService(serviceID ServiceID, cloudIntegrationID valuer.UUID, provider CloudProviderType, config *ServiceConfig) (*CloudIntegrationService, error) { switch provider { case CloudProviderTypeAWS: @@ -164,11 +178,41 @@ func NewServiceMetadata(definition ServiceDefinition, enabled bool) *ServiceMeta } } -func NewService(def ServiceDefinition, storableService *CloudIntegrationService) *Service { - return &Service{ - ServiceDefinition: def, - CloudIntegrationService: storableService, +func NewService(provider CloudProviderType, def *ServiceDefinition, integrationService *CloudIntegrationService, integrationDashboards []*StorableIntegrationDashboard) *Service { + service := &Service{ + ServiceDefinitionMetadata: def.ServiceDefinitionMetadata, + Overview: def.Overview, + SupportedSignals: def.SupportedSignals, + DataCollected: def.DataCollected, + CloudIntegrationService: integrationService, + ServiceAssets: ServiceAssets{Dashboards: make([]*ServiceDashboard, 0, len(def.Assets.Dashboards))}, + } + + integrationDashboardsMap := make(map[string]*IntegrationDashboard) + for _, d := range integrationDashboards { + integrationDashboardsMap[d.Slug] = d + } + + for _, d := range def.Assets.Dashboards { + dashboard := &ServiceDashboard{ + Title: d.Title, + Description: d.Description, + } + + if integrationService != nil { + slug := CloudIntegrationDashboardSlug(provider, integrationService.Type, d.ID) + + if integrationDashboard, exists := integrationDashboardsMap[slug]; exists { + if integrationDashboard != nil { + dashboard.IntegrationDashboard = integrationDashboard + } + } + } + + service.ServiceAssets.Dashboards = append(service.ServiceAssets.Dashboards, dashboard) } + + return service } func NewGettableServicesMetadata(services []*ServiceMetadata) *GettableServicesMetadata { From ce26458b9f5791eeb19178b8c33e78c579224211 Mon Sep 17 00:00:00 2001 From: swapnil-signoz Date: Thu, 4 Jun 2026 23:37:01 +0530 Subject: [PATCH 2/4] feat(cloud-integrations): adding endpoint services metadata for account (#11563) * feat: adding endpoint services metadata for account * fix: adding missing response writer in error * refactor(cloud-integrations): use account-scoped ListAccountServices endpoint when an account is connected (#11569) * fix: frontend changes for issue 4616 * fix: update frontend changes --------- Co-authored-by: Gaurav Tewari --------- Co-authored-by: Gaurav Tewari Co-authored-by: Gaurav Tewari --- docs/api/openapi.yml | 58 +++++++++ .../services/cloudintegration/index.ts | 112 ++++++++++++++++++ .../api/generated/services/sigNoz.schemas.ts | 12 ++ .../ServiceDetails/ServiceDetails.tsx | 33 ++---- .../CloudIntegration/ServicesList.tsx | 44 +++++-- .../signozapiserver/cloudintegration.go | 20 ++++ .../cloudintegration/cloudintegration.go | 1 + .../implcloudintegration/handler.go | 38 ++++++ .../tests/cloudintegrations/05_services.py | 43 +++++++ 9 files changed, 328 insertions(+), 33 deletions(-) diff --git a/docs/api/openapi.yml b/docs/api/openapi.yml index 6481ebeac13..47c1592f5de 100644 --- a/docs/api/openapi.yml +++ b/docs/api/openapi.yml @@ -8103,6 +8103,64 @@ paths: summary: Update account tags: - cloudintegration + /api/v1/cloud_integrations/{cloud_provider}/accounts/{id}/services: + get: + deprecated: false + description: This endpoint lists the services metadata for the specified account + and cloud provider + operationId: ListAccountServicesMetadata + parameters: + - in: path + name: cloud_provider + required: true + schema: + type: string + - in: path + name: id + required: true + schema: + type: string + responses: + "200": + content: + application/json: + schema: + properties: + data: + $ref: '#/components/schemas/CloudintegrationtypesGettableServicesMetadata' + status: + type: string + required: + - status + - data + type: object + description: OK + "401": + content: + application/json: + schema: + $ref: '#/components/schemas/RenderErrorResponse' + description: Unauthorized + "403": + content: + application/json: + schema: + $ref: '#/components/schemas/RenderErrorResponse' + description: Forbidden + "500": + content: + application/json: + schema: + $ref: '#/components/schemas/RenderErrorResponse' + description: Internal Server Error + security: + - api_key: + - ADMIN + - tokenizer: + - ADMIN + summary: List account services metadata + tags: + - cloudintegration /api/v1/cloud_integrations/{cloud_provider}/accounts/{id}/services/{service_id}: put: deprecated: false diff --git a/frontend/src/api/generated/services/cloudintegration/index.ts b/frontend/src/api/generated/services/cloudintegration/index.ts index 1b5a6396fb0..7a6049e5bce 100644 --- a/frontend/src/api/generated/services/cloudintegration/index.ts +++ b/frontend/src/api/generated/services/cloudintegration/index.ts @@ -36,6 +36,8 @@ import type { GetService200, GetServiceParams, GetServicePathParameters, + ListAccountServicesMetadata200, + ListAccountServicesMetadataPathParameters, ListAccounts200, ListAccountsPathParameters, ListServicesMetadata200, @@ -631,6 +633,116 @@ export const useUpdateAccount = < > => { return useMutation(getUpdateAccountMutationOptions(options)); }; +/** + * This endpoint lists the services metadata for the specified account and cloud provider + * @summary List account services metadata + */ +export const listAccountServicesMetadata = ( + { cloudProvider, id }: ListAccountServicesMetadataPathParameters, + signal?: AbortSignal, +) => { + return GeneratedAPIInstance({ + url: `/api/v1/cloud_integrations/${cloudProvider}/accounts/${id}/services`, + method: 'GET', + signal, + }); +}; + +export const getListAccountServicesMetadataQueryKey = ({ + cloudProvider, + id, +}: ListAccountServicesMetadataPathParameters) => { + return [ + `/api/v1/cloud_integrations/${cloudProvider}/accounts/${id}/services`, + ] as const; +}; + +export const getListAccountServicesMetadataQueryOptions = < + TData = Awaited>, + TError = ErrorType, +>( + { cloudProvider, id }: ListAccountServicesMetadataPathParameters, + options?: { + query?: UseQueryOptions< + Awaited>, + TError, + TData + >; + }, +) => { + const { query: queryOptions } = options ?? {}; + + const queryKey = + queryOptions?.queryKey ?? + getListAccountServicesMetadataQueryKey({ cloudProvider, id }); + + const queryFn: QueryFunction< + Awaited> + > = ({ signal }) => listAccountServicesMetadata({ cloudProvider, id }, signal); + + return { + queryKey, + queryFn, + enabled: !!(cloudProvider && id), + ...queryOptions, + } as UseQueryOptions< + Awaited>, + TError, + TData + > & { queryKey: QueryKey }; +}; + +export type ListAccountServicesMetadataQueryResult = NonNullable< + Awaited> +>; +export type ListAccountServicesMetadataQueryError = + ErrorType; + +/** + * @summary List account services metadata + */ + +export function useListAccountServicesMetadata< + TData = Awaited>, + TError = ErrorType, +>( + { cloudProvider, id }: ListAccountServicesMetadataPathParameters, + options?: { + query?: UseQueryOptions< + Awaited>, + TError, + TData + >; + }, +): UseQueryResult & { queryKey: QueryKey } { + const queryOptions = getListAccountServicesMetadataQueryOptions( + { cloudProvider, id }, + options, + ); + + const query = useQuery(queryOptions) as UseQueryResult & { + queryKey: QueryKey; + }; + + return { ...query, queryKey: queryOptions.queryKey }; +} + +/** + * @summary List account services metadata + */ +export const invalidateListAccountServicesMetadata = async ( + queryClient: QueryClient, + { cloudProvider, id }: ListAccountServicesMetadataPathParameters, + options?: InvalidateOptions, +): Promise => { + await queryClient.invalidateQueries( + { queryKey: getListAccountServicesMetadataQueryKey({ cloudProvider, id }) }, + options, + ); + + return queryClient; +}; + /** * This endpoint updates a service for the specified cloud provider * @summary Update service diff --git a/frontend/src/api/generated/services/sigNoz.schemas.ts b/frontend/src/api/generated/services/sigNoz.schemas.ts index 142d57e2c5a..23698cf1182 100644 --- a/frontend/src/api/generated/services/sigNoz.schemas.ts +++ b/frontend/src/api/generated/services/sigNoz.schemas.ts @@ -8695,6 +8695,18 @@ export type UpdateAccountPathParameters = { cloudProvider: string; id: string; }; +export type ListAccountServicesMetadataPathParameters = { + cloudProvider: string; + id: string; +}; +export type ListAccountServicesMetadata200 = { + data: CloudintegrationtypesGettableServicesMetadataDTO; + /** + * @type string + */ + status: string; +}; + export type UpdateServicePathParameters = { cloudProvider: string; id: string; diff --git a/frontend/src/container/Integrations/CloudIntegration/AmazonWebServices/ServiceDetails/ServiceDetails.tsx b/frontend/src/container/Integrations/CloudIntegration/AmazonWebServices/ServiceDetails/ServiceDetails.tsx index 10e2e01328d..9cb5be99088 100644 --- a/frontend/src/container/Integrations/CloudIntegration/AmazonWebServices/ServiceDetails/ServiceDetails.tsx +++ b/frontend/src/container/Integrations/CloudIntegration/AmazonWebServices/ServiceDetails/ServiceDetails.tsx @@ -8,16 +8,16 @@ import { Tabs } from '@signozhq/ui/tabs'; import { Skeleton } from 'antd'; import logEvent from 'api/common/logEvent'; import { - getListServicesMetadataQueryKey, + getListAccountServicesMetadataQueryKey, invalidateGetService, - invalidateListServicesMetadata, + invalidateListAccountServicesMetadata, useGetService, useUpdateService, } from 'api/generated/services/cloudintegration'; import { CloudintegrationtypesServiceConfigDTO, CloudintegrationtypesServiceDTO, - ListServicesMetadata200, + ListAccountServicesMetadata200, } from 'api/generated/services/sigNoz.schemas'; import CloudServiceDataCollected from 'components/CloudIntegrations/CloudServiceDataCollected/CloudServiceDataCollected'; import { MarkdownRenderer } from 'components/MarkdownRenderer/MarkdownRenderer'; @@ -240,16 +240,12 @@ function ServiceDetails({ // instead of waiting for the refetch to complete. reset(nextFormValues); - const servicesListQueryKey = getListServicesMetadataQueryKey( - { - cloudProvider: type, - }, - { - cloud_integration_id: cloudAccountId, - }, - ); + const servicesListQueryKey = getListAccountServicesMetadataQueryKey({ + cloudProvider: type, + id: cloudAccountId, + }); - queryClient.setQueryData( + queryClient.setQueryData( servicesListQueryKey, (prev) => { if (!prev?.data?.services?.length) { @@ -283,15 +279,10 @@ function ServiceDetails({ }, ); - invalidateListServicesMetadata( - queryClient, - { - cloudProvider: type, - }, - { - cloud_integration_id: cloudAccountId, - }, - ); + invalidateListAccountServicesMetadata(queryClient, { + cloudProvider: type, + id: cloudAccountId, + }); logEvent(`${type} Integration: Service settings saved`, { cloudAccountId, diff --git a/frontend/src/container/Integrations/CloudIntegration/ServicesList.tsx b/frontend/src/container/Integrations/CloudIntegration/ServicesList.tsx index 9de64b00018..966926e7e52 100644 --- a/frontend/src/container/Integrations/CloudIntegration/ServicesList.tsx +++ b/frontend/src/container/Integrations/CloudIntegration/ServicesList.tsx @@ -1,7 +1,11 @@ import { useCallback, useEffect, useMemo } from 'react'; import { useNavigate } from 'react-router-dom-v5-compat'; import { Skeleton } from 'antd'; -import { useListServicesMetadata } from 'api/generated/services/cloudintegration'; +import { + useListAccounts, + useListAccountServicesMetadata, + useListServicesMetadata, +} from 'api/generated/services/cloudintegration'; import type { CloudintegrationtypesServiceMetadataDTO } from 'api/generated/services/sigNoz.schemas'; import cx from 'classnames'; import { IntegrationType } from 'container/Integrations/types'; @@ -20,17 +24,33 @@ function ServicesList({ }: ServicesListProps): JSX.Element { const urlQuery = useUrlQuery(); const navigate = useNavigate(); - const hasValidCloudAccountId = Boolean(cloudAccountId); - const serviceQueryParams = hasValidCloudAccountId - ? { cloud_integration_id: cloudAccountId } - : undefined; - - const { data: servicesMetadata, isLoading } = useListServicesMetadata( - { - cloudProvider: type, - }, - serviceQueryParams, - ); + const isAccountConnected = Boolean(cloudAccountId); + const { data: listAccountsResponse, isLoading: isAccountsLoading } = + useListAccounts({ cloudProvider: type }); + const hasConnectedAccounts = + (listAccountsResponse?.data?.accounts?.length ?? 0) > 0; + + const { data: accountServicesMetadata, isLoading: isAccountServicesLoading } = + useListAccountServicesMetadata( + { cloudProvider: type, id: cloudAccountId }, + { query: { enabled: isAccountConnected } }, + ); + + const { + data: providerServicesMetadata, + isLoading: isProviderServicesLoading, + } = useListServicesMetadata({ cloudProvider: type }, undefined, { + query: { enabled: !isAccountsLoading && !hasConnectedAccounts }, + }); + + const servicesMetadata = hasConnectedAccounts + ? accountServicesMetadata + : providerServicesMetadata; + const isLoading = + isAccountsLoading || + (hasConnectedAccounts + ? isAccountServicesLoading || !isAccountConnected + : isProviderServicesLoading); const awsServices = useMemo( () => servicesMetadata?.data?.services ?? [], diff --git a/pkg/apiserver/signozapiserver/cloudintegration.go b/pkg/apiserver/signozapiserver/cloudintegration.go index 8885e88085f..fb879124ed5 100644 --- a/pkg/apiserver/signozapiserver/cloudintegration.go +++ b/pkg/apiserver/signozapiserver/cloudintegration.go @@ -151,6 +151,26 @@ func (provider *provider) addCloudIntegrationRoutes(router *mux.Router) error { return err } + if err := router.Handle("/api/v1/cloud_integrations/{cloud_provider}/accounts/{id}/services", handler.New( + provider.authzMiddleware.AdminAccess(provider.cloudIntegrationHandler.ListAccountServicesMetadata), + handler.OpenAPIDef{ + ID: "ListAccountServicesMetadata", + Tags: []string{"cloudintegration"}, + Summary: "List account services metadata", + Description: "This endpoint lists the services metadata for the specified account and cloud provider", + Request: nil, + RequestContentType: "", + Response: new(citypes.GettableServicesMetadata), + ResponseContentType: "application/json", + SuccessStatusCode: http.StatusOK, + ErrorStatusCodes: []int{}, + Deprecated: false, + SecuritySchemes: newSecuritySchemes(types.RoleAdmin), + }, + )).Methods(http.MethodGet).GetError(); err != nil { + return err + } + if err := router.Handle("/api/v1/cloud_integrations/{cloud_provider}/services/{service_id}", handler.New( provider.authzMiddleware.AdminAccess(provider.cloudIntegrationHandler.GetService), handler.OpenAPIDef{ diff --git a/pkg/modules/cloudintegration/cloudintegration.go b/pkg/modules/cloudintegration/cloudintegration.go index ba9bbc319f0..1f99a59f460 100644 --- a/pkg/modules/cloudintegration/cloudintegration.go +++ b/pkg/modules/cloudintegration/cloudintegration.go @@ -75,6 +75,7 @@ type Handler interface { UpdateAccount(http.ResponseWriter, *http.Request) DisconnectAccount(http.ResponseWriter, *http.Request) ListServicesMetadata(http.ResponseWriter, *http.Request) + ListAccountServicesMetadata(http.ResponseWriter, *http.Request) GetService(http.ResponseWriter, *http.Request) UpdateService(http.ResponseWriter, *http.Request) AgentCheckIn(http.ResponseWriter, *http.Request) diff --git a/pkg/modules/cloudintegration/implcloudintegration/handler.go b/pkg/modules/cloudintegration/implcloudintegration/handler.go index 7a9764cb604..b65148366fd 100644 --- a/pkg/modules/cloudintegration/implcloudintegration/handler.go +++ b/pkg/modules/cloudintegration/implcloudintegration/handler.go @@ -276,6 +276,44 @@ func (handler *handler) ListServicesMetadata(rw http.ResponseWriter, r *http.Req render.Success(rw, http.StatusOK, cloudintegrationtypes.NewGettableServicesMetadata(services)) } +func (handler *handler) ListAccountServicesMetadata(rw http.ResponseWriter, r *http.Request) { + ctx, cancel := context.WithTimeout(r.Context(), 10*time.Second) + defer cancel() + + claims, err := authtypes.ClaimsFromContext(ctx) + if err != nil { + render.Error(rw, err) + return + } + + provider, err := cloudintegrationtypes.NewCloudProvider(mux.Vars(r)["cloud_provider"]) + if err != nil { + render.Error(rw, err) + return + } + + accountID, err := valuer.NewUUID(mux.Vars(r)["id"]) + if err != nil { + render.Error(rw, err) + return + } + + // check if integration account exists and is not removed. + _, err = handler.module.GetConnectedAccount(ctx, valuer.MustNewUUID(claims.OrgID), accountID, provider) + if err != nil { + render.Error(rw, err) + return + } + + services, err := handler.module.ListServicesMetadata(ctx, valuer.MustNewUUID(claims.OrgID), provider, accountID) + if err != nil { + render.Error(rw, err) + return + } + + render.Success(rw, http.StatusOK, cloudintegrationtypes.NewGettableServicesMetadata(services)) +} + func (handler *handler) GetService(rw http.ResponseWriter, r *http.Request) { ctx, cancel := context.WithTimeout(r.Context(), 10*time.Second) defer cancel() diff --git a/tests/integration/tests/cloudintegrations/05_services.py b/tests/integration/tests/cloudintegrations/05_services.py index 7ab67bbe24f..6731851c57d 100644 --- a/tests/integration/tests/cloudintegrations/05_services.py +++ b/tests/integration/tests/cloudintegrations/05_services.py @@ -86,6 +86,49 @@ def test_list_services_with_account( assert svc["enabled"] is False, f"Service {svc['id']} should be disabled before any config is set" +EC2_SERVICE_ID = "ec2" + + +def test_list_account_services( + signoz: types.SigNoz, + create_user_admin: types.Operation, # pylint: disable=unused-argument + get_token: Callable[[str, str], str], + create_cloud_integration_account: Callable, +) -> None: + """ListAccountServicesMetadata reflects enabled state after enabling a service.""" + admin_token = get_token(USER_ADMIN_EMAIL, USER_ADMIN_PASSWORD) + + account = create_cloud_integration_account(admin_token, CLOUD_PROVIDER) + account_id = account["id"] + + checkin = simulate_agent_checkin(signoz, admin_token, CLOUD_PROVIDER, account_id, str(uuid.uuid4())) + assert checkin.status_code == HTTPStatus.OK, f"Check-in failed: {checkin.text}" + + put_response = requests.put( + signoz.self.host_configs["8080"].get(f"/api/v1/cloud_integrations/{CLOUD_PROVIDER}/accounts/{account_id}/services/{EC2_SERVICE_ID}"), + headers={"Authorization": f"Bearer {admin_token}"}, + json={"config": {"aws": {"metrics": {"enabled": True}, "logs": {"enabled": True}}}}, + timeout=10, + ) + assert put_response.status_code == HTTPStatus.NO_CONTENT, f"Enable ec2 failed: {put_response.status_code}: {put_response.text}" + + list_response = requests.get( + signoz.self.host_configs["8080"].get(f"/api/v1/cloud_integrations/{CLOUD_PROVIDER}/accounts/{account_id}/services"), + headers={"Authorization": f"Bearer {admin_token}"}, + timeout=10, + ) + assert list_response.status_code == HTTPStatus.OK, f"Expected 200, got {list_response.status_code}" + + data = list_response.json()["data"] + assert "services" in data, "Response should contain 'services' field" + assert isinstance(data["services"], list), "services should be a list" + assert len(data["services"]) > 0, "services list should be non-empty" + + ec2_service = next((s for s in data["services"] if s["id"] == EC2_SERVICE_ID), None) + assert ec2_service is not None, f"EC2 service '{EC2_SERVICE_ID}' not found in services list" + assert ec2_service["enabled"] is True, f"EC2 service should be enabled, got: {ec2_service['enabled']}" + + def test_get_service_details_without_account( signoz: types.SigNoz, create_user_admin: types.Operation, # pylint: disable=unused-argument From eec1c45e3fae6e7d21ce4d11396e17695129999b Mon Sep 17 00:00:00 2001 From: Aditya Singh Date: Fri, 5 Jun 2026 01:32:13 +0530 Subject: [PATCH 3/4] fix(trace-flamegraph): prevent layout hang on very wide traces (#11578) * feat: algo change * feat: update test --- .../__tests__/computeVisualLayout.test.ts | 178 +++++++++++++++++- .../TraceFlamegraph/computeVisualLayout.ts | 123 +++++++++++- .../hooks/useVisualLayoutWorker.ts | 2 +- 3 files changed, 300 insertions(+), 3 deletions(-) diff --git a/frontend/src/pages/TraceDetailsV3/TraceFlamegraph/__tests__/computeVisualLayout.test.ts b/frontend/src/pages/TraceDetailsV3/TraceFlamegraph/__tests__/computeVisualLayout.test.ts index 53a0eb5ff5c..64674225373 100644 --- a/frontend/src/pages/TraceDetailsV3/TraceFlamegraph/__tests__/computeVisualLayout.test.ts +++ b/frontend/src/pages/TraceDetailsV3/TraceFlamegraph/__tests__/computeVisualLayout.test.ts @@ -1,6 +1,9 @@ import { FlamegraphSpan } from 'types/api/trace/getTraceFlamegraph'; -import { computeVisualLayout } from '../computeVisualLayout'; +import { + computeVisualLayout, + WIDE_GROUP_THRESHOLD, +} from '../computeVisualLayout'; function makeSpan( overrides: Partial & { @@ -472,4 +475,177 @@ describe('computeVisualLayout', () => { expect(aRow).toBeGreaterThan(1); // must NOT be at row 1 expect(aRow).toBe(3); // next free row after B at row 2 (A overlaps B) }); + + // --- Wide-group fast path (> WIDE_GROUP_THRESHOLD siblings) --- + // Past the threshold the layout switches to exact overlap-only packing to + // avoid the O(N^2) connector-avoidance spiral. These lock in correctness and + // the no-overlap invariant at scale. + + function noRowHasOverlap( + layout: ReturnType, + ): void { + for (const row of layout.visualRows) { + const sorted = [...row].sort((a, b) => a.timestamp - b.timestamp); + for (let i = 1; i < sorted.length; i++) { + const prevEnd = sorted[i - 1].timestamp + sorted[i - 1].durationNano / 1e6; + expect(sorted[i].timestamp).toBeGreaterThanOrEqual(prevEnd); + } + } + } + + it('should pack thousands of sequential leaf siblings into 1 row (wide path)', () => { + const root = makeSpan({ spanId: 'root', timestamp: 0, durationNano: 1e12 }); + const kids: FlamegraphSpan[] = []; + // 2000 strictly sequential (non-overlapping) children + for (let i = 0; i < 2000; i++) { + kids.push( + makeSpan({ + spanId: `k${i}`, + parentSpanId: 'root', + timestamp: i * 10, + durationNano: 5e6, // 5ms, ends before next starts + }), + ); + } + + const layout = computeVisualLayout([[root], kids]); + + expect(layout.spanToVisualRow['root']).toBe(0); + expect(layout.totalVisualRows).toBe(2); // all siblings share row 1 + for (const k of kids) { + expect(layout.spanToVisualRow[k.spanId]).toBe(1); + } + noRowHasOverlap(layout); + }); + + it('should pack thousands of fully-overlapping leaf siblings without violations (wide path)', () => { + const root = makeSpan({ spanId: 'root', timestamp: 0, durationNano: 1e12 }); + const kids: FlamegraphSpan[] = []; + // 1000 children all spanning the same window → each needs its own row + for (let i = 0; i < 1000; i++) { + kids.push( + makeSpan({ + spanId: `k${i}`, + parentSpanId: 'root', + timestamp: 0, + durationNano: 100e6, + }), + ); + } + + const layout = computeVisualLayout([[root], kids]); + + expect(layout.totalVisualRows).toBe(1001); // root + 1000 stacked rows + expect(Object.keys(layout.spanToVisualRow)).toHaveLength(1001); + noRowHasOverlap(layout); + }); + + it('should keep non-leaf subtrees adjacent within a wide mixed group (wide path)', () => { + const root = makeSpan({ spanId: 'root', timestamp: 0, durationNano: 1e12 }); + const kids: FlamegraphSpan[] = []; + for (let i = 0; i < 1000; i++) { + kids.push( + makeSpan({ + spanId: `k${i}`, + parentSpanId: 'root', + timestamp: i * 10, + durationNano: 5e6, + }), + ); + } + // One of the wide siblings has a child of its own + const grandchild = makeSpan({ + spanId: 'gc', + parentSpanId: 'k500', + timestamp: 5000, + durationNano: 2e6, + }); + + const layout = computeVisualLayout([[root], kids, [grandchild]]); + + const parentRow = layout.spanToVisualRow['k500']; + const gcRow = layout.spanToVisualRow['gc']; + expect(gcRow - parentRow).toBe(1); // subtree adjacency preserved + expect(Object.keys(layout.spanToVisualRow)).toHaveLength(1002); + noRowHasOverlap(layout); + }); + + // --- Regression guards for the wide-trace layout spiral --- + // The pre-fix algorithm's connector-avoidance checks formed a positive- + // feedback loop on wide SCATTERED groups (short spans spread across the + // parent window with modest concurrency): each pushed-down child stamped + // connector points on intermediate rows, pushing later children even + // higher. Sequential and fully-overlapping groups (tests above) do NOT + // trigger it — these two shapes do, and encode its failure signatures. + + function makeScatteredStar( + childCount: number, + spacingMs: number, + durationMs: number, + ): FlamegraphSpan[][] { + const root = makeSpan({ + spanId: 'root', + timestamp: 0, + durationNano: (childCount * spacingMs + durationMs) * 1e6, + }); + const kids: FlamegraphSpan[] = []; + for (let i = 0; i < childCount; i++) { + kids.push( + makeSpan({ + spanId: `k${i}`, + parentSpanId: 'root', + timestamp: i * spacingMs, + durationNano: durationMs * 1e6, + }), + ); + } + return [[root], kids]; + } + + it('should not spiral row count on a scattered group just above the threshold', () => { + // Children of 50ms each, starting every 10ms → max temporal concurrency + // is 6 (each span overlaps the next 5). The old algorithm spiraled this + // shape to ~1 row per child; overlap-only packing needs ~7 including the + // root. Sized just above WIDE_GROUP_THRESHOLD so a regression fails fast + // (sub-second) rather than burning CI time. + const count = WIDE_GROUP_THRESHOLD + 88; + const layout = computeVisualLayout(makeScatteredStar(count, 10, 50)); + + expect(Object.keys(layout.spanToVisualRow)).toHaveLength(count + 1); + // Generous slack over the optimal ~7 — but orders of magnitude below the + // one-row-per-child the spiral produced. + expect(layout.totalVisualRows).toBeLessThan(30); + noRowHasOverlap(layout); + }); + + it('should be faster through the fast path despite one extra span', () => { + // Identical scattered shape on both sides of the gate: THRESHOLD children + // run the original connector-avoidance path, THRESHOLD + 1 run the + // overlap-only fast path. With one MORE span to place, the fast path can + // only win because the algorithm is cheaper — a self-calibrating + // comparison (same machine, same process) with no absolute time budget. + // On this shape the avoidance path spirals (~tens of ms) while the fast + // path stays ~1ms, so the margin is well past timer noise. Also guards + // the gate itself: if everything routed to one path, one extra span can + // never be faster. + const avoidancePathInput = makeScatteredStar(WIDE_GROUP_THRESHOLD, 10, 50); + const fastPathInput = makeScatteredStar(WIDE_GROUP_THRESHOLD + 1, 10, 50); + + // Warm-up runs so JIT compilation doesn't skew either side. + computeVisualLayout(avoidancePathInput); + computeVisualLayout(fastPathInput); + + const timeOf = (input: FlamegraphSpan[][]): number => { + const start = performance.now(); + computeVisualLayout(input); + return performance.now() - start; + }; + + // Best-of-3 per side to shave off GC pauses and scheduler noise. + const RUNS = [0, 1, 2]; + const avoidanceMs = Math.min(...RUNS.map(() => timeOf(avoidancePathInput))); + const fastMs = Math.min(...RUNS.map(() => timeOf(fastPathInput))); + + expect(fastMs).toBeLessThan(avoidanceMs); + }); }); diff --git a/frontend/src/pages/TraceDetailsV3/TraceFlamegraph/computeVisualLayout.ts b/frontend/src/pages/TraceDetailsV3/TraceFlamegraph/computeVisualLayout.ts index ca97b89878e..48547600f7b 100644 --- a/frontend/src/pages/TraceDetailsV3/TraceFlamegraph/computeVisualLayout.ts +++ b/frontend/src/pages/TraceDetailsV3/TraceFlamegraph/computeVisualLayout.ts @@ -18,6 +18,82 @@ export interface VisualLayout { totalVisualRows: number; } +// Above this many siblings under one parent, the connector-avoidance refinement +// (Checks 2 & 3) is both visually meaningless — the row is already a dense wall — +// and quadratic: every child deposits a connector point on each intermediate row, +// which pushes later children even higher, which deposits more points. That +// feedback loop inflates a layout needing ~50 rows to thousands and never +// finishes on wide traces. Past the threshold we pack by overlap only. +// Exported so the regression tests stay anchored to the real gate value. +export const WIDE_GROUP_THRESHOLD = 512; + +/** + * Segment tree over rows that answers "lowest row index >= `from` whose smallest + * span start-time is >= `end`" in O(log rows). Used to place a large group of + * leaf siblings by overlap only: because siblings are processed in descending + * start order, every already-placed span on a row starts at or after the current + * one, so [start, end] overlaps a row iff some span there starts before `end` — + * i.e. the row is free iff its minimum start >= end. Each node stores the max of + * its subtree's per-row minimum starts so a free row can be found by descent. + */ +class LowestFreeRow { + private readonly size: number; + + private readonly tree: Float64Array; + + constructor(rows: number) { + let size = 1; + while (size < rows) { + size *= 2; + } + this.size = size; + this.tree = new Float64Array(size * 2).fill(Infinity); + } + + place(row: number, start: number): void { + let i = row + this.size; + // A row's key is the minimum start among its spans. Children are processed + // in descending start order so a leaf's start is the new minimum, but a + // non-leaf subtree's descendant can land on a row out of order — take min. + if (start >= this.tree[i]) { + return; + } + this.tree[i] = start; + for (i >>= 1; i >= 1; i >>= 1) { + const next = Math.max(this.tree[2 * i], this.tree[2 * i + 1]); + if (this.tree[i] === next) { + break; + } + this.tree[i] = next; + } + } + + lowestFrom(from: number, end: number): number { + return this.descend(1, 0, this.size - 1, from, end); + } + + private descend( + node: number, + lo: number, + hi: number, + from: number, + end: number, + ): number { + if (hi < from || this.tree[node] < end) { + return -1; + } + if (lo === hi) { + return lo; + } + const mid = (lo + hi) >> 1; + const left = this.descend(2 * node, lo, mid, from, end); + if (left !== -1) { + return left; + } + return this.descend(2 * node + 1, mid + 1, hi, from, end); + } +} + /** * Computes an overlap-safe visual layout for flamegraph spans using DFS ordering. * @@ -214,7 +290,53 @@ export function computeVisualLayout(spans: FlamegraphSpan[][]): VisualLayout { arr.push(point); } + // Fast path for a parent with a very large group of children: pack by overlap + // only (descending greedy), skipping the quadratic connector-avoidance that + // spirals at this scale. Leaf children — the bulk of a wide trace — are placed + // in O(log rows) via the segment tree; the rare non-leaf subtree falls back to + // findPlacement against the shared interval map. Both structures are kept in + // sync so each placement sees all prior occupancy. Same ShapeEntry[] contract. + function computeWideShape( + rootSpan: FlamegraphSpan, + children: FlamegraphSpan[], + ): ShapeEntry[] { + const shape: ShapeEntry[] = [{ span: rootSpan, relativeRow: 0 }]; + const localIntervals = new Map>(); + // Children occupy relative rows 1..children.length in the worst case. + const finder = new LowestFreeRow(children.length + 2); + + const occupy = (row: number, span: FlamegraphSpan): void => { + const s = span.timestamp; + const e = span.timestamp + span.durationNano / 1e6; + shape.push({ span, relativeRow: row }); + addIntervalTo(localIntervals, row, s, e); + finder.place(row, s); + }; + + for (const child of children) { + if (childrenMap.has(child.spanId)) { + // Non-leaf: place its whole subtree shape as a unit via findPlacement. + const childShape = computeSubtreeShape(child); + const offset = findPlacement(childShape, 1, localIntervals); + for (const entry of childShape) { + occupy(entry.relativeRow + offset, entry.span); + } + } else { + const end = child.timestamp + child.durationNano / 1e6; + occupy(finder.lowestFrom(1, end), child); + } + } + + return shape; + } + function computeSubtreeShape(rootSpan: FlamegraphSpan): ShapeEntry[] { + const children = childrenMap.get(rootSpan.spanId); + + if (children && children.length > WIDE_GROUP_THRESHOLD) { + return computeWideShape(rootSpan, children); + } + const localIntervals = new Map>(); const localConnectorPoints = new Map(); const shape: ShapeEntry[] = []; @@ -225,7 +347,6 @@ export function computeVisualLayout(spans: FlamegraphSpan[][]): VisualLayout { shape.push({ span: rootSpan, relativeRow: 0 }); addIntervalTo(localIntervals, 0, rootStart, rootEnd); - const children = childrenMap.get(rootSpan.spanId); if (children) { for (const child of children) { const childShape = computeSubtreeShape(child); diff --git a/frontend/src/pages/TraceDetailsV3/TraceFlamegraph/hooks/useVisualLayoutWorker.ts b/frontend/src/pages/TraceDetailsV3/TraceFlamegraph/hooks/useVisualLayoutWorker.ts index 0a25c6ea503..9369850d806 100644 --- a/frontend/src/pages/TraceDetailsV3/TraceFlamegraph/hooks/useVisualLayoutWorker.ts +++ b/frontend/src/pages/TraceDetailsV3/TraceFlamegraph/hooks/useVisualLayoutWorker.ts @@ -94,7 +94,7 @@ export function useVisualLayoutWorker(spans: FlamegraphSpan[][]): { cleanup(); }; - // Timeout: if worker doesn't respond in 30s, terminate and error + // Timeout: if worker doesn't respond in 15s, terminate and error const WORKER_TIMEOUT_MS = 15000; const timeoutId = setTimeout(() => { if (requestIdRef.current === currentId && isComputingRef.current) { From 81eadac3a83ebe6d9321b723ea805845152d4c91 Mon Sep 17 00:00:00 2001 From: Aditya Singh Date: Fri, 5 Jun 2026 01:39:44 +0530 Subject: [PATCH 4/4] feat(trace-details): lazy aggregations + waterfall v4 cutover (#11556) * feat: wire available colors on spans instead of aggregations from waterfall api * feat: integrate aggregation api * feat: integrate v4 waterfall * feat: dropdown style fix * feat: move to open api spec * feat: move to open api spec * feat: minor changes --- .../trace/{getTraceV3.tsx => getTraceV4.tsx} | 47 +++--- frontend/src/constants/reactQueryKeys.ts | 3 +- .../useGetTraceAggregations.test.tsx | 66 ++++++++ .../hooks/trace/useGetTraceAggregations.tsx | 34 +++++ .../{useGetTraceV3.tsx => useGetTraceV4.tsx} | 19 ++- .../AnalyticsPanel/AnalyticsPanel.module.scss | 9 ++ .../AnalyticsPanel/AnalyticsPanel.tsx | 123 +++++++-------- .../AnalyticsPanel/AnalyticsTabContent.tsx | 84 ++++++++++ .../__tests__/AnalyticsPanel.test.tsx | 144 ++++++++++++++++++ .../TraceOptionsMenu.module.scss | 3 + .../TraceDetailsHeader/TraceOptionsMenu.tsx | 130 +++++++--------- .../TraceWaterfall/TraceWaterfall.tsx | 4 +- .../getAvailableColorByFieldNames.test.ts | 34 +++++ frontend/src/pages/TraceDetailsV3/index.tsx | 37 ++--- .../TraceDetailsV3/stores/TraceStoreSync.tsx | 13 +- .../stores/__tests__/traceStore.test.ts | 71 +++++++++ .../pages/TraceDetailsV3/stores/traceStore.ts | 50 +++--- frontend/src/pages/TraceDetailsV3/utils.ts | 12 ++ .../TraceDetailsV3/utils/aggregations.ts | 20 +-- frontend/src/types/api/trace/getTraceV3.ts | 22 +-- 20 files changed, 659 insertions(+), 266 deletions(-) rename frontend/src/api/trace/{getTraceV3.tsx => getTraceV4.tsx} (57%) create mode 100644 frontend/src/hooks/trace/__tests__/useGetTraceAggregations.test.tsx create mode 100644 frontend/src/hooks/trace/useGetTraceAggregations.tsx rename frontend/src/hooks/trace/{useGetTraceV3.tsx => useGetTraceV4.tsx} (52%) create mode 100644 frontend/src/pages/TraceDetailsV3/SpanDetailsPanel/AnalyticsPanel/AnalyticsTabContent.tsx create mode 100644 frontend/src/pages/TraceDetailsV3/SpanDetailsPanel/AnalyticsPanel/__tests__/AnalyticsPanel.test.tsx create mode 100644 frontend/src/pages/TraceDetailsV3/__tests__/getAvailableColorByFieldNames.test.ts create mode 100644 frontend/src/pages/TraceDetailsV3/stores/__tests__/traceStore.test.ts diff --git a/frontend/src/api/trace/getTraceV3.tsx b/frontend/src/api/trace/getTraceV4.tsx similarity index 57% rename from frontend/src/api/trace/getTraceV3.tsx rename to frontend/src/api/trace/getTraceV4.tsx index 6f99a4c96ed..907856a1be1 100644 --- a/frontend/src/api/trace/getTraceV3.tsx +++ b/frontend/src/api/trace/getTraceV4.tsx @@ -1,15 +1,14 @@ -import { ApiV3Instance as axios } from 'api'; -import { omit } from 'lodash-es'; +import { getWaterfallV4 } from 'api/generated/services/tracedetail'; import { ErrorResponse, SuccessResponse } from 'types/api'; import { - GetTraceV3PayloadProps, - GetTraceV3SuccessResponse, + GetTraceV4PayloadProps, + GetTraceV4SuccessResponse, SpanV3, } from 'types/api/trace/getTraceV3'; -const getTraceV3 = async ( - props: GetTraceV3PayloadProps, -): Promise | ErrorResponse> => { +const getTraceV4 = async ( + props: GetTraceV4PayloadProps, +): Promise | ErrorResponse> => { let uncollapsedSpans = [...props.uncollapsedSpans]; if (!props.isSelectedSpanIDUnCollapsed) { uncollapsedSpans = uncollapsedSpans.filter( @@ -19,31 +18,37 @@ const getTraceV3 = async ( props.selectedSpanId && !uncollapsedSpans.includes(props.selectedSpanId) ) { - // V3 backend only uses uncollapsedSpans list (unlike V2 which also interprets + // Backend only uses the uncollapsedSpans list (unlike V2 which also interprets // isSelectedSpanIDUnCollapsed server-side), so explicitly add the selected span uncollapsedSpans.push(props.selectedSpanId); } - const postData: GetTraceV3PayloadProps = { - ...props, - uncollapsedSpans, - limit: 10000, - }; - const response = await axios.post( - `/traces/${props.traceId}/waterfall`, - omit(postData, 'traceId'), + const response = await getWaterfallV4( + { traceID: props.traceId }, + { + selectedSpanId: props.selectedSpanId, + uncollapsedSpans, + limit: 10000, + }, ); - // V3 API wraps response in { status, data } - const rawPayload = (response.data as any).data || response.data; + // Generated client unwraps the axios response; .data is the waterfall payload. + // Wire spans carry time_unix; SpanV3's timestamp + 'service.name' are derived below. + type WireSpan = Omit & { + time_unix: number; + }; + const rawPayload = response.data as unknown as Omit< + GetTraceV4SuccessResponse, + 'spans' + > & { spans: WireSpan[] | null }; // Derive 'service.name' from resource for convenience — only derived field - const spans: SpanV3[] = (rawPayload.spans || []).map((span: any) => ({ + const spans: SpanV3[] = (rawPayload.spans || []).map((span) => ({ ...span, 'service.name': span.resource?.['service.name'] || '', timestamp: span.time_unix, })); - // V3 API returns startTimestampMillis/endTimestampMillis as relative durations (ms from epoch offset), + // API returns startTimestampMillis/endTimestampMillis as relative durations (ms from epoch offset), // not absolute unix millis like V2. The span timestamps are absolute unix millis. // Convert by using the first span's timestamp as the base if there's a mismatch. let { startTimestampMillis, endTimestampMillis } = rawPayload; @@ -70,4 +75,4 @@ const getTraceV3 = async ( }; }; -export default getTraceV3; +export default getTraceV4; diff --git a/frontend/src/constants/reactQueryKeys.ts b/frontend/src/constants/reactQueryKeys.ts index 2946f5e8688..9620bcfc5ac 100644 --- a/frontend/src/constants/reactQueryKeys.ts +++ b/frontend/src/constants/reactQueryKeys.ts @@ -33,7 +33,8 @@ export const REACT_QUERY_KEY = { UPDATE_ALERT_RULE: 'UPDATE_ALERT_RULE', GET_ACTIVE_LICENSE_V3: 'GET_ACTIVE_LICENSE_V3', GET_TRACE_V2_WATERFALL: 'GET_TRACE_V2_WATERFALL', - GET_TRACE_V3_WATERFALL: 'GET_TRACE_V3_WATERFALL', + GET_TRACE_V4_WATERFALL: 'GET_TRACE_V4_WATERFALL', + GET_TRACE_AGGREGATIONS: 'GET_TRACE_AGGREGATIONS', GET_TRACE_V2_FLAMEGRAPH: 'GET_TRACE_V2_FLAMEGRAPH', GET_POD_LIST: 'GET_POD_LIST', GET_NODE_LIST: 'GET_NODE_LIST', diff --git a/frontend/src/hooks/trace/__tests__/useGetTraceAggregations.test.tsx b/frontend/src/hooks/trace/__tests__/useGetTraceAggregations.test.tsx new file mode 100644 index 00000000000..e56c23e09a0 --- /dev/null +++ b/frontend/src/hooks/trace/__tests__/useGetTraceAggregations.test.tsx @@ -0,0 +1,66 @@ +import { renderHook, waitFor } from '@testing-library/react'; +import { getTraceAggregations } from 'api/generated/services/tracedetail'; +import { ReactNode } from 'react'; +import { QueryClient, QueryClientProvider } from 'react-query'; + +import useGetTraceAggregations from '../useGetTraceAggregations'; + +jest.mock('api/generated/services/tracedetail', () => ({ + __esModule: true, + getTraceAggregations: jest + .fn() + .mockResolvedValue({ status: 'success', data: { aggregations: [] } }), +})); + +const mockApi = getTraceAggregations as jest.Mock; + +const wrapper = ({ children }: { children: ReactNode }): JSX.Element => { + const client = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + return {children}; +}; + +const aggregations = [ + { field: { name: 'service.name' }, aggregation: 'execution_time_percentage' }, +] as never; + +describe('useGetTraceAggregations', () => { + beforeEach(() => mockApi.mockClear()); + + it('fetches when enabled with a traceId and aggregations', async () => { + renderHook( + () => + useGetTraceAggregations({ traceId: 't1', aggregations, enabled: true }), + { wrapper }, + ); + await waitFor(() => expect(mockApi).toHaveBeenCalledTimes(1)); + expect(mockApi).toHaveBeenCalledWith({ traceID: 't1' }, { aggregations }); + }); + + it('does not fetch when disabled', () => { + renderHook( + () => + useGetTraceAggregations({ traceId: 't1', aggregations, enabled: false }), + { wrapper }, + ); + expect(mockApi).not.toHaveBeenCalled(); + }); + + it('does not fetch without a traceId', () => { + renderHook( + () => useGetTraceAggregations({ traceId: '', aggregations, enabled: true }), + { wrapper }, + ); + expect(mockApi).not.toHaveBeenCalled(); + }); + + it('does not fetch with no aggregations requested', () => { + renderHook( + () => + useGetTraceAggregations({ traceId: 't1', aggregations: [], enabled: true }), + { wrapper }, + ); + expect(mockApi).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/hooks/trace/useGetTraceAggregations.tsx b/frontend/src/hooks/trace/useGetTraceAggregations.tsx new file mode 100644 index 00000000000..d925855ee18 --- /dev/null +++ b/frontend/src/hooks/trace/useGetTraceAggregations.tsx @@ -0,0 +1,34 @@ +import { useQuery, UseQueryResult } from 'react-query'; +import { + GetTraceAggregations200, + SpantypesSpanAggregationDTO, +} from 'api/generated/services/sigNoz.schemas'; +import { getTraceAggregations } from 'api/generated/services/tracedetail'; +import { REACT_QUERY_KEY } from 'constants/reactQueryKeys'; + +interface UseGetTraceAggregationsProps { + traceId: string; + aggregations: SpantypesSpanAggregationDTO[]; + enabled: boolean; +} + +type UseGetTraceAggregations = UseQueryResult; + +/** + * Fetches trace aggregations on demand — gate via `enabled` so the request + * fires only when the Analytics panel is open. The query key includes the + * requested fields, so changing the color-by field refetches. + */ +const useGetTraceAggregations = ({ + traceId, + aggregations, + enabled, +}: UseGetTraceAggregationsProps): UseGetTraceAggregations => + useQuery({ + queryFn: () => getTraceAggregations({ traceID: traceId }, { aggregations }), + queryKey: [REACT_QUERY_KEY.GET_TRACE_AGGREGATIONS, traceId, aggregations], + enabled: enabled && !!traceId && aggregations.length > 0, + refetchOnWindowFocus: false, + }); + +export default useGetTraceAggregations; diff --git a/frontend/src/hooks/trace/useGetTraceV3.tsx b/frontend/src/hooks/trace/useGetTraceV4.tsx similarity index 52% rename from frontend/src/hooks/trace/useGetTraceV3.tsx rename to frontend/src/hooks/trace/useGetTraceV4.tsx index fdf582ef307..07aa7b53916 100644 --- a/frontend/src/hooks/trace/useGetTraceV3.tsx +++ b/frontend/src/hooks/trace/useGetTraceV4.tsx @@ -1,30 +1,29 @@ import { useQuery, UseQueryResult } from 'react-query'; -import getTraceV3 from 'api/trace/getTraceV3'; +import getTraceV4 from 'api/trace/getTraceV4'; import { REACT_QUERY_KEY } from 'constants/reactQueryKeys'; import { ErrorResponse, SuccessResponse } from 'types/api'; import { - GetTraceV3PayloadProps, - GetTraceV3SuccessResponse, + GetTraceV4PayloadProps, + GetTraceV4SuccessResponse, } from 'types/api/trace/getTraceV3'; -const useGetTraceV3 = (props: GetTraceV3PayloadProps): UseTraceV3 => +const useGetTraceV4 = (props: GetTraceV4PayloadProps): UseTraceV4 => useQuery({ - queryFn: () => getTraceV3(props), + queryFn: () => getTraceV4(props), queryKey: [ - REACT_QUERY_KEY.GET_TRACE_V3_WATERFALL, + REACT_QUERY_KEY.GET_TRACE_V4_WATERFALL, props.traceId, props.selectedSpanId, props.isSelectedSpanIDUnCollapsed, - props.aggregations, ], enabled: !!props.traceId, keepPreviousData: true, refetchOnWindowFocus: false, }); -type UseTraceV3 = UseQueryResult< - SuccessResponse | ErrorResponse, +type UseTraceV4 = UseQueryResult< + SuccessResponse | ErrorResponse, unknown >; -export default useGetTraceV3; +export default useGetTraceV4; diff --git a/frontend/src/pages/TraceDetailsV3/SpanDetailsPanel/AnalyticsPanel/AnalyticsPanel.module.scss b/frontend/src/pages/TraceDetailsV3/SpanDetailsPanel/AnalyticsPanel/AnalyticsPanel.module.scss index e130fe20f06..02f368a39fd 100644 --- a/frontend/src/pages/TraceDetailsV3/SpanDetailsPanel/AnalyticsPanel/AnalyticsPanel.module.scss +++ b/frontend/src/pages/TraceDetailsV3/SpanDetailsPanel/AnalyticsPanel/AnalyticsPanel.module.scss @@ -33,6 +33,15 @@ scrollbar-width: none; } +.state { + display: flex; + align-items: center; + justify-content: center; + flex: 1; + min-height: 120px; + padding: 24px 12px; +} + .list { display: grid; grid-template-columns: auto auto 1fr; diff --git a/frontend/src/pages/TraceDetailsV3/SpanDetailsPanel/AnalyticsPanel/AnalyticsPanel.tsx b/frontend/src/pages/TraceDetailsV3/SpanDetailsPanel/AnalyticsPanel/AnalyticsPanel.tsx index fd845193ecf..4f9f3cadd42 100644 --- a/frontend/src/pages/TraceDetailsV3/SpanDetailsPanel/AnalyticsPanel/AnalyticsPanel.tsx +++ b/frontend/src/pages/TraceDetailsV3/SpanDetailsPanel/AnalyticsPanel/AnalyticsPanel.tsx @@ -1,21 +1,28 @@ import { useMemo } from 'react'; +import { useParams } from 'react-router-dom'; import { TabsContent, TabsList, TabsRoot, TabsTrigger, } from '@signozhq/ui/tabs'; -import cx from 'classnames'; import { DetailsHeader } from 'components/DetailsPanel'; import { useIsDarkMode } from 'hooks/useDarkMode'; +import useGetTraceAggregations from 'hooks/trace/useGetTraceAggregations'; import { generateColorPair } from 'pages/TraceDetailsV3/utils/generateColorPair'; import { FloatingPanel } from 'periscope/components/FloatingPanel'; +import { + SpantypesSpanAggregationDTO, + TelemetrytypesTelemetryFieldKeyDTO, +} from 'api/generated/services/sigNoz.schemas'; +import { TraceDetailV3URLProps } from 'types/api/trace/getTraceV3'; import { useTraceStore } from '../../stores/traceStore'; import { AGGREGATIONS, getAggregationMap as findAggregationMap, } from '../../utils/aggregations'; +import AnalyticsTabContent, { AnalyticsRow } from './AnalyticsTabContent'; import styles from './AnalyticsPanel.module.scss'; @@ -35,10 +42,31 @@ function AnalyticsPanel({ onClose, onTabChange, }: AnalyticsPanelProps): JSX.Element | null { - const aggregations = useTraceStore((s) => s.aggregations); - const colorByFieldName = useTraceStore((s) => s.colorByField.name); + const { id: traceId } = useParams(); + const colorByField = useTraceStore((s) => s.colorByField); + const colorByFieldName = colorByField.name; const isDarkMode = useIsDarkMode(); + // Fetch exec-time % + span count for the current color-by field only, and + // only while the panel is open. Changing the field refetches via the key. + const aggregationsRequest = useMemo(() => { + // v5 TelemetryFieldKey and the generated DTO are runtime-identical; only + // the literal-union vs enum nominal types differ + const field = colorByField as unknown as TelemetrytypesTelemetryFieldKeyDTO; + return [ + { field, aggregation: AGGREGATIONS.EXEC_TIME_PCT }, + { field, aggregation: AGGREGATIONS.SPAN_COUNT }, + ]; + }, [colorByField]); + + const { data, isLoading, isError } = useGetTraceAggregations({ + traceId: traceId || '', + aggregations: aggregationsRequest, + enabled: isOpen, + }); + + const aggregations = data?.data.aggregations; + const execTimePct = useMemo( () => findAggregationMap( @@ -55,38 +83,39 @@ function AnalyticsPanel({ [aggregations, colorByFieldName], ); - const execTimeRows = useMemo(() => { + const execTimeRows = useMemo(() => { if (!execTimePct) { return []; } return Object.entries(execTimePct) + .sort(([, a], [, b]) => b - a) .map(([group, percentage]) => { const pair = generateColorPair(group); return { group, - percentage, color: isDarkMode ? pair.color : pair.colorDark, + widthPct: Math.min(percentage, 100), + label: `${percentage.toFixed(2)}%`, }; - }) - .sort((a, b) => b.percentage - a.percentage); + }); }, [execTimePct, isDarkMode]); - const spanCountRows = useMemo(() => { + const spanCountRows = useMemo(() => { if (!spanCounts) { return []; } const max = Math.max(...Object.values(spanCounts), 1); return Object.entries(spanCounts) + .sort(([, a], [, b]) => b - a) .map(([group, count]) => { const pair = generateColorPair(group); return { group, - count, - max, color: isDarkMode ? pair.color : pair.colorDark, + widthPct: (count / max) * 100, + label: String(count), }; - }) - .sort((a, b) => b.count - a.count); + }); }, [spanCounts, isDarkMode]); if (!isOpen) { @@ -132,65 +161,23 @@ function AnalyticsPanel({
-
- {execTimeRows.map((row) => ( - <> -
- - {row.group} - -
-
-
-
- - {row.percentage.toFixed(2)}% - -
- - ))} -
+ -
- {spanCountRows.map((row) => ( - <> -
- - {row.group} - -
-
-
-
- - {row.count} - -
- - ))} -
+
diff --git a/frontend/src/pages/TraceDetailsV3/SpanDetailsPanel/AnalyticsPanel/AnalyticsTabContent.tsx b/frontend/src/pages/TraceDetailsV3/SpanDetailsPanel/AnalyticsPanel/AnalyticsTabContent.tsx new file mode 100644 index 00000000000..c9f56254473 --- /dev/null +++ b/frontend/src/pages/TraceDetailsV3/SpanDetailsPanel/AnalyticsPanel/AnalyticsTabContent.tsx @@ -0,0 +1,84 @@ +import { Fragment } from 'react'; +import { Typography } from '@signozhq/ui/typography'; +import cx from 'classnames'; +import Spinner from 'components/Spinner'; + +import styles from './AnalyticsPanel.module.scss'; + +export interface AnalyticsRow { + group: string; + color: string; + widthPct: number; + label: string; +} + +interface AnalyticsTabContentProps { + isLoading: boolean; + isError: boolean; + fieldName: string; + rows: AnalyticsRow[]; + valueVariant: 'wide' | 'narrow'; +} + +// Loading / error / empty render in place of the rows so the tabs stay visible. +function AnalyticsTabContent({ + isLoading, + isError, + fieldName, + rows, + valueVariant, +}: AnalyticsTabContentProps): JSX.Element { + if (isLoading) { + return ( +
+ +
+ ); + } + if (isError) { + return ( +
+ Couldn't load analytics +
+ ); + } + if (rows.length === 0) { + return ( +
+ No data for {fieldName} +
+ ); + } + + return ( +
+ {rows.map((row) => ( + +
+ {row.group} +
+
+
+
+ + {row.label} + +
+ + ))} +
+ ); +} + +export default AnalyticsTabContent; diff --git a/frontend/src/pages/TraceDetailsV3/SpanDetailsPanel/AnalyticsPanel/__tests__/AnalyticsPanel.test.tsx b/frontend/src/pages/TraceDetailsV3/SpanDetailsPanel/AnalyticsPanel/__tests__/AnalyticsPanel.test.tsx new file mode 100644 index 00000000000..1187eca828d --- /dev/null +++ b/frontend/src/pages/TraceDetailsV3/SpanDetailsPanel/AnalyticsPanel/__tests__/AnalyticsPanel.test.tsx @@ -0,0 +1,144 @@ +import { screen } from '@testing-library/react'; +import useGetTraceAggregations from 'hooks/trace/useGetTraceAggregations'; +import { render } from 'tests/test-utils'; + +import { DEFAULT_COLOR_BY_FIELD } from '../../../constants'; +import { useTraceStore } from '../../../stores/traceStore'; +import AnalyticsPanel from '../AnalyticsPanel'; + +jest.mock('react-router-dom', () => ({ + ...jest.requireActual('react-router-dom'), + useParams: (): { id: string } => ({ id: 'trace-123' }), +})); + +jest.mock('hooks/trace/useGetTraceAggregations', () => ({ + __esModule: true, + default: jest.fn(), +})); + +// Isolate the panel's own logic from the floating-panel chrome. +jest.mock('periscope/components/FloatingPanel', () => ({ + __esModule: true, + FloatingPanel: ({ children }: { children: React.ReactNode }): JSX.Element => ( +
{children}
+ ), +})); +jest.mock('components/DetailsPanel', () => ({ + __esModule: true, + DetailsHeader: (): JSX.Element =>
, +})); +jest.mock('components/Spinner', () => ({ + __esModule: true, + default: (): JSX.Element =>
, +})); + +const mockHook = useGetTraceAggregations as jest.Mock; + +const noop = (): void => undefined; + +const renderPanel = (isOpen = true): ReturnType => + render(); + +const aggregationsResponse = { + status: 'success', + data: { + aggregations: [ + { + field: { name: 'service.name' }, + aggregation: 'execution_time_percentage', + value: { api: 80, db: 20 }, + }, + { + field: { name: 'service.name' }, + aggregation: 'span_count', + value: { api: 5, db: 2 }, + }, + ], + }, +}; + +describe('AnalyticsPanel', () => { + beforeEach(() => { + mockHook.mockReset(); + useTraceStore.setState({ colorByField: DEFAULT_COLOR_BY_FIELD }); + }); + + it('renders nothing when closed and does not enable the fetch', () => { + mockHook.mockReturnValue({ + data: undefined, + isLoading: false, + isError: false, + }); + const { container } = renderPanel(false); + expect(container).toBeEmptyDOMElement(); + expect(mockHook).toHaveBeenCalledWith( + expect.objectContaining({ enabled: false }), + ); + }); + + it('requests both aggregations for the current color-by field when open', () => { + mockHook.mockReturnValue({ + data: undefined, + isLoading: true, + isError: false, + }); + renderPanel(); + expect(mockHook).toHaveBeenCalledWith( + expect.objectContaining({ + traceId: 'trace-123', + enabled: true, + aggregations: [ + { + field: DEFAULT_COLOR_BY_FIELD, + aggregation: 'execution_time_percentage', + }, + { field: DEFAULT_COLOR_BY_FIELD, aggregation: 'span_count' }, + ], + }), + ); + }); + + it('shows the loading state with the tabs still visible', () => { + mockHook.mockReturnValue({ + data: undefined, + isLoading: true, + isError: false, + }); + renderPanel(); + expect(screen.getByTestId('spinner')).toBeInTheDocument(); + // tabs stay visible while loading + expect(screen.getByText('% exec time')).toBeInTheDocument(); + expect(screen.getByText('Spans')).toBeInTheDocument(); + }); + + it('shows an error state when the request fails', () => { + mockHook.mockReturnValue({ + data: undefined, + isLoading: false, + isError: true, + }); + renderPanel(); + expect(screen.getByText(/couldn't load analytics/i)).toBeInTheDocument(); + }); + + it('renders rows for the current field on success', () => { + mockHook.mockReturnValue({ + data: aggregationsResponse, + isLoading: false, + isError: false, + }); + renderPanel(); + expect(screen.getByText('api')).toBeInTheDocument(); + expect(screen.getByText('80.00%')).toBeInTheDocument(); + }); + + it('shows an empty state when the field has no data', () => { + mockHook.mockReturnValue({ + data: { status: 'success', data: { aggregations: [] } }, + isLoading: false, + isError: false, + }); + renderPanel(); + expect(screen.getByText(/no data for service.name/i)).toBeInTheDocument(); + }); +}); diff --git a/frontend/src/pages/TraceDetailsV3/TraceDetailsHeader/TraceOptionsMenu.module.scss b/frontend/src/pages/TraceDetailsV3/TraceDetailsHeader/TraceOptionsMenu.module.scss index 6705346b2c3..e17112b4175 100644 --- a/frontend/src/pages/TraceDetailsV3/TraceDetailsHeader/TraceOptionsMenu.module.scss +++ b/frontend/src/pages/TraceDetailsV3/TraceDetailsHeader/TraceOptionsMenu.module.scss @@ -1,3 +1,6 @@ +// Applied to both the menu content and the submenu content (each renders in +// its own portal with a default z-index of 50) so both stack above +// FloatingPanel (z-index 999). .traceOptionsDropdown { z-index: 1100; } diff --git a/frontend/src/pages/TraceDetailsV3/TraceDetailsHeader/TraceOptionsMenu.tsx b/frontend/src/pages/TraceDetailsV3/TraceDetailsHeader/TraceOptionsMenu.tsx index 6ba24b468e5..880bafa22ef 100644 --- a/frontend/src/pages/TraceDetailsV3/TraceDetailsHeader/TraceOptionsMenu.tsx +++ b/frontend/src/pages/TraceDetailsV3/TraceDetailsHeader/TraceOptionsMenu.tsx @@ -1,7 +1,16 @@ -import { useMemo } from 'react'; -import type { MenuItem } from '@signozhq/ui/dropdown-menu'; import { Button } from '@signozhq/ui/button'; -import { DropdownMenuSimple as Dropdown } from '@signozhq/ui/dropdown-menu'; +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuLabel, + DropdownMenuRadioGroup, + DropdownMenuRadioItem, + DropdownMenuSub, + DropdownMenuSubContent, + DropdownMenuSubTrigger, + DropdownMenuTrigger, +} from '@signozhq/ui/dropdown-menu'; import { Settings2 } from '@signozhq/icons'; import { useTraceStore } from '../stores/traceStore'; @@ -14,6 +23,9 @@ interface TraceOptionsMenuProps { onOpenPreviewFields: () => void; } +// Composed from dropdown-menu primitives (instead of DropdownMenuSimple) +// because the simple preset offers no way to style the submenu content, +// which renders in its own portal and needs a z-index above FloatingPanel. function TraceOptionsMenu({ showTraceDetails, onToggleTraceDetails, @@ -25,78 +37,52 @@ function TraceOptionsMenu({ (s) => s.availableColorByOptions, ); - const menuItems: MenuItem[] = useMemo(() => { - const items: MenuItem[] = [ - { - key: 'toggle-trace-details', - label: showTraceDetails ? 'Hide trace details' : 'Show trace details', - onClick: onToggleTraceDetails, - }, - { - key: 'preview-fields', - label: 'Preview fields', - onClick: onOpenPreviewFields, - }, - ]; - - // Only show the "Colour by" submenu if there's an actual choice to make. - if (availableColorByOptions.length > 1) { - items.push({ - key: 'colour-by', - label: 'Colour by', - children: [ - { - type: 'group', - label: 'COLOUR BY', - children: [ - { - type: 'radio-group', - value: colorByField.name, - onChange: (name: string): void => { - const next = availableColorByOptions.find( - (o) => o.field.name === name, - ); - if (next) { - setColorByField(next.field); - } - }, - children: availableColorByOptions.map((opt) => ({ - type: 'radio', - key: opt.field.name, - label: opt.label, - value: opt.field.name, - })), - }, - ], - }, - ], - }); + const handleColorByChange = (name: string): void => { + const next = availableColorByOptions.find((o) => o.field.name === name); + if (next) { + setColorByField(next.field); } - - return items; - }, [ - showTraceDetails, - onToggleTraceDetails, - onOpenPreviewFields, - colorByField.name, - setColorByField, - availableColorByOptions, - ]); + }; return ( - -