Skip to content

fix(observability-tracing-openobserve): return empty result when stream not yet created#114

Open
mvanhorn wants to merge 2 commits into
openchoreo:mainfrom
mvanhorn:osc/3435-traces-empty-stream
Open

fix(observability-tracing-openobserve): return empty result when stream not yet created#114
mvanhorn wants to merge 2 commits into
openchoreo:mainfrom
mvanhorn:osc/3435-traces-empty-stream

Conversation

@mvanhorn
Copy link
Copy Markdown

@mvanhorn mvanhorn commented May 17, 2026

Purpose

Moves the OpenObserve empty-stream handling from the observer interface (openchoreo/openchoreo) into this adapter, per @akila-i's review feedback on openchoreo/openchoreo#3439. The observer interface stays adapter-agnostic; OpenObserve-specific codes (20002 = "stream not found") live in the adapter that actually knows them.

Refs:

Bug

When a component has never emitted spans, OpenObserve responds to a search with HTTP 400 and body {"code":20002,"message":"Search stream not found: default"} because the search stream is created lazily on first write. The adapter's executeSearchQuery currently treats any non-200 as a hard failure, so "no data yet" surfaces as HTTP 500 to the API client through the observer interface.

Approach

Single file edit on the adapter side, plus tests. All changes are confined to observability-tracing-openobserve/internal/openobserve/.

  • New constant openObserveStreamNotFoundCode = 20002 and a small isStreamNotFound([]byte) bool helper that parses the OpenObserve error envelope. Tolerant: empty body, malformed JSON, or JSON without code all return false, so existing error paths are unchanged for everything except this one specific OpenObserve response.
  • New exported sentinel ErrStreamNotFound (Go convention for "expected non-error state", like sql.ErrNoRows).
  • executeSearchQuery returns the sentinel when the body indicates code 20002.
  • Each public client method (GetTraces, GetSpans, GetSpanDetail) translates the sentinel into the appropriate zero-value result and nil error. For GetTraces/GetSpans, if only the count query returns the sentinel (rare), the search results are returned with Total = len(results) rather than failing.

Tests

Four new tests in client_test.go:

  • TestExecuteSearchQuery_StreamNotFound - asserts the sentinel is returned for a 400+code:20002 response.
  • TestGetTraces_StreamNotFound - asserts the public API returns an empty *TracesResult and nil error.
  • TestGetSpans_StreamNotFound - symmetric for spans.
  • TestIsStreamNotFound - table-driven test for the parser (matching code, other code, missing code, empty body, malformed JSON).

All pass; existing tests unaffected; go vet clean.

What this doesn't change

  • The observer interface in openchoreo/openchoreo. The companion PR there (#3439) can now be closed in favor of this one - the OpenObserve-specific behavior lives where it should.
  • The metrics or logs adapters. The same pattern likely applies if those streams are also lazy, but I kept this PR scoped to tracing for reviewability. Happy to follow up with symmetric PRs against observability-logs-openobserve and observability-metrics-* if you'd like.

This contribution was developed with AI assistance (Claude Code).

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling when a configured search stream is missing: trace and span list queries now return empty results (with preserved timing/total metadata) instead of failing, and span detail requests return a consistent "span not found" response.
  • Tests

    • Added unit tests covering missing-stream scenarios, varied response bodies, and mixed count/list outcomes.
  • Chores

    • Bumped chart version and appVersion for the observability tracing package.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 17, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5cffeef3-e0a8-41b2-b083-04b7ba5c110c

📥 Commits

Reviewing files that changed from the base of the PR and between 8061aa1 and b9dba36.

📒 Files selected for processing (2)
  • observability-tracing-openobserve/helm/Chart.yaml
  • observability-tracing-openobserve/internal/openobserve/client_test.go
✅ Files skipped from review due to trivial changes (1)
  • observability-tracing-openobserve/helm/Chart.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
  • observability-tracing-openobserve/internal/openobserve/client_test.go

📝 Walkthrough

Walkthrough

Adds ErrStreamNotFound sentinel and detection helper, makes executeSearchQuery return it when OpenObserve signals a missing stream, and updates GetTraces/GetSpans/GetSpanDetail to treat that case as "no data" (or "span not found"); tests and a Helm chart version bump included.

Changes

Stream-not-found error handling

Layer / File(s) Summary
Stream-not-found error contract and detection primitives
observability-tracing-openobserve/internal/openobserve/client.go
Adds errors import, defines ErrStreamNotFound sentinel, introduces internal error-code constant, and implements isStreamNotFound helper to parse the error envelope.
Query execution error detection
observability-tracing-openobserve/internal/openobserve/client.go
executeSearchQuery inspects non-200 response bodies for the stream-not-found error code and returns ErrStreamNotFound instead of a generic status error.
GetTraces stream-not-found handling
observability-tracing-openobserve/internal/openobserve/client.go
Main list query converts ErrStreamNotFound to an empty TracesResult; the separate count query also treats it as non-fatal, returning totals derived from parsed traces and setting TookMs from the successful response.
GetSpans stream-not-found handling
observability-tracing-openobserve/internal/openobserve/client.go
Main list query converts ErrStreamNotFound to an empty SpansResult; the separate count query also treats it as non-fatal, returning totals derived from parsed spans and setting TookMs from the successful response.
GetSpanDetail stream-not-found handling
observability-tracing-openobserve/internal/openobserve/client.go
Converts ErrStreamNotFound into a "span not found" error message keyed by traceId and spanId, consistent with zero-hit behavior.
Test coverage for stream-not-found behavior
observability-tracing-openobserve/internal/openobserve/client_test.go
Adds errors import and tests verifying executeSearchQuery returns ErrStreamNotFound, that GetTraces/GetSpans return non-nil empty results (and CountQueryOnly scenarios), and that isStreamNotFound detects the condition across multiple JSON shapes.
Helm chart metadata
observability-tracing-openobserve/helm/Chart.yaml
Bumps version and appVersion from 0.2.2 to 0.2.3.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Poem

I sniffed the stream and found it thin,
The query answered with a grin.
No traces there, the logs are mild —
I hop along, a satisfied child.
🐇✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: handling the OpenObserve stream-not-found case by returning empty results instead of errors when a stream hasn't been created yet.
Description check ✅ Passed The description comprehensively covers Purpose, Approach, related issues, and testing strategy with clear context on the bug, solution, and scope boundaries.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@observability-tracing-openobserve/internal/openobserve/client_test.go`:
- Around line 971-1046: Add a unit test for GetSpanDetail that verifies
stream-not-found is translated into a "span not found" error: spin up an
httptest server that returns HTTP 400 with body {"code":20002,"message":"Search
stream not found: default"}, create the client with newTestClient(server.URL),
call client.GetSpanDetail(context.Background(),
TracesQueryParams{TraceID:"trace-1", SpanID:"span-1"}), assert that err is
non-nil and that err.Error() contains both "span" and "not found" (use
TestGetSpanDetail_StreamNotFound as the test name to match existing style).
- Around line 985-1003: Add tests that exercise the count-query-only
stream-not-found path: create TestGetTraces_StreamNotFound_CountQueryOnly (and a
symmetric TestGetSpans_StreamNotFound_CountQueryOnly) that spins up an
httptest.Server which returns a successful body for the main query endpoint
(containing one or more traces/spans) but returns HTTP 400 with JSON
{"code":20002,...} for the count endpoint; call client.GetTraces /
client.GetSpans (using newTestClient and TracesQueryParams/SpansQueryParams),
assert err == nil, result is non-nil, len(result.Traces or result.Spans) matches
the returned items and result.Total == len(result.Traces or result.Spans) to
verify the sentinel on the count query sets Total to the number of returned
items.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5cc3f913-f93f-4be1-9021-2b8c11a65af5

📥 Commits

Reviewing files that changed from the base of the PR and between fe64eb8 and 6edec74.

📒 Files selected for processing (2)
  • observability-tracing-openobserve/internal/openobserve/client.go
  • observability-tracing-openobserve/internal/openobserve/client_test.go

…am not yet created

OpenObserve returns HTTP 400 with code 20002 when a search target stream
has not yet been created (the stream is lazy - created on first write).
The adapter currently treats this as a hard failure, which surfaces as
HTTP 500 from the observer API. "No data yet" is not a server error;
the API should return an empty result set.

Adds a sentinel ErrStreamNotFound and a tolerant isStreamNotFound helper
that parses the OpenObserve error envelope. executeSearchQuery returns
the sentinel when the body says code == 20002; GetTraces, GetSpans, and
GetSpanDetail each translate the sentinel into the appropriate
zero-value result.

Companion to openchoreo/openchoreo#3439 - moves the OpenObserve-specific
error-code handling out of the observer interface (which should stay
adapter-agnostic) and into the adapter that actually knows the codes,
per @akila-i's review feedback.

Refs openchoreo/openchoreo#3435

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
@mvanhorn mvanhorn force-pushed the osc/3435-traces-empty-stream branch from 6edec74 to 8061aa1 Compare May 17, 2026 22:21
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 14 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...tracing-openobserve/internal/openobserve/client.go 50.00% 11 Missing and 3 partials ⚠️

📢 Thoughts on this report? Let us know!

…m-not-found test coverage

Addresses CI + review feedback on openchoreo#114:

- helm/Chart.yaml: bump version + appVersion from 0.2.2 to 0.2.3 so
  check-version-bump accepts the module changes.
- client_test.go: add three tests coderabbit flagged as missing on the
  review.
  - TestGetSpanDetail_StreamNotFound verifies the sentinel maps to a
    "span not found" error.
  - TestGetTraces_StreamNotFound_CountQueryOnly and the spans variant
    verify the documented partial-failure path: main query succeeds,
    count query returns code 20002, GetTraces / GetSpans return the
    returned hits with Total = len(results) and err == nil.

Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
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.

2 participants