Skip to content

Add OTLP trace export and bump spannerotel to v0.2.0#67

Merged
apstndb merged 3 commits into
mainfrom
feat/tracing-v0.2.0
Jun 28, 2026
Merged

Add OTLP trace export and bump spannerotel to v0.2.0#67
apstndb merged 3 commits into
mainfrom
feat/tracing-v0.2.0

Conversation

@apstndb

@apstndb apstndb commented Jun 28, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add --experimental-trace-otlp and --experimental-trace-otlp-endpoint for local OpenTelemetry collectors (xor with Cloud Trace and stderr export).
  • Bump spannerotel to v0.2.0 (OTLP export, service.name resource).
  • Expand README tracing docs (PROFILE plan spans, exporter matrix).

Test plan

  • go test ./...
  • Integration test covers tracing without OpenCensus bridge

Made with Cursor

apstndb and others added 3 commits June 29, 2026 05:21
Introduce --experimental-trace-otlp for local OpenTelemetry collectors,
set service.name via spannerotel v0.2.0, and document PROFILE plan spans.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds support for exporting OpenTelemetry spans via OTLP/gRPC to a local collector, introducing the --experimental-trace-otlp and --experimental-trace-otlp-endpoint CLI flags, updating the documentation, and upgrading dependencies. A critical issue was identified in trace.go where checking n != 1 for mutually exclusive flags prevents the optional case (where no tracing flags are set) from falling through to the default configuration, incorrectly triggering an error instead.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread trace.go
Comment on lines +30 to +32
if n != 1 {
return tracing.Config{}, fmt.Errorf("exactly one of --experimental-trace-otlp, --experimental-trace-stdout, or --experimental-trace-project must be set")
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

By checking n != 1, any case where no tracing flags are set (n == 0) will trigger the error exactly one of ... must be set. This makes the default branch in the switch statement (which returns tracing is not configured) completely unreachable.

Additionally, if traceConfig is called when tracing is disabled, the error message claiming that one of the flags must be set is misleading. Changing the check to n > 1 allows the n == 0 case to correctly fall through to the default branch.

Suggested change
if n != 1 {
return tracing.Config{}, fmt.Errorf("exactly one of --experimental-trace-otlp, --experimental-trace-stdout, or --experimental-trace-project must be set")
}
if n > 1 {
return tracing.Config{}, fmt.Errorf("at most one of --experimental-trace-otlp, --experimental-trace-stdout, or --experimental-trace-project can be set")
}
References
  1. When configuring mutually exclusive CLI flags, omit the required constraint if the flags are optional as a group, allowing the user to specify none of them.

@apstndb apstndb merged commit 6b4d5d2 into main Jun 28, 2026
2 checks passed
@apstndb apstndb deleted the feat/tracing-v0.2.0 branch June 28, 2026 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant