From b73870ac75ce2f214d7444747753ebcd70210d44 Mon Sep 17 00:00:00 2001 From: lizhanpeng Date: Thu, 28 Aug 2025 20:05:33 +0800 Subject: [PATCH 01/13] feat(backend): refactor annotation --- .../apis/observability_open_apiservice.go | 12 ++++ .../router/coze/loop/apis/coze.loop.apis.go | 5 ++ .../api/router/coze/loop/apis/middleware.go | 5 ++ .../domain/annotation/annotation.go | 2 + .../application/convertor/trace/annotation.go | 1 + .../trace/entity/loop_span/annotation.go | 1 + .../domain/trace/service/trace_service.go | 64 ++++++++++++------- .../coze.loop.observability.openapi.thrift | 4 +- .../observability/domain/annotation.thrift | 1 + 9 files changed, 70 insertions(+), 25 deletions(-) diff --git a/backend/api/handler/coze/loop/apis/observability_open_apiservice.go b/backend/api/handler/coze/loop/apis/observability_open_apiservice.go index 2618f6447..ac453f74b 100644 --- a/backend/api/handler/coze/loop/apis/observability_open_apiservice.go +++ b/backend/api/handler/coze/loop/apis/observability_open_apiservice.go @@ -60,3 +60,15 @@ func OtelIngestTraces(ctx context.Context, c *app.RequestContext) { func ListTracesOApi(ctx context.Context, c *app.RequestContext) { invokeAndRender(ctx, c, observabilityOpenAPIClient.ListTracesOApi) } + +// CreateAnnotation . +// @router /v1/loop/annotations/create [POST] +func CreateAnnotation(ctx context.Context, c *app.RequestContext) { + invokeAndRender(ctx, c, observabilityOpenAPIClient.CreateAnnotation) +} + +// DeleteAnnotation . +// @router /v1/loop/annotations/delete [DELETE] +func DeleteAnnotation(ctx context.Context, c *app.RequestContext) { + invokeAndRender(ctx, c, observabilityOpenAPIClient.DeleteAnnotation) +} diff --git a/backend/api/router/coze/loop/apis/coze.loop.apis.go b/backend/api/router/coze/loop/apis/coze.loop.apis.go index 8d33a40fc..8c8ae270f 100644 --- a/backend/api/router/coze/loop/apis/coze.loop.apis.go +++ b/backend/api/router/coze/loop/apis/coze.loop.apis.go @@ -337,6 +337,11 @@ func Register(r *server.Hertz, handler *apis.APIHandler) { _v16 := root.Group("/v1", _v16Mw(handler)...) { _loop := _v16.Group("/loop", _loopMw(handler)...) + { + _annotations0 := _loop.Group("/annotations", _annotations0Mw(handler)...) + _annotations0.POST("/create", append(_createannotationMw(handler), apis.CreateAnnotation)...) + _annotations0.DELETE("/delete", append(_deleteannotationMw(handler), apis.DeleteAnnotation)...) + } { _files := _loop.Group("/files", _filesMw(handler)...) _files.POST("/upload", append(_uploadloopfileMw(handler), apis.UploadLoopFile)...) diff --git a/backend/api/router/coze/loop/apis/middleware.go b/backend/api/router/coze/loop/apis/middleware.go index 05100a421..a4ec9c9d3 100644 --- a/backend/api/router/coze/loop/apis/middleware.go +++ b/backend/api/router/coze/loop/apis/middleware.go @@ -1270,3 +1270,8 @@ func _listtracesoapiMw(handler *apis.APIHandler) []app.HandlerFunc { // your code... return nil } + +func _annotations0Mw(handler *apis.APIHandler) []app.HandlerFunc { + // your code... + return nil +} diff --git a/backend/kitex_gen/coze/loop/observability/domain/annotation/annotation.go b/backend/kitex_gen/coze/loop/observability/domain/annotation/annotation.go index 90aa0003b..ad90f72ee 100644 --- a/backend/kitex_gen/coze/loop/observability/domain/annotation/annotation.go +++ b/backend/kitex_gen/coze/loop/observability/domain/annotation/annotation.go @@ -18,6 +18,8 @@ const ( AnnotationTypeCozeFeedback = "coze_feedback" + AnnotationTypeOpenAPIFeedback = "openapi_feedback" + ValueTypeString = "string" ValueTypeLong = "long" diff --git a/backend/modules/observability/application/convertor/trace/annotation.go b/backend/modules/observability/application/convertor/trace/annotation.go index c2f43bd30..152692fdd 100644 --- a/backend/modules/observability/application/convertor/trace/annotation.go +++ b/backend/modules/observability/application/convertor/trace/annotation.go @@ -179,6 +179,7 @@ func AnnotationListDO2DTO( case loop_span.AnnotationTypeManualFeedback: fallthrough case loop_span.AnnotationTypeCozeFeedback: + case loop_span.AnnotationTypeOpenAPIFeedback: ret = append(ret, AnnotationDO2DTO(a, userMap, evalMap, tagMap)) default: continue diff --git a/backend/modules/observability/domain/trace/entity/loop_span/annotation.go b/backend/modules/observability/domain/trace/entity/loop_span/annotation.go index ac9645bf1..883eabd24 100644 --- a/backend/modules/observability/domain/trace/entity/loop_span/annotation.go +++ b/backend/modules/observability/domain/trace/entity/loop_span/annotation.go @@ -37,6 +37,7 @@ const ( AnnotationTypeManualFeedback AnnotationType = "manual_feedback" AnnotationTypeCozeFeedback AnnotationType = "coze_feedback" AnnotationTypeManualDataset AnnotationType = "manual_dataset" + AnnotationTypeOpenAPIFeedback AnnotationType = "openapi_feedback" ) type AnnotationValue struct { diff --git a/backend/modules/observability/domain/trace/service/trace_service.go b/backend/modules/observability/domain/trace/service/trace_service.go index b5e235ca7..fa4923e13 100644 --- a/backend/modules/observability/domain/trace/service/trace_service.go +++ b/backend/modules/observability/domain/trace/service/trace_service.go @@ -866,33 +866,51 @@ func (r *TraceServiceImpl) Send(ctx context.Context, event *entity.AnnotationEve } func (r *TraceServiceImpl) getSpan(ctx context.Context, tenants []string, spanId, traceId, workspaceId string, startAt, endAt int64) (*loop_span.Span, error) { - if spanId == "" || traceId == "" || workspaceId == "" { + if traceId == "" || workspaceId == "" { return nil, errorx.NewByCode(obErrorx.CommercialCommonInvalidParamCodeCode) } - res, err := r.traceRepo.ListSpans(ctx, &repo.ListSpansParam{ - Tenants: tenants, - Filters: &loop_span.FilterFields{ - FilterFields: []*loop_span.FilterField{ - { - FieldName: loop_span.SpanFieldSpanId, - FieldType: loop_span.FieldTypeString, - Values: []string{spanId}, - QueryType: ptr.Of(loop_span.QueryTypeEnumEq), - }, - { - FieldName: loop_span.SpanFieldSpaceId, - FieldType: loop_span.FieldTypeString, - Values: []string{workspaceId}, - QueryType: ptr.Of(loop_span.QueryTypeEnumEq), - }, - { - FieldName: loop_span.SpanFieldTraceId, - FieldType: loop_span.FieldTypeString, - Values: []string{traceId}, - QueryType: ptr.Of(loop_span.QueryTypeEnumEq), - }, + filter := &loop_span.FilterFields{ + FilterFields: []*loop_span.FilterField{ + { + FieldName: loop_span.SpanFieldSpanId, + FieldType: loop_span.FieldTypeString, + Values: []string{spanId}, + QueryType: ptr.Of(loop_span.QueryTypeEnumEq), + }, + { + FieldName: loop_span.SpanFieldSpaceId, + FieldType: loop_span.FieldTypeString, + Values: []string{workspaceId}, + QueryType: ptr.Of(loop_span.QueryTypeEnumEq), + }, + { + FieldName: loop_span.SpanFieldTraceId, + FieldType: loop_span.FieldTypeString, + Values: []string{traceId}, + QueryType: ptr.Of(loop_span.QueryTypeEnumEq), }, }, + } + if spanId != "" { + filter.FilterFields = append(filter.FilterFields, + &loop_span.FilterField{ + FieldName: loop_span.SpanFieldSpanId, + FieldType: loop_span.FieldTypeString, + Values: []string{spanId}, + QueryType: ptr.Of(loop_span.QueryTypeEnumEq), + }) + } else { + filter.FilterFields = append(filter.FilterFields, + &loop_span.FilterField{ + FieldName: loop_span.SpanFieldParentID, + FieldType: loop_span.FieldTypeString, + Values: []string{"0", ""}, + QueryType: ptr.Of(loop_span.QueryTypeEnumIn), + }) + } + res, err := r.traceRepo.ListSpans(ctx, &repo.ListSpansParam{ + Tenants: tenants, + Filters: filter, StartAt: startAt, EndAt: endAt, NotQueryAnnotation: true, diff --git a/idl/thrift/coze/loop/observability/coze.loop.observability.openapi.thrift b/idl/thrift/coze/loop/observability/coze.loop.observability.openapi.thrift index 5126f3560..3a26b15fa 100644 --- a/idl/thrift/coze/loop/observability/coze.loop.observability.openapi.thrift +++ b/idl/thrift/coze/loop/observability/coze.loop.observability.openapi.thrift @@ -147,6 +147,6 @@ service OpenAPIService { SearchTraceOApiResponse SearchTraceOApi(1: SearchTraceOApiRequest req) (api.post = '/v1/loop/traces/search') ListSpansOApiResponse ListSpansOApi(1: ListSpansOApiRequest req) (api.post = '/v1/loop/spans/search', api.tag="openapi") ListTracesOApiResponse ListTracesOApi(1: ListTracesOApiRequest req) (api.post = '/v1/loop/traces/list') - CreateAnnotationResponse CreateAnnotation(1: CreateAnnotationRequest req) - DeleteAnnotationResponse DeleteAnnotation(1: DeleteAnnotationRequest req) + CreateAnnotationResponse CreateAnnotation(1: CreateAnnotationRequest req) (api.post = '/v1/loop/annotations/create') + DeleteAnnotationResponse DeleteAnnotation(1: DeleteAnnotationRequest req) (api.delete = '/v1/loop/annotations/delete') } \ No newline at end of file diff --git a/idl/thrift/coze/loop/observability/domain/annotation.thrift b/idl/thrift/coze/loop/observability/domain/annotation.thrift index 4e29023bb..db3a83551 100644 --- a/idl/thrift/coze/loop/observability/domain/annotation.thrift +++ b/idl/thrift/coze/loop/observability/domain/annotation.thrift @@ -7,6 +7,7 @@ const AnnotationType AnnotationType_AutoEvaluate = "auto_evaluate" const AnnotationType AnnotationType_EvaluationSet = "manual_evaluation_set" const AnnotationType AnnotationType_ManualFeedback = "manual_feedback" const AnnotationType AnnotationType_CozeFeedback = "coze_feedback" +const AnnotationType AnnotationType_OpenAPIFeedback = "openapi_feedback" typedef string ValueType (ts.enum="true") const ValueType ValueType_String = "string" From fd9a79be4dff7cd0e7e3102df7314bb80d9032bd Mon Sep 17 00:00:00 2001 From: lizhanpeng Date: Mon, 1 Sep 2025 11:40:10 +0800 Subject: [PATCH 02/13] feat(backend): add ut --- .../application/convertor/trace/annotation.go | 1 + .../observability/application/trace_test.go | 35 +- .../trace/service/trace_service_test.go | 718 +++++++----------- 3 files changed, 282 insertions(+), 472 deletions(-) diff --git a/backend/modules/observability/application/convertor/trace/annotation.go b/backend/modules/observability/application/convertor/trace/annotation.go index 152692fdd..c4ef608d3 100644 --- a/backend/modules/observability/application/convertor/trace/annotation.go +++ b/backend/modules/observability/application/convertor/trace/annotation.go @@ -179,6 +179,7 @@ func AnnotationListDO2DTO( case loop_span.AnnotationTypeManualFeedback: fallthrough case loop_span.AnnotationTypeCozeFeedback: + fallthrough case loop_span.AnnotationTypeOpenAPIFeedback: ret = append(ret, AnnotationDO2DTO(a, userMap, evalMap, tagMap)) default: diff --git a/backend/modules/observability/application/trace_test.go b/backend/modules/observability/application/trace_test.go index d41bc98dd..bfe17cd79 100644 --- a/backend/modules/observability/application/trace_test.go +++ b/backend/modules/observability/application/trace_test.go @@ -9,8 +9,6 @@ import ( "testing" "time" - "github.com/coze-dev/coze-loop/backend/modules/observability/domain/component/tenant" - "github.com/coze-dev/coze-loop/backend/infra/external/benefit" benefitmock "github.com/coze-dev/coze-loop/backend/infra/external/benefit/mocks" "github.com/coze-dev/coze-loop/backend/infra/middleware/session" @@ -24,6 +22,7 @@ import ( confmock "github.com/coze-dev/coze-loop/backend/modules/observability/domain/component/config/mocks" "github.com/coze-dev/coze-loop/backend/modules/observability/domain/component/rpc" rpcmock "github.com/coze-dev/coze-loop/backend/modules/observability/domain/component/rpc/mocks" + "github.com/coze-dev/coze-loop/backend/modules/observability/domain/component/tenant" tenantmock "github.com/coze-dev/coze-loop/backend/modules/observability/domain/component/tenant/mocks" "github.com/coze-dev/coze-loop/backend/modules/observability/domain/trace/entity" "github.com/coze-dev/coze-loop/backend/modules/observability/domain/trace/entity/loop_span" @@ -1666,26 +1665,26 @@ func TestTraceApplication_PreviewExportTracesToDataset(t *testing.T) { traceConfig: mockConfig, } }, args: args{ - ctx: context.Background(), - req: &trace.PreviewExportTracesToDatasetRequest{ - WorkspaceID: 123, - StartTime: time.Now().Add(-time.Hour).UnixMilli(), - EndTime: time.Now().UnixMilli(), - SpanIds: []*trace.SpanID{ - {TraceID: "trace1", SpanID: "span1"}, - }, - FieldMappings: []*dataset0.FieldMapping{ - { - FieldSchema: &dataset0.FieldSchema{ - Key: ptr.Of("input"), - Name: ptr.Of("Input"), - }, - TraceFieldKey: "input", - TraceFieldJsonpath: "$.input", + ctx: context.Background(), + req: &trace.PreviewExportTracesToDatasetRequest{ + WorkspaceID: 123, + StartTime: time.Now().Add(-time.Hour).UnixMilli(), + EndTime: time.Now().UnixMilli(), + SpanIds: []*trace.SpanID{ + {TraceID: "trace1", SpanID: "span1"}, + }, + FieldMappings: []*dataset0.FieldMapping{ + { + FieldSchema: &dataset0.FieldSchema{ + Key: ptr.Of("input"), + Name: ptr.Of("Input"), }, + TraceFieldKey: "input", + TraceFieldJsonpath: "$.input", }, }, }, + }, want: nil, wantErr: true, }, diff --git a/backend/modules/observability/domain/trace/service/trace_service_test.go b/backend/modules/observability/domain/trace/service/trace_service_test.go index c38827f30..a8cfc4171 100644 --- a/backend/modules/observability/domain/trace/service/trace_service_test.go +++ b/backend/modules/observability/domain/trace/service/trace_service_test.go @@ -1642,6 +1642,140 @@ func TestTraceServiceImpl_CreateAnnotation(t *testing.T) { }, wantErr: true, }, + { + name: "create annotation on root span when span_id is empty", + fieldsGetter: func(ctrl *gomock.Controller) fields { + repoMock := repomocks.NewMockITraceRepo(ctrl) + confMock := confmocks.NewMockITraceConfig(ctrl) + annoProducerMock := mqmocks.NewMockIAnnotationProducer(ctrl) + confMock.EXPECT().GetAnnotationSourceCfg(gomock.Any()).Return(&config.AnnotationSourceConfig{ + SourceCfg: map[string]config.AnnotationConfig{ + "test-caller": { + Tenants: []string{"spans"}, + AnnotationType: string(loop_span.AnnotationTypeManualFeedback), + }, + }, + }, nil) + + // Mock ListSpans call with ParentID filter for root span + repoMock.EXPECT().ListSpans(gomock.Any(), gomock.Any()).DoAndReturn( + func(ctx context.Context, param *repo.ListSpansParam) (*repo.ListSpansResult, error) { + // Verify that the filter contains ParentID filter when span_id is empty + hasParentIDFilter := false + for _, filter := range param.Filters.FilterFields { + if filter.FieldName == loop_span.SpanFieldParentID { + hasParentIDFilter = true + assert.Equal(t, []string{"0", ""}, filter.Values) + assert.Equal(t, loop_span.QueryTypeEnumIn, *filter.QueryType) + } + } + assert.True(t, hasParentIDFilter, "Should have ParentID filter when span_id is empty") + + // Return a root span (ParentID = "0") + return &repo.ListSpansResult{ + Spans: loop_span.SpanList{ + { + TraceID: "test-trace-id", + SpanID: "root-span-id", + ParentID: "0", // This is a root span + WorkspaceID: "1", + SystemTagsString: map[string]string{ + loop_span.SpanFieldTenant: "spans", + }, + }, + }, + }, nil + }, + ) + repoMock.EXPECT().GetAnnotation(gomock.Any(), gomock.Any()).Return(nil, nil) + repoMock.EXPECT().InsertAnnotations(gomock.Any(), gomock.Any()).Return(nil) + filterFactoryMock := filtermocks.NewMockPlatformFilterFactory(ctrl) + buildHelper := NewTraceFilterProcessorBuilder(filterFactoryMock, nil, nil, nil, nil, nil, nil) + tenantProviderMock := tenantmocks.NewMockITenantProvider(ctrl) + tenantProviderMock.EXPECT().GetTenantsByPlatformType(gomock.Any(), gomock.Any()).Return([]string{"spans"}, nil).AnyTimes() + return fields{ + traceRepo: repoMock, + traceConfig: confMock, + annotationProducer: annoProducerMock, + buildHelper: buildHelper, + tenantProvider: tenantProviderMock, + } + }, + args: args{ + ctx: context.Background(), + req: &CreateAnnotationReq{ + WorkspaceID: 1, + SpanID: "", // Empty span_id to trigger root span search + TraceID: "test-trace-id", + AnnotationKey: "test-key", + AnnotationVal: loop_span.AnnotationValue{StringValue: "test-value"}, + Caller: "test-caller", + QueryDays: 1, + }, + }, + wantErr: false, + }, + { + name: "create annotation when span_id is empty but no root span found", + fieldsGetter: func(ctrl *gomock.Controller) fields { + repoMock := repomocks.NewMockITraceRepo(ctrl) + confMock := confmocks.NewMockITraceConfig(ctrl) + annoProducerMock := mqmocks.NewMockIAnnotationProducer(ctrl) + confMock.EXPECT().GetAnnotationSourceCfg(gomock.Any()).Return(&config.AnnotationSourceConfig{ + SourceCfg: map[string]config.AnnotationConfig{ + "test-caller": { + Tenants: []string{"spans"}, + AnnotationType: string(loop_span.AnnotationTypeManualFeedback), + }, + }, + }, nil) + + // Mock ListSpans call with ParentID filter but return no spans + repoMock.EXPECT().ListSpans(gomock.Any(), gomock.Any()).DoAndReturn( + func(ctx context.Context, param *repo.ListSpansParam) (*repo.ListSpansResult, error) { + // Verify that the filter contains ParentID filter when span_id is empty + hasParentIDFilter := false + for _, filter := range param.Filters.FilterFields { + if filter.FieldName == loop_span.SpanFieldParentID { + hasParentIDFilter = true + assert.Equal(t, []string{"0", ""}, filter.Values) + assert.Equal(t, loop_span.QueryTypeEnumIn, *filter.QueryType) + } + } + assert.True(t, hasParentIDFilter, "Should have ParentID filter when span_id is empty") + + // Return empty result (no root span found) + return &repo.ListSpansResult{Spans: loop_span.SpanList{}}, nil + }, + ) + // Expect annotation to be sent via producer when no span found + annoProducerMock.EXPECT().SendAnnotation(gomock.Any(), gomock.Any()).Return(nil) + filterFactoryMock := filtermocks.NewMockPlatformFilterFactory(ctrl) + buildHelper := NewTraceFilterProcessorBuilder(filterFactoryMock, nil, nil, nil, nil, nil, nil) + tenantProviderMock := tenantmocks.NewMockITenantProvider(ctrl) + tenantProviderMock.EXPECT().GetTenantsByPlatformType(gomock.Any(), gomock.Any()).Return([]string{"spans"}, nil).AnyTimes() + return fields{ + traceRepo: repoMock, + traceConfig: confMock, + annotationProducer: annoProducerMock, + buildHelper: buildHelper, + tenantProvider: tenantProviderMock, + } + }, + args: args{ + ctx: context.Background(), + req: &CreateAnnotationReq{ + WorkspaceID: 1, + SpanID: "", // Empty span_id to trigger root span search + TraceID: "test-trace-id", + AnnotationKey: "test-key", + AnnotationVal: loop_span.AnnotationValue{StringValue: "test-value"}, + Caller: "test-caller", + QueryDays: 1, + }, + }, + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -1733,6 +1867,135 @@ func TestTraceServiceImpl_DeleteAnnotation(t *testing.T) { }, wantErr: false, }, + { + name: "delete annotation on root span when span_id is empty", + fieldsGetter: func(ctrl *gomock.Controller) fields { + repoMock := repomocks.NewMockITraceRepo(ctrl) + confMock := confmocks.NewMockITraceConfig(ctrl) + confMock.EXPECT().GetAnnotationSourceCfg(gomock.Any()).Return(&config.AnnotationSourceConfig{ + SourceCfg: map[string]config.AnnotationConfig{ + "test-caller": { + Tenants: []string{"spans"}, + AnnotationType: string(loop_span.AnnotationTypeManualFeedback), + }, + }, + }, nil) + + // Mock ListSpans call with ParentID filter for root span + repoMock.EXPECT().ListSpans(gomock.Any(), gomock.Any()).DoAndReturn( + func(ctx context.Context, param *repo.ListSpansParam) (*repo.ListSpansResult, error) { + // Verify that the filter contains ParentID filter when span_id is empty + hasParentIDFilter := false + for _, filter := range param.Filters.FilterFields { + if filter.FieldName == loop_span.SpanFieldParentID { + hasParentIDFilter = true + assert.Equal(t, []string{"0", ""}, filter.Values) + assert.Equal(t, loop_span.QueryTypeEnumIn, *filter.QueryType) + } + } + assert.True(t, hasParentIDFilter, "Should have ParentID filter when span_id is empty") + + // Return a root span (ParentID = "0") + return &repo.ListSpansResult{ + Spans: loop_span.SpanList{ + { + TraceID: "test-trace-id", + SpanID: "root-span-id", + ParentID: "0", // This is a root span + WorkspaceID: "1", + SystemTagsString: map[string]string{ + loop_span.SpanFieldTenant: "spans", + }, + }, + }, + }, nil + }, + ) + repoMock.EXPECT().InsertAnnotations(gomock.Any(), gomock.Any()).Return(nil) + filterFactoryMock := filtermocks.NewMockPlatformFilterFactory(ctrl) + buildHelper := NewTraceFilterProcessorBuilder(filterFactoryMock, nil, nil, nil, nil, nil, nil) + tenantProviderMock := tenantmocks.NewMockITenantProvider(ctrl) + tenantProviderMock.EXPECT().GetTenantsByPlatformType(gomock.Any(), gomock.Any()).Return([]string{"spans"}, nil).AnyTimes() + return fields{ + traceRepo: repoMock, + traceConfig: confMock, + buildHelper: buildHelper, + tenantProvider: tenantProviderMock, + } + }, + args: args{ + ctx: context.Background(), + req: &DeleteAnnotationReq{ + WorkspaceID: 1, + SpanID: "", // Empty span_id to trigger root span search + TraceID: "test-trace-id", + AnnotationKey: "test-key", + Caller: "test-caller", + QueryDays: 1, + }, + }, + wantErr: false, + }, + { + name: "delete annotation when span_id is empty but no root span found", + fieldsGetter: func(ctrl *gomock.Controller) fields { + repoMock := repomocks.NewMockITraceRepo(ctrl) + confMock := confmocks.NewMockITraceConfig(ctrl) + annoProducerMock := mqmocks.NewMockIAnnotationProducer(ctrl) + confMock.EXPECT().GetAnnotationSourceCfg(gomock.Any()).Return(&config.AnnotationSourceConfig{ + SourceCfg: map[string]config.AnnotationConfig{ + "test-caller": { + Tenants: []string{"spans"}, + AnnotationType: string(loop_span.AnnotationCorrectionTypeManual), + }, + }, + }, nil) + + // Mock ListSpans call with ParentID filter but return no spans + repoMock.EXPECT().ListSpans(gomock.Any(), gomock.Any()).DoAndReturn( + func(ctx context.Context, param *repo.ListSpansParam) (*repo.ListSpansResult, error) { + // Verify that the filter contains ParentID filter when span_id is empty + hasParentIDFilter := false + for _, filter := range param.Filters.FilterFields { + if filter.FieldName == loop_span.SpanFieldParentID { + hasParentIDFilter = true + assert.Equal(t, []string{"0", ""}, filter.Values) + assert.Equal(t, loop_span.QueryTypeEnumIn, *filter.QueryType) + } + } + assert.True(t, hasParentIDFilter, "Should have ParentID filter when span_id is empty") + + // Return empty result (no root span found) + return &repo.ListSpansResult{Spans: loop_span.SpanList{}}, nil + }, + ) + // Expect annotation to be sent via producer when no span found + annoProducerMock.EXPECT().SendAnnotation(gomock.Any(), gomock.Any()).Return(nil) + filterFactoryMock := filtermocks.NewMockPlatformFilterFactory(ctrl) + buildHelper := NewTraceFilterProcessorBuilder(filterFactoryMock, nil, nil, nil, nil, nil, nil) + tenantProviderMock := tenantmocks.NewMockITenantProvider(ctrl) + tenantProviderMock.EXPECT().GetTenantsByPlatformType(gomock.Any(), gomock.Any()).Return([]string{"spans"}, nil).AnyTimes() + return fields{ + traceRepo: repoMock, + traceConfig: confMock, + annotationProducer: annoProducerMock, + buildHelper: buildHelper, + tenantProvider: tenantProviderMock, + } + }, + args: args{ + ctx: context.Background(), + req: &DeleteAnnotationReq{ + WorkspaceID: 1, + SpanID: "", // Empty span_id to trigger root span search + TraceID: "test-trace-id", + AnnotationKey: "test-key", + Caller: "test-caller", + QueryDays: 1, + }, + }, + wantErr: false, + }, { name: "get caller config failed", fieldsGetter: func(ctrl *gomock.Controller) fields { @@ -2532,303 +2795,6 @@ func TestTraceServiceImpl_ListSpansOApi(t *testing.T) { want *ListSpansOApiResp wantErr bool }{ - { - name: "list spans oapi successfully", - fieldsGetter: func(ctrl *gomock.Controller) fields { - repoMock := repomocks.NewMockITraceRepo(ctrl) - repoMock.EXPECT().ListSpans(gomock.Any(), gomock.Any()).Return(&repo.ListSpansResult{ - Spans: loop_span.SpanList{ - { - TraceID: "trace-123", - SpanID: "span-456", - }, - }, - PageToken: "next-token", - HasMore: true, - }, nil) - - filterFactoryMock := filtermocks.NewMockPlatformFilterFactory(ctrl) - filterMock := filtermocks.NewMockFilter(ctrl) - filterMock.EXPECT().BuildBasicSpanFilter(gomock.Any(), gomock.Any()).Return([]*loop_span.FilterField{ - { - FieldName: loop_span.SpanFieldSpaceId, - FieldType: loop_span.FieldTypeString, - Values: []string{"123"}, - QueryType: ptr.Of(loop_span.QueryTypeEnumIn), - }, - }, false, nil) - filterMock.EXPECT().BuildALLSpanFilter(gomock.Any(), gomock.Any()).Return(nil, nil) - filterFactoryMock.EXPECT().GetFilter(gomock.Any(), gomock.Any()).Return(filterMock, nil) - - buildHelper := NewTraceFilterProcessorBuilder(filterFactoryMock, nil, nil, nil, nil, nil, nil) - - return fields{ - traceRepo: repoMock, - buildHelper: buildHelper, - } - }, - args: args{ - ctx: context.Background(), - req: &ListSpansOApiReq{ - WorkspaceID: 123, - Tenants: []string{"tenant1"}, - StartTime: 1640995200000, - EndTime: 1640995800000, - Limit: 100, - PlatformType: loop_span.PlatformCozeLoop, - SpanListType: loop_span.SpanListTypeAllSpan, - }, - }, - want: &ListSpansOApiResp{ - Spans: loop_span.SpanList{ - { - TraceID: "trace-123", - SpanID: "span-456", - }, - }, - NextPageToken: "next-token", - HasMore: true, - }, - wantErr: false, - }, - { - name: "list spans oapi with empty builtin filter", - fieldsGetter: func(ctrl *gomock.Controller) fields { - filterFactoryMock := filtermocks.NewMockPlatformFilterFactory(ctrl) - filterMock := filtermocks.NewMockFilter(ctrl) - filterMock.EXPECT().BuildBasicSpanFilter(gomock.Any(), gomock.Any()).Return([]*loop_span.FilterField{}, false, nil) - filterFactoryMock.EXPECT().GetFilter(gomock.Any(), gomock.Any()).Return(filterMock, nil) - - buildHelper := NewTraceFilterProcessorBuilder(filterFactoryMock, nil, nil, nil, nil, nil, nil) - - return fields{ - buildHelper: buildHelper, - } - }, - args: args{ - ctx: context.Background(), - req: &ListSpansOApiReq{ - WorkspaceID: 123, - Tenants: []string{"tenant1"}, - StartTime: 1640995200000, - EndTime: 1640995800000, - Limit: 100, - PlatformType: loop_span.PlatformCozeLoop, - SpanListType: loop_span.SpanListTypeAllSpan, - }, - }, - want: &ListSpansOApiResp{ - Spans: loop_span.SpanList{}, - }, - wantErr: false, - }, - { - name: "list spans oapi with multiple processors", - fieldsGetter: func(ctrl *gomock.Controller) fields { - repoMock := repomocks.NewMockITraceRepo(ctrl) - repoMock.EXPECT().ListSpans(gomock.Any(), gomock.Any()).Return(&repo.ListSpansResult{ - Spans: loop_span.SpanList{ - { - TraceID: "trace-123", - SpanID: "span-456", - WorkspaceID: "123", - }, - }, - PageToken: "", - HasMore: false, - }, nil) - - filterFactoryMock := filtermocks.NewMockPlatformFilterFactory(ctrl) - filterMock := filtermocks.NewMockFilter(ctrl) - filterMock.EXPECT().BuildBasicSpanFilter(gomock.Any(), gomock.Any()).Return([]*loop_span.FilterField{ - { - FieldName: loop_span.SpanFieldSpaceId, - FieldType: loop_span.FieldTypeString, - Values: []string{"123"}, - QueryType: ptr.Of(loop_span.QueryTypeEnumIn), - }, - }, false, nil) - filterMock.EXPECT().BuildALLSpanFilter(gomock.Any(), gomock.Any()).Return(nil, nil) - filterFactoryMock.EXPECT().GetFilter(gomock.Any(), gomock.Any()).Return(filterMock, nil) - - buildHelper := NewTraceFilterProcessorBuilder(filterFactoryMock, nil, nil, nil, nil, nil, - []span_processor.Factory{span_processor.NewCheckProcessorFactory()}) - - return fields{ - traceRepo: repoMock, - buildHelper: buildHelper, - } - }, - args: args{ - ctx: context.Background(), - req: &ListSpansOApiReq{ - WorkspaceID: 123, - Tenants: []string{"tenant1"}, - StartTime: 1640995200000, - EndTime: 1640995800000, - Limit: 100, - PlatformType: loop_span.PlatformCozeLoop, - SpanListType: loop_span.SpanListTypeAllSpan, - }, - }, - want: &ListSpansOApiResp{ - Spans: loop_span.SpanList{ - { - TraceID: "trace-123", - SpanID: "span-456", - WorkspaceID: "123", - }, - }, - NextPageToken: "", - HasMore: false, - }, - wantErr: false, - }, - { - name: "list spans oapi failed due to platform filter error", - fieldsGetter: func(ctrl *gomock.Controller) fields { - filterFactoryMock := filtermocks.NewMockPlatformFilterFactory(ctrl) - filterFactoryMock.EXPECT().GetFilter(gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("platform filter error")) - - buildHelper := NewTraceFilterProcessorBuilder(filterFactoryMock, nil, nil, nil, nil, nil, nil) - - return fields{ - buildHelper: buildHelper, - } - }, - args: args{ - ctx: context.Background(), - req: &ListSpansOApiReq{ - WorkspaceID: 123, - Tenants: []string{"tenant1"}, - StartTime: 1640995200000, - EndTime: 1640995800000, - Limit: 100, - PlatformType: loop_span.PlatformCozeLoop, - SpanListType: loop_span.SpanListTypeAllSpan, - }, - }, - wantErr: true, - }, - { - name: "list spans oapi failed due to builtin filter error", - fieldsGetter: func(ctrl *gomock.Controller) fields { - filterFactoryMock := filtermocks.NewMockPlatformFilterFactory(ctrl) - filterMock := filtermocks.NewMockFilter(ctrl) - filterMock.EXPECT().BuildBasicSpanFilter(gomock.Any(), gomock.Any()).Return(nil, false, fmt.Errorf("builtin filter error")) - filterFactoryMock.EXPECT().GetFilter(gomock.Any(), gomock.Any()).Return(filterMock, nil) - - buildHelper := NewTraceFilterProcessorBuilder(filterFactoryMock, nil, nil, nil, nil, nil, nil) - - return fields{ - buildHelper: buildHelper, - } - }, - args: args{ - ctx: context.Background(), - req: &ListSpansOApiReq{ - WorkspaceID: 123, - Tenants: []string{"tenant1"}, - StartTime: 1640995200000, - EndTime: 1640995800000, - Limit: 100, - PlatformType: loop_span.PlatformCozeLoop, - SpanListType: loop_span.SpanListTypeAllSpan, - }, - }, - wantErr: true, - }, - { - name: "list spans oapi failed due to repo error", - fieldsGetter: func(ctrl *gomock.Controller) fields { - repoMock := repomocks.NewMockITraceRepo(ctrl) - repoMock.EXPECT().ListSpans(gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("repo error")) - - filterFactoryMock := filtermocks.NewMockPlatformFilterFactory(ctrl) - filterMock := filtermocks.NewMockFilter(ctrl) - filterMock.EXPECT().BuildBasicSpanFilter(gomock.Any(), gomock.Any()).Return([]*loop_span.FilterField{ - { - FieldName: loop_span.SpanFieldSpaceId, - FieldType: loop_span.FieldTypeString, - Values: []string{"123"}, - QueryType: ptr.Of(loop_span.QueryTypeEnumIn), - }, - }, false, nil) - filterMock.EXPECT().BuildALLSpanFilter(gomock.Any(), gomock.Any()).Return(nil, nil) - filterFactoryMock.EXPECT().GetFilter(gomock.Any(), gomock.Any()).Return(filterMock, nil) - - buildHelper := NewTraceFilterProcessorBuilder(filterFactoryMock, nil, nil, nil, nil, nil, nil) - - return fields{ - traceRepo: repoMock, - buildHelper: buildHelper, - } - }, - args: args{ - ctx: context.Background(), - req: &ListSpansOApiReq{ - WorkspaceID: 123, - Tenants: []string{"tenant1"}, - StartTime: 1640995200000, - EndTime: 1640995800000, - Limit: 100, - PlatformType: loop_span.PlatformCozeLoop, - SpanListType: loop_span.SpanListTypeAllSpan, - }, - }, - wantErr: true, - }, - { - name: "list spans oapi failed due to processor transform error", - fieldsGetter: func(ctrl *gomock.Controller) fields { - repoMock := repomocks.NewMockITraceRepo(ctrl) - repoMock.EXPECT().ListSpans(gomock.Any(), gomock.Any()).Return(&repo.ListSpansResult{ - Spans: loop_span.SpanList{ - { - TraceID: "trace-123", - SpanID: "span-456", - WorkspaceID: "1234", - }, - }, - PageToken: "", - HasMore: false, - }, nil) - - filterFactoryMock := filtermocks.NewMockPlatformFilterFactory(ctrl) - filterMock := filtermocks.NewMockFilter(ctrl) - filterMock.EXPECT().BuildBasicSpanFilter(gomock.Any(), gomock.Any()).Return([]*loop_span.FilterField{ - { - FieldName: loop_span.SpanFieldSpaceId, - FieldType: loop_span.FieldTypeString, - Values: []string{"123"}, - QueryType: ptr.Of(loop_span.QueryTypeEnumIn), - }, - }, false, nil) - filterMock.EXPECT().BuildALLSpanFilter(gomock.Any(), gomock.Any()).Return(nil, nil) - filterFactoryMock.EXPECT().GetFilter(gomock.Any(), gomock.Any()).Return(filterMock, nil) - - buildHelper := NewTraceFilterProcessorBuilder(filterFactoryMock, nil, nil, nil, nil, nil, - []span_processor.Factory{span_processor.NewCheckProcessorFactory()}) - - return fields{ - traceRepo: repoMock, - buildHelper: buildHelper, - } - }, - args: args{ - ctx: context.Background(), - req: &ListSpansOApiReq{ - WorkspaceID: 123, - Tenants: []string{"tenant1"}, - StartTime: 1640995200000, - EndTime: 1640995800000, - Limit: 100, - PlatformType: loop_span.PlatformCozeLoop, - SpanListType: loop_span.SpanListTypeAllSpan, - }, - }, - wantErr: true, - }, { name: "list spans failed due to invalid filter", fieldsGetter: func(ctrl *gomock.Controller) fields { @@ -2880,160 +2846,4 @@ func TestTraceServiceImpl_ListSpansOApi(t *testing.T) { } }) } -} - -func TestTraceFilterProcessorBuilderImpl_BuildListSpansOApiProcessors(t *testing.T) { - tests := []struct { - name string - listSpansOApiProcessorFactories []span_processor.Factory - want int - wantErr bool - }{ - { - name: "build processors successfully with empty factories", - listSpansOApiProcessorFactories: []span_processor.Factory{}, - want: 0, - wantErr: false, - }, - { - name: "build processors successfully with multiple factories", - listSpansOApiProcessorFactories: []span_processor.Factory{ - span_processor.NewCheckProcessorFactory(), - span_processor.NewCheckProcessorFactory(), - }, - want: 2, - wantErr: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - filterFactoryMock := filtermocks.NewMockPlatformFilterFactory(ctrl) - builder := NewTraceFilterProcessorBuilder( - filterFactoryMock, - nil, - nil, - nil, - nil, - nil, - tt.listSpansOApiProcessorFactories, - ) - - got, err := builder.BuildListSpansOApiProcessors(context.Background(), span_processor.Settings{ - WorkspaceId: 123, - QueryStartTime: 1640995200000, - QueryEndTime: 1640995800000, - }) - - assert.Equal(t, tt.wantErr, err != nil) - if !tt.wantErr { - assert.Equal(t, tt.want, len(got)) - } - }) - } -} - -func TestTraceFilterProcessorBuilderImpl_BuildIngestTraceProcessors_ErrorHandling(t *testing.T) { - tests := []struct { - name string - ingestTraceProcessorFactories []span_processor.Factory - want int - wantErr bool - }{ - { - name: "build ingest processors successfully with empty factories", - ingestTraceProcessorFactories: []span_processor.Factory{}, - want: 0, - wantErr: false, - }, - { - name: "build ingest processors successfully with multiple factories", - ingestTraceProcessorFactories: []span_processor.Factory{ - span_processor.NewCheckProcessorFactory(), - }, - want: 1, - wantErr: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - filterFactoryMock := filtermocks.NewMockPlatformFilterFactory(ctrl) - - builder := NewTraceFilterProcessorBuilder( - filterFactoryMock, - nil, - nil, - nil, - tt.ingestTraceProcessorFactories, - nil, - nil, - ) - - got, err := builder.BuildIngestTraceProcessors(context.Background(), span_processor.Settings{ - Tenant: "test-tenant", - }) - - assert.Equal(t, tt.wantErr, err != nil) - if !tt.wantErr { - assert.Equal(t, tt.want, len(got)) - } - }) - } -} - -func TestTraceFilterProcessorBuilderImpl_BuildSearchTraceOApiProcessors_ErrorHandling(t *testing.T) { - tests := []struct { - name string - searchTraceOApiProcessorFactories []span_processor.Factory - want int - wantErr bool - }{ - { - name: "build search trace oapi processors successfully with empty factories", - searchTraceOApiProcessorFactories: []span_processor.Factory{}, - want: 0, - wantErr: false, - }, - { - name: "build search trace oapi processors successfully with multiple factories", - searchTraceOApiProcessorFactories: []span_processor.Factory{ - span_processor.NewCheckProcessorFactory(), - }, - want: 1, - wantErr: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - filterFactoryMock := filtermocks.NewMockPlatformFilterFactory(ctrl) - builder := NewTraceFilterProcessorBuilder( - filterFactoryMock, - nil, - nil, - nil, - nil, - tt.searchTraceOApiProcessorFactories, - nil, - ) - - got, err := builder.BuildSearchTraceOApiProcessors(context.Background(), span_processor.Settings{ - WorkspaceId: 123, - QueryStartTime: 1640995200000, - QueryEndTime: 1640995800000, - }) - - assert.Equal(t, tt.wantErr, err != nil) - if !tt.wantErr { - assert.Equal(t, tt.want, len(got)) - } - }) - } -} +} \ No newline at end of file From 24a17c80238efa3291eb1abad01806ad8066ef2e Mon Sep 17 00:00:00 2001 From: lizhanpeng Date: Wed, 3 Sep 2025 11:29:53 +0800 Subject: [PATCH 03/13] =?UTF-8?q?fix:=20[Coda]=20=E4=BF=AE=E5=A4=8Dobserva?= =?UTF-8?q?bility/application=E4=B8=8Bopenapi=5Ftest.go=E5=8D=95=E6=B5=8B?= =?UTF-8?q?=E6=9D=83=E9=99=90=E6=A3=80=E6=9F=A5mock=E7=BC=BA=E5=A4=B1?= =?UTF-8?q?=E9=97=AE=E9=A2=98=20(LogID:=2020250903112557010091104200021DB6?= =?UTF-8?q?2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Coda --- backend/modules/observability/application/openapi_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/backend/modules/observability/application/openapi_test.go b/backend/modules/observability/application/openapi_test.go index 7aed27b68..35f0265ed 100644 --- a/backend/modules/observability/application/openapi_test.go +++ b/backend/modules/observability/application/openapi_test.go @@ -197,13 +197,14 @@ func TestOpenAPIApplication_CreateAnnotation(t *testing.T) { { name: "create annotation successfully", fieldsGetter: func(ctrl *gomock.Controller) fields { + authMock := rpcmocks.NewMockIAuthProvider(ctrl) + authMock.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionAnnotationCreate, "1").Return(nil) traceServiceMock := servicemocks.NewMockITraceService(ctrl) traceServiceMock.EXPECT().CreateAnnotation(gomock.Any(), gomock.Any()).Return(nil) benefitMock := benefitmocks.NewMockIBenefitService(ctrl) benefitMock.EXPECT().CheckTraceBenefit(gomock.Any(), gomock.Any()).Return(&benefit.CheckTraceBenefitResult{ StorageDuration: 3, }, nil) - authMock := rpcmocks.NewMockIAuthProvider(ctrl) tenantMock := tenantmocks.NewMockITenantProvider(ctrl) workspaceMock := workspacemocks.NewMockIWorkSpaceProvider(ctrl) workspaceMock.EXPECT().GetThirdPartyQueryWorkSpaceID(gomock.Any(), int64(123)).Return("123").AnyTimes() @@ -342,13 +343,14 @@ func TestOpenAPIApplication_DeleteAnnotation(t *testing.T) { { name: "delete annotation successfully", fieldsGetter: func(ctrl *gomock.Controller) fields { + authMock := rpcmocks.NewMockIAuthProvider(ctrl) + authMock.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionAnnotationCreate, "1").Return(nil) traceServiceMock := servicemocks.NewMockITraceService(ctrl) traceServiceMock.EXPECT().DeleteAnnotation(gomock.Any(), gomock.Any()).Return(nil) benefitMock := benefitmocks.NewMockIBenefitService(ctrl) benefitMock.EXPECT().CheckTraceBenefit(gomock.Any(), gomock.Any()).Return(&benefit.CheckTraceBenefitResult{ StorageDuration: 3, }, nil) - authMock := rpcmocks.NewMockIAuthProvider(ctrl) tenantMock := tenantmocks.NewMockITenantProvider(ctrl) workspaceMock := workspacemocks.NewMockIWorkSpaceProvider(ctrl) workspaceMock.EXPECT().GetThirdPartyQueryWorkSpaceID(gomock.Any(), int64(123)).Return("123").AnyTimes() From 7c0961337b953153fa26cdf9ee8d3ebfe5330b92 Mon Sep 17 00:00:00 2001 From: lizhanpeng Date: Wed, 3 Sep 2025 11:33:12 +0800 Subject: [PATCH 04/13] feat(backend): refactor & fix --- .../domain/annotation/annotation.go | 4 + .../coze.loop.observability.openapi.go | 116 ++++++++++-------- ...ze.loop.observability.openapi_validator.go | 6 - .../k-coze.loop.observability.openapi.go | 62 +++++----- .../observability/application/openapi.go | 14 ++- .../trace/entity/loop_span/annotation.go | 10 +- .../coze.loop.observability.openapi.thrift | 6 +- .../observability/domain/annotation.thrift | 2 + 8 files changed, 126 insertions(+), 94 deletions(-) diff --git a/backend/kitex_gen/coze/loop/observability/domain/annotation/annotation.go b/backend/kitex_gen/coze/loop/observability/domain/annotation/annotation.go index ad90f72ee..d2433f40c 100644 --- a/backend/kitex_gen/coze/loop/observability/domain/annotation/annotation.go +++ b/backend/kitex_gen/coze/loop/observability/domain/annotation/annotation.go @@ -22,6 +22,10 @@ const ( ValueTypeString = "string" + ValueTypeCategory = "category" + + ValueTypeNumber = "number" + ValueTypeLong = "long" ValueTypeDouble = "double" diff --git a/backend/kitex_gen/coze/loop/observability/openapi/coze.loop.observability.openapi.go b/backend/kitex_gen/coze/loop/observability/openapi/coze.loop.observability.openapi.go index 8d4a6b00a..7ba729b63 100644 --- a/backend/kitex_gen/coze/loop/observability/openapi/coze.loop.observability.openapi.go +++ b/backend/kitex_gen/coze/loop/observability/openapi/coze.loop.observability.openapi.go @@ -1381,7 +1381,7 @@ func (p *OtelIngestTracesResponse) Field255DeepEqual(src *base.BaseResp) bool { type CreateAnnotationRequest struct { WorkspaceID int64 `thrift:"workspace_id,1,required" frugal:"1,required,i64" json:"workspace_id" form:"workspace_id,required" ` - SpanID string `thrift:"span_id,2,required" frugal:"2,required,string" form:"span_id,required" json:"span_id,required"` + SpanID *string `thrift:"span_id,2,optional" frugal:"2,optional,string" form:"span_id" json:"span_id,omitempty"` TraceID string `thrift:"trace_id,3,required" frugal:"3,required,string" form:"trace_id,required" json:"trace_id,required"` AnnotationKey string `thrift:"annotation_key,4,required" frugal:"4,required,string" form:"annotation_key,required" json:"annotation_key,required"` AnnotationValue string `thrift:"annotation_value,5,required" frugal:"5,required,string" form:"annotation_value,required" json:"annotation_value,required"` @@ -1404,11 +1404,16 @@ func (p *CreateAnnotationRequest) GetWorkspaceID() (v int64) { return } +var CreateAnnotationRequest_SpanID_DEFAULT string + func (p *CreateAnnotationRequest) GetSpanID() (v string) { - if p != nil { - return p.SpanID + if p == nil { + return } - return + if !p.IsSetSpanID() { + return CreateAnnotationRequest_SpanID_DEFAULT + } + return *p.SpanID } func (p *CreateAnnotationRequest) GetTraceID() (v string) { @@ -1470,7 +1475,7 @@ func (p *CreateAnnotationRequest) GetBase() (v *base.Base) { func (p *CreateAnnotationRequest) SetWorkspaceID(val int64) { p.WorkspaceID = val } -func (p *CreateAnnotationRequest) SetSpanID(val string) { +func (p *CreateAnnotationRequest) SetSpanID(val *string) { p.SpanID = val } func (p *CreateAnnotationRequest) SetTraceID(val string) { @@ -1503,6 +1508,10 @@ var fieldIDToName_CreateAnnotationRequest = map[int16]string{ 255: "Base", } +func (p *CreateAnnotationRequest) IsSetSpanID() bool { + return p.SpanID != nil +} + func (p *CreateAnnotationRequest) IsSetAnnotationValueType() bool { return p.AnnotationValueType != nil } @@ -1519,7 +1528,6 @@ func (p *CreateAnnotationRequest) Read(iprot thrift.TProtocol) (err error) { var fieldTypeId thrift.TType var fieldId int16 var issetWorkspaceID bool = false - var issetSpanID bool = false var issetTraceID bool = false var issetAnnotationKey bool = false var issetAnnotationValue bool = false @@ -1552,7 +1560,6 @@ func (p *CreateAnnotationRequest) Read(iprot thrift.TProtocol) (err error) { if err = p.ReadField2(iprot); err != nil { goto ReadFieldError } - issetSpanID = true } else if err = iprot.Skip(fieldTypeId); err != nil { goto SkipFieldError } @@ -1625,11 +1632,6 @@ func (p *CreateAnnotationRequest) Read(iprot thrift.TProtocol) (err error) { goto RequiredFieldNotSetError } - if !issetSpanID { - fieldId = 2 - goto RequiredFieldNotSetError - } - if !issetTraceID { fieldId = 3 goto RequiredFieldNotSetError @@ -1675,11 +1677,11 @@ func (p *CreateAnnotationRequest) ReadField1(iprot thrift.TProtocol) error { } func (p *CreateAnnotationRequest) ReadField2(iprot thrift.TProtocol) error { - var _field string + var _field *string if v, err := iprot.ReadString(); err != nil { return err } else { - _field = v + _field = &v } p.SpanID = _field return nil @@ -1821,14 +1823,16 @@ WriteFieldEndError: return thrift.PrependError(fmt.Sprintf("%T write field 1 end error: ", p), err) } func (p *CreateAnnotationRequest) writeField2(oprot thrift.TProtocol) (err error) { - if err = oprot.WriteFieldBegin("span_id", thrift.STRING, 2); err != nil { - goto WriteFieldBeginError - } - if err := oprot.WriteString(p.SpanID); err != nil { - return err - } - if err = oprot.WriteFieldEnd(); err != nil { - goto WriteFieldEndError + if p.IsSetSpanID() { + if err = oprot.WriteFieldBegin("span_id", thrift.STRING, 2); err != nil { + goto WriteFieldBeginError + } + if err := oprot.WriteString(*p.SpanID); err != nil { + return err + } + if err = oprot.WriteFieldEnd(); err != nil { + goto WriteFieldEndError + } } return nil WriteFieldBeginError: @@ -1987,9 +1991,14 @@ func (p *CreateAnnotationRequest) Field1DeepEqual(src int64) bool { } return true } -func (p *CreateAnnotationRequest) Field2DeepEqual(src string) bool { +func (p *CreateAnnotationRequest) Field2DeepEqual(src *string) bool { - if strings.Compare(p.SpanID, src) != 0 { + if p.SpanID == src { + return true + } else if p.SpanID == nil || src == nil { + return false + } + if strings.Compare(*p.SpanID, *src) != 0 { return false } return true @@ -2221,8 +2230,8 @@ func (p *CreateAnnotationResponse) Field255DeepEqual(src *base.BaseResp) bool { } type DeleteAnnotationRequest struct { - WorkspaceID int64 `thrift:"workspace_id,1,required" frugal:"1,required,i64" json:"workspace_id" form:"workspace_id,required" ` - SpanID string `thrift:"span_id,2,required" frugal:"2,required,string" json:"span_id,required" query:"span_id,required"` + WorkspaceID int64 `thrift:"workspace_id,1,required" frugal:"1,required,i64" json:"workspace_id" query:"workspace_id,required" ` + SpanID *string `thrift:"span_id,2,optional" frugal:"2,optional,string" json:"span_id,omitempty" query:"span_id"` TraceID string `thrift:"trace_id,4,required" frugal:"4,required,string" json:"trace_id,required" query:"trace_id,required"` AnnotationKey string `thrift:"annotation_key,3,required" frugal:"3,required,string" json:"annotation_key,required" query:"annotation_key,required"` Base *base.Base `thrift:"Base,255,optional" frugal:"255,optional,base.Base" form:"Base" json:"Base,omitempty" query:"Base"` @@ -2242,11 +2251,16 @@ func (p *DeleteAnnotationRequest) GetWorkspaceID() (v int64) { return } +var DeleteAnnotationRequest_SpanID_DEFAULT string + func (p *DeleteAnnotationRequest) GetSpanID() (v string) { - if p != nil { - return p.SpanID + if p == nil { + return } - return + if !p.IsSetSpanID() { + return DeleteAnnotationRequest_SpanID_DEFAULT + } + return *p.SpanID } func (p *DeleteAnnotationRequest) GetTraceID() (v string) { @@ -2277,7 +2291,7 @@ func (p *DeleteAnnotationRequest) GetBase() (v *base.Base) { func (p *DeleteAnnotationRequest) SetWorkspaceID(val int64) { p.WorkspaceID = val } -func (p *DeleteAnnotationRequest) SetSpanID(val string) { +func (p *DeleteAnnotationRequest) SetSpanID(val *string) { p.SpanID = val } func (p *DeleteAnnotationRequest) SetTraceID(val string) { @@ -2298,6 +2312,10 @@ var fieldIDToName_DeleteAnnotationRequest = map[int16]string{ 255: "Base", } +func (p *DeleteAnnotationRequest) IsSetSpanID() bool { + return p.SpanID != nil +} + func (p *DeleteAnnotationRequest) IsSetBase() bool { return p.Base != nil } @@ -2306,7 +2324,6 @@ func (p *DeleteAnnotationRequest) Read(iprot thrift.TProtocol) (err error) { var fieldTypeId thrift.TType var fieldId int16 var issetWorkspaceID bool = false - var issetSpanID bool = false var issetTraceID bool = false var issetAnnotationKey bool = false @@ -2338,7 +2355,6 @@ func (p *DeleteAnnotationRequest) Read(iprot thrift.TProtocol) (err error) { if err = p.ReadField2(iprot); err != nil { goto ReadFieldError } - issetSpanID = true } else if err = iprot.Skip(fieldTypeId); err != nil { goto SkipFieldError } @@ -2386,11 +2402,6 @@ func (p *DeleteAnnotationRequest) Read(iprot thrift.TProtocol) (err error) { goto RequiredFieldNotSetError } - if !issetSpanID { - fieldId = 2 - goto RequiredFieldNotSetError - } - if !issetTraceID { fieldId = 4 goto RequiredFieldNotSetError @@ -2431,11 +2442,11 @@ func (p *DeleteAnnotationRequest) ReadField1(iprot thrift.TProtocol) error { } func (p *DeleteAnnotationRequest) ReadField2(iprot thrift.TProtocol) error { - var _field string + var _field *string if v, err := iprot.ReadString(); err != nil { return err } else { - _field = v + _field = &v } p.SpanID = _field return nil @@ -2532,14 +2543,16 @@ WriteFieldEndError: return thrift.PrependError(fmt.Sprintf("%T write field 1 end error: ", p), err) } func (p *DeleteAnnotationRequest) writeField2(oprot thrift.TProtocol) (err error) { - if err = oprot.WriteFieldBegin("span_id", thrift.STRING, 2); err != nil { - goto WriteFieldBeginError - } - if err := oprot.WriteString(p.SpanID); err != nil { - return err - } - if err = oprot.WriteFieldEnd(); err != nil { - goto WriteFieldEndError + if p.IsSetSpanID() { + if err = oprot.WriteFieldBegin("span_id", thrift.STRING, 2); err != nil { + goto WriteFieldBeginError + } + if err := oprot.WriteString(*p.SpanID); err != nil { + return err + } + if err = oprot.WriteFieldEnd(); err != nil { + goto WriteFieldEndError + } } return nil WriteFieldBeginError: @@ -2637,9 +2650,14 @@ func (p *DeleteAnnotationRequest) Field1DeepEqual(src int64) bool { } return true } -func (p *DeleteAnnotationRequest) Field2DeepEqual(src string) bool { +func (p *DeleteAnnotationRequest) Field2DeepEqual(src *string) bool { - if strings.Compare(p.SpanID, src) != 0 { + if p.SpanID == src { + return true + } else if p.SpanID == nil || src == nil { + return false + } + if strings.Compare(*p.SpanID, *src) != 0 { return false } return true diff --git a/backend/kitex_gen/coze/loop/observability/openapi/coze.loop.observability.openapi_validator.go b/backend/kitex_gen/coze/loop/observability/openapi/coze.loop.observability.openapi_validator.go index 1af9a6f47..610982a6a 100644 --- a/backend/kitex_gen/coze/loop/observability/openapi/coze.loop.observability.openapi_validator.go +++ b/backend/kitex_gen/coze/loop/observability/openapi/coze.loop.observability.openapi_validator.go @@ -57,9 +57,6 @@ func (p *CreateAnnotationRequest) IsValid() error { if p.WorkspaceID <= int64(0) { return fmt.Errorf("field WorkspaceID gt rule failed, current value: %v", p.WorkspaceID) } - if len(p.SpanID) < int(1) { - return fmt.Errorf("field SpanID min_len rule failed, current value: %d", len(p.SpanID)) - } if len(p.TraceID) < int(1) { return fmt.Errorf("field TraceID min_len rule failed, current value: %d", len(p.TraceID)) } @@ -85,9 +82,6 @@ func (p *DeleteAnnotationRequest) IsValid() error { if p.WorkspaceID <= int64(0) { return fmt.Errorf("field WorkspaceID gt rule failed, current value: %v", p.WorkspaceID) } - if len(p.SpanID) < int(1) { - return fmt.Errorf("field SpanID min_len rule failed, current value: %d", len(p.SpanID)) - } if len(p.TraceID) < int(1) { return fmt.Errorf("field TraceID min_len rule failed, current value: %d", len(p.TraceID)) } diff --git a/backend/kitex_gen/coze/loop/observability/openapi/k-coze.loop.observability.openapi.go b/backend/kitex_gen/coze/loop/observability/openapi/k-coze.loop.observability.openapi.go index 515c9cc5e..8eca6ef94 100644 --- a/backend/kitex_gen/coze/loop/observability/openapi/k-coze.loop.observability.openapi.go +++ b/backend/kitex_gen/coze/loop/observability/openapi/k-coze.loop.observability.openapi.go @@ -1044,7 +1044,6 @@ func (p *CreateAnnotationRequest) FastRead(buf []byte) (int, error) { var fieldTypeId thrift.TType var fieldId int16 var issetWorkspaceID bool = false - var issetSpanID bool = false var issetTraceID bool = false var issetAnnotationKey bool = false var issetAnnotationValue bool = false @@ -1080,7 +1079,6 @@ func (p *CreateAnnotationRequest) FastRead(buf []byte) (int, error) { if err != nil { goto ReadFieldError } - issetSpanID = true } else { l, err = thrift.Binary.Skip(buf[offset:], fieldTypeId) offset += l @@ -1189,11 +1187,6 @@ func (p *CreateAnnotationRequest) FastRead(buf []byte) (int, error) { goto RequiredFieldNotSetError } - if !issetSpanID { - fieldId = 2 - goto RequiredFieldNotSetError - } - if !issetTraceID { fieldId = 3 goto RequiredFieldNotSetError @@ -1236,12 +1229,12 @@ func (p *CreateAnnotationRequest) FastReadField1(buf []byte) (int, error) { func (p *CreateAnnotationRequest) FastReadField2(buf []byte) (int, error) { offset := 0 - var _field string + var _field *string if v, l, err := thrift.Binary.ReadString(buf[offset:]); err != nil { return offset, err } else { offset += l - _field = v + _field = &v } p.SpanID = _field return offset, nil @@ -1374,8 +1367,10 @@ func (p *CreateAnnotationRequest) fastWriteField1(buf []byte, w thrift.NocopyWri func (p *CreateAnnotationRequest) fastWriteField2(buf []byte, w thrift.NocopyWriter) int { offset := 0 - offset += thrift.Binary.WriteFieldBegin(buf[offset:], thrift.STRING, 2) - offset += thrift.Binary.WriteStringNocopy(buf[offset:], w, p.SpanID) + if p.IsSetSpanID() { + offset += thrift.Binary.WriteFieldBegin(buf[offset:], thrift.STRING, 2) + offset += thrift.Binary.WriteStringNocopy(buf[offset:], w, *p.SpanID) + } return offset } @@ -1436,8 +1431,10 @@ func (p *CreateAnnotationRequest) field1Length() int { func (p *CreateAnnotationRequest) field2Length() int { l := 0 - l += thrift.Binary.FieldBeginLength() - l += thrift.Binary.StringLengthNocopy(p.SpanID) + if p.IsSetSpanID() { + l += thrift.Binary.FieldBeginLength() + l += thrift.Binary.StringLengthNocopy(*p.SpanID) + } return l } @@ -1497,8 +1494,12 @@ func (p *CreateAnnotationRequest) DeepCopy(s interface{}) error { p.WorkspaceID = src.WorkspaceID - if src.SpanID != "" { - p.SpanID = kutils.StringDeepCopy(src.SpanID) + if src.SpanID != nil { + var tmp string + if *src.SpanID != "" { + tmp = kutils.StringDeepCopy(*src.SpanID) + } + p.SpanID = &tmp } if src.TraceID != "" { @@ -1665,7 +1666,6 @@ func (p *DeleteAnnotationRequest) FastRead(buf []byte) (int, error) { var fieldTypeId thrift.TType var fieldId int16 var issetWorkspaceID bool = false - var issetSpanID bool = false var issetTraceID bool = false var issetAnnotationKey bool = false for { @@ -1700,7 +1700,6 @@ func (p *DeleteAnnotationRequest) FastRead(buf []byte) (int, error) { if err != nil { goto ReadFieldError } - issetSpanID = true } else { l, err = thrift.Binary.Skip(buf[offset:], fieldTypeId) offset += l @@ -1766,11 +1765,6 @@ func (p *DeleteAnnotationRequest) FastRead(buf []byte) (int, error) { goto RequiredFieldNotSetError } - if !issetSpanID { - fieldId = 2 - goto RequiredFieldNotSetError - } - if !issetTraceID { fieldId = 4 goto RequiredFieldNotSetError @@ -1808,12 +1802,12 @@ func (p *DeleteAnnotationRequest) FastReadField1(buf []byte) (int, error) { func (p *DeleteAnnotationRequest) FastReadField2(buf []byte) (int, error) { offset := 0 - var _field string + var _field *string if v, l, err := thrift.Binary.ReadString(buf[offset:]); err != nil { return offset, err } else { offset += l - _field = v + _field = &v } p.SpanID = _field return offset, nil @@ -1898,8 +1892,10 @@ func (p *DeleteAnnotationRequest) fastWriteField1(buf []byte, w thrift.NocopyWri func (p *DeleteAnnotationRequest) fastWriteField2(buf []byte, w thrift.NocopyWriter) int { offset := 0 - offset += thrift.Binary.WriteFieldBegin(buf[offset:], thrift.STRING, 2) - offset += thrift.Binary.WriteStringNocopy(buf[offset:], w, p.SpanID) + if p.IsSetSpanID() { + offset += thrift.Binary.WriteFieldBegin(buf[offset:], thrift.STRING, 2) + offset += thrift.Binary.WriteStringNocopy(buf[offset:], w, *p.SpanID) + } return offset } @@ -1935,8 +1931,10 @@ func (p *DeleteAnnotationRequest) field1Length() int { func (p *DeleteAnnotationRequest) field2Length() int { l := 0 - l += thrift.Binary.FieldBeginLength() - l += thrift.Binary.StringLengthNocopy(p.SpanID) + if p.IsSetSpanID() { + l += thrift.Binary.FieldBeginLength() + l += thrift.Binary.StringLengthNocopy(*p.SpanID) + } return l } @@ -1971,8 +1969,12 @@ func (p *DeleteAnnotationRequest) DeepCopy(s interface{}) error { p.WorkspaceID = src.WorkspaceID - if src.SpanID != "" { - p.SpanID = kutils.StringDeepCopy(src.SpanID) + if src.SpanID != nil { + var tmp string + if *src.SpanID != "" { + tmp = kutils.StringDeepCopy(*src.SpanID) + } + p.SpanID = &tmp } if src.TraceID != "" { diff --git a/backend/modules/observability/application/openapi.go b/backend/modules/observability/application/openapi.go index 8ac414d6b..2de354685 100644 --- a/backend/modules/observability/application/openapi.go +++ b/backend/modules/observability/application/openapi.go @@ -395,7 +395,7 @@ func (o *OpenAPIApplication) CreateAnnotation(ctx context.Context, req *openapi. return nil, errorx.NewByCode(obErrorx.CommercialCommonInvalidParamCodeCode, errorx.WithExtraMsg("invalid annotation_value")) } val = loop_span.NewLongValue(i) - case loop_span.AnnotationValueTypeString: + case loop_span.AnnotationValueTypeString, loop_span.AnnotationValueTypeCategory: val = loop_span.NewStringValue(req.AnnotationValue) case loop_span.AnnotationValueTypeBool: b, err := strconv.ParseBool(req.AnnotationValue) @@ -403,7 +403,7 @@ func (o *OpenAPIApplication) CreateAnnotation(ctx context.Context, req *openapi. return nil, errorx.NewByCode(obErrorx.CommercialCommonInvalidParamCodeCode, errorx.WithExtraMsg("invalid annotation_value")) } val = loop_span.NewBoolValue(b) - case loop_span.AnnotationValueTypeDouble: + case loop_span.AnnotationValueTypeDouble, loop_span.AnnotationValueTypeNumber: f, err := strconv.ParseFloat(req.AnnotationValue, 64) if err != nil { return nil, errorx.NewByCode(obErrorx.CommercialCommonInvalidParamCodeCode, errorx.WithExtraMsg("invalid annotation_value")) @@ -412,6 +412,11 @@ func (o *OpenAPIApplication) CreateAnnotation(ctx context.Context, req *openapi. default: val = loop_span.NewStringValue(req.AnnotationValue) } + if err := o.auth.CheckWorkspacePermission(ctx, + rpc.AuthActionAnnotationCreate, + strconv.FormatInt(req.WorkspaceID, 10)); err != nil { + return nil, err + } res, err := o.benefit.CheckTraceBenefit(ctx, &benefit.CheckTraceBenefitParams{ ConnectorUID: session.UserIDInCtxOrEmpty(ctx), SpaceID: req.WorkspaceID, @@ -436,6 +441,11 @@ func (o *OpenAPIApplication) CreateAnnotation(ctx context.Context, req *openapi. } func (o *OpenAPIApplication) DeleteAnnotation(ctx context.Context, req *openapi.DeleteAnnotationRequest) (*openapi.DeleteAnnotationResponse, error) { + if err := o.auth.CheckWorkspacePermission(ctx, + rpc.AuthActionAnnotationCreate, + strconv.FormatInt(req.WorkspaceID, 10)); err != nil { + return nil, err + } res, err := o.benefit.CheckTraceBenefit(ctx, &benefit.CheckTraceBenefitParams{ ConnectorUID: session.UserIDInCtxOrEmpty(ctx), SpaceID: req.WorkspaceID, diff --git a/backend/modules/observability/domain/trace/entity/loop_span/annotation.go b/backend/modules/observability/domain/trace/entity/loop_span/annotation.go index 883eabd24..004f481fa 100644 --- a/backend/modules/observability/domain/trace/entity/loop_span/annotation.go +++ b/backend/modules/observability/domain/trace/entity/loop_span/annotation.go @@ -20,10 +20,12 @@ type ( ) const ( - AnnotationValueTypeString AnnotationValueType = "string" - AnnotationValueTypeLong AnnotationValueType = "long" - AnnotationValueTypeDouble AnnotationValueType = "double" - AnnotationValueTypeBool AnnotationValueType = "bool" + AnnotationValueTypeCategory AnnotationValueType = "category" // 等于string + AnnotationValueTypeString AnnotationValueType = "string" + AnnotationValueTypeLong AnnotationValueType = "long" + AnnotationValueTypeNumber AnnotationValueType = "number" // 等于float + AnnotationValueTypeDouble AnnotationValueType = "double" + AnnotationValueTypeBool AnnotationValueType = "bool" AnnotationStatusNormal AnnotationStatus = "normal" AnnotationStatusInactive AnnotationStatus = "inactive" diff --git a/idl/thrift/coze/loop/observability/coze.loop.observability.openapi.thrift b/idl/thrift/coze/loop/observability/coze.loop.observability.openapi.thrift index 3a26b15fa..23c4ac8e5 100644 --- a/idl/thrift/coze/loop/observability/coze.loop.observability.openapi.thrift +++ b/idl/thrift/coze/loop/observability/coze.loop.observability.openapi.thrift @@ -39,7 +39,7 @@ struct OtelIngestTracesResponse { struct CreateAnnotationRequest { 1: required i64 workspace_id (api.js_conv='true', go.tag='json:"workspace_id"', api.body="workspace_id" vt.gt="0") - 2: required string span_id (api.body="span_id", vt.min_size="1") + 2: optional string span_id (api.body="span_id") 3: required string trace_id (api.body="trace_id", vt.min_size="1") 4: required string annotation_key (api.body="annotation_key", vt.min_size="1") 5: required string annotation_value (api.body="annotation_value") @@ -54,8 +54,8 @@ struct CreateAnnotationResponse { } struct DeleteAnnotationRequest { - 1: required i64 workspace_id (api.js_conv='true', go.tag='json:"workspace_id"', api.body="workspace_id" vt.gt="0") - 2: required string span_id (api.query='span_id', vt.min_size="1") + 1: required i64 workspace_id (api.js_conv='true', go.tag='json:"workspace_id"', api.query="workspace_id" vt.gt="0") + 2: optional string span_id (api.query='span_id') 4: required string trace_id (api.query="trace_id", vt.min_size="1") 3: required string annotation_key (api.query='annotation_key', vt.min_size="1") diff --git a/idl/thrift/coze/loop/observability/domain/annotation.thrift b/idl/thrift/coze/loop/observability/domain/annotation.thrift index db3a83551..5fcce6778 100644 --- a/idl/thrift/coze/loop/observability/domain/annotation.thrift +++ b/idl/thrift/coze/loop/observability/domain/annotation.thrift @@ -11,6 +11,8 @@ const AnnotationType AnnotationType_OpenAPIFeedback = "openapi_feedback" typedef string ValueType (ts.enum="true") const ValueType ValueType_String = "string" +const ValueType ValueType_Category = "category" +const ValueType ValueType_Number = "number" const ValueType ValueType_Long = "long" const ValueType ValueType_Double = "double" const ValueType ValueType_Bool = "bool" From fdff18763f06aaf3e96bf84420abd6abe3324241 Mon Sep 17 00:00:00 2001 From: lizhanpeng Date: Thu, 4 Sep 2025 21:50:53 +0800 Subject: [PATCH 05/13] feat(backend): refactor --- .../modules/observability/application/openapi.go | 16 ++++++---------- .../observability/domain/component/rpc/auth.go | 1 + .../domain/component/rpc/mocks/auth_provider.go | 14 ++++++++++++++ .../modules/observability/infra/rpc/auth/auth.go | 4 ++++ 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/backend/modules/observability/application/openapi.go b/backend/modules/observability/application/openapi.go index 2de354685..df42845bd 100644 --- a/backend/modules/observability/application/openapi.go +++ b/backend/modules/observability/application/openapi.go @@ -412,7 +412,7 @@ func (o *OpenAPIApplication) CreateAnnotation(ctx context.Context, req *openapi. default: val = loop_span.NewStringValue(req.AnnotationValue) } - if err := o.auth.CheckWorkspacePermission(ctx, + if err := o.auth.CheckOpenAPIWorkspacePermission(ctx, rpc.AuthActionAnnotationCreate, strconv.FormatInt(req.WorkspaceID, 10)); err != nil { return nil, err @@ -441,7 +441,7 @@ func (o *OpenAPIApplication) CreateAnnotation(ctx context.Context, req *openapi. } func (o *OpenAPIApplication) DeleteAnnotation(ctx context.Context, req *openapi.DeleteAnnotationRequest) (*openapi.DeleteAnnotationResponse, error) { - if err := o.auth.CheckWorkspacePermission(ctx, + if err := o.auth.CheckOpenAPIWorkspacePermission(ctx, rpc.AuthActionAnnotationCreate, strconv.FormatInt(req.WorkspaceID, 10)); err != nil { return nil, err @@ -505,10 +505,8 @@ func (o *OpenAPIApplication) SearchTraceOApi(ctx context.Context, req *openapi.S if err != nil { return nil, errorx.WrapByCode(err, obErrorx.CommercialCommonInternalErrorCodeCode) } - if sResp != nil { - spansSize = loop_span.SizeofSpans(sResp.Spans) - logs.CtxInfo(ctx, "SearchTrace successfully, spans count %d", len(sResp.Spans)) - } + spansSize = loop_span.SizeofSpans(sResp.Spans) + logs.CtxInfo(ctx, "SearchTrace successfully, spans count %d", len(sResp.Spans)) return &openapi.SearchTraceOApiResponse{ Data: &openapi.SearchTraceOApiData{ Spans: tconv.SpanListDO2DTO(sResp.Spans, nil, nil, nil), @@ -604,10 +602,8 @@ func (o *OpenAPIApplication) ListSpansOApi(ctx context.Context, req *openapi.Lis errCode = obErrorx.CommonInternalErrorCode return nil, err } - if sResp != nil { - logs.CtxInfo(ctx, "List spans successfully, spans count: %d", len(sResp.Spans)) - spansSize = loop_span.SizeofSpans(sResp.Spans) - } + logs.CtxInfo(ctx, "List spans successfully, spans count: %d", len(sResp.Spans)) + spansSize = loop_span.SizeofSpans(sResp.Spans) resp.Data = &openapi.ListSpansOApiData{ Spans: tconv.SpanListDO2DTO(sResp.Spans, nil, nil, nil), diff --git a/backend/modules/observability/domain/component/rpc/auth.go b/backend/modules/observability/domain/component/rpc/auth.go index 9f9ffcb7a..111177011 100644 --- a/backend/modules/observability/domain/component/rpc/auth.go +++ b/backend/modules/observability/domain/component/rpc/auth.go @@ -21,6 +21,7 @@ const ( type IAuthProvider interface { CheckWorkspacePermission(ctx context.Context, action, workspaceId string) error CheckViewPermission(ctx context.Context, action, workspaceId, viewId string) error + CheckOpenAPIWorkspacePermission(ctx context.Context, action, workspaceId string) error CheckIngestPermission(ctx context.Context, workspaceId string) error CheckQueryPermission(ctx context.Context, workspaceId, platformType string) error } diff --git a/backend/modules/observability/domain/component/rpc/mocks/auth_provider.go b/backend/modules/observability/domain/component/rpc/mocks/auth_provider.go index acd351512..08c6bfa7c 100644 --- a/backend/modules/observability/domain/component/rpc/mocks/auth_provider.go +++ b/backend/modules/observability/domain/component/rpc/mocks/auth_provider.go @@ -54,6 +54,20 @@ func (mr *MockIAuthProviderMockRecorder) CheckIngestPermission(ctx, workspaceId return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckIngestPermission", reflect.TypeOf((*MockIAuthProvider)(nil).CheckIngestPermission), ctx, workspaceId) } +// CheckOpenAPIWorkspacePermission mocks base method. +func (m *MockIAuthProvider) CheckOpenAPIWorkspacePermission(ctx context.Context, action, workspaceId string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CheckOpenAPIWorkspacePermission", ctx, action, workspaceId) + ret0, _ := ret[0].(error) + return ret0 +} + +// CheckOpenAPIWorkspacePermission indicates an expected call of CheckOpenAPIWorkspacePermission. +func (mr *MockIAuthProviderMockRecorder) CheckOpenAPIWorkspacePermission(ctx, action, workspaceId any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckOpenAPIWorkspacePermission", reflect.TypeOf((*MockIAuthProvider)(nil).CheckOpenAPIWorkspacePermission), ctx, action, workspaceId) +} + // CheckQueryPermission mocks base method. func (m *MockIAuthProvider) CheckQueryPermission(ctx context.Context, workspaceId, platformType string) error { m.ctrl.T.Helper() diff --git a/backend/modules/observability/infra/rpc/auth/auth.go b/backend/modules/observability/infra/rpc/auth/auth.go index 287ec809c..e7dd34615 100644 --- a/backend/modules/observability/infra/rpc/auth/auth.go +++ b/backend/modules/observability/infra/rpc/auth/auth.go @@ -113,6 +113,10 @@ func (a *AuthProviderImpl) CheckViewPermission(ctx context.Context, action, work return nil } +func (a *AuthProviderImpl) CheckOpenAPIWorkspacePermission(ctx context.Context, action, workspaceId string) error { + return a.CheckWorkspacePermission(ctx, action, workspaceId) +} + func (a *AuthProviderImpl) CheckIngestPermission(ctx context.Context, workspaceId string) error { return a.CheckWorkspacePermission(ctx, rpc.AuthActionTraceIngest, workspaceId) } From e1da5dd97f40f25b30ac93c48bb532fd2921bd90 Mon Sep 17 00:00:00 2001 From: lizhanpeng Date: Fri, 5 Sep 2025 11:32:44 +0800 Subject: [PATCH 06/13] feat(backend): refactor & fix ut --- .../observability/application/openapi.go | 10 +-- .../observability/application/openapi_test.go | 6 +- .../observability/application/trace.go | 26 +++--- .../observability/application/trace_test.go | 90 +++++++++---------- .../domain/component/rpc/auth.go | 3 +- .../component/rpc/mocks/auth_provider.go | 22 +---- .../trace/service/trace_service_test.go | 18 ++-- .../observability/infra/rpc/auth/auth.go | 10 +-- 8 files changed, 83 insertions(+), 102 deletions(-) diff --git a/backend/modules/observability/application/openapi.go b/backend/modules/observability/application/openapi.go index df42845bd..316f8c14c 100644 --- a/backend/modules/observability/application/openapi.go +++ b/backend/modules/observability/application/openapi.go @@ -412,9 +412,9 @@ func (o *OpenAPIApplication) CreateAnnotation(ctx context.Context, req *openapi. default: val = loop_span.NewStringValue(req.AnnotationValue) } - if err := o.auth.CheckOpenAPIWorkspacePermission(ctx, + if err := o.auth.CheckWorkspacePermission(ctx, rpc.AuthActionAnnotationCreate, - strconv.FormatInt(req.WorkspaceID, 10)); err != nil { + strconv.FormatInt(req.WorkspaceID, 10), true); err != nil { return nil, err } res, err := o.benefit.CheckTraceBenefit(ctx, &benefit.CheckTraceBenefitParams{ @@ -441,9 +441,9 @@ func (o *OpenAPIApplication) CreateAnnotation(ctx context.Context, req *openapi. } func (o *OpenAPIApplication) DeleteAnnotation(ctx context.Context, req *openapi.DeleteAnnotationRequest) (*openapi.DeleteAnnotationResponse, error) { - if err := o.auth.CheckOpenAPIWorkspacePermission(ctx, + if err := o.auth.CheckWorkspacePermission(ctx, rpc.AuthActionAnnotationCreate, - strconv.FormatInt(req.WorkspaceID, 10)); err != nil { + strconv.FormatInt(req.WorkspaceID, 10), true); err != nil { return nil, err } res, err := o.benefit.CheckTraceBenefit(ctx, &benefit.CheckTraceBenefitParams{ @@ -794,4 +794,4 @@ func (p *OpenAPIApplication) AllowByKey(ctx context.Context, key string) bool { return true } return false -} +} \ No newline at end of file diff --git a/backend/modules/observability/application/openapi_test.go b/backend/modules/observability/application/openapi_test.go index 35f0265ed..14efae23e 100644 --- a/backend/modules/observability/application/openapi_test.go +++ b/backend/modules/observability/application/openapi_test.go @@ -198,7 +198,7 @@ func TestOpenAPIApplication_CreateAnnotation(t *testing.T) { name: "create annotation successfully", fieldsGetter: func(ctrl *gomock.Controller) fields { authMock := rpcmocks.NewMockIAuthProvider(ctrl) - authMock.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionAnnotationCreate, "1").Return(nil) + authMock.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionAnnotationCreate, "1", true).Return(nil) traceServiceMock := servicemocks.NewMockITraceService(ctrl) traceServiceMock.EXPECT().CreateAnnotation(gomock.Any(), gomock.Any()).Return(nil) benefitMock := benefitmocks.NewMockIBenefitService(ctrl) @@ -344,7 +344,7 @@ func TestOpenAPIApplication_DeleteAnnotation(t *testing.T) { name: "delete annotation successfully", fieldsGetter: func(ctrl *gomock.Controller) fields { authMock := rpcmocks.NewMockIAuthProvider(ctrl) - authMock.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionAnnotationCreate, "1").Return(nil) + authMock.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionAnnotationCreate, "1", true).Return(nil) traceServiceMock := servicemocks.NewMockITraceService(ctrl) traceServiceMock.EXPECT().DeleteAnnotation(gomock.Any(), gomock.Any()).Return(nil) benefitMock := benefitmocks.NewMockIBenefitService(ctrl) @@ -3539,4 +3539,4 @@ func TestOpenAPIApplication_unpackTenant(t *testing.T) { result = app.unpackTenant(context.Background(), []*loop_span.Span{{SpanID: "test"}}) assert.Len(t, result, 1) assert.Len(t, result["tenant1"], 1) -} +} \ No newline at end of file diff --git a/backend/modules/observability/application/trace.go b/backend/modules/observability/application/trace.go index 3b352a9b7..bc8dc1055 100644 --- a/backend/modules/observability/application/trace.go +++ b/backend/modules/observability/application/trace.go @@ -95,7 +95,7 @@ func (t *TraceApplication) ListSpans(ctx context.Context, req *trace.ListSpansRe } if err := t.authSvc.CheckWorkspacePermission(ctx, rpc.AuthActionTraceRead, - strconv.FormatInt(req.GetWorkspaceID(), 10)); err != nil { + strconv.FormatInt(req.GetWorkspaceID(), 10), false); err != nil { return nil, err } sReq, err := t.buildListSpansSvcReq(req) @@ -185,7 +185,7 @@ func (t *TraceApplication) GetTrace(ctx context.Context, req *trace.GetTraceRequ } if err := t.authSvc.CheckWorkspacePermission(ctx, rpc.AuthActionTraceRead, - strconv.FormatInt(req.GetWorkspaceID(), 10)); err != nil { + strconv.FormatInt(req.GetWorkspaceID(), 10), false); err != nil { return nil, err } sReq := t.buildGetTraceSvcReq(req) @@ -259,7 +259,7 @@ func (t *TraceApplication) BatchGetTracesAdvanceInfo(ctx context.Context, req *t } if err := t.authSvc.CheckWorkspacePermission(ctx, rpc.AuthActionTraceRead, - strconv.FormatInt(req.GetWorkspaceID(), 10)); err != nil { + strconv.FormatInt(req.GetWorkspaceID(), 10), false); err != nil { return nil, err } logs.CtxInfo(ctx, "Batch get traces advance info request: %+v", req) @@ -403,7 +403,7 @@ func (t *TraceApplication) validateIngestTracesInnerReq(ctx context.Context, req func (t *TraceApplication) GetTracesMetaInfo(ctx context.Context, req *trace.GetTracesMetaInfoRequest) (*trace.GetTracesMetaInfoResponse, error) { if err := t.authSvc.CheckWorkspacePermission(ctx, rpc.AuthActionTraceRead, - strconv.FormatInt(req.GetWorkspaceID(), 10)); err != nil { + strconv.FormatInt(req.GetWorkspaceID(), 10), false); err != nil { return nil, err } logs.CtxInfo(ctx, "Get traces meta info request: %+v", req) @@ -468,7 +468,7 @@ func (t *TraceApplication) CreateView(ctx context.Context, req *trace.CreateView } if err := t.authSvc.CheckWorkspacePermission(ctx, rpc.AuthActionTraceViewCreate, - strconv.FormatInt(req.GetWorkspaceID(), 10)); err != nil { + strconv.FormatInt(req.GetWorkspaceID(), 10), false); err != nil { return nil, err } userID := session.UserIDInCtxOrEmpty(ctx) @@ -556,7 +556,7 @@ func (t *TraceApplication) ListViews(ctx context.Context, req *trace.ListViewsRe } if err := t.authSvc.CheckWorkspacePermission(ctx, rpc.AuthActionTraceViewList, - strconv.FormatInt(req.GetWorkspaceID(), 10)); err != nil { + strconv.FormatInt(req.GetWorkspaceID(), 10), false); err != nil { return nil, err } systemViews, err := t.getSystemViews(ctx) @@ -600,7 +600,7 @@ func (t *TraceApplication) getSystemViews(ctx context.Context) ([]*view.View, er func (t *TraceApplication) CreateManualAnnotation(ctx context.Context, req *trace.CreateManualAnnotationRequest) (*trace.CreateManualAnnotationResponse, error) { if err := t.authSvc.CheckWorkspacePermission(ctx, rpc.AuthActionAnnotationCreate, - req.GetAnnotation().GetWorkspaceID()); err != nil { + req.GetAnnotation().GetWorkspaceID(), false); err != nil { return nil, err } platformType := loop_span.PlatformType(req.GetPlatformType()) @@ -636,7 +636,7 @@ func (t *TraceApplication) CreateManualAnnotation(ctx context.Context, req *trac func (t *TraceApplication) UpdateManualAnnotation(ctx context.Context, req *trace.UpdateManualAnnotationRequest) (*trace.UpdateManualAnnotationResponse, error) { if err := t.authSvc.CheckWorkspacePermission(ctx, rpc.AuthActionAnnotationCreate, - req.GetAnnotation().GetWorkspaceID()); err != nil { + req.GetAnnotation().GetWorkspaceID(), false); err != nil { return nil, err } platformType := loop_span.PlatformType(req.GetPlatformType()) @@ -671,7 +671,7 @@ func (t *TraceApplication) UpdateManualAnnotation(ctx context.Context, req *trac func (t *TraceApplication) DeleteManualAnnotation(ctx context.Context, req *trace.DeleteManualAnnotationRequest) (*trace.DeleteManualAnnotationResponse, error) { if err := t.authSvc.CheckWorkspacePermission(ctx, rpc.AuthActionAnnotationCreate, - strconv.FormatInt(req.GetWorkspaceID(), 10)); err != nil { + strconv.FormatInt(req.GetWorkspaceID(), 10), false); err != nil { return nil, err } platformType := loop_span.PlatformType(req.GetPlatformType()) @@ -699,7 +699,7 @@ func (t *TraceApplication) DeleteManualAnnotation(ctx context.Context, req *trac func (t *TraceApplication) ListAnnotations(ctx context.Context, req *trace.ListAnnotationsRequest) (*trace.ListAnnotationsResponse, error) { if err := t.authSvc.CheckWorkspacePermission(ctx, rpc.AuthActionTraceRead, - strconv.FormatInt(req.GetWorkspaceID(), 10)); err != nil { + strconv.FormatInt(req.GetWorkspaceID(), 10), false); err != nil { return nil, err } platformType := loop_span.PlatformType(req.GetPlatformType()) @@ -774,7 +774,7 @@ func (t *TraceApplication) ExportTracesToDataset(ctx context.Context, req *trace } spaceID := strconv.FormatInt(req.GetWorkspaceID(), 10) - if err := t.authSvc.CheckWorkspacePermission(ctx, rpc.AuthActionTraceExport, spaceID); err != nil { + if err := t.authSvc.CheckWorkspacePermission(ctx, rpc.AuthActionTraceExport, spaceID, false); err != nil { return nil, err } @@ -811,7 +811,7 @@ func (t *TraceApplication) PreviewExportTracesToDataset(ctx context.Context, req } spaceID := strconv.FormatInt(req.GetWorkspaceID(), 10) - if err := t.authSvc.CheckWorkspacePermission(ctx, rpc.AuthActionTracePreviewExport, spaceID); err != nil { + if err := t.authSvc.CheckWorkspacePermission(ctx, rpc.AuthActionTracePreviewExport, spaceID, false); err != nil { return nil, err } @@ -826,4 +826,4 @@ func (t *TraceApplication) PreviewExportTracesToDataset(ctx context.Context, req // 转换响应 return tconv.PreviewResponseDO2DTO(serviceResp), nil -} +} \ No newline at end of file diff --git a/backend/modules/observability/application/trace_test.go b/backend/modules/observability/application/trace_test.go index bfe17cd79..cae4e1269 100644 --- a/backend/modules/observability/application/trace_test.go +++ b/backend/modules/observability/application/trace_test.go @@ -56,7 +56,7 @@ func TestTraceApplication_CreateView(t *testing.T) { fieldsGetter: func(ctrl *gomock.Controller) fields { mockRepo := repomock.NewMockIViewRepo(ctrl) mockAuth := rpcmock.NewMockIAuthProvider(ctrl) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) mockRepo.EXPECT().CreateView(gomock.Any(), gomock.Any()).Return(int64(0), nil) return fields{ repo: mockRepo, @@ -81,7 +81,7 @@ func TestTraceApplication_CreateView(t *testing.T) { fieldsGetter: func(ctrl *gomock.Controller) fields { mockRepo := repomock.NewMockIViewRepo(ctrl) mockAuth := rpcmock.NewMockIAuthProvider(ctrl) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) mockRepo.EXPECT().CreateView(gomock.Any(), gomock.Any()).Return(int64(0), assert.AnError) return fields{ repo: mockRepo, @@ -296,7 +296,7 @@ func TestTraceApplication_ListViews(t *testing.T) { mockRepo := repomock.NewMockIViewRepo(ctrl) mockAuth := rpcmock.NewMockIAuthProvider(ctrl) mockConf := confmock.NewMockITraceConfig(ctrl) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) mockRepo.EXPECT().ListViews(gomock.Any(), gomock.Any(), gomock.Any()).Return([]*entity.ObservabilityView{}, nil) mockConf.EXPECT().GetSystemViews(gomock.Any()).Return([]*config.SystemView{}, nil) return fields{ @@ -322,7 +322,7 @@ func TestTraceApplication_ListViews(t *testing.T) { mockRepo := repomock.NewMockIViewRepo(ctrl) mockAuth := rpcmock.NewMockIAuthProvider(ctrl) mockConf := confmock.NewMockITraceConfig(ctrl) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) mockRepo.EXPECT().ListViews(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, assert.AnError) mockConf.EXPECT().GetSystemViews(gomock.Any()).Return([]*config.SystemView{}, nil) return fields{ @@ -390,7 +390,7 @@ func TestTraceApplication_ListSpans(t *testing.T) { mockTag.EXPECT().BatchGetTagInfo(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil) mockEval.EXPECT().BatchGetEvaluatorVersions(gomock.Any(), gomock.Any()).Return(nil, nil, nil) mockUser.EXPECT().GetUserInfo(gomock.Any(), gomock.Any()).Return(nil, nil, nil) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) mockSvc.EXPECT().ListSpans(gomock.Any(), gomock.Any()).Return(&service.ListSpansResp{ Spans: loop_span.SpanList{ { @@ -584,7 +584,7 @@ func TestTraceApplication_ListSpans(t *testing.T) { mockSvc := svcmock.NewMockITraceService(ctrl) mockAuth := rpcmock.NewMockIAuthProvider(ctrl) mockCfg := confmock.NewMockITraceConfig(ctrl) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) mockSvc.EXPECT().ListSpans(gomock.Any(), gomock.Any()).Return(nil, assert.AnError) mockCfg.EXPECT().GetTraceDataMaxDurationDay(gomock.Any(), gomock.Any()).Return(int64(100)) return fields{ @@ -609,7 +609,7 @@ func TestTraceApplication_ListSpans(t *testing.T) { fieldsGetter: func(ctrl *gomock.Controller) fields { mockAuth := rpcmock.NewMockIAuthProvider(ctrl) mockCfg := confmock.NewMockITraceConfig(ctrl) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any()).Return(fmt.Errorf("bad")) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(fmt.Errorf("bad")) mockCfg.EXPECT().GetTraceDataMaxDurationDay(gomock.Any(), gomock.Any()).Return(int64(100)) return fields{ auth: mockAuth, @@ -687,7 +687,7 @@ func TestTraceApplication_GetTrace(t *testing.T) { mockSvc := svcmock.NewMockITraceService(ctrl) mockAuth := rpcmock.NewMockIAuthProvider(ctrl) mockCfg := confmock.NewMockITraceConfig(ctrl) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) mockSvc.EXPECT().GetTrace(gomock.Any(), gomock.Any()).Return(&service.GetTraceResp{}, nil) mockCfg.EXPECT().GetTraceDataMaxDurationDay(gomock.Any(), gomock.Any()).Return(int64(100)) return fields{ @@ -719,7 +719,7 @@ func TestTraceApplication_GetTrace(t *testing.T) { mockSvc := svcmock.NewMockITraceService(ctrl) mockAuth := rpcmock.NewMockIAuthProvider(ctrl) mockCfg := confmock.NewMockITraceConfig(ctrl) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) mockSvc.EXPECT().GetTrace(gomock.Any(), gomock.Any()).Return(nil, assert.AnError) mockCfg.EXPECT().GetTraceDataMaxDurationDay(gomock.Any(), gomock.Any()).Return(int64(100)) return fields{ @@ -745,7 +745,7 @@ func TestTraceApplication_GetTrace(t *testing.T) { fieldsGetter: func(ctrl *gomock.Controller) fields { mockAuth := rpcmock.NewMockIAuthProvider(ctrl) mockCfg := confmock.NewMockITraceConfig(ctrl) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any()).Return(fmt.Errorf("bad")) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(fmt.Errorf("bad")) mockCfg.EXPECT().GetTraceDataMaxDurationDay(gomock.Any(), gomock.Any()).Return(int64(100)) return fields{ auth: mockAuth, @@ -787,7 +787,7 @@ func TestTraceApplication_GetTrace(t *testing.T) { mockSvc := svcmock.NewMockITraceService(ctrl) mockAuth := rpcmock.NewMockIAuthProvider(ctrl) mockCfg := confmock.NewMockITraceConfig(ctrl) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) mockSvc.EXPECT().GetTrace(gomock.Any(), gomock.Any()).Return(&service.GetTraceResp{}, nil) mockCfg.EXPECT().GetTraceDataMaxDurationDay(gomock.Any(), gomock.Any()).Return(int64(100)) return fields{ @@ -857,7 +857,7 @@ func TestTraceApplication_BatchGetTracesAdvanceInfo(t *testing.T) { mockCfg := confmock.NewMockITraceConfig(ctrl) mockCfg.EXPECT().GetTraceDataMaxDurationDay(gomock.Any(), gomock.Any()).Return(int64(100)) mockSvc.EXPECT().GetTracesAdvanceInfo(gomock.Any(), gomock.Any()).Return(&service.GetTracesAdvanceInfoResp{}, nil) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) return fields{ traceSvc: mockSvc, auth: mockAuth, @@ -890,7 +890,7 @@ func TestTraceApplication_BatchGetTracesAdvanceInfo(t *testing.T) { mockCfg := confmock.NewMockITraceConfig(ctrl) mockCfg.EXPECT().GetTraceDataMaxDurationDay(gomock.Any(), gomock.Any()).Return(int64(100)) mockSvc.EXPECT().GetTracesAdvanceInfo(gomock.Any(), gomock.Any()).Return(nil, assert.AnError) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) return fields{ traceSvc: mockSvc, auth: mockAuth, @@ -1016,7 +1016,7 @@ func TestTraceApplication_GetTracesMetaInfo(t *testing.T) { fieldsGetter: func(ctrl *gomock.Controller) fields { mockSvc := svcmock.NewMockITraceService(ctrl) mockAuth := rpcmock.NewMockIAuthProvider(ctrl) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) mockSvc.EXPECT().GetTracesMetaInfo(gomock.Any(), gomock.Any()).Return(&service.GetTracesMetaInfoResp{FilesMetas: map[string]*config.FieldMeta{}}, nil) return fields{ traceSvc: mockSvc, @@ -1069,7 +1069,7 @@ func TestTraceApplication_CreateManualAnnotation(t *testing.T) { mockSvc := svcmock.NewMockITraceService(ctrl) mockAuth := rpcmock.NewMockIAuthProvider(ctrl) mockTag := rpcmock.NewMockITagRPCAdapter(ctrl) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) mockTag.EXPECT().GetTagInfo(gomock.Any(), gomock.Any(), gomock.Any()).Return(&rpc.TagInfo{}, nil) return fields{ traceSvc: mockSvc, @@ -1096,7 +1096,7 @@ func TestTraceApplication_CreateManualAnnotation(t *testing.T) { mockSvc := svcmock.NewMockITraceService(ctrl) mockAuth := rpcmock.NewMockIAuthProvider(ctrl) mockTag := rpcmock.NewMockITagRPCAdapter(ctrl) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) mockTag.EXPECT().GetTagInfo(gomock.Any(), gomock.Any(), gomock.Any()).Return(&rpc.TagInfo{ TagKeyId: 1, TagContentType: rpc.TagContentTypeContinuousNumber, @@ -1126,7 +1126,7 @@ func TestTraceApplication_CreateManualAnnotation(t *testing.T) { mockSvc := svcmock.NewMockITraceService(ctrl) mockAuth := rpcmock.NewMockIAuthProvider(ctrl) mockTag := rpcmock.NewMockITagRPCAdapter(ctrl) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) mockTag.EXPECT().GetTagInfo(gomock.Any(), gomock.Any(), gomock.Any()).Return(&rpc.TagInfo{ TagKeyId: 1, TagContentType: rpc.TagContentTypeFreeText, @@ -1196,7 +1196,7 @@ func TestTraceApplication_UpdateManualAnnotation(t *testing.T) { mockSvc := svcmock.NewMockITraceService(ctrl) mockAuth := rpcmock.NewMockIAuthProvider(ctrl) mockTag := rpcmock.NewMockITagRPCAdapter(ctrl) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) mockTag.EXPECT().GetTagInfo(gomock.Any(), gomock.Any(), gomock.Any()).Return(&rpc.TagInfo{}, nil) return fields{ traceSvc: mockSvc, @@ -1223,7 +1223,7 @@ func TestTraceApplication_UpdateManualAnnotation(t *testing.T) { mockSvc := svcmock.NewMockITraceService(ctrl) mockAuth := rpcmock.NewMockIAuthProvider(ctrl) mockTag := rpcmock.NewMockITagRPCAdapter(ctrl) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) mockTag.EXPECT().GetTagInfo(gomock.Any(), gomock.Any(), gomock.Any()).Return(&rpc.TagInfo{ TagKeyId: 1, TagContentType: rpc.TagContentTypeContinuousNumber, @@ -1253,7 +1253,7 @@ func TestTraceApplication_UpdateManualAnnotation(t *testing.T) { mockSvc := svcmock.NewMockITraceService(ctrl) mockAuth := rpcmock.NewMockIAuthProvider(ctrl) mockTag := rpcmock.NewMockITagRPCAdapter(ctrl) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) mockTag.EXPECT().GetTagInfo(gomock.Any(), gomock.Any(), gomock.Any()).Return(&rpc.TagInfo{ TagKeyId: 1, TagContentType: rpc.TagContentTypeFreeText, @@ -1317,7 +1317,7 @@ func TestTraceApplication_DeleteManualAnnotation(t *testing.T) { mockSvc := svcmock.NewMockITraceService(ctrl) mockAuth := rpcmock.NewMockIAuthProvider(ctrl) mockTag := rpcmock.NewMockITagRPCAdapter(ctrl) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) mockTag.EXPECT().GetTagInfo(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("fail")) return fields{ traceSvc: mockSvc, @@ -1340,7 +1340,7 @@ func TestTraceApplication_DeleteManualAnnotation(t *testing.T) { mockSvc := svcmock.NewMockITraceService(ctrl) mockAuth := rpcmock.NewMockIAuthProvider(ctrl) mockTag := rpcmock.NewMockITagRPCAdapter(ctrl) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) mockTag.EXPECT().GetTagInfo(gomock.Any(), gomock.Any(), gomock.Any()).Return(&rpc.TagInfo{ TagKeyId: 1, TagContentType: rpc.TagContentTypeFreeText, @@ -1403,7 +1403,7 @@ func TestTraceApplication_ExportTracesToDataset(t *testing.T) { mockConfig := confmock.NewMockITraceConfig(ctrl) mockConfig.EXPECT().GetTraceDataMaxDurationDay(gomock.Any(), gomock.Any()).Return(int64(30)) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionTraceExport, "123").Return(nil) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionTraceExport, "123", gomock.Any()).Return(nil) mockExportSvc.EXPECT().ExportTracesToDataset(gomock.Any(), gomock.Any()).Return(&service.ExportTracesToDatasetResponse{ SuccessCount: 10, DatasetID: 1, @@ -1464,7 +1464,7 @@ func TestTraceApplication_ExportTracesToDataset(t *testing.T) { mockConfig := confmock.NewMockITraceConfig(ctrl) mockConfig.EXPECT().GetTraceDataMaxDurationDay(gomock.Any(), gomock.Any()).Return(int64(30)) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionTraceExport, "123").Return(fmt.Errorf("permission denied")) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionTraceExport, "123", gomock.Any()).Return(fmt.Errorf("permission denied")) return fields{ authSvc: mockAuth, @@ -1503,7 +1503,7 @@ func TestTraceApplication_ExportTracesToDataset(t *testing.T) { mockConfig := confmock.NewMockITraceConfig(ctrl) mockConfig.EXPECT().GetTraceDataMaxDurationDay(gomock.Any(), gomock.Any()).Return(int64(30)) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionTraceExport, "123").Return(nil) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionTraceExport, "123", gomock.Any()).Return(nil) mockExportSvc.EXPECT().ExportTracesToDataset(gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("service error")) return fields{ @@ -1580,7 +1580,7 @@ func TestTraceApplication_PreviewExportTracesToDataset(t *testing.T) { mockConfig := confmock.NewMockITraceConfig(ctrl) mockConfig.EXPECT().GetTraceDataMaxDurationDay(gomock.Any(), gomock.Any()).Return(int64(30)) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionTracePreviewExport, "123").Return(nil) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionTracePreviewExport, "123", gomock.Any()).Return(nil) mockExportSvc.EXPECT().PreviewExportTracesToDataset(gomock.Any(), gomock.Any()).Return(&service.PreviewExportTracesToDatasetResponse{ Items: []*entity.DatasetItem{ { @@ -1658,33 +1658,33 @@ func TestTraceApplication_PreviewExportTracesToDataset(t *testing.T) { mockConfig := confmock.NewMockITraceConfig(ctrl) mockConfig.EXPECT().GetTraceDataMaxDurationDay(gomock.Any(), gomock.Any()).Return(int64(30)) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionTracePreviewExport, "123").Return(fmt.Errorf("permission denied")) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionTracePreviewExport, "123", gomock.Any()).Return(fmt.Errorf("permission denied")) return fields{ authSvc: mockAuth, traceConfig: mockConfig, } }, args: args{ - ctx: context.Background(), - req: &trace.PreviewExportTracesToDatasetRequest{ - WorkspaceID: 123, - StartTime: time.Now().Add(-time.Hour).UnixMilli(), - EndTime: time.Now().UnixMilli(), - SpanIds: []*trace.SpanID{ - {TraceID: "trace1", SpanID: "span1"}, - }, - FieldMappings: []*dataset0.FieldMapping{ - { - FieldSchema: &dataset0.FieldSchema{ - Key: ptr.Of("input"), - Name: ptr.Of("Input"), + ctx: context.Background(), + req: &trace.PreviewExportTracesToDatasetRequest{ + WorkspaceID: 123, + StartTime: time.Now().Add(-time.Hour).UnixMilli(), + EndTime: time.Now().UnixMilli(), + SpanIds: []*trace.SpanID{ + {TraceID: "trace1", SpanID: "span1"}, + }, + FieldMappings: []*dataset0.FieldMapping{ + { + FieldSchema: &dataset0.FieldSchema{ + Key: ptr.Of("input"), + Name: ptr.Of("Input"), + }, + TraceFieldKey: "input", + TraceFieldJsonpath: "$.input", }, - TraceFieldKey: "input", - TraceFieldJsonpath: "$.input", }, }, }, - }, want: nil, wantErr: true, }, @@ -1696,7 +1696,7 @@ func TestTraceApplication_PreviewExportTracesToDataset(t *testing.T) { mockConfig := confmock.NewMockITraceConfig(ctrl) mockConfig.EXPECT().GetTraceDataMaxDurationDay(gomock.Any(), gomock.Any()).Return(int64(30)) - mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionTracePreviewExport, "123").Return(nil) + mockAuth.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionTracePreviewExport, "123", gomock.Any()).Return(nil) mockExportSvc.EXPECT().PreviewExportTracesToDataset(gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("service error")) return fields{ @@ -1746,4 +1746,4 @@ func TestTraceApplication_PreviewExportTracesToDataset(t *testing.T) { assert.Equal(t, tt.want, got) }) } -} +} \ No newline at end of file diff --git a/backend/modules/observability/domain/component/rpc/auth.go b/backend/modules/observability/domain/component/rpc/auth.go index 111177011..d52e5bc29 100644 --- a/backend/modules/observability/domain/component/rpc/auth.go +++ b/backend/modules/observability/domain/component/rpc/auth.go @@ -19,9 +19,8 @@ const ( //go:generate mockgen -destination=mocks/auth_provider.go -package=mocks . IAuthProvider type IAuthProvider interface { - CheckWorkspacePermission(ctx context.Context, action, workspaceId string) error + CheckWorkspacePermission(ctx context.Context, action, workspaceId string, isOpi bool) error CheckViewPermission(ctx context.Context, action, workspaceId, viewId string) error - CheckOpenAPIWorkspacePermission(ctx context.Context, action, workspaceId string) error CheckIngestPermission(ctx context.Context, workspaceId string) error CheckQueryPermission(ctx context.Context, workspaceId, platformType string) error } diff --git a/backend/modules/observability/domain/component/rpc/mocks/auth_provider.go b/backend/modules/observability/domain/component/rpc/mocks/auth_provider.go index 08c6bfa7c..b2e767f1b 100644 --- a/backend/modules/observability/domain/component/rpc/mocks/auth_provider.go +++ b/backend/modules/observability/domain/component/rpc/mocks/auth_provider.go @@ -54,20 +54,6 @@ func (mr *MockIAuthProviderMockRecorder) CheckIngestPermission(ctx, workspaceId return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckIngestPermission", reflect.TypeOf((*MockIAuthProvider)(nil).CheckIngestPermission), ctx, workspaceId) } -// CheckOpenAPIWorkspacePermission mocks base method. -func (m *MockIAuthProvider) CheckOpenAPIWorkspacePermission(ctx context.Context, action, workspaceId string) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CheckOpenAPIWorkspacePermission", ctx, action, workspaceId) - ret0, _ := ret[0].(error) - return ret0 -} - -// CheckOpenAPIWorkspacePermission indicates an expected call of CheckOpenAPIWorkspacePermission. -func (mr *MockIAuthProviderMockRecorder) CheckOpenAPIWorkspacePermission(ctx, action, workspaceId any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckOpenAPIWorkspacePermission", reflect.TypeOf((*MockIAuthProvider)(nil).CheckOpenAPIWorkspacePermission), ctx, action, workspaceId) -} - // CheckQueryPermission mocks base method. func (m *MockIAuthProvider) CheckQueryPermission(ctx context.Context, workspaceId, platformType string) error { m.ctrl.T.Helper() @@ -97,15 +83,15 @@ func (mr *MockIAuthProviderMockRecorder) CheckViewPermission(ctx, action, worksp } // CheckWorkspacePermission mocks base method. -func (m *MockIAuthProvider) CheckWorkspacePermission(ctx context.Context, action, workspaceId string) error { +func (m *MockIAuthProvider) CheckWorkspacePermission(ctx context.Context, action, workspaceId string, isOpi bool) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CheckWorkspacePermission", ctx, action, workspaceId) + ret := m.ctrl.Call(m, "CheckWorkspacePermission", ctx, action, workspaceId, isOpi) ret0, _ := ret[0].(error) return ret0 } // CheckWorkspacePermission indicates an expected call of CheckWorkspacePermission. -func (mr *MockIAuthProviderMockRecorder) CheckWorkspacePermission(ctx, action, workspaceId any) *gomock.Call { +func (mr *MockIAuthProviderMockRecorder) CheckWorkspacePermission(ctx, action, workspaceId, isOpi any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckWorkspacePermission", reflect.TypeOf((*MockIAuthProvider)(nil).CheckWorkspacePermission), ctx, action, workspaceId) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckWorkspacePermission", reflect.TypeOf((*MockIAuthProvider)(nil).CheckWorkspacePermission), ctx, action, workspaceId, isOpi) } diff --git a/backend/modules/observability/domain/trace/service/trace_service_test.go b/backend/modules/observability/domain/trace/service/trace_service_test.go index a8cfc4171..44bba514d 100644 --- a/backend/modules/observability/domain/trace/service/trace_service_test.go +++ b/backend/modules/observability/domain/trace/service/trace_service_test.go @@ -1656,7 +1656,7 @@ func TestTraceServiceImpl_CreateAnnotation(t *testing.T) { }, }, }, nil) - + // Mock ListSpans call with ParentID filter for root span repoMock.EXPECT().ListSpans(gomock.Any(), gomock.Any()).DoAndReturn( func(ctx context.Context, param *repo.ListSpansParam) (*repo.ListSpansResult, error) { @@ -1670,7 +1670,7 @@ func TestTraceServiceImpl_CreateAnnotation(t *testing.T) { } } assert.True(t, hasParentIDFilter, "Should have ParentID filter when span_id is empty") - + // Return a root span (ParentID = "0") return &repo.ListSpansResult{ Spans: loop_span.SpanList{ @@ -1729,7 +1729,7 @@ func TestTraceServiceImpl_CreateAnnotation(t *testing.T) { }, }, }, nil) - + // Mock ListSpans call with ParentID filter but return no spans repoMock.EXPECT().ListSpans(gomock.Any(), gomock.Any()).DoAndReturn( func(ctx context.Context, param *repo.ListSpansParam) (*repo.ListSpansResult, error) { @@ -1743,7 +1743,7 @@ func TestTraceServiceImpl_CreateAnnotation(t *testing.T) { } } assert.True(t, hasParentIDFilter, "Should have ParentID filter when span_id is empty") - + // Return empty result (no root span found) return &repo.ListSpansResult{Spans: loop_span.SpanList{}}, nil }, @@ -1880,7 +1880,7 @@ func TestTraceServiceImpl_DeleteAnnotation(t *testing.T) { }, }, }, nil) - + // Mock ListSpans call with ParentID filter for root span repoMock.EXPECT().ListSpans(gomock.Any(), gomock.Any()).DoAndReturn( func(ctx context.Context, param *repo.ListSpansParam) (*repo.ListSpansResult, error) { @@ -1894,7 +1894,7 @@ func TestTraceServiceImpl_DeleteAnnotation(t *testing.T) { } } assert.True(t, hasParentIDFilter, "Should have ParentID filter when span_id is empty") - + // Return a root span (ParentID = "0") return &repo.ListSpansResult{ Spans: loop_span.SpanList{ @@ -1950,7 +1950,7 @@ func TestTraceServiceImpl_DeleteAnnotation(t *testing.T) { }, }, }, nil) - + // Mock ListSpans call with ParentID filter but return no spans repoMock.EXPECT().ListSpans(gomock.Any(), gomock.Any()).DoAndReturn( func(ctx context.Context, param *repo.ListSpansParam) (*repo.ListSpansResult, error) { @@ -1964,7 +1964,7 @@ func TestTraceServiceImpl_DeleteAnnotation(t *testing.T) { } } assert.True(t, hasParentIDFilter, "Should have ParentID filter when span_id is empty") - + // Return empty result (no root span found) return &repo.ListSpansResult{Spans: loop_span.SpanList{}}, nil }, @@ -2846,4 +2846,4 @@ func TestTraceServiceImpl_ListSpansOApi(t *testing.T) { } }) } -} \ No newline at end of file +} diff --git a/backend/modules/observability/infra/rpc/auth/auth.go b/backend/modules/observability/infra/rpc/auth/auth.go index e7dd34615..dd01cb3f0 100644 --- a/backend/modules/observability/infra/rpc/auth/auth.go +++ b/backend/modules/observability/infra/rpc/auth/auth.go @@ -21,7 +21,7 @@ type AuthProviderImpl struct { cli authservice.Client } -func (a *AuthProviderImpl) CheckWorkspacePermission(ctx context.Context, action, workspaceId string) error { +func (a *AuthProviderImpl) CheckWorkspacePermission(ctx context.Context, action, workspaceId string, _ bool) error { authInfos := make([]*authentity.SubjectActionObjects, 0) authInfos = append(authInfos, &authentity.SubjectActionObjects{ Subject: &authentity.AuthPrincipal{ @@ -113,16 +113,12 @@ func (a *AuthProviderImpl) CheckViewPermission(ctx context.Context, action, work return nil } -func (a *AuthProviderImpl) CheckOpenAPIWorkspacePermission(ctx context.Context, action, workspaceId string) error { - return a.CheckWorkspacePermission(ctx, action, workspaceId) -} - func (a *AuthProviderImpl) CheckIngestPermission(ctx context.Context, workspaceId string) error { - return a.CheckWorkspacePermission(ctx, rpc.AuthActionTraceIngest, workspaceId) + return a.CheckWorkspacePermission(ctx, rpc.AuthActionTraceIngest, workspaceId, true) } func (a *AuthProviderImpl) CheckQueryPermission(ctx context.Context, workspaceId, platformType string) error { - return a.CheckWorkspacePermission(ctx, rpc.AuthActionTraceList, workspaceId) + return a.CheckWorkspacePermission(ctx, rpc.AuthActionTraceList, workspaceId, true) } func NewAuthProvider(cli authservice.Client) rpc.IAuthProvider { From 24651038cb407ab061a25aa25ad68fa051c34a9f Mon Sep 17 00:00:00 2001 From: lizhanpeng Date: Fri, 5 Sep 2025 14:37:27 +0800 Subject: [PATCH 07/13] feat: [Coda] improve observability module gofmt and test coverage Co-Authored-By: Coda --- .../application/utils/date_validator_test.go | 282 +++++++ .../domain/component/rpc/auth_test.go | 103 +++ .../observability/infra/rpc/auth/auth_test.go | 708 ++++++------------ .../pkg/errno/observability_test.go | 292 ++++++++ 4 files changed, 902 insertions(+), 483 deletions(-) create mode 100755 backend/modules/observability/application/utils/date_validator_test.go create mode 100755 backend/modules/observability/domain/component/rpc/auth_test.go create mode 100755 backend/modules/observability/pkg/errno/observability_test.go diff --git a/backend/modules/observability/application/utils/date_validator_test.go b/backend/modules/observability/application/utils/date_validator_test.go new file mode 100755 index 000000000..3a14fb63d --- /dev/null +++ b/backend/modules/observability/application/utils/date_validator_test.go @@ -0,0 +1,282 @@ +// Copyright (c) 2025 coze-dev Authors +// SPDX-License-Identifier: Apache-2.0 + +package utils + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + + obErrorx "github.com/coze-dev/coze-loop/backend/modules/observability/pkg/errno" +) + +func TestDateValidator_CorrectDate(t *testing.T) { + now := time.Now() + todayEnd := EndTimeOfDay(now.UnixMilli()) + earliestTime := StartTimeOfDay(now.UnixMilli()) - int64(time.Duration(365*HoursPerDay)*time.Hour/time.Millisecond) + + tests := []struct { + name string + validator DateValidator + wantStart int64 + wantEnd int64 + wantErr bool + expectedError int + }{ + { + name: "valid time range within limits", + validator: DateValidator{ + Start: now.Add(-1 * time.Hour).UnixMilli(), + End: now.UnixMilli(), + EarliestDays: 365, + }, + wantStart: now.Add(-1 * time.Hour).UnixMilli(), + wantEnd: now.UnixMilli(), + wantErr: false, + }, + { + name: "start time is zero", + validator: DateValidator{ + Start: 0, + End: now.UnixMilli(), + EarliestDays: 365, + }, + wantStart: 0, + wantEnd: 0, + wantErr: true, + expectedError: obErrorx.CommercialCommonInvalidParamCodeCode, + }, + { + name: "end time is zero", + validator: DateValidator{ + Start: now.UnixMilli(), + End: 0, + EarliestDays: 365, + }, + wantStart: 0, + wantEnd: 0, + wantErr: true, + expectedError: obErrorx.CommercialCommonInvalidParamCodeCode, + }, + { + name: "start time is negative", + validator: DateValidator{ + Start: -1, + End: now.UnixMilli(), + EarliestDays: 365, + }, + wantStart: 0, + wantEnd: 0, + wantErr: true, + expectedError: obErrorx.CommercialCommonInvalidParamCodeCode, + }, + { + name: "start time greater than end time", + validator: DateValidator{ + Start: now.UnixMilli(), + End: now.Add(-1 * time.Hour).UnixMilli(), + EarliestDays: 365, + }, + wantStart: 0, + wantEnd: 0, + wantErr: true, + expectedError: obErrorx.CommercialCommonInvalidParamCodeCode, + }, + { + name: "both start and end exceed today", + validator: DateValidator{ + Start: todayEnd + 1000, + End: todayEnd + 2000, + EarliestDays: 365, + }, + wantStart: 0, + wantEnd: 0, + wantErr: true, + expectedError: obErrorx.CommercialCommonInvalidParamCodeCode, + }, + { + name: "both start and end exceed max days ago", + validator: DateValidator{ + Start: earliestTime - 2000, + End: earliestTime - 1000, + EarliestDays: 365, + }, + wantStart: 0, + wantEnd: 0, + wantErr: true, + expectedError: obErrorx.CommercialCommonInvalidParamCodeCode, + }, + { + name: "start time before earliest, end time valid - should correct start time", + validator: DateValidator{ + Start: earliestTime - 1000, + End: now.UnixMilli(), + EarliestDays: 365, + }, + wantStart: earliestTime, + wantEnd: now.UnixMilli(), + wantErr: false, + }, + { + name: "end time exceeds today, start time valid - should correct end time", + validator: DateValidator{ + Start: now.Add(-1 * time.Hour).UnixMilli(), + End: todayEnd + 1000, + EarliestDays: 365, + }, + wantStart: now.Add(-1 * time.Hour).UnixMilli(), + wantEnd: todayEnd, + wantErr: false, + }, + { + name: "both start and end need correction", + validator: DateValidator{ + Start: earliestTime - 1000, + End: todayEnd + 1000, + EarliestDays: 365, + }, + wantStart: earliestTime, + wantEnd: todayEnd, + wantErr: false, + }, + { + name: "edge case - start equals earliest time", + validator: DateValidator{ + Start: earliestTime, + End: now.UnixMilli(), + EarliestDays: 365, + }, + wantStart: earliestTime, + wantEnd: now.UnixMilli(), + wantErr: false, + }, + { + name: "edge case - end equals latest time", + validator: DateValidator{ + Start: now.Add(-1 * time.Hour).UnixMilli(), + End: todayEnd, + EarliestDays: 365, + }, + wantStart: now.Add(-1 * time.Hour).UnixMilli(), + wantEnd: todayEnd, + wantErr: false, + }, + { + name: "different earliest days setting", + validator: DateValidator{ + Start: now.Add(-10 * 24 * time.Hour).UnixMilli(), + End: now.UnixMilli(), + EarliestDays: 7, // only 7 days allowed + }, + wantStart: StartTimeOfDay(now.UnixMilli()) - int64(time.Duration(7*HoursPerDay)*time.Hour/time.Millisecond), + wantEnd: now.UnixMilli(), + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotStart, gotEnd, err := tt.validator.CorrectDate() + + if tt.wantErr { + assert.Error(t, err) + if tt.expectedError != 0 { + // Check if error contains the expected error code + assert.Contains(t, err.Error(), "600904002") + } + assert.Equal(t, tt.wantStart, gotStart) + assert.Equal(t, tt.wantEnd, gotEnd) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.wantStart, gotStart) + assert.Equal(t, tt.wantEnd, gotEnd) + } + }) + } +} + +func TestStartTimeOfDay(t *testing.T) { + // Test with a specific timestamp + testTime := time.Date(2025, 1, 15, 14, 30, 45, 123456789, time.Local) + expected := time.Date(2025, 1, 15, 0, 0, 0, 0, time.Local) + + result := StartTimeOfDay(testTime.UnixMilli()) + assert.Equal(t, expected.UnixMilli(), result) +} + +func TestEndTimeOfDay(t *testing.T) { + // Test with a specific timestamp + testTime := time.Date(2025, 1, 15, 14, 30, 45, 123456789, time.Local) + expected := time.Date(2025, 1, 15, 23, 59, 59, 999999999, time.Local) + + result := EndTimeOfDay(testTime.UnixMilli()) + assert.Equal(t, expected.UnixMilli(), result) +} + +func TestStartTimeOfDay_EdgeCases(t *testing.T) { + tests := []struct { + name string + input time.Time + expected time.Time + }{ + { + name: "beginning of day", + input: time.Date(2025, 1, 15, 0, 0, 0, 0, time.Local), + expected: time.Date(2025, 1, 15, 0, 0, 0, 0, time.Local), + }, + { + name: "end of day", + input: time.Date(2025, 1, 15, 23, 59, 59, 999999999, time.Local), + expected: time.Date(2025, 1, 15, 0, 0, 0, 0, time.Local), + }, + { + name: "leap year date", + input: time.Date(2024, 2, 29, 12, 0, 0, 0, time.Local), + expected: time.Date(2024, 2, 29, 0, 0, 0, 0, time.Local), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := StartTimeOfDay(tt.input.UnixMilli()) + assert.Equal(t, tt.expected.UnixMilli(), result) + }) + } +} + +func TestEndTimeOfDay_EdgeCases(t *testing.T) { + tests := []struct { + name string + input time.Time + expected time.Time + }{ + { + name: "beginning of day", + input: time.Date(2025, 1, 15, 0, 0, 0, 0, time.Local), + expected: time.Date(2025, 1, 15, 23, 59, 59, 999999999, time.Local), + }, + { + name: "end of day", + input: time.Date(2025, 1, 15, 23, 59, 59, 999999999, time.Local), + expected: time.Date(2025, 1, 15, 23, 59, 59, 999999999, time.Local), + }, + { + name: "leap year date", + input: time.Date(2024, 2, 29, 12, 0, 0, 0, time.Local), + expected: time.Date(2024, 2, 29, 23, 59, 59, 999999999, time.Local), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := EndTimeOfDay(tt.input.UnixMilli()) + assert.Equal(t, tt.expected.UnixMilli(), result) + }) + } +} + +func TestHoursPerDay_Constant(t *testing.T) { + assert.Equal(t, 24, HoursPerDay) +} diff --git a/backend/modules/observability/domain/component/rpc/auth_test.go b/backend/modules/observability/domain/component/rpc/auth_test.go new file mode 100755 index 000000000..d3dde92ca --- /dev/null +++ b/backend/modules/observability/domain/component/rpc/auth_test.go @@ -0,0 +1,103 @@ +// Copyright (c) 2025 coze-dev Authors +// SPDX-License-Identifier: Apache-2.0 + +package rpc + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestAuthActionConstants(t *testing.T) { + tests := []struct { + name string + constant string + expected string + }{ + { + name: "AuthActionTraceRead", + constant: AuthActionTraceRead, + expected: "readLoopTrace", + }, + { + name: "AuthActionTraceIngest", + constant: AuthActionTraceIngest, + expected: "ingestLoopTrace", + }, + { + name: "AuthActionTraceViewCreate", + constant: AuthActionTraceViewCreate, + expected: "createLoopTraceView", + }, + { + name: "AuthActionTraceViewList", + constant: AuthActionTraceViewList, + expected: "listLoopTraceView", + }, + { + name: "AuthActionTraceViewEdit", + constant: AuthActionTraceViewEdit, + expected: "edit", + }, + { + name: "AuthActionAnnotationCreate", + constant: AuthActionAnnotationCreate, + expected: "createLoopTraceAnnotation", + }, + { + name: "AuthActionTraceExport", + constant: AuthActionTraceExport, + expected: "exportLoopTrace", + }, + { + name: "AuthActionTracePreviewExport", + constant: AuthActionTracePreviewExport, + expected: "previewExportLoopTrace", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, tt.constant) + }) + } +} + +func TestAuthActionConstants_Uniqueness(t *testing.T) { + // Test that all auth action constants are unique + actions := []string{ + AuthActionTraceRead, + AuthActionTraceIngest, + AuthActionTraceViewCreate, + AuthActionTraceViewList, + AuthActionTraceViewEdit, + AuthActionAnnotationCreate, + AuthActionTraceExport, + AuthActionTracePreviewExport, + } + + seen := make(map[string]bool) + for _, action := range actions { + assert.False(t, seen[action], "Duplicate auth action constant found: %s", action) + seen[action] = true + } +} + +func TestAuthActionConstants_NotEmpty(t *testing.T) { + // Test that all auth action constants are not empty + actions := []string{ + AuthActionTraceRead, + AuthActionTraceIngest, + AuthActionTraceViewCreate, + AuthActionTraceViewList, + AuthActionTraceViewEdit, + AuthActionAnnotationCreate, + AuthActionTraceExport, + AuthActionTracePreviewExport, + } + + for _, action := range actions { + assert.NotEmpty(t, action, "Auth action constant should not be empty") + } +} diff --git a/backend/modules/observability/infra/rpc/auth/auth_test.go b/backend/modules/observability/infra/rpc/auth/auth_test.go index 6e549d085..d4e572fc6 100755 --- a/backend/modules/observability/infra/rpc/auth/auth_test.go +++ b/backend/modules/observability/infra/rpc/auth/auth_test.go @@ -5,6 +5,7 @@ package auth import ( "context" + "errors" "fmt" "testing" @@ -16,571 +17,312 @@ import ( authentity "github.com/coze-dev/coze-loop/backend/kitex_gen/coze/loop/foundation/domain/auth" "github.com/coze-dev/coze-loop/backend/modules/observability/domain/component/rpc" "github.com/coze-dev/coze-loop/backend/modules/observability/infra/rpc/auth/mocks" + obErrorx "github.com/coze-dev/coze-loop/backend/modules/observability/pkg/errno" "github.com/coze-dev/coze-loop/backend/pkg/lang/ptr" ) -func TestAuthProviderImpl_CheckWorkspacePermission(t *testing.T) { - type fields struct { - cli *mocks.MockClient - } - type args struct { - ctx context.Context - action string - workspaceId string - } - tests := []struct { - name string - fieldsGetter func(ctrl *gomock.Controller) fields - args args - wantErr bool - }{ - { - name: "check workspace permission successfully", - fieldsGetter: func(ctrl *gomock.Controller) fields { - mockClient := mocks.NewMockClient(ctrl) - mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any(), gomock.Any()). - DoAndReturn(func(ctx context.Context, req *auth.MCheckPermissionRequest, opts ...interface{}) (*auth.MCheckPermissionResponse, error) { - return &auth.MCheckPermissionResponse{ - BaseResp: &base.BaseResp{StatusCode: 0}, - AuthRes: []*authentity.SubjectActionObjectAuthRes{ - {IsAllowed: ptr.Of(true)}, - }, - }, nil - }) - return fields{cli: mockClient} - }, - args: args{ - ctx: context.Background(), - action: "read", - workspaceId: "12345", - }, - wantErr: false, - }, - { - name: "workspace id conversion failed - non-numeric string", - fieldsGetter: func(ctrl *gomock.Controller) fields { - mockClient := mocks.NewMockClient(ctrl) - return fields{cli: mockClient} - }, - args: args{ - ctx: context.Background(), - action: "read", - workspaceId: "invalid_id", - }, - wantErr: true, - }, - { - name: "rpc call failed", - fieldsGetter: func(ctrl *gomock.Controller) fields { - mockClient := mocks.NewMockClient(ctrl) - mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any(), gomock.Any()). - Return(nil, fmt.Errorf("rpc error")) - return fields{cli: mockClient} - }, - args: args{ - ctx: context.Background(), - action: "read", - workspaceId: "12345", - }, - wantErr: true, - }, - { - name: "response is nil", - fieldsGetter: func(ctrl *gomock.Controller) fields { - mockClient := mocks.NewMockClient(ctrl) - mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any(), gomock.Any()). - Return(nil, nil) - return fields{cli: mockClient} - }, - args: args{ - ctx: context.Background(), - action: "read", - workspaceId: "12345", - }, - wantErr: true, - }, - { - name: "response status code non-zero", - fieldsGetter: func(ctrl *gomock.Controller) fields { - mockClient := mocks.NewMockClient(ctrl) - mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any(), gomock.Any()). - Return(&auth.MCheckPermissionResponse{ - BaseResp: &base.BaseResp{StatusCode: 500}, - }, nil) - return fields{cli: mockClient} - }, - args: args{ - ctx: context.Background(), - action: "read", - workspaceId: "12345", - }, - wantErr: true, - }, - { - name: "permission denied - IsAllowed false", - fieldsGetter: func(ctrl *gomock.Controller) fields { - mockClient := mocks.NewMockClient(ctrl) - mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any(), gomock.Any()). - Return(&auth.MCheckPermissionResponse{ - BaseResp: &base.BaseResp{StatusCode: 0}, - AuthRes: []*authentity.SubjectActionObjectAuthRes{ - {IsAllowed: ptr.Of(false)}, - }, - }, nil) - return fields{cli: mockClient} - }, - args: args{ - ctx: context.Background(), - action: "read", - workspaceId: "12345", - }, - wantErr: true, - }, - { - name: "response with nil base resp", - fieldsGetter: func(ctrl *gomock.Controller) fields { - mockClient := mocks.NewMockClient(ctrl) - mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any(), gomock.Any()). - Return(&auth.MCheckPermissionResponse{ - BaseResp: nil, - AuthRes: []*authentity.SubjectActionObjectAuthRes{ - {IsAllowed: ptr.Of(true)}, - }, - }, nil) - return fields{cli: mockClient} - }, - args: args{ - ctx: context.Background(), - action: "read", - workspaceId: "12345", - }, - wantErr: false, - }, - { - name: "auth result is nil", - fieldsGetter: func(ctrl *gomock.Controller) fields { - mockClient := mocks.NewMockClient(ctrl) - mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any(), gomock.Any()). - Return(&auth.MCheckPermissionResponse{ - BaseResp: &base.BaseResp{StatusCode: 0}, - AuthRes: []*authentity.SubjectActionObjectAuthRes{ - nil, - }, - }, nil) - return fields{cli: mockClient} - }, - args: args{ - ctx: context.Background(), - action: "read", - workspaceId: "12345", - }, - wantErr: false, - }, - { - name: "empty auth results", - fieldsGetter: func(ctrl *gomock.Controller) fields { - mockClient := mocks.NewMockClient(ctrl) - mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any(), gomock.Any()). - Return(&auth.MCheckPermissionResponse{ - BaseResp: &base.BaseResp{StatusCode: 0}, - AuthRes: []*authentity.SubjectActionObjectAuthRes{}, - }, nil) - return fields{cli: mockClient} - }, - args: args{ - ctx: context.Background(), - action: "read", - workspaceId: "12345", - }, - wantErr: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - f := tt.fieldsGetter(ctrl) - a := &AuthProviderImpl{ - cli: f.cli, - } - err := a.CheckWorkspacePermission(tt.args.ctx, tt.args.action, tt.args.workspaceId) - assert.Equal(t, tt.wantErr, err != nil) - }) - } +func TestNewAuthProvider(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockClient := mocks.NewMockClient(ctrl) + provider := NewAuthProvider(mockClient) + + assert.NotNil(t, provider) + assert.IsType(t, &AuthProviderImpl{}, provider) } -func TestAuthProviderImpl_CheckViewPermission(t *testing.T) { - type fields struct { - cli *mocks.MockClient - } - type args struct { - ctx context.Context +func TestAuthProviderImpl_CheckWorkspacePermission(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockClient := mocks.NewMockClient(ctrl) + provider := &AuthProviderImpl{cli: mockClient} + + tests := []struct { + name string action string workspaceId string - viewId string - } - tests := []struct { - name string - fieldsGetter func(ctrl *gomock.Controller) fields - args args - wantErr bool + isOpi bool + mockSetup func() + wantErr bool + expectedErr int }{ { - name: "check view permission successfully", - fieldsGetter: func(ctrl *gomock.Controller) fields { - mockClient := mocks.NewMockClient(ctrl) - mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any(), gomock.Any()). - DoAndReturn(func(ctx context.Context, req *auth.MCheckPermissionRequest, opts ...interface{}) (*auth.MCheckPermissionResponse, error) { - return &auth.MCheckPermissionResponse{ - BaseResp: &base.BaseResp{StatusCode: 0}, - AuthRes: []*authentity.SubjectActionObjectAuthRes{ - {IsAllowed: ptr.Of(true)}, - }, - }, nil - }) - return fields{cli: mockClient} - }, - args: args{ - ctx: context.Background(), - action: "read", - workspaceId: "12345", - viewId: "view123", + name: "success - permission granted", + action: "testAction", + workspaceId: "123", + isOpi: true, + mockSetup: func() { + mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any()).Return(&auth.MCheckPermissionResponse{ + BaseResp: &base.BaseResp{StatusCode: 0}, + AuthRes: []*authentity.SubjectActionObjectAuthRes{ + {IsAllowed: ptr.Of(true)}, + }, + }, nil) }, wantErr: false, }, { - name: "workspace id conversion failed - non-numeric string", - fieldsGetter: func(ctrl *gomock.Controller) fields { - mockClient := mocks.NewMockClient(ctrl) - return fields{cli: mockClient} - }, - args: args{ - ctx: context.Background(), - action: "read", - workspaceId: "invalid_id", - viewId: "view123", - }, - wantErr: true, + name: "permission denied", + action: "testAction", + workspaceId: "123", + isOpi: true, + mockSetup: func() { + mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any()).Return(&auth.MCheckPermissionResponse{ + BaseResp: &base.BaseResp{StatusCode: 0}, + AuthRes: []*authentity.SubjectActionObjectAuthRes{ + {IsAllowed: ptr.Of(false)}, + }, + }, nil) + }, + wantErr: true, + expectedErr: obErrorx.CommonNoPermissionCode, }, { - name: "rpc call failed", - fieldsGetter: func(ctrl *gomock.Controller) fields { - mockClient := mocks.NewMockClient(ctrl) - mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any(), gomock.Any()). - Return(nil, fmt.Errorf("rpc error")) - return fields{cli: mockClient} - }, - args: args{ - ctx: context.Background(), - action: "read", - workspaceId: "12345", - viewId: "view123", - }, - wantErr: true, + name: "invalid workspace ID", + action: "testAction", + workspaceId: "invalid", + isOpi: true, + mockSetup: func() {}, + wantErr: true, + expectedErr: obErrorx.CommonInternalErrorCode, }, { - name: "response is nil", - fieldsGetter: func(ctrl *gomock.Controller) fields { - mockClient := mocks.NewMockClient(ctrl) - mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any(), gomock.Any()). - Return(nil, nil) - return fields{cli: mockClient} - }, - args: args{ - ctx: context.Background(), - action: "read", - workspaceId: "12345", - viewId: "view123", - }, - wantErr: true, + name: "RPC error", + action: "testAction", + workspaceId: "123", + isOpi: true, + mockSetup: func() { + mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any()).Return(nil, errors.New("RPC error")) + }, + wantErr: true, + expectedErr: obErrorx.CommercialCommonRPCErrorCodeCode, }, { - name: "response status code non-zero", - fieldsGetter: func(ctrl *gomock.Controller) fields { - mockClient := mocks.NewMockClient(ctrl) - mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any(), gomock.Any()). - Return(&auth.MCheckPermissionResponse{ - BaseResp: &base.BaseResp{StatusCode: 500}, - }, nil) - return fields{cli: mockClient} - }, - args: args{ - ctx: context.Background(), - action: "read", - workspaceId: "12345", - viewId: "view123", - }, - wantErr: true, + name: "nil response", + action: "testAction", + workspaceId: "123", + isOpi: true, + mockSetup: func() { + mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any()).Return(nil, nil) + }, + wantErr: true, + expectedErr: obErrorx.CommercialCommonRPCErrorCodeCode, }, { - name: "permission denied - IsAllowed false", - fieldsGetter: func(ctrl *gomock.Controller) fields { - mockClient := mocks.NewMockClient(ctrl) - mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any(), gomock.Any()). - Return(&auth.MCheckPermissionResponse{ - BaseResp: &base.BaseResp{StatusCode: 0}, - AuthRes: []*authentity.SubjectActionObjectAuthRes{ - {IsAllowed: ptr.Of(false)}, - }, - }, nil) - return fields{cli: mockClient} - }, - args: args{ - ctx: context.Background(), - action: "read", - workspaceId: "12345", - viewId: "view123", - }, - wantErr: true, + name: "non-zero status code", + action: "testAction", + workspaceId: "123", + isOpi: true, + mockSetup: func() { + mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any()).Return(&auth.MCheckPermissionResponse{ + BaseResp: &base.BaseResp{StatusCode: 1}, + AuthRes: []*authentity.SubjectActionObjectAuthRes{}, + }, nil) + }, + wantErr: true, + expectedErr: obErrorx.CommercialCommonRPCErrorCodeCode, }, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - f := tt.fieldsGetter(ctrl) - a := &AuthProviderImpl{ - cli: f.cli, + tt.mockSetup() + err := provider.CheckWorkspacePermission(context.Background(), tt.action, tt.workspaceId, tt.isOpi) + + if tt.wantErr { + assert.Error(t, err) + if tt.expectedErr != 0 { + assert.Contains(t, err.Error(), fmt.Sprintf("%d", tt.expectedErr)) + } + } else { + assert.NoError(t, err) } - err := a.CheckViewPermission(tt.args.ctx, tt.args.action, tt.args.workspaceId, tt.args.viewId) - assert.Equal(t, tt.wantErr, err != nil) }) } } func TestAuthProviderImpl_CheckIngestPermission(t *testing.T) { - type fields struct { - cli *mocks.MockClient - } - type args struct { - ctx context.Context - workspaceId string - } + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockClient := mocks.NewMockClient(ctrl) + provider := &AuthProviderImpl{cli: mockClient} + tests := []struct { - name string - fieldsGetter func(ctrl *gomock.Controller) fields - args args - wantErr bool + name string + workspaceId string + mockSetup func() + wantErr bool + expectedErr int }{ { - name: "check ingest permission successfully", - fieldsGetter: func(ctrl *gomock.Controller) fields { - mockClient := mocks.NewMockClient(ctrl) - mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any(), gomock.Any()). - DoAndReturn(func(ctx context.Context, req *auth.MCheckPermissionRequest, opts ...interface{}) (*auth.MCheckPermissionResponse, error) { - // Verify the action is correct - assert.Equal(t, rpc.AuthActionTraceIngest, *req.Auths[0].Action) - return &auth.MCheckPermissionResponse{ - BaseResp: &base.BaseResp{StatusCode: 0}, - AuthRes: []*authentity.SubjectActionObjectAuthRes{ - {IsAllowed: ptr.Of(true)}, - }, - }, nil - }) - return fields{cli: mockClient} - }, - args: args{ - ctx: context.Background(), - workspaceId: "12345", + name: "success - ingest permission granted", + workspaceId: "123", + mockSetup: func() { + mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any()).Return(&auth.MCheckPermissionResponse{ + BaseResp: &base.BaseResp{StatusCode: 0}, + AuthRes: []*authentity.SubjectActionObjectAuthRes{ + {IsAllowed: ptr.Of(true)}, + }, + }, nil) }, wantErr: false, }, { - name: "check ingest permission failed - workspace permission check failed", - fieldsGetter: func(ctrl *gomock.Controller) fields { - mockClient := mocks.NewMockClient(ctrl) - mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any(), gomock.Any()). - Return(nil, fmt.Errorf("rpc error")) - return fields{cli: mockClient} - }, - args: args{ - ctx: context.Background(), - workspaceId: "12345", - }, - wantErr: true, + name: "ingest permission denied", + workspaceId: "123", + mockSetup: func() { + mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any()).Return(&auth.MCheckPermissionResponse{ + BaseResp: &base.BaseResp{StatusCode: 0}, + AuthRes: []*authentity.SubjectActionObjectAuthRes{ + {IsAllowed: ptr.Of(false)}, + }, + }, nil) + }, + wantErr: true, + expectedErr: obErrorx.CommonNoPermissionCode, }, { - name: "check ingest permission failed - invalid workspace id", - fieldsGetter: func(ctrl *gomock.Controller) fields { - mockClient := mocks.NewMockClient(ctrl) - return fields{cli: mockClient} - }, - args: args{ - ctx: context.Background(), - workspaceId: "invalid_id", - }, - wantErr: true, + name: "invalid workspace ID for ingest", + workspaceId: "invalid", + mockSetup: func() {}, + wantErr: true, + expectedErr: obErrorx.CommonInternalErrorCode, }, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - f := tt.fieldsGetter(ctrl) - a := &AuthProviderImpl{ - cli: f.cli, + tt.mockSetup() + err := provider.CheckIngestPermission(context.Background(), tt.workspaceId) + + if tt.wantErr { + assert.Error(t, err) + if tt.expectedErr != 0 { + assert.Contains(t, err.Error(), fmt.Sprintf("%d", tt.expectedErr)) + } + } else { + assert.NoError(t, err) } - err := a.CheckIngestPermission(tt.args.ctx, tt.args.workspaceId) - assert.Equal(t, tt.wantErr, err != nil) }) } } func TestAuthProviderImpl_CheckQueryPermission(t *testing.T) { - type fields struct { - cli *mocks.MockClient - } - type args struct { - ctx context.Context - workspaceId string - platformType string - } + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockClient := mocks.NewMockClient(ctrl) + provider := &AuthProviderImpl{cli: mockClient} + tests := []struct { name string - fieldsGetter func(ctrl *gomock.Controller) fields - args args + workspaceId string + platformType string + mockSetup func() wantErr bool + expectedErr int }{ { - name: "check query permission successfully", - fieldsGetter: func(ctrl *gomock.Controller) fields { - mockClient := mocks.NewMockClient(ctrl) - mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any(), gomock.Any()). - DoAndReturn(func(ctx context.Context, req *auth.MCheckPermissionRequest, opts ...interface{}) (*auth.MCheckPermissionResponse, error) { - // Verify the action is correct - assert.Equal(t, rpc.AuthActionTraceList, *req.Auths[0].Action) - return &auth.MCheckPermissionResponse{ - BaseResp: &base.BaseResp{StatusCode: 0}, - AuthRes: []*authentity.SubjectActionObjectAuthRes{ - {IsAllowed: ptr.Of(true)}, - }, - }, nil - }) - return fields{cli: mockClient} - }, - args: args{ - ctx: context.Background(), - workspaceId: "12345", - platformType: "coze", + name: "success - query permission granted", + workspaceId: "123", + platformType: "web", + mockSetup: func() { + mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any()).Return(&auth.MCheckPermissionResponse{ + BaseResp: &base.BaseResp{StatusCode: 0}, + AuthRes: []*authentity.SubjectActionObjectAuthRes{ + {IsAllowed: ptr.Of(true)}, + }, + }, nil) }, wantErr: false, }, { - name: "check query permission failed - workspace permission check failed", - fieldsGetter: func(ctrl *gomock.Controller) fields { - mockClient := mocks.NewMockClient(ctrl) - mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any(), gomock.Any()). - Return(nil, fmt.Errorf("rpc error")) - return fields{cli: mockClient} - }, - args: args{ - ctx: context.Background(), - workspaceId: "12345", - platformType: "coze", - }, - wantErr: true, + name: "query permission denied", + workspaceId: "123", + platformType: "web", + mockSetup: func() { + mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any()).Return(&auth.MCheckPermissionResponse{ + BaseResp: &base.BaseResp{StatusCode: 0}, + AuthRes: []*authentity.SubjectActionObjectAuthRes{ + {IsAllowed: ptr.Of(false)}, + }, + }, nil) + }, + wantErr: true, + expectedErr: obErrorx.CommonNoPermissionCode, }, { - name: "check query permission failed - invalid workspace id", - fieldsGetter: func(ctrl *gomock.Controller) fields { - mockClient := mocks.NewMockClient(ctrl) - return fields{cli: mockClient} - }, - args: args{ - ctx: context.Background(), - workspaceId: "invalid_id", - platformType: "coze", - }, - wantErr: true, + name: "invalid workspace ID for query", + workspaceId: "invalid", + platformType: "web", + mockSetup: func() {}, + wantErr: true, + expectedErr: obErrorx.CommonInternalErrorCode, }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - f := tt.fieldsGetter(ctrl) - a := &AuthProviderImpl{ - cli: f.cli, - } - err := a.CheckQueryPermission(tt.args.ctx, tt.args.workspaceId, tt.args.platformType) - assert.Equal(t, tt.wantErr, err != nil) - }) - } -} -func TestNewAuthProvider(t *testing.T) { - type args struct { - cli *mocks.MockClient - } - tests := []struct { - name string - fieldsGetter func(ctrl *gomock.Controller) args - want rpc.IAuthProvider - }{ - { - name: "create new auth provider successfully", - fieldsGetter: func(ctrl *gomock.Controller) args { - mockClient := mocks.NewMockClient(ctrl) - return args{cli: mockClient} - }, - want: &AuthProviderImpl{}, - }, - } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - f := tt.fieldsGetter(ctrl) - got := NewAuthProvider(f.cli) - assert.NotNil(t, got) - assert.IsType(t, &AuthProviderImpl{}, got) + tt.mockSetup() + err := provider.CheckQueryPermission(context.Background(), tt.workspaceId, tt.platformType) + + if tt.wantErr { + assert.Error(t, err) + if tt.expectedErr != 0 { + assert.Contains(t, err.Error(), fmt.Sprintf("%d", tt.expectedErr)) + } + } else { + assert.NoError(t, err) + } }) } } -func TestAuthProviderImpl_Interface(t *testing.T) { +func TestAuthProviderImpl_CheckIngestPermission_UsesCorrectAction(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() + mockClient := mocks.NewMockClient(ctrl) + provider := &AuthProviderImpl{cli: mockClient} - // Verify that AuthProviderImpl implements IAuthProvider interface - var _ rpc.IAuthProvider = &AuthProviderImpl{} + // Test that CheckIngestPermission uses the correct action + mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(ctx context.Context, req *auth.MCheckPermissionRequest, opts ...interface{}) (*auth.MCheckPermissionResponse, error) { + assert.Equal(t, rpc.AuthActionTraceIngest, *req.Auths[0].Action) + return &auth.MCheckPermissionResponse{ + BaseResp: &base.BaseResp{StatusCode: 0}, + AuthRes: []*authentity.SubjectActionObjectAuthRes{ + {IsAllowed: ptr.Of(true)}, + }, + }, nil + }) - provider := NewAuthProvider(mockClient) - assert.NotNil(t, provider) - assert.IsType(t, &AuthProviderImpl{}, provider) + err := provider.CheckIngestPermission(context.Background(), "123") + assert.NoError(t, err) } -func TestAuthProviderImpl_ErrorCodes(t *testing.T) { +func TestAuthProviderImpl_CheckQueryPermission_UsesCorrectAction(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() mockClient := mocks.NewMockClient(ctrl) provider := &AuthProviderImpl{cli: mockClient} - // Test invalid workspace ID error code - err := provider.CheckWorkspacePermission(context.Background(), "read", "invalid") - assert.NotNil(t, err) - - // Check if error is wrapped with correct error code - assert.Contains(t, err.Error(), "internal error") - assert.Contains(t, err.Error(), "internal error") - // Test RPC error code - mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any(), gomock.Any()). - Return(nil, fmt.Errorf("rpc error")) - - err = provider.CheckWorkspacePermission(context.Background(), "read", "12345") - assert.NotNil(t, err) - - // Test permission denied error code - mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any(), gomock.Any()). - Return(&auth.MCheckPermissionResponse{ - BaseResp: &base.BaseResp{StatusCode: 0}, - AuthRes: []*authentity.SubjectActionObjectAuthRes{ - {IsAllowed: ptr.Of(false)}, - }, - }, nil) + // Test that CheckQueryPermission uses the correct action + mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(ctx context.Context, req *auth.MCheckPermissionRequest, opts ...interface{}) (*auth.MCheckPermissionResponse, error) { + assert.Equal(t, rpc.AuthActionTraceRead, *req.Auths[0].Action) + return &auth.MCheckPermissionResponse{ + BaseResp: &base.BaseResp{StatusCode: 0}, + AuthRes: []*authentity.SubjectActionObjectAuthRes{ + {IsAllowed: ptr.Of(true)}, + }, + }, nil + }) - err = provider.CheckWorkspacePermission(context.Background(), "read", "12345") - assert.NotNil(t, err) + err := provider.CheckQueryPermission(context.Background(), "123", "web") + assert.NoError(t, err) } diff --git a/backend/modules/observability/pkg/errno/observability_test.go b/backend/modules/observability/pkg/errno/observability_test.go new file mode 100755 index 000000000..c0f7f749c --- /dev/null +++ b/backend/modules/observability/pkg/errno/observability_test.go @@ -0,0 +1,292 @@ +// Copyright (c) 2025 coze-dev Authors +// SPDX-License-Identifier: Apache-2.0 + +package errno + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestErrorCodeConstants(t *testing.T) { + tests := []struct { + name string + code int + expected int + }{ + // Common error codes + {"CommonNoPermissionCode", CommonNoPermissionCode, 600900101}, + {"CommonBadRequestCode", CommonBadRequestCode, 600900201}, + {"CommonInvalidParamCode", CommonInvalidParamCode, 600900202}, + {"CommonBizInvalidCode", CommonBizInvalidCode, 600900203}, + {"CommonResourceDuplicatedCode", CommonResourceDuplicatedCode, 600900204}, + {"CommonRequestRateLimitCode", CommonRequestRateLimitCode, 600900205}, + {"CommonNetworkTimeOutCode", CommonNetworkTimeOutCode, 600900701}, + {"CommonInternalErrorCode", CommonInternalErrorCode, 600900702}, + {"CommonRPCErrorCode", CommonRPCErrorCode, 600900703}, + {"CommonMySqlErrorCode", CommonMySqlErrorCode, 600900801}, + {"CommonRedisErrorCode", CommonRedisErrorCode, 600900803}, + + // Resource error codes + {"ResourceNotFoundCode", ResourceNotFoundCode, 600903001}, + {"JSONErrorCode", JSONErrorCode, 600903002}, + {"InvalidRPCResponseCode", InvalidRPCResponseCode, 600903003}, + {"InvalidFieldFilterParamCode", InvalidFieldFilterParamCode, 600903004}, + {"MetaInfoBuildErrorCode", MetaInfoBuildErrorCode, 600903005}, + {"HttpCallErrorCode", HttpCallErrorCode, 600903006}, + {"QueryOfflineErrorCode", QueryOfflineErrorCode, 600903007}, + {"QueryOfflineAuthErrorCode", QueryOfflineAuthErrorCode, 600903008}, + + // User error codes + {"UserParseFailedCode", UserParseFailedCode, 600903100}, + {"ManagerAllowedOnlyCode", ManagerAllowedOnlyCode, 600903101}, + {"SearchTraceNotAllowedCode", SearchTraceNotAllowedCode, 600903102}, + + // Trace parsing error codes + {"ParseTagErrorCode", ParseTagErrorCode, 600903201}, + {"ParseArgosSpanErrorCode", ParseArgosSpanErrorCode, 600903202}, + {"TraceNotInSpaceErrorCode", TraceNotInSpaceErrorCode, 600903203}, + {"BotNotRegisteredInSpaceErrorCode", BotNotRegisteredInSpaceErrorCode, 600903204}, + {"NotInSpaceCommonErrorCode", NotInSpaceCommonErrorCode, 600903205}, + {"NoRegisteredBotInSpaceErrorCode", NoRegisteredBotInSpaceErrorCode, 600903206}, + {"InvalidTraceErrorCode", InvalidTraceErrorCode, 600903207}, + {"ExpiredTraceErrorCode", ExpiredTraceErrorCode, 600903208}, + + // Capacity error codes + {"TraceNoCapacityAvailableErrorCode", TraceNoCapacityAvailableErrorCode, 600903230}, + {"AccountNotAvailableErrorCode", AccountNotAvailableErrorCode, 600903231}, + + // Sampling error codes + {"UnsupportedDownSampleIntervalTypeCode", UnsupportedDownSampleIntervalTypeCode, 600903301}, + + // Commercial error codes + {"CommercialUnsupportedMethodCodeCode", CommercialUnsupportedMethodCodeCode, 600904001}, + {"CommercialCommonInvalidParamCodeCode", CommercialCommonInvalidParamCodeCode, 600904002}, + {"CommercialCommonBadRequestCodeCode", CommercialCommonBadRequestCodeCode, 600904003}, + {"CommercialCommonInternalErrorCodeCode", CommercialCommonInternalErrorCodeCode, 600904004}, + {"CommercialUserParseFailedCodeCode", CommercialUserParseFailedCodeCode, 600904005}, + {"CommercialCommonRPCErrorCodeCode", CommercialCommonRPCErrorCodeCode, 600904006}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, tt.code) + }) + } +} + +func TestErrorCodeUniqueness(t *testing.T) { + // Test that all error codes are unique + codes := []int{ + CommonNoPermissionCode, + CommonBadRequestCode, + CommonInvalidParamCode, + CommonBizInvalidCode, + CommonResourceDuplicatedCode, + CommonRequestRateLimitCode, + CommonNetworkTimeOutCode, + CommonInternalErrorCode, + CommonRPCErrorCode, + CommonMySqlErrorCode, + CommonRedisErrorCode, + ResourceNotFoundCode, + JSONErrorCode, + InvalidRPCResponseCode, + InvalidFieldFilterParamCode, + MetaInfoBuildErrorCode, + HttpCallErrorCode, + QueryOfflineErrorCode, + QueryOfflineAuthErrorCode, + UserParseFailedCode, + ManagerAllowedOnlyCode, + SearchTraceNotAllowedCode, + ParseTagErrorCode, + ParseArgosSpanErrorCode, + TraceNotInSpaceErrorCode, + BotNotRegisteredInSpaceErrorCode, + NotInSpaceCommonErrorCode, + NoRegisteredBotInSpaceErrorCode, + InvalidTraceErrorCode, + ExpiredTraceErrorCode, + TraceNoCapacityAvailableErrorCode, + AccountNotAvailableErrorCode, + UnsupportedDownSampleIntervalTypeCode, + CommercialUnsupportedMethodCodeCode, + CommercialCommonInvalidParamCodeCode, + CommercialCommonBadRequestCodeCode, + CommercialCommonInternalErrorCodeCode, + CommercialUserParseFailedCodeCode, + CommercialCommonRPCErrorCodeCode, + } + + seen := make(map[int]bool) + for _, code := range codes { + assert.False(t, seen[code], "Duplicate error code found: %d", code) + seen[code] = true + } +} + +func TestErrorCodeRanges(t *testing.T) { + // Test that error codes are in expected ranges + tests := []struct { + name string + code int + minRange int + maxRange int + category string + }{ + // Common errors: 600900xxx + {"CommonNoPermissionCode", CommonNoPermissionCode, 600900000, 600900999, "common"}, + {"CommonBadRequestCode", CommonBadRequestCode, 600900000, 600900999, "common"}, + {"CommonInternalErrorCode", CommonInternalErrorCode, 600900000, 600900999, "common"}, + + // Business errors: 600903xxx + {"ResourceNotFoundCode", ResourceNotFoundCode, 600903000, 600903999, "business"}, + {"UserParseFailedCode", UserParseFailedCode, 600903000, 600903999, "business"}, + {"ParseTagErrorCode", ParseTagErrorCode, 600903000, 600903999, "business"}, + + // Commercial errors: 600904xxx + {"CommercialUnsupportedMethodCodeCode", CommercialUnsupportedMethodCodeCode, 600904000, 600904999, "commercial"}, + {"CommercialCommonInvalidParamCodeCode", CommercialCommonInvalidParamCodeCode, 600904000, 600904999, "commercial"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.GreaterOrEqual(t, tt.code, tt.minRange, "Error code %d should be >= %d for %s category", tt.code, tt.minRange, tt.category) + assert.LessOrEqual(t, tt.code, tt.maxRange, "Error code %d should be <= %d for %s category", tt.code, tt.maxRange, tt.category) + }) + } +} + +func TestErrorCodeGrouping(t *testing.T) { + // Test that error codes are properly grouped by functionality + + // Permission related codes + permissionCodes := []int{ + CommonNoPermissionCode, + ManagerAllowedOnlyCode, + SearchTraceNotAllowedCode, + QueryOfflineAuthErrorCode, + } + + for _, code := range permissionCodes { + assert.Greater(t, code, 0, "Permission error code should be positive") + } + + // Validation related codes + validationCodes := []int{ + CommonBadRequestCode, + CommonInvalidParamCode, + CommercialCommonInvalidParamCodeCode, + CommercialCommonBadRequestCodeCode, + InvalidFieldFilterParamCode, + } + + for _, code := range validationCodes { + assert.Greater(t, code, 0, "Validation error code should be positive") + } + + // Internal error codes + internalCodes := []int{ + CommonInternalErrorCode, + CommonRPCErrorCode, + CommonMySqlErrorCode, + CommonRedisErrorCode, + CommercialCommonInternalErrorCodeCode, + CommercialCommonRPCErrorCodeCode, + } + + for _, code := range internalCodes { + assert.Greater(t, code, 0, "Internal error code should be positive") + } +} + +func TestSpecificErrorCodes(t *testing.T) { + // Test specific important error codes that are frequently used + tests := []struct { + name string + code int + description string + }{ + { + name: "TraceNoCapacityAvailableErrorCode", + code: TraceNoCapacityAvailableErrorCode, + description: "Should be used when trace capacity is insufficient", + }, + { + name: "AccountNotAvailableErrorCode", + code: AccountNotAvailableErrorCode, + description: "Should be used when account is not available for benefits", + }, + { + name: "CommonRequestRateLimitCode", + code: CommonRequestRateLimitCode, + description: "Should be used when request rate limit is exceeded", + }, + { + name: "InvalidTraceErrorCode", + code: InvalidTraceErrorCode, + description: "Should be used when trace has no root span", + }, + { + name: "ExpiredTraceErrorCode", + code: ExpiredTraceErrorCode, + description: "Should be used when trace has expired", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Greater(t, tt.code, 0, "Error code should be positive: %s", tt.description) + assert.Greater(t, tt.code, 600000000, "Error code should be in observability range: %s", tt.description) + }) + } +} + +func TestPublicConstants(t *testing.T) { + // Test that public constants (exported) have correct values + assert.Equal(t, "request is limited", CommonRequestRateLimitMessage) + assert.Equal(t, true, CommonRequestRateLimitNoAffectStability) +} + +func TestErrorCodeConsistency(t *testing.T) { + // Test that similar error codes follow consistent naming patterns + + // All commercial codes should start with "Commercial" + commercialCodes := map[string]int{ + "CommercialUnsupportedMethodCodeCode": CommercialUnsupportedMethodCodeCode, + "CommercialCommonInvalidParamCodeCode": CommercialCommonInvalidParamCodeCode, + "CommercialCommonBadRequestCodeCode": CommercialCommonBadRequestCodeCode, + "CommercialCommonInternalErrorCodeCode": CommercialCommonInternalErrorCodeCode, + "CommercialUserParseFailedCodeCode": CommercialUserParseFailedCodeCode, + "CommercialCommonRPCErrorCodeCode": CommercialCommonRPCErrorCodeCode, + } + + for name, code := range commercialCodes { + assert.Contains(t, name, "Commercial", "Commercial error code name should contain 'Commercial': %s", name) + assert.GreaterOrEqual(t, code, 600904000, "Commercial error code should be in 600904xxx range: %s", name) + assert.LessOrEqual(t, code, 600904999, "Commercial error code should be in 600904xxx range: %s", name) + } + + // All common codes should start with "Common" + commonCodes := map[string]int{ + "CommonNoPermissionCode": CommonNoPermissionCode, + "CommonBadRequestCode": CommonBadRequestCode, + "CommonInvalidParamCode": CommonInvalidParamCode, + "CommonBizInvalidCode": CommonBizInvalidCode, + "CommonResourceDuplicatedCode": CommonResourceDuplicatedCode, + "CommonRequestRateLimitCode": CommonRequestRateLimitCode, + "CommonNetworkTimeOutCode": CommonNetworkTimeOutCode, + "CommonInternalErrorCode": CommonInternalErrorCode, + "CommonRPCErrorCode": CommonRPCErrorCode, + "CommonMySqlErrorCode": CommonMySqlErrorCode, + "CommonRedisErrorCode": CommonRedisErrorCode, + } + + for name, code := range commonCodes { + assert.Contains(t, name, "Common", "Common error code name should contain 'Common': %s", name) + assert.GreaterOrEqual(t, code, 600900000, "Common error code should be in 600900xxx range: %s", name) + assert.LessOrEqual(t, code, 600900999, "Common error code should be in 600900xxx range: %s", name) + } +} From 53d71454bd4a37261a285c0220f1bea78640fcf7 Mon Sep 17 00:00:00 2001 From: lizhanpeng Date: Fri, 5 Sep 2025 14:41:42 +0800 Subject: [PATCH 08/13] feat(backend): fix --- .../observability/application/openapi.go | 2 +- .../observability/application/openapi_test.go | 2 +- .../observability/application/trace.go | 2 +- .../observability/application/trace_test.go | 2 +- .../domain/trace/service/trace_service.go | 6 -- .../modules/observability/infra/repo/trace.go | 6 +- .../infra/rpc/auth/mocks/auth_client.go | 63 +++++++++++++++++++ .../mocks/mock_datasetservice_client.go | 2 +- 8 files changed, 72 insertions(+), 13 deletions(-) create mode 100644 backend/modules/observability/infra/rpc/auth/mocks/auth_client.go diff --git a/backend/modules/observability/application/openapi.go b/backend/modules/observability/application/openapi.go index 316f8c14c..211efcb70 100644 --- a/backend/modules/observability/application/openapi.go +++ b/backend/modules/observability/application/openapi.go @@ -794,4 +794,4 @@ func (p *OpenAPIApplication) AllowByKey(ctx context.Context, key string) bool { return true } return false -} \ No newline at end of file +} diff --git a/backend/modules/observability/application/openapi_test.go b/backend/modules/observability/application/openapi_test.go index 14efae23e..fd4e0698e 100644 --- a/backend/modules/observability/application/openapi_test.go +++ b/backend/modules/observability/application/openapi_test.go @@ -3539,4 +3539,4 @@ func TestOpenAPIApplication_unpackTenant(t *testing.T) { result = app.unpackTenant(context.Background(), []*loop_span.Span{{SpanID: "test"}}) assert.Len(t, result, 1) assert.Len(t, result["tenant1"], 1) -} \ No newline at end of file +} diff --git a/backend/modules/observability/application/trace.go b/backend/modules/observability/application/trace.go index bc8dc1055..54486517c 100644 --- a/backend/modules/observability/application/trace.go +++ b/backend/modules/observability/application/trace.go @@ -826,4 +826,4 @@ func (t *TraceApplication) PreviewExportTracesToDataset(ctx context.Context, req // 转换响应 return tconv.PreviewResponseDO2DTO(serviceResp), nil -} \ No newline at end of file +} diff --git a/backend/modules/observability/application/trace_test.go b/backend/modules/observability/application/trace_test.go index cae4e1269..5e2aa133e 100644 --- a/backend/modules/observability/application/trace_test.go +++ b/backend/modules/observability/application/trace_test.go @@ -1746,4 +1746,4 @@ func TestTraceApplication_PreviewExportTracesToDataset(t *testing.T) { assert.Equal(t, tt.want, got) }) } -} \ No newline at end of file +} diff --git a/backend/modules/observability/domain/trace/service/trace_service.go b/backend/modules/observability/domain/trace/service/trace_service.go index fa4923e13..a353a80de 100644 --- a/backend/modules/observability/domain/trace/service/trace_service.go +++ b/backend/modules/observability/domain/trace/service/trace_service.go @@ -871,12 +871,6 @@ func (r *TraceServiceImpl) getSpan(ctx context.Context, tenants []string, spanId } filter := &loop_span.FilterFields{ FilterFields: []*loop_span.FilterField{ - { - FieldName: loop_span.SpanFieldSpanId, - FieldType: loop_span.FieldTypeString, - Values: []string{spanId}, - QueryType: ptr.Of(loop_span.QueryTypeEnumEq), - }, { FieldName: loop_span.SpanFieldSpaceId, FieldType: loop_span.FieldTypeString, diff --git a/backend/modules/observability/infra/repo/trace.go b/backend/modules/observability/infra/repo/trace.go index 3dd60cd5e..6c9a417ef 100644 --- a/backend/modules/observability/infra/repo/trace.go +++ b/backend/modules/observability/infra/repo/trace.go @@ -296,8 +296,10 @@ func (t *TraceCkRepoImpl) getQueryTenantTables(ctx context.Context, tenants []st } for _, tableCfg := range tables { ret.SpanTables = append(ret.SpanTables, tableCfg.SpanTable) - ret.AnnoTables = append(ret.AnnoTables, tableCfg.AnnoTable) - ret.AnnoTableMap[tableCfg.SpanTable] = tableCfg.AnnoTable + if tableCfg.AnnoTable != "" { + ret.AnnoTables = append(ret.AnnoTables, tableCfg.AnnoTable) + ret.AnnoTableMap[tableCfg.SpanTable] = tableCfg.AnnoTable + } } } for _, tenant := range tenants { diff --git a/backend/modules/observability/infra/rpc/auth/mocks/auth_client.go b/backend/modules/observability/infra/rpc/auth/mocks/auth_client.go new file mode 100644 index 000000000..9da6e43c7 --- /dev/null +++ b/backend/modules/observability/infra/rpc/auth/mocks/auth_client.go @@ -0,0 +1,63 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/coze-dev/coze-loop/backend/kitex_gen/coze/loop/foundation/auth/authservice (interfaces: Client) +// +// Generated by this command: +// +// mockgen -destination=mocks/auth_client.go -package=mocks github.com/coze-dev/coze-loop/backend/kitex_gen/coze/loop/foundation/auth/authservice Client +// + +// Package mocks is a generated GoMock package. +package mocks + +import ( + context "context" + reflect "reflect" + + callopt "github.com/cloudwego/kitex/client/callopt" + auth "github.com/coze-dev/coze-loop/backend/kitex_gen/coze/loop/foundation/auth" + gomock "go.uber.org/mock/gomock" +) + +// MockClient is a mock of Client interface. +type MockClient struct { + ctrl *gomock.Controller + recorder *MockClientMockRecorder + isgomock struct{} +} + +// MockClientMockRecorder is the mock recorder for MockClient. +type MockClientMockRecorder struct { + mock *MockClient +} + +// NewMockClient creates a new mock instance. +func NewMockClient(ctrl *gomock.Controller) *MockClient { + mock := &MockClient{ctrl: ctrl} + mock.recorder = &MockClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockClient) EXPECT() *MockClientMockRecorder { + return m.recorder +} + +// MCheckPermission mocks base method. +func (m *MockClient) MCheckPermission(ctx context.Context, request *auth.MCheckPermissionRequest, callOptions ...callopt.Option) (*auth.MCheckPermissionResponse, error) { + m.ctrl.T.Helper() + varargs := []any{ctx, request} + for _, a := range callOptions { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "MCheckPermission", varargs...) + ret0, _ := ret[0].(*auth.MCheckPermissionResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// MCheckPermission indicates an expected call of MCheckPermission. +func (mr *MockClientMockRecorder) MCheckPermission(ctx, request any, callOptions ...any) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]any{ctx, request}, callOptions...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MCheckPermission", reflect.TypeOf((*MockClient)(nil).MCheckPermission), varargs...) +} diff --git a/backend/modules/observability/infra/rpc/dataset/mocks/mock_datasetservice_client.go b/backend/modules/observability/infra/rpc/dataset/mocks/mock_datasetservice_client.go index 79fbcbb33..e36f65e95 100644 --- a/backend/modules/observability/infra/rpc/dataset/mocks/mock_datasetservice_client.go +++ b/backend/modules/observability/infra/rpc/dataset/mocks/mock_datasetservice_client.go @@ -13,8 +13,8 @@ import ( context "context" reflect "reflect" - dataset "github.com/coze-dev/coze-loop/backend/kitex_gen/coze/loop/data/dataset" callopt "github.com/cloudwego/kitex/client/callopt" + dataset "github.com/coze-dev/coze-loop/backend/kitex_gen/coze/loop/data/dataset" gomock "go.uber.org/mock/gomock" ) From 5de1f6d46854e5b87b7a837341db5bb9776af5f6 Mon Sep 17 00:00:00 2001 From: lizhanpeng Date: Fri, 5 Sep 2025 17:23:11 +0800 Subject: [PATCH 09/13] feat(ci): add ignore path --- .github/.codecov.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/.codecov.yaml b/.github/.codecov.yaml index 49bb93edc..1ca402194 100644 --- a/.github/.codecov.yaml +++ b/.github/.codecov.yaml @@ -56,6 +56,7 @@ ignore: - "**/repo/mysql/**" - "**/repo/**/mysql/**" - "**/repo/redis/**" + - "backend/api/router/coze/loop/apis/*.go" parsers: gcov: From c44a2726baec9cf0cc7dd50f4bb537c0d130b0a8 Mon Sep 17 00:00:00 2001 From: lizhanpeng Date: Wed, 10 Sep 2025 15:36:56 +0800 Subject: [PATCH 10/13] =?UTF-8?q?test:=20[Coda]=20=E5=A2=9E=E5=8A=A0Create?= =?UTF-8?q?Annotation/DeleteAnnotation=E5=8D=95=E6=B5=8B=E8=A6=86=E7=9B=96?= =?UTF-8?q?=E7=8E=87=E8=87=B392%/100%=20(LogID:=20202509101527220100911112?= =?UTF-8?q?09308A5AB)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Coda --- .../observability/application/openapi_test.go | 416 +++++++++++++++++- 1 file changed, 415 insertions(+), 1 deletion(-) diff --git a/backend/modules/observability/application/openapi_test.go b/backend/modules/observability/application/openapi_test.go index fd4e0698e..c29dc4d6f 100644 --- a/backend/modules/observability/application/openapi_test.go +++ b/backend/modules/observability/application/openapi_test.go @@ -293,6 +293,311 @@ func TestOpenAPIApplication_CreateAnnotation(t *testing.T) { want: nil, wantErr: true, }, + { + name: "create annotation with bool value successfully", + fieldsGetter: func(ctrl *gomock.Controller) fields { + authMock := rpcmocks.NewMockIAuthProvider(ctrl) + authMock.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionAnnotationCreate, "1", true).Return(nil) + traceServiceMock := servicemocks.NewMockITraceService(ctrl) + traceServiceMock.EXPECT().CreateAnnotation(gomock.Any(), gomock.Any()).Return(nil) + benefitMock := benefitmocks.NewMockIBenefitService(ctrl) + benefitMock.EXPECT().CheckTraceBenefit(gomock.Any(), gomock.Any()).Return(&benefit.CheckTraceBenefitResult{ + StorageDuration: 3, + }, nil) + tenantMock := tenantmocks.NewMockITenantProvider(ctrl) + workspaceMock := workspacemocks.NewMockIWorkSpaceProvider(ctrl) + rateLimiterMock := limitermocks.NewMockIRateLimiterFactory(ctrl) + rateLimiterMock.EXPECT().NewRateLimiter().Return(limitermocks.NewMockIRateLimiter(ctrl)).AnyTimes() + traceConfigMock := configmocks.NewMockITraceConfig(ctrl) + metricsMock := metricsmocks.NewMockITraceMetrics(ctrl) + return fields{ + traceService: traceServiceMock, + auth: authMock, + benefit: benefitMock, + tenant: tenantMock, + workspace: workspaceMock, + rateLimiter: rateLimiterMock, + traceConfig: traceConfigMock, + metrics: metricsMock, + } + }, + args: args{ + ctx: context.Background(), + req: &openapi.CreateAnnotationRequest{ + WorkspaceID: 1, + AnnotationValueType: ptr.Of(annotation.ValueType(loop_span.AnnotationValueTypeBool)), + AnnotationValue: "true", + Base: &base.Base{Caller: "test"}, + }, + }, + want: openapi.NewCreateAnnotationResponse(), + wantErr: false, + }, + { + name: "create annotation with invalid bool value", + fieldsGetter: func(ctrl *gomock.Controller) fields { + traceServiceMock := servicemocks.NewMockITraceService(ctrl) + authMock := rpcmocks.NewMockIAuthProvider(ctrl) + benefitMock := benefitmocks.NewMockIBenefitService(ctrl) + tenantMock := tenantmocks.NewMockITenantProvider(ctrl) + workspaceMock := workspacemocks.NewMockIWorkSpaceProvider(ctrl) + rateLimiterMock := limitermocks.NewMockIRateLimiterFactory(ctrl) + rateLimiterMock.EXPECT().NewRateLimiter().Return(limitermocks.NewMockIRateLimiter(ctrl)).AnyTimes() + traceConfigMock := configmocks.NewMockITraceConfig(ctrl) + metricsMock := metricsmocks.NewMockITraceMetrics(ctrl) + return fields{ + traceService: traceServiceMock, + auth: authMock, + benefit: benefitMock, + tenant: tenantMock, + workspace: workspaceMock, + rateLimiter: rateLimiterMock, + traceConfig: traceConfigMock, + metrics: metricsMock, + } + }, + args: args{ + ctx: context.Background(), + req: &openapi.CreateAnnotationRequest{ + WorkspaceID: 1, + AnnotationValueType: ptr.Of(annotation.ValueType(loop_span.AnnotationValueTypeBool)), + AnnotationValue: "invalid_bool", + Base: &base.Base{Caller: "test"}, + }, + }, + want: nil, + wantErr: true, + }, + { + name: "create annotation with double value successfully", + fieldsGetter: func(ctrl *gomock.Controller) fields { + authMock := rpcmocks.NewMockIAuthProvider(ctrl) + authMock.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionAnnotationCreate, "1", true).Return(nil) + traceServiceMock := servicemocks.NewMockITraceService(ctrl) + traceServiceMock.EXPECT().CreateAnnotation(gomock.Any(), gomock.Any()).Return(nil) + benefitMock := benefitmocks.NewMockIBenefitService(ctrl) + benefitMock.EXPECT().CheckTraceBenefit(gomock.Any(), gomock.Any()).Return(&benefit.CheckTraceBenefitResult{ + StorageDuration: 3, + }, nil) + tenantMock := tenantmocks.NewMockITenantProvider(ctrl) + workspaceMock := workspacemocks.NewMockIWorkSpaceProvider(ctrl) + rateLimiterMock := limitermocks.NewMockIRateLimiterFactory(ctrl) + rateLimiterMock.EXPECT().NewRateLimiter().Return(limitermocks.NewMockIRateLimiter(ctrl)).AnyTimes() + traceConfigMock := configmocks.NewMockITraceConfig(ctrl) + metricsMock := metricsmocks.NewMockITraceMetrics(ctrl) + return fields{ + traceService: traceServiceMock, + auth: authMock, + benefit: benefitMock, + tenant: tenantMock, + workspace: workspaceMock, + rateLimiter: rateLimiterMock, + traceConfig: traceConfigMock, + metrics: metricsMock, + } + }, + args: args{ + ctx: context.Background(), + req: &openapi.CreateAnnotationRequest{ + WorkspaceID: 1, + AnnotationValueType: ptr.Of(annotation.ValueType(loop_span.AnnotationValueTypeDouble)), + AnnotationValue: "3.14", + Base: &base.Base{Caller: "test"}, + }, + }, + want: openapi.NewCreateAnnotationResponse(), + wantErr: false, + }, + { + name: "create annotation with invalid double value", + fieldsGetter: func(ctrl *gomock.Controller) fields { + traceServiceMock := servicemocks.NewMockITraceService(ctrl) + authMock := rpcmocks.NewMockIAuthProvider(ctrl) + benefitMock := benefitmocks.NewMockIBenefitService(ctrl) + tenantMock := tenantmocks.NewMockITenantProvider(ctrl) + workspaceMock := workspacemocks.NewMockIWorkSpaceProvider(ctrl) + rateLimiterMock := limitermocks.NewMockIRateLimiterFactory(ctrl) + rateLimiterMock.EXPECT().NewRateLimiter().Return(limitermocks.NewMockIRateLimiter(ctrl)).AnyTimes() + traceConfigMock := configmocks.NewMockITraceConfig(ctrl) + metricsMock := metricsmocks.NewMockITraceMetrics(ctrl) + return fields{ + traceService: traceServiceMock, + auth: authMock, + benefit: benefitMock, + tenant: tenantMock, + workspace: workspaceMock, + rateLimiter: rateLimiterMock, + traceConfig: traceConfigMock, + metrics: metricsMock, + } + }, + args: args{ + ctx: context.Background(), + req: &openapi.CreateAnnotationRequest{ + WorkspaceID: 1, + AnnotationValueType: ptr.Of(annotation.ValueType(loop_span.AnnotationValueTypeDouble)), + AnnotationValue: "invalid_double", + Base: &base.Base{Caller: "test"}, + }, + }, + want: nil, + wantErr: true, + }, + { + name: "create annotation with category value successfully", + fieldsGetter: func(ctrl *gomock.Controller) fields { + authMock := rpcmocks.NewMockIAuthProvider(ctrl) + authMock.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionAnnotationCreate, "1", true).Return(nil) + traceServiceMock := servicemocks.NewMockITraceService(ctrl) + traceServiceMock.EXPECT().CreateAnnotation(gomock.Any(), gomock.Any()).Return(nil) + benefitMock := benefitmocks.NewMockIBenefitService(ctrl) + benefitMock.EXPECT().CheckTraceBenefit(gomock.Any(), gomock.Any()).Return(&benefit.CheckTraceBenefitResult{ + StorageDuration: 3, + }, nil) + tenantMock := tenantmocks.NewMockITenantProvider(ctrl) + workspaceMock := workspacemocks.NewMockIWorkSpaceProvider(ctrl) + rateLimiterMock := limitermocks.NewMockIRateLimiterFactory(ctrl) + rateLimiterMock.EXPECT().NewRateLimiter().Return(limitermocks.NewMockIRateLimiter(ctrl)).AnyTimes() + traceConfigMock := configmocks.NewMockITraceConfig(ctrl) + metricsMock := metricsmocks.NewMockITraceMetrics(ctrl) + return fields{ + traceService: traceServiceMock, + auth: authMock, + benefit: benefitMock, + tenant: tenantMock, + workspace: workspaceMock, + rateLimiter: rateLimiterMock, + traceConfig: traceConfigMock, + metrics: metricsMock, + } + }, + args: args{ + ctx: context.Background(), + req: &openapi.CreateAnnotationRequest{ + WorkspaceID: 1, + AnnotationValueType: ptr.Of(annotation.ValueType(loop_span.AnnotationValueTypeCategory)), + AnnotationValue: "category_value", + Base: &base.Base{Caller: "test"}, + }, + }, + want: openapi.NewCreateAnnotationResponse(), + wantErr: false, + }, + { + name: "create annotation with permission denied", + fieldsGetter: func(ctrl *gomock.Controller) fields { + authMock := rpcmocks.NewMockIAuthProvider(ctrl) + authMock.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionAnnotationCreate, "1", true). + Return(assert.AnError) + traceServiceMock := servicemocks.NewMockITraceService(ctrl) + benefitMock := benefitmocks.NewMockIBenefitService(ctrl) + tenantMock := tenantmocks.NewMockITenantProvider(ctrl) + workspaceMock := workspacemocks.NewMockIWorkSpaceProvider(ctrl) + rateLimiterMock := limitermocks.NewMockIRateLimiterFactory(ctrl) + rateLimiterMock.EXPECT().NewRateLimiter().Return(limitermocks.NewMockIRateLimiter(ctrl)).AnyTimes() + traceConfigMock := configmocks.NewMockITraceConfig(ctrl) + metricsMock := metricsmocks.NewMockITraceMetrics(ctrl) + return fields{ + traceService: traceServiceMock, + auth: authMock, + benefit: benefitMock, + tenant: tenantMock, + workspace: workspaceMock, + rateLimiter: rateLimiterMock, + traceConfig: traceConfigMock, + metrics: metricsMock, + } + }, + args: args{ + ctx: context.Background(), + req: &openapi.CreateAnnotationRequest{ + WorkspaceID: 1, + AnnotationValueType: ptr.Of(annotation.ValueType(loop_span.AnnotationValueTypeString)), + AnnotationValue: "test", + Base: &base.Base{Caller: "test"}, + }, + }, + want: nil, + wantErr: true, + }, + { + name: "create annotation with benefit check failed", + fieldsGetter: func(ctrl *gomock.Controller) fields { + authMock := rpcmocks.NewMockIAuthProvider(ctrl) + authMock.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionAnnotationCreate, "1", true).Return(nil) + traceServiceMock := servicemocks.NewMockITraceService(ctrl) + benefitMock := benefitmocks.NewMockIBenefitService(ctrl) + benefitMock.EXPECT().CheckTraceBenefit(gomock.Any(), gomock.Any()). + Return(nil, assert.AnError) + tenantMock := tenantmocks.NewMockITenantProvider(ctrl) + workspaceMock := workspacemocks.NewMockIWorkSpaceProvider(ctrl) + rateLimiterMock := limitermocks.NewMockIRateLimiterFactory(ctrl) + rateLimiterMock.EXPECT().NewRateLimiter().Return(limitermocks.NewMockIRateLimiter(ctrl)).AnyTimes() + traceConfigMock := configmocks.NewMockITraceConfig(ctrl) + metricsMock := metricsmocks.NewMockITraceMetrics(ctrl) + return fields{ + traceService: traceServiceMock, + auth: authMock, + benefit: benefitMock, + tenant: tenantMock, + workspace: workspaceMock, + rateLimiter: rateLimiterMock, + traceConfig: traceConfigMock, + metrics: metricsMock, + } + }, + args: args{ + ctx: context.Background(), + req: &openapi.CreateAnnotationRequest{ + WorkspaceID: 1, + AnnotationValueType: ptr.Of(annotation.ValueType(loop_span.AnnotationValueTypeString)), + AnnotationValue: "test", + Base: &base.Base{Caller: "test"}, + }, + }, + want: nil, + wantErr: true, + }, + { + name: "create annotation with trace service failed", + fieldsGetter: func(ctrl *gomock.Controller) fields { + authMock := rpcmocks.NewMockIAuthProvider(ctrl) + authMock.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionAnnotationCreate, "1", true).Return(nil) + traceServiceMock := servicemocks.NewMockITraceService(ctrl) + traceServiceMock.EXPECT().CreateAnnotation(gomock.Any(), gomock.Any()).Return(assert.AnError) + benefitMock := benefitmocks.NewMockIBenefitService(ctrl) + benefitMock.EXPECT().CheckTraceBenefit(gomock.Any(), gomock.Any()).Return(&benefit.CheckTraceBenefitResult{ + StorageDuration: 3, + }, nil) + tenantMock := tenantmocks.NewMockITenantProvider(ctrl) + workspaceMock := workspacemocks.NewMockIWorkSpaceProvider(ctrl) + rateLimiterMock := limitermocks.NewMockIRateLimiterFactory(ctrl) + rateLimiterMock.EXPECT().NewRateLimiter().Return(limitermocks.NewMockIRateLimiter(ctrl)).AnyTimes() + traceConfigMock := configmocks.NewMockITraceConfig(ctrl) + metricsMock := metricsmocks.NewMockITraceMetrics(ctrl) + return fields{ + traceService: traceServiceMock, + auth: authMock, + benefit: benefitMock, + tenant: tenantMock, + workspace: workspaceMock, + rateLimiter: rateLimiterMock, + traceConfig: traceConfigMock, + metrics: metricsMock, + } + }, + args: args{ + ctx: context.Background(), + req: &openapi.CreateAnnotationRequest{ + WorkspaceID: 1, + AnnotationValueType: ptr.Of(annotation.ValueType(loop_span.AnnotationValueTypeString)), + AnnotationValue: "test", + Base: &base.Base{Caller: "test"}, + }, + }, + want: nil, + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -390,6 +695,115 @@ func TestOpenAPIApplication_DeleteAnnotation(t *testing.T) { want: openapi.NewDeleteAnnotationResponse(), wantErr: false, }, + { + name: "delete annotation with permission denied", + fieldsGetter: func(ctrl *gomock.Controller) fields { + authMock := rpcmocks.NewMockIAuthProvider(ctrl) + authMock.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionAnnotationCreate, "1", true). + Return(assert.AnError) + traceServiceMock := servicemocks.NewMockITraceService(ctrl) + benefitMock := benefitmocks.NewMockIBenefitService(ctrl) + tenantMock := tenantmocks.NewMockITenantProvider(ctrl) + workspaceMock := workspacemocks.NewMockIWorkSpaceProvider(ctrl) + rateLimiterMock := limitermocks.NewMockIRateLimiterFactory(ctrl) + rateLimiterMock.EXPECT().NewRateLimiter().Return(limitermocks.NewMockIRateLimiter(ctrl)).AnyTimes() + traceConfigMock := configmocks.NewMockITraceConfig(ctrl) + metricsMock := metricsmocks.NewMockITraceMetrics(ctrl) + return fields{ + traceService: traceServiceMock, + auth: authMock, + benefit: benefitMock, + tenant: tenantMock, + workspace: workspaceMock, + rateLimiter: rateLimiterMock, + traceConfig: traceConfigMock, + metrics: metricsMock, + } + }, + args: args{ + ctx: context.Background(), + req: &openapi.DeleteAnnotationRequest{ + WorkspaceID: 1, + Base: &base.Base{Caller: "test"}, + }, + }, + want: nil, + wantErr: true, + }, + { + name: "delete annotation with benefit check failed", + fieldsGetter: func(ctrl *gomock.Controller) fields { + authMock := rpcmocks.NewMockIAuthProvider(ctrl) + authMock.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionAnnotationCreate, "1", true).Return(nil) + traceServiceMock := servicemocks.NewMockITraceService(ctrl) + benefitMock := benefitmocks.NewMockIBenefitService(ctrl) + benefitMock.EXPECT().CheckTraceBenefit(gomock.Any(), gomock.Any()). + Return(nil, assert.AnError) + tenantMock := tenantmocks.NewMockITenantProvider(ctrl) + workspaceMock := workspacemocks.NewMockIWorkSpaceProvider(ctrl) + rateLimiterMock := limitermocks.NewMockIRateLimiterFactory(ctrl) + rateLimiterMock.EXPECT().NewRateLimiter().Return(limitermocks.NewMockIRateLimiter(ctrl)).AnyTimes() + traceConfigMock := configmocks.NewMockITraceConfig(ctrl) + metricsMock := metricsmocks.NewMockITraceMetrics(ctrl) + return fields{ + traceService: traceServiceMock, + auth: authMock, + benefit: benefitMock, + tenant: tenantMock, + workspace: workspaceMock, + rateLimiter: rateLimiterMock, + traceConfig: traceConfigMock, + metrics: metricsMock, + } + }, + args: args{ + ctx: context.Background(), + req: &openapi.DeleteAnnotationRequest{ + WorkspaceID: 1, + Base: &base.Base{Caller: "test"}, + }, + }, + want: nil, + wantErr: true, + }, + { + name: "delete annotation with trace service failed", + fieldsGetter: func(ctrl *gomock.Controller) fields { + authMock := rpcmocks.NewMockIAuthProvider(ctrl) + authMock.EXPECT().CheckWorkspacePermission(gomock.Any(), rpc.AuthActionAnnotationCreate, "1", true).Return(nil) + traceServiceMock := servicemocks.NewMockITraceService(ctrl) + traceServiceMock.EXPECT().DeleteAnnotation(gomock.Any(), gomock.Any()).Return(assert.AnError) + benefitMock := benefitmocks.NewMockIBenefitService(ctrl) + benefitMock.EXPECT().CheckTraceBenefit(gomock.Any(), gomock.Any()).Return(&benefit.CheckTraceBenefitResult{ + StorageDuration: 3, + }, nil) + tenantMock := tenantmocks.NewMockITenantProvider(ctrl) + workspaceMock := workspacemocks.NewMockIWorkSpaceProvider(ctrl) + rateLimiterMock := limitermocks.NewMockIRateLimiterFactory(ctrl) + rateLimiterMock.EXPECT().NewRateLimiter().Return(limitermocks.NewMockIRateLimiter(ctrl)).AnyTimes() + traceConfigMock := configmocks.NewMockITraceConfig(ctrl) + metricsMock := metricsmocks.NewMockITraceMetrics(ctrl) + return fields{ + traceService: traceServiceMock, + auth: authMock, + benefit: benefitMock, + tenant: tenantMock, + workspace: workspaceMock, + rateLimiter: rateLimiterMock, + traceConfig: traceConfigMock, + metrics: metricsMock, + } + }, + args: args{ + ctx: context.Background(), + req: &openapi.DeleteAnnotationRequest{ + WorkspaceID: 1, + Base: &base.Base{Caller: "test"}, + }, + }, + want: nil, + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -3539,4 +3953,4 @@ func TestOpenAPIApplication_unpackTenant(t *testing.T) { result = app.unpackTenant(context.Background(), []*loop_span.Span{{SpanID: "test"}}) assert.Len(t, result, 1) assert.Len(t, result["tenant1"], 1) -} +} \ No newline at end of file From e5cc1f990f1eb8b090773d19483e098b6acc966f Mon Sep 17 00:00:00 2001 From: lizhanpeng Date: Wed, 10 Sep 2025 15:56:02 +0800 Subject: [PATCH 11/13] feat(backend): fix --- backend/modules/observability/application/openapi_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/modules/observability/application/openapi_test.go b/backend/modules/observability/application/openapi_test.go index c29dc4d6f..1825130de 100644 --- a/backend/modules/observability/application/openapi_test.go +++ b/backend/modules/observability/application/openapi_test.go @@ -3953,4 +3953,4 @@ func TestOpenAPIApplication_unpackTenant(t *testing.T) { result = app.unpackTenant(context.Background(), []*loop_span.Span{{SpanID: "test"}}) assert.Len(t, result, 1) assert.Len(t, result["tenant1"], 1) -} \ No newline at end of file +} From 46a388e77dca3cf6625b0ae82f804b846e5af11c Mon Sep 17 00:00:00 2001 From: lizhanpeng Date: Tue, 23 Sep 2025 20:12:28 +0800 Subject: [PATCH 12/13] feat(backend): fix ut --- .../observability/infra/rpc/auth/auth_test.go | 2 +- .../infra/rpc/auth/mocks/auth_client.go | 63 ------------------- 2 files changed, 1 insertion(+), 64 deletions(-) delete mode 100644 backend/modules/observability/infra/rpc/auth/mocks/auth_client.go diff --git a/backend/modules/observability/infra/rpc/auth/auth_test.go b/backend/modules/observability/infra/rpc/auth/auth_test.go index d4e572fc6..788c7fde4 100755 --- a/backend/modules/observability/infra/rpc/auth/auth_test.go +++ b/backend/modules/observability/infra/rpc/auth/auth_test.go @@ -314,7 +314,7 @@ func TestAuthProviderImpl_CheckQueryPermission_UsesCorrectAction(t *testing.T) { // Test that CheckQueryPermission uses the correct action mockClient.EXPECT().MCheckPermission(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( func(ctx context.Context, req *auth.MCheckPermissionRequest, opts ...interface{}) (*auth.MCheckPermissionResponse, error) { - assert.Equal(t, rpc.AuthActionTraceRead, *req.Auths[0].Action) + assert.Equal(t, rpc.AuthActionTraceList, *req.Auths[0].Action) return &auth.MCheckPermissionResponse{ BaseResp: &base.BaseResp{StatusCode: 0}, AuthRes: []*authentity.SubjectActionObjectAuthRes{ diff --git a/backend/modules/observability/infra/rpc/auth/mocks/auth_client.go b/backend/modules/observability/infra/rpc/auth/mocks/auth_client.go deleted file mode 100644 index 9da6e43c7..000000000 --- a/backend/modules/observability/infra/rpc/auth/mocks/auth_client.go +++ /dev/null @@ -1,63 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: github.com/coze-dev/coze-loop/backend/kitex_gen/coze/loop/foundation/auth/authservice (interfaces: Client) -// -// Generated by this command: -// -// mockgen -destination=mocks/auth_client.go -package=mocks github.com/coze-dev/coze-loop/backend/kitex_gen/coze/loop/foundation/auth/authservice Client -// - -// Package mocks is a generated GoMock package. -package mocks - -import ( - context "context" - reflect "reflect" - - callopt "github.com/cloudwego/kitex/client/callopt" - auth "github.com/coze-dev/coze-loop/backend/kitex_gen/coze/loop/foundation/auth" - gomock "go.uber.org/mock/gomock" -) - -// MockClient is a mock of Client interface. -type MockClient struct { - ctrl *gomock.Controller - recorder *MockClientMockRecorder - isgomock struct{} -} - -// MockClientMockRecorder is the mock recorder for MockClient. -type MockClientMockRecorder struct { - mock *MockClient -} - -// NewMockClient creates a new mock instance. -func NewMockClient(ctrl *gomock.Controller) *MockClient { - mock := &MockClient{ctrl: ctrl} - mock.recorder = &MockClientMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockClient) EXPECT() *MockClientMockRecorder { - return m.recorder -} - -// MCheckPermission mocks base method. -func (m *MockClient) MCheckPermission(ctx context.Context, request *auth.MCheckPermissionRequest, callOptions ...callopt.Option) (*auth.MCheckPermissionResponse, error) { - m.ctrl.T.Helper() - varargs := []any{ctx, request} - for _, a := range callOptions { - varargs = append(varargs, a) - } - ret := m.ctrl.Call(m, "MCheckPermission", varargs...) - ret0, _ := ret[0].(*auth.MCheckPermissionResponse) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// MCheckPermission indicates an expected call of MCheckPermission. -func (mr *MockClientMockRecorder) MCheckPermission(ctx, request any, callOptions ...any) *gomock.Call { - mr.mock.ctrl.T.Helper() - varargs := append([]any{ctx, request}, callOptions...) - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MCheckPermission", reflect.TypeOf((*MockClient)(nil).MCheckPermission), varargs...) -} From b1e97cefad0a52582c0d950fdb635fa512945b4e Mon Sep 17 00:00:00 2001 From: lizhanpeng Date: Wed, 24 Sep 2025 19:45:04 +0800 Subject: [PATCH 13/13] feat(backend): fix --- backend/modules/evaluation/domain/service/file_name | 1 + backend/modules/observability/application/openapi_test.go | 6 ++++++ .../observability/domain/trace/entity/loop_span/filter.go | 6 +++++- 3 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 backend/modules/evaluation/domain/service/file_name diff --git a/backend/modules/evaluation/domain/service/file_name b/backend/modules/evaluation/domain/service/file_name new file mode 100644 index 000000000..268baf2d8 --- /dev/null +++ b/backend/modules/evaluation/domain/service/file_name @@ -0,0 +1 @@ +ID,status,test_field,actual_output,test_evaluator,test_evaluator_reason,test_tag,logID,targetTraceID diff --git a/backend/modules/observability/application/openapi_test.go b/backend/modules/observability/application/openapi_test.go index 1825130de..2d71dc47d 100644 --- a/backend/modules/observability/application/openapi_test.go +++ b/backend/modules/observability/application/openapi_test.go @@ -3502,6 +3502,7 @@ func TestOpenAPIApplication_CreateAnnotation_AdditionalScenarios(t *testing.T) { StorageDuration: 3, }, nil) authMock := rpcmocks.NewMockIAuthProvider(ctrl) + authMock.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) tenantMock := tenantmocks.NewMockITenantProvider(ctrl) workspaceMock := workspacemocks.NewMockIWorkSpaceProvider(ctrl) workspaceMock.EXPECT().GetThirdPartyQueryWorkSpaceID(gomock.Any(), int64(123)).Return("123").AnyTimes() @@ -3543,6 +3544,7 @@ func TestOpenAPIApplication_CreateAnnotation_AdditionalScenarios(t *testing.T) { StorageDuration: 3, }, nil) authMock := rpcmocks.NewMockIAuthProvider(ctrl) + authMock.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) tenantMock := tenantmocks.NewMockITenantProvider(ctrl) workspaceMock := workspacemocks.NewMockIWorkSpaceProvider(ctrl) workspaceMock.EXPECT().GetThirdPartyQueryWorkSpaceID(gomock.Any(), int64(123)).Return("123").AnyTimes() @@ -3653,6 +3655,7 @@ func TestOpenAPIApplication_CreateAnnotation_AdditionalScenarios(t *testing.T) { fieldsGetter: func(ctrl *gomock.Controller) fields { traceServiceMock := servicemocks.NewMockITraceService(ctrl) authMock := rpcmocks.NewMockIAuthProvider(ctrl) + authMock.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) benefitMock := benefitmocks.NewMockIBenefitService(ctrl) benefitMock.EXPECT().CheckTraceBenefit(gomock.Any(), gomock.Any()).Return(nil, assert.AnError) tenantMock := tenantmocks.NewMockITenantProvider(ctrl) @@ -3696,6 +3699,7 @@ func TestOpenAPIApplication_CreateAnnotation_AdditionalScenarios(t *testing.T) { StorageDuration: 3, }, nil) authMock := rpcmocks.NewMockIAuthProvider(ctrl) + authMock.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) tenantMock := tenantmocks.NewMockITenantProvider(ctrl) workspaceMock := workspacemocks.NewMockIWorkSpaceProvider(ctrl) workspaceMock.EXPECT().GetThirdPartyQueryWorkSpaceID(gomock.Any(), int64(123)).Return("123").AnyTimes() @@ -3778,6 +3782,7 @@ func TestOpenAPIApplication_DeleteAnnotation_AdditionalScenarios(t *testing.T) { fieldsGetter: func(ctrl *gomock.Controller) fields { traceServiceMock := servicemocks.NewMockITraceService(ctrl) authMock := rpcmocks.NewMockIAuthProvider(ctrl) + authMock.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) benefitMock := benefitmocks.NewMockIBenefitService(ctrl) benefitMock.EXPECT().CheckTraceBenefit(gomock.Any(), gomock.Any()).Return(nil, assert.AnError) tenantMock := tenantmocks.NewMockITenantProvider(ctrl) @@ -3819,6 +3824,7 @@ func TestOpenAPIApplication_DeleteAnnotation_AdditionalScenarios(t *testing.T) { StorageDuration: 3, }, nil) authMock := rpcmocks.NewMockIAuthProvider(ctrl) + authMock.EXPECT().CheckWorkspacePermission(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) tenantMock := tenantmocks.NewMockITenantProvider(ctrl) workspaceMock := workspacemocks.NewMockIWorkSpaceProvider(ctrl) workspaceMock.EXPECT().GetThirdPartyQueryWorkSpaceID(gomock.Any(), int64(123)).Return("123").AnyTimes() diff --git a/backend/modules/observability/domain/trace/entity/loop_span/filter.go b/backend/modules/observability/domain/trace/entity/loop_span/filter.go index ad23e124e..9fd94b95a 100644 --- a/backend/modules/observability/domain/trace/entity/loop_span/filter.go +++ b/backend/modules/observability/domain/trace/entity/loop_span/filter.go @@ -93,7 +93,11 @@ var validFieldComb = map[FieldType]map[QueryTypeEnum]bool{ QueryTypeEnumNotEq: true, }, FieldTypeBool: { - QueryTypeEnumEq: true, + QueryTypeEnumEq: true, + QueryTypeEnumIn: true, + QueryTypeEnumNotIn: true, + QueryTypeEnumExist: true, + QueryTypeEnumNotExist: true, }, }