jaegerquery: propagate query server health to collector runtime #7992
Conversation
Signed-off-by: aaryan359 <aaryanmeena96@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7992 +/- ##
=======================================
Coverage 95.47% 95.47%
=======================================
Files 316 316
Lines 16756 16756
=======================================
Hits 15997 15997
Misses 593 593
Partials 166 166
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: aaryan359 <aaryanmeena96@gmail.com>
|
@jkowall, @yurishkuro, can you review this? |
| if err != nil { | ||
| s.telset.Logger.Error("Could not start GRPC server", zap.Error(err)) | ||
| s.reportHealthStatus(componentstatus.StatusFatalError) | ||
| s.telset.ReportStatus(componentstatus.NewFatalErrorEvent(err)) |
There was a problem hiding this comment.
there is literally a call to report status in the next line. What's the point of duplicating it?
| } | ||
|
|
||
| // Report initial status as OK | ||
| s.reportHealthStatus(componentstatus.StatusOK) |
There was a problem hiding this comment.
what's the point of this if nothing got started yet?
Signed-off-by: aaryan359 <aaryanmeena96@gmail.com>
Signed-off-by: aaryan359 <aaryanmeena96@gmail.com>
|
@yurishkuro, I initially added explicit status reporting (OK / Stopped) based on a literal reading of the issue, but I understand now that in collector semantics, health is implicit and only failures should be reported. |
There was a problem hiding this comment.
Pull request overview
Propagates Jaeger query server health into the OpenTelemetry Collector runtime by removing outdated TODO/comment references and aligning server lifecycle code with updated status reporting behavior and tests.
Changes:
- Removed the outdated TODO about propagating healthcheck updates to the collector runtime.
- Removed/cleaned up a stale inline comment and adjusted formatting in server lifecycle methods.
- Minor whitespace tweaks for readability and consistency.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| cmd/jaeger/internal/extension/jaegerquery/server.go | Removes outdated TODO related to health propagation. |
| cmd/jaeger/internal/extension/jaegerquery/internal/server.go | Removes stale comment and applies minor formatting/whitespace adjustments in Start/Close. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
2 participants
Which problem is this PR solving?
Description of the changes
StatusOKwhen the query server starts successfullyStatusFatalErrorwhen HTTP or gRPC servers fail at runtimecomponentstatusandtelemetry.Settings.ReportStatusmechanismsHow was this change tested?
Checklist
jaeger:make lint testjaeger-ui: not applicable