Skip to content

feat(search): add search request observation logs and result metrics - #501

Open
eguguchkin wants to merge 3 commits into
mainfrom
0-extra-search-logs2
Open

feat(search): add search request observation logs and result metrics#501
eguguchkin wants to merge 3 commits into
mainfrom
0-extra-search-logs2

Conversation

@eguguchkin

Copy link
Copy Markdown
Collaborator

Description

This change adds request-level observability to the search path and refactors the way search execution statistics are propagated from the ingestor to the gRPC handlers.

Observability

  • Introduce requestObservation (proxyapi/request_observation.go) that collects timing and storage-tier data for a single search request. It is populated in doSearch and finalized via obs.finish(method, retErr) from each handler's
    deferred call — after the document stream has been consumed, so the recorded total_duration includes stream reading.
  • Emit a structured search request stat log line per request with method, agg/hist flags, docs count, storage tier, and per-phase durations (hot_duration, cold_duration, total_search_duration, fetch_duration,
    total_duration), plus the result category and error.
  • Add a new seq_db_ingestor_search_results_total Prometheus counter (metric/ingestor.go) labeled by result and tier.
  • Classify each request's outcome into success / client_error / server_error / timeout via classifySearchResult. The raw ingestor error is preserved separately (rawErr) so timeouts — which processSearchErrors wraps into
    codes.Internal — are still detected.

SearchStats refactor

  • Ingestor.Search now returns *search.SearchStats instead of a bare overallDuration. SearchStats carries size, agg/hist flags, storage tier (hot/cold/none), and hot/cold/total search durations. The SearchIngestor interface and
    its mock are updated accordingly.
  • doSearch returns the requestObservation alongside the response; all handlers (Search, StreamSearch, Export, Fetch, aggregations, histogram) wire up a defer obs.finish(...) and a named retErr return.

Streaming helpers

  • Add search.DocsIteratorSeq, an iter.Seq2[StreamingDoc, error] adapter, used to replace manual Next() loops in StreamSearch and Export.
  • Add SetDocsIteratorEvents to hook onStart/onFinish callbacks onto a DocsIterator, used to measure the actual document fetch duration (from first Next() to stream end/EOF) independently of the ingestor-side search duration.

Merged stream balance

  • newNMergedStreams now builds a balanced binary merge tree (split-in-half recursion) instead of a left-leaning chain, producing more even merge depth across input streams.

Tests

  • Integration tests and handler tests are updated for the new doSearch/Search signatures and the DocsIteratorSeq iteration pattern.

  • I have read and followed all requirements in CONTRIBUTING.md;
  • I used LLM/AI assistance to make this pull request;

@eguguchkin
eguguchkin requested review from cheb0 and forshev August 18, 2026 21:51
@eguguchkin eguguchkin added this to the v0.77.0 milestone Aug 18, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔴 Performance Degradation

Some benchmarks have degraded compared to the previous run.
Click on Show table button to see full list of degraded benchmarks.

Show table
Name Previous Current Ratio Verdict
Indexer-4 a73114 5781f5
678752766.00 B/op 763859882.00 B/op 1.13 🔴

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 78.04878% with 36 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.23%. Comparing base (a73114c) to head (29c4585).

Files with missing lines Patch % Lines
proxyapi/request_observation.go 80.39% 6 Missing and 4 partials ⚠️
proxy/search/ingestor.go 63.15% 7 Missing ⚠️
proxyapi/grpc_v1.go 74.07% 7 Missing ⚠️
proxy/search/docs_iterator.go 86.95% 2 Missing and 1 partial ⚠️
proxyapi/grpc_async_search.go 33.33% 1 Missing and 1 partial ⚠️
proxyapi/grpc_export.go 66.66% 1 Missing and 1 partial ⚠️
proxyapi/grpc_fetch.go 33.33% 1 Missing and 1 partial ⚠️
proxyapi/grpc_stream_search.go 66.66% 1 Missing and 1 partial ⚠️
proxy/search/merged_docs_iterator.go 88.88% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #501      +/-   ##
==========================================
- Coverage   71.33%   71.23%   -0.10%     
==========================================
  Files         233      234       +1     
  Lines       18999    19108     +109     
==========================================
+ Hits        13552    13612      +60     
- Misses       4419     4455      +36     
- Partials     1028     1041      +13     

☔ View full report in Codecov by Harness.
📢 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

Copy link
Copy Markdown
Contributor

🔴 Performance Degradation

Some benchmarks have degraded compared to the previous run.
Click on Show table button to see full list of degraded benchmarks.

Show table
Name Previous Current Ratio Verdict
MutexListAppend-4 a73114 8957f9
194.53 MB/s 165.95 MB/s 0.85 🔴
83148539.00 ns/op 96412632.00 ns/op 1.16 🔴

if retErr != nil {
fields = append(fields, zap.NamedError("error", retErr))
}
logger.Info("search request stat", fields...)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

{"level":"info","ts":"2026-08-19T13:24:51.249+0300","message":"search request stat","method":"ComplexSearch","agg":false,"hist":false,"docs":100,"tier":"hot","hot_duration":0.031910833,"total_search_duration":0.033224184,"fetch_duration":0,"total_duration":0.033482858,"result":"success"}

It's not possible to correlate the incoming search request and the result. I.e. if we don't log the request itlsef as well, then it looks like we just duplicate telemetry in logs. Some of that telemetry we have already deleted (like bulk stats) as far as I remember.

zap.Int("docs", o.stats.Size),
zap.String("tier", tier),
zap.Duration("hot_duration", o.stats.HotSearchDuration),
zap.Duration("total_search_duration", o.stats.TotalSearchDuration),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

it looks like total_search_duration is always 0 if it's search error:

{"level":"info","ts":"2026-08-19T13:28:51.822+0300","message":"search request stat","method":"ComplexSearch","agg":true,"hist":false,"docs":10,"tier":"hot","hot_duration":0.002872493,"total_search_duration":0,"fetch_duration":0,"total_duration":0.002891408,"result":"server_error","error":"rpc error: code = Internal desc = store forbids aggregation request: aggregation has too many fraction tokens"}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

btw just noticed there are parameters total_search_duration and total_duration, hard to tell how they are different without reading the code

zap.String("method", method),
zap.Bool("agg", o.stats.HasAgg),
zap.Bool("hist", o.stats.HasHist),
zap.Int("docs", o.stats.Size),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: If I don't look at the corresponding code, it looks like docs name is ambiguos. It's actually a size parameter of the incoming reuqest, but some might expected it to be the actual search result count (like found docs).

type requestObservation struct {
start time.Time
fetchStart time.Time
fetchDuration time.Duration

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: a liitle bit unclear with this field. This observation is not enabled for Fetch operation, yet in logs we have fetch_duration logged.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

strangely for Export operation I also hit fetch_duration equal to 0

searchResultSuccess = "success"
searchResultClientErr = "client_error"
searchResultServerErr = "server_error"
searchResultTimeout = "timeout"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

just a note: this "timeout" is only possible when store is timed out (we got context canceled from store). If a user presses Ctrl+C it's also a context canceled but we have server_error logged.

{"level":"info","ts":"2026-08-19T14:19:13.880+0300","message":"search request stat","method":"ComplexSearch","agg":false,"hist":false,"docs":100,"tier":"hot","hot_duration":0.822881595,"total_search_duration":0,"fetch_duration":0,"total_duration":0.822918864,"result":"server_error","error":"rpc error: code = Internal desc = rpc error: code = Canceled desc = context canceled"}

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.

4 participants