feat(tracing): adding tracing to DA client

[chatton]

Overview

Add tracing wrapper around DA interactions

[claude]

Claude finished @chatton's task in 2m 9s —— View job

---

Code Review Complete ✓

I've conducted a comprehensive review of this PR that adds OpenTelemetry tracing to the DA client. Overall, this is a solid implementation that follows established patterns from the eth client tracing (PR #2960). Below are my findings organized by focus area:

---

1. Code Quality & Architecture ✅

Strengths:

• Clean decorator pattern matching the existing tracedEthRPCClient implementation
• Proper interface compliance: tracedClient correctly implements FullClient
• Good separation of concerns: tracing logic isolated from business logic
• Consistent tracer naming: "ev-node/da" follows project conventions
• Pass-through methods (lines 126-133) correctly delegate without adding overhead

Observations:

• The implementation is idiomatic Go with proper use of OpenTelemetry SDK
• No cyclic dependencies or architectural concerns
• Integration point in block/public.go:59-62 is clean and conditional

---

2. Error Handling & Resilience ⚠️

Issues Identified:

Critical: Inconsistent error recording (line 62 in tracing.go)

The variable name is misleading:

span.RecordError(&submitError{msg: res.Message})

This is used for both Submit (line 42) and Retrieve (line 62), but it's always called submitError. Consider:

Fix naming →

Minor: StatusNotFound handling inconsistency (line 61)

The Retrieve method excludes StatusNotFound from error recording (line 61), which is appropriate since "not found" is a valid state, but this could be documented:

// StatusNotFound is not an error condition for Retrieve
if res.Code != datypes.StatusSuccess && res.Code != datypes.StatusNotFound {

Strengths:

• Proper error propagation: all errors returned with spans marked
• Consistent use of codes.Error for failed operations
• No panic risks in the tracing layer

---

3. Security 🔒

Potential Issues:

Low Risk: Namespace exposure in traces (lines 35, 54, 74, 93, 112)

All methods log the namespace as hex-encoded strings:

attribute.String("da.namespace", hex.EncodeToString(namespace))

Question: Could namespace values be considered sensitive in your threat model? If namespaces contain identifiable information, this could leak in traces. However, given that:

• Namespaces are configured in config.DA and appear to be system-level identifiers
• The data itself is not logged, only metadata
• This follows the eth tracing pattern

Recommendation: Document any sensitivity considerations in CLAUDE.md if applicable.

Strengths:

• No credential or key exposure
• No PII in trace attributes
• Blob data itself is never logged (only sizes/counts)

---

4. Performance & Resource Efficiency ✅

Strengths:

• Zero allocation for pass-through methods (lines 126-133)
• Efficient attribute construction: all attributes set at span creation or before span.End()
• Proper span lifecycle: all spans have defer span.End() immediately after creation
• No blocking operations or resource leaks
• Blob size calculation in Submit (lines 27-30) is O(n) but necessary

Observations:

• hex.EncodeToString() allocates, but unavoidable for human-readable traces
• Context propagation is correct: traced context passed to inner client

---

5. Testing & Reliability ⚠️

Coverage Issues:

40 lines missing coverage (per Codecov)

The test file has good coverage of happy paths but is missing:

1. Missing GetProofs tests - No tests for GetProofs method (lines 89-106 in tracing.go)
2. Missing Validate tests - No tests for Validate method (lines 108-124 in tracing.go)
3. Missing namespace accessor tests - Pass-through methods untested (lines 126-133)

Add missing tests →

Strengths:

• Well-structured tests using testify and OTEL test recorder
• Good test helper: setupDATrace reduces boilerplate
• Both success and error paths tested for Submit, Retrieve, and Get
• Mock implementation is clean and focused

Test quality observations:

• Lines 93-101 (TestTracedDA_Submit_Success): The namespace hex length assertion is creative but fragile - direct value comparison would be clearer
• The requireAttribute helper (lines 202-222) is well-designed for maintainability

---

6. Documentation 📝

Needs Improvement:

Missing godoc comments

Only the tracedClient type has a comment (line 15). Add comments for:

• WithTracingClient function (line 22) - should explain when/why to use
• submitError type (line 135) - explain its purpose

Example:

// WithTracingClient wraps a FullClient with OpenTelemetry tracing.
// Each DA operation (Submit, Retrieve, Get, GetProofs, Validate) will create
// a span with relevant attributes. Errors are automatically recorded.
// Returns the original client if tracing is not configured.
func WithTracingClient(inner FullClient) FullClient {

Positive:

• Integration in block/public.go is self-documenting with clear conditional logic

---

Additional Observations

Regarding the review comment asking about "da.test":

Looking at the .gitignore change (line 9), the pattern *.test was added. This is correct and standard Go practice - it ignores compiled test binaries like da.test. This should not be removed as it prevents accidentally committing test binaries. ✅

Attribute naming consistency:

The attributes follow OpenTelemetry semantic conventions reasonably well:

• blob.count, blob.total_size_bytes - clear and namespaced
• da.namespace, da.height - domain-specific and appropriate
• id.count, proof.count, result.count - consistent pattern

---

Summary & Recommendations

Must Fix Before Merge:

None - the code is production-ready as-is.

Should Fix (Priority Order):

1. Add tests for GetProofs and Validate to achieve >70% patch coverage
2. Rename submitError to daError or resultError for clarity
3. Add godoc comments for exported function and types

Nice to Have:

• Document namespace sensitivity considerations if applicable
• Simplify the hex length test assertion in TestTracedDA_Submit_Success

Strengths to Highlight:

• Clean implementation following established patterns
• Proper error handling and resource management
• Good integration with existing codebase
• No blocking issues for production use

---

Overall Assessment: This is a well-crafted PR that maintains consistency with the existing tracing implementation. The code is safe for production use. I recommend addressing the test coverage gap and documentation improvements in a follow-up PR if not in this one.

[github-actions]

The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
✅ passed	⏩ skipped	✅ passed	✅ passed	Jan 13, 2026, 3:41 PM

[codecov]

Codecov Report

❌ Patch coverage is 56.52174% with 40 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.73%. Comparing base (7726a64) to head (8df2f12).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
block/internal/da/tracing.go	59.09%	36 Missing ⚠️
block/public.go	0.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2968      +/-   ##
==========================================
- Coverage   57.85%   57.73%   -0.12%     
==========================================
  Files          97       98       +1     
  Lines        9303     9394      +91     
==========================================
+ Hits         5382     5424      +42     
- Misses       3318     3367      +49     
  Partials      603      603              

Flag	Coverage Δ
combined	57.73% <56.52%> (-0.12%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tac0turtle]

can this be removed?

[chatton]

yeah just saw the comment, removed it.

[tac0turtle]

utACK