[exporter/elasticsearch] report connection health via componentstatus #39562
Conversation
faec
changed the title
…y-collector-contrib into elasticsearch-exporter-componentstatus
| } else if resp.StatusCode >= 300 { | ||
| // Error results | ||
| err := fmt.Errorf("Elasticsearch request failed: %v", resp.Status) | ||
| componentstatus.ReportStatus( | ||
| cl.componentHost, componentstatus.NewRecoverableErrorEvent(err)) | ||
| } |
There was a problem hiding this comment.
Are all of these recoverable? I would think a 401 would be non-recoverable without human intervention.
409s are not an error at all necessarily, when documents are truly duplicates. Is it worth special casing those? There should probably be an indicator they are happening but if someone explicitly is doing de-duplication via an _id in the document this will mark the exporter as degraded unnecessarily.
There was a problem hiding this comment.
"non-recoverable" in the collector is very unforgiving, to the extent that almost nothing we could encounter truly satisfies its definition (which is that the component can never again return to a healthy state no matter what for the life of the process -- so its real meaning isn't "requires human intervention" it's "requires intervention and a restart of the process"). For example I've seen some setups where ingest credentials are synced to the ingest workers before they're actually activated upstream (I don't know why, but it shouldn't break us), or similarly a 401 can be a side effect of broken or unreliable proxy settings that can be resolved without restarting the client process.
Good point about 409s though, those should be telemetry numbers rather than a component error state, I will add a special case for that.
There was a problem hiding this comment.
Thanks, just special casing 409s makes sense to me then.
There was a problem hiding this comment.
Change looks good, probably worth a test case for 409s specifically since they are now meant not to trigger recovery from an error state.
…y-collector-contrib into elasticsearch-exporter-componentstatus
|
Please address CI |
…y-collector-contrib into elasticsearch-exporter-componentstatus
|
Maybe I'm not understanding how to sync module versions -- "go get" on |
…y-collector-contrib into elasticsearch-exporter-componentstatus
…open-telemetry#39562) #### Description Report component status of the Elasticsearch exporter based on the response code when making ingestion requests. #### Testing Added unit test confirming status is reported on http response. Also tested manually with the collector and confirmed that the error conditions appear when querying collector health status. --------- Co-authored-by: Andrzej Stencel <[email protected]>
Description
Report component status of the Elasticsearch exporter based on the response code when making ingestion requests.
Testing
Added unit test confirming status is reported on http response. Also tested manually with the collector and confirmed that the error conditions appear when querying collector health status.