[feat][backend]trace impl mult repo #323
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 修复backend/modules/observability/infra/repo/ck/spans_test.go中的未定义QueryParam类型错误 - 修复backend/modules/observability/infra/repo/trace_test.go中的多个构建和运行时错误 - 创建backend/modules/observability/application/convertor/page_test.go用于测试新的convertor函数 - 创建backend/modules/observability/application/convertor/task/filter_test.go用于测试新的TaskFilters转换函数 - 修复mock文件中的包导入和类型引用错误 - 确保所有trace repo测试用例通过 主要修复内容包括： - 修正结构体字段名称不匹配问题(spansDao → spanDaos) - 添加缺失的mockStorageProvider实现 - 更新所有构造函数调用以使用正确的TraceRepoOption模式 - 修复mock期望以匹配实际方法签名 - 解决参数匹配问题 所有测试现已通过，确保代码库保持高质量和可靠性。
- 修复backend/modules/observability/domain/task/service/task_service_test.go中的构建错误 - 修复NewTaskServiceImpl构造函数调用，添加缺失的必需参数 - 创建stubIDGenerator、stubStorageProvider、stubTenantProvider实现 - 修复测试函数中直接使用未导出类型的问题 - 修复backend/modules/observability/domain/task/service/task_callback_test.go中的mock期望问题 - 添加缺失的GetTask方法期望 - 修复TaskCallbackServiceImpl初始化时缺少的taskRepo字段 - 修复backend/modules/observability/domain/trace/service/trace_service_test.go中的参数匹配问题 - 修复SearchTraceOApi测试中GetTrace参数期望不匹配问题 所有TaskServiceImpl、TaskCallbackServiceImpl和TraceServiceImpl相关的测试现已通过，确保代码库的稳定性和可靠性。
CozeLoop
suggested changes
Dec 2, 2025
Collaborator
CozeLoop
left a comment
CozeLoop
left a comment
There was a problem hiding this comment.
Summary of key findings:
- [🚫Must-Fix] Remove hard-coded environment injection via
context.WithValuein task_service.go (line 387). Use configuration/wiring instead; contexts should carry cancellation/timeouts and request-scoped values. - [🚫Must-Fix] Fix invalid
godirective in backend/go.mod (go 1.24.6→go 1.24; optionally keeptoolchain go1.24.6). - [🚫Must-Fix] Fix invalid
godirective in modules/observability/lib/go.mod (go 1.24.6→go 1.24; optionally keeptoolchain go1.24.6).
Other changes reviewed are scoped workspace additions, DAO refactors, wiring updates, and OTEL/loop span conversions. No correctness/concurrency/security risks identified within the modified lines.
| return errorx.NewByCode(obErrorx.CommonInternalErrorCode, errorx.WithExtraMsg("backfill producer not initialized")) | ||
| } | ||
| // todo ppe test | ||
| ctx = context.WithValue(ctx, "K_ENV", "ppe_6788399583") |
Collaborator
There was a problem hiding this comment.
[🚫Must-Fix] Hard-coded environment injected via context
- Problem: This line injects a hard-coded environment value into the request context ("ppe_6788399583"). Using
context.WithValuefor config and hard-coding an env breaks correctness across environments and violates Go’s context usage guidance (context is for cancellation, deadlines, and request-scoped values, not global config). It will force all backfill sends to use PPE and can leak into production unexpectedly. - Fix: Remove the hard-coded
WithValueand pass environment via typed configuration or explicit parameter wiring. Example:
// remove: ctx = context.WithValue(ctx, "K_ENV", "ppe_6788399583")
// use injected config/env instead (e.g., from t.config or service wiring)
return t.backfillProducer.SendBackfill(ctx, event)
- Reference: Go Code Review Comments – Context; Effective Go – Package context (use for cancellation/timeouts, avoid ad-hoc key/value globals).
| limit = 10000 | ||
| } | ||
| spans, err := r.traceRepo.GetTrace(ctx, &repo.GetTraceParam{ | ||
| WorkSpaceID: strconv.FormatInt(req.WorkspaceID, 10), |
Collaborator
There was a problem hiding this comment.
No risk found in this change.
| go 1.24.1 | ||
|
|
||
| toolchain go1.24.6 | ||
| go 1.24.6 |
Collaborator
There was a problem hiding this comment.
[🚫Must-Fix] Invalid go directive version in go.mod
- Problem: The
godirective must specify the Go language version asmajor.minor(e.g.,go 1.24). Usinggo 1.24.6is invalid and will cause tooling/build issues. If you need to pin a patch toolchain, use thetoolchaindirective instead. - Fix: Change to
go 1.24and optionally (re-)addtoolchain go1.24.6if your build requires a specific patch toolchain. - Reference: Go Modules Reference –
godirective semantics; Effective Go module guidelines.
| const PlatformType PlatformType_Workflow = "coze_workflow" | ||
| const PlatformType PlatformType_Ark = "ark" | ||
| const PlatformType PlatformType_VeADK = "veadk" | ||
| const PlatformType PlatformType_VeAgentkit = "ve_agentkit" |
Collaborator
There was a problem hiding this comment.
No risk found in this change.
| spans := otel.OtelSpansConvertToSendSpans(ctx, workspaceId, otelSpans) | ||
|
|
||
| tenantSpanMap := o.unpackTenant(ctx, spans) | ||
| tenantSpanMap := o.unpackTenant(ctx, tconv.OtelSpans2LoopSpans(spans)) |
Collaborator
There was a problem hiding this comment.
No risk found in this change.
|
|
||
| func (t *TraceCkRepoImpl) ListSpans(ctx context.Context, req *repo.ListSpansParam) (*repo.ListSpansResult, error) { | ||
| func (t *TraceRepoImpl) ListSpans(ctx context.Context, req *repo.ListSpansParam) (*repo.ListSpansResult, error) { | ||
| spanStorage := t.storageProvider.GetTraceStorage(ctx, req.WorkSpaceID, req.Tenants) |
Collaborator
There was a problem hiding this comment.
No risk found in this change.
| ) | ||
| ) | ||
|
|
||
| func provideTraceRepo( |
Collaborator
There was a problem hiding this comment.
No risk found in this change.
| List(context.Context, *ListAnnotationsParam) ([]*Annotation, error) | ||
| } | ||
|
|
||
| type Annotation struct { |
Collaborator
There was a problem hiding this comment.
No risk found in this change.
| } | ||
|
|
||
| func NewAnnotationCkDaoImpl(db ck.Provider) (IAnnotationDao, error) { | ||
| func NewAnnotationCkDaoImpl(db ck.Provider) (dao.IAnnotationDao, error) { |
Collaborator
There was a problem hiding this comment.
No risk found in this change.
| @@ -0,0 +1,33 @@ | |||
| module github.com/coze-dev/coze-loop/backend/modules/observability/lib | |||
|
|
|||
| go 1.24.6 | |||
Collaborator
There was a problem hiding this comment.
[🚫Must-Fix] Invalid go directive version in module go.mod
- Problem: The
godirective must bemajor.minor(e.g.,go 1.24).go 1.24.6is invalid for thegodirective and can break tooling. If pinning a patch toolchain is required, usetoolchain. - Fix: Change to
go 1.24and optionally addtoolchain go1.24.6. - Reference: Go Modules Reference –
godirective.
Change-Id: Idef45fe757a98f87c5f731934a47d11e57fcd953
Change-Id: I92b5ca667d4bf5aac67ca7ccd069902b664c4713
- 修复了buildTestTask函数中缺失的SpanFilter字段导致的空指针解引用问题 - 修正了SpanFilter相关的类型定义错误 - 补充了buildEvalTargetParam函数的测试用例，覆盖不同平台类型 - 添加了NewAutoEvaluteProcessor构造函数测试 - 补充了错误处理路径的测试用例 - 整体测试覆盖率从74.3%提升到76.5% - 关键函数buildEvalTargetParam覆盖率从66.7%提升到100%
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
Check the PR title
(Optional) Translate the PR title into Chinese
(Optional) More detailed description for this PR(en: English/zh: Chinese)
en:
zh(optional):
(Optional) Which issue(s) this PR fixes