fix(otelutil,search-service): env-skip OTLP and env-drive search indices#166
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR introduces two independent changes: conditional OTLP tracer initialization that skips exporter creation when endpoint environment variables are unset, and configurable Elasticsearch index selection in the search service via required environment variables and handler configuration wiring. ChangesOTLP Tracer Conditional Initialization
Configurable Elasticsearch Indices
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
search-service/handler_test.go (1)
87-87: ⚡ Quick winRemove the dead
SpotlightIndexconstant from production code or move it to_test.go.The
SpotlightIndexconstant is declared inquery_rooms.gobut not used anywhere in production code. Handler and main use the config value (h.cfg.SpotlightIndexandcfg.Search.SpotlightIndex) instead. The constant only appears in tests (lines 87 and 238). Either remove it and replace test references with a local sentinel value, or move it into a test helper file.🤖 Prompt for 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. In `@search-service/handler_test.go` at line 87, Remove the unused production constant SpotlightIndex from query_rooms.go and update tests to reference a test-local sentinel instead: delete the SpotlightIndex declaration, then in tests (e.g., handler_test.go lines referencing SpotlightIndex) replace those references with a test-only constant defined in the _test.go file (or a test helper), ensuring production uses h.cfg.SpotlightIndex / cfg.Search.SpotlightIndex unchanged; update imports if you move the sentinel to a helper test file.
🤖 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.
Nitpick comments:
In `@search-service/handler_test.go`:
- Line 87: Remove the unused production constant SpotlightIndex from
query_rooms.go and update tests to reference a test-local sentinel instead:
delete the SpotlightIndex declaration, then in tests (e.g., handler_test.go
lines referencing SpotlightIndex) replace those references with a test-only
constant defined in the _test.go file (or a test helper), ensuring production
uses h.cfg.SpotlightIndex / cfg.Search.SpotlightIndex unchanged; update imports
if you move the sentinel to a helper test file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0eb23879-aadc-451f-8226-35ab38f9a5b6
📒 Files selected for processing (5)
pkg/otelutil/otel.gosearch-service/deploy/docker-compose.ymlsearch-service/handler.gosearch-service/handler_test.gosearch-service/main.go
Two small dev-loop quality fixes bundled into one PR. pkg/otelutil: - InitTracer skips the OTLP gRPC exporter setup when neither OTEL_EXPORTER_OTLP_ENDPOINT nor OTEL_EXPORTER_OTLP_TRACES_ENDPOINT is set, installing only the W3C TraceContext propagator and returning a no-op shutdown. Stops `traces export: connection refused` log spam in local dev / CI when no OTel collector runs. With the env set, behavior is unchanged. search-service: - New SPOTLIGHT_INDEX env var (required), plumbed through handlerConfig into searchRooms. handler.go was hardcoding the package-level const `SpotlightIndex = "spotlight"` which 404s against site-suffixed indices written by search-sync-worker. - Tighten USER_ROOM_INDEX from `envDefault:""` to `,required` so the empty-default 404 is caught at startup instead of silently returning zero results. - deploy/docker-compose.yml pins both env vars to the concrete site-local indices so `make up` works against a fresh dev stack. Prod uses aliases owned by ops/IaC. https://claude.ai/code/session_01UkLD7hpaypxjeh5zbEWTjp
Joey0538
force-pushed
the
claude/otelutil-and-spotlight-env
branch
from
b56c932 to
8b890a9
Compare
3 participants
Summary
Two small dev-loop quality fixes pulled out of the PR #148 dev-stack rebase. Both are env-driven and have no prod behavior change beyond their explicit env triggers.
pkg/otelutilInitTracerno longer attempts to dial an OTLP gRPC collector when neitherOTEL_EXPORTER_OTLP_ENDPOINTnorOTEL_EXPORTER_OTLP_TRACES_ENDPOINTis set. Installs only the W3C TraceContext propagator (so trace IDs still flow between services) and returns a no-op shutdown. Stops thetraces export: connection refusedlog spam in local dev / CI runs that don't have a collector. Behavior is unchanged once either env is set.search-serviceSPOTLIGHT_INDEXenv var (,required), plumbed throughhandlerConfigintosearchRooms.handler.gowas hardcoding the package-level constSpotlightIndex = "spotlight"which 404s against the site-suffixed concrete indices written bysearch-sync-worker.USER_ROOM_INDEXtightened fromenvDefault:""to,required— same failure mode otherwise (empty default → silently zero results).deploy/docker-compose.ymlpins both env vars to the concretesite-localindices somake upworks against a fresh dev stack. Prod uses aliases owned by ops/IaC.Test plan
make lintcleanmake testclean (full repo)go build ./...cleanmake upagainst a fresh stack: search-service starts,searchRoomsreturns data instead of 404OTEL_EXPORTER_OTLP_*env: notraces export: connection refusedlinesNotes
make lintfailure on main fromhistory-service/internal/mongorepowas already fixed in feat(searchengine): support Basic Auth for ES connections #165, so this PR rebases cleanly without bundling the workaround.https://claude.ai/code/session_01UkLD7hpaypxjeh5zbEWTjp
Generated by Claude Code
Summary by CodeRabbit
New Features
Chores