feat: HQL tracing via datadog

[devinat1]

• Added a new traceDecorator.ts utility that provides a flexible @Trace decorator for methods, supporting custom span names, tags, and lifecycle hooks for tracing.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
helicone	Error			Sep 12, 2025 8:31pm
helicone-bifrost	Ready	Preview	Comment	Sep 12, 2025 8:31pm
helicone-eu	Error			Sep 12, 2025 8:31pm

[greptile-apps]

Greptile Summary

This PR introduces DataDog distributed tracing capabilities for Helicone's HQL (Helicone Query Language) feature, which is currently in beta. The implementation includes two main components:

1. New Trace Decorator Utility (traceDecorator.ts): A flexible @Trace decorator that can be applied to methods to automatically instrument them with DataDog spans. The decorator supports custom span names, tags, lifecycle hooks (onStart, onSuccess, onError), and handles both synchronous and asynchronous functions.

2. Tracing Integration: The tracing is applied in two ways:

   • Manual instrumentation in HeliconeSqlManager.ts where spans are manually created and managed for core SQL operations (getClickhouseSchema, executeSql, downloadCsv) with comprehensive tagging for performance metrics, error states, and business context
   • Decorator-based instrumentation in heliconeSqlController.ts where the @Trace decorator is applied to all 10 public API endpoints for HQL functionality

The tracing implementation provides comprehensive observability with span tagging for execution times, row counts, data sizes, error states, organization IDs, and other business-specific metrics. This observability infrastructure is designed to monitor the performance and health of HQL operations as the feature transitions from beta to production, enabling better debugging, performance optimization, and system monitoring.

Confidence score: 2/5

• This PR has significant implementation issues that could cause runtime errors and needs careful review before merging
• Score reflects critical bugs in the trace decorator implementation, specifically double span finishing and inadequate error handling
• Pay close attention to traceDecorator.ts which contains the core bug that could impact all traced methods

_{3 files reviewed, 3 comments}

_{Edit Code Review Bot Settings | Greptile}

[greptile-apps]

logic: Potential inconsistency: rows variable is defined but result.data is still used in the return object. Consider using rows consistently.

Suggested change

	const rows = result.data ?? [];
	const size = Buffer.byteLength(JSON.stringify(rows), "utf8");
	span.setTag("hql.row_count", rows.length);
	span.setTag("hql.size_bytes", size);
	return ok({
	rows: result.data ?? [],
	const rows = result.data ?? [];
	const size = Buffer.byteLength(JSON.stringify(rows), "utf8");
	span.setTag("hql.row_count", rows.length);
	span.setTag("hql.size_bytes", size);
	return ok({
	rows: rows,

[greptile-apps]

Greptile Summary

This review covers only the changes made since the last review, not the entire PR. The developer has made changes to the traceDecorator.ts file that refactor how spans are finished in the @Trace decorator. The changes move span finishing logic from a centralized finally block to explicit calls in each execution path - adding span.finish() calls directly in both the synchronous success path (line 81) and synchronous error path (line 93), while keeping the existing call in the asynchronous finally block (line 71).

The stated goal was to provide more explicit control over span lifecycle management and remove defensive error handling that was silently swallowing potential span finishing errors. However, this refactoring introduces a critical bug where synchronous functions will have span.finish() called multiple times, which can cause runtime errors or corrupted tracing data in DataDog.

Confidence score: 1/5

• This PR introduces a critical bug that will cause immediate runtime issues with duplicate span finishing calls
• Score reflects a fundamental flaw in the span lifecycle management logic that breaks basic tracing functionality
• The traceDecorator.ts file requires immediate attention to fix the duplicate span finishing issue for synchronous functions

_{1 file reviewed, no comments}

_{Edit Code Review Bot Settings | Greptile}

[claude]

Claude finished @devinat1's task —— View job

---

PR Review: HQL Tracing via DataDog

Confidence Score: 2/10 - Do NOT merge this PR

Critical Issues Found 🚨

Major Bug: Duplicate Method Declaration in HeliconeSqlManager.ts

Location: valhalla/jawn/src/managers/HeliconeSqlManager.ts:394-397

There's a duplicate method declaration that will cause a compilation error:

async downloadCsv(sql: string): Promise<Result<string, HqlError>> {
  const span = tracer.startSpan("hql.downloadCsv");
  span.setTag("resource.name", "hql.downloadCsv");
  async downloadCsv(sql: string): Promise<Result<string, HqlError>> { // ❌ DUPLICATE

This appears to be a copy-paste error that duplicates the method signature and initial lines.

Critical Bug: Missing Finally Block in downloadCsv

Location: valhalla/jawn/src/managers/HeliconeSqlManager.ts:439-440

The downloadCsv method creates a span but has an incomplete finally block:

} finally {
  // Missing span.finish() call - will cause memory leaks
}

This will cause memory leaks as spans are never properly closed.

Additional Issues

1. Inconsistent Error Handling: The manual tracing in HeliconeSqlManager.ts doesn't consistently handle span finishing in error cases, while the decorator handles this automatically.

2. Mixed Tracing Patterns: The codebase uses both manual span management (in HeliconeSqlManager.ts) and decorator-based tracing (in heliconeSqlController.ts), which creates maintenance overhead.

3. Performance Impact: The decorator creates spans for every API call, which adds overhead. Consider implementing sampling for production environments.

Code Quality Assessment

Positive aspects:

• Comprehensive tracing coverage across HQL operations
• Good span tagging with business context (organization_id, row_count, etc.)
• Clean decorator API that's easy to use

Areas for improvement:

• Fix compilation errors before merge
• Consider standardizing on one tracing pattern (decorator vs manual)
• Add unit tests for the trace decorator
• Implement span sampling for production

Security Considerations

• Tracing data should not include sensitive information (SQL parameters, user data)
• Consider redacting or hashing sensitive span tags

Performance Considerations

• Tracing adds ~1-2ms overhead per operation
• Large result sets will increase serialization time for span tags
• Consider implementing trace sampling in high-traffic scenarios

---

Summary: This PR cannot be merged due to critical compilation errors and incomplete implementation. The duplicate method declaration and missing finally block must be fixed first. The tracing approach is sound but needs refinement.

Next Steps:

1. Fix the duplicate downloadCsv method declaration
2. Complete the finally block in downloadCsv
3. Add proper tests for the trace decorator
4. Consider standardizing the tracing approach

[devinat1]

Subsumed by #4750