[exporter/cassandra] fix error handling for insert failures #45205
Conversation
currently if a span or log insert fails, we log the error but return nil. this causes silent data loss since the collector thinks the export succeeded. fixed by returning the error immediately when insert fails, allowing the collector's retry logic to handle it properly.
|
Welcome, contributor! Thank you for your contribution to opentelemetry-collector-contrib. Important reminders:
A maintainer will review your pull request soon. Thank you for helping make OpenTelemetry better! |
|
Please add a test case showcasing the error being returned. What is the behavior you want? Retry? Permanent failure? |
Co-authored-by: Antoine Toulme <[email protected]>
added test file that documents the expected behavior when cassandra inserts fail. the test is currently skipped because properly mocking gocql.Session requires additional infrastructure, but it clearly documents what should be tested. happy to add a full mock-based test if you can point me to the right mocking pattern used elsewhere in the project.
BashfulHippo
force-pushed
the
fix-cassandra-error-handling
branch
from
6aa5bdc to
e671743
Compare
|
thanks for the review! regarding retry behavior: the fix enables retry by returning errors to the collector's built-in retry process. this handles momentary failures (network issues, busy server, etc) which is standard exporter behavior. regarding the test: i added tests that verify:
testing actual cassandra insert failures seems to be tricky because gocql.Session has no interface to mock against. I looked at clickhouseexporter (a similar database exporter) and they only test export errors in integration tests, not unit tests. happy to add integration tests or create some mock infrastructure if you can point me toward the right pattern, but the current tests follow the existing conventions in the codebase. |
|
Please add tests for logs too. |
similar to traces tests, verifies that: - pushLogsData properly returns errors - empty logs don't cause errors - error handling contract is documented
|
added logs tests as requested |
3 participants
Description
currently if a span or log insert fails in the cassandra exporter, we log the error but return nil to the collector. this causes silent data loss since the collector thinks the export succeeded and moves on.
this PR fixes the bug by returning the error immediately when an insert fails, which allows the collector's retry process to handle it properly.
changes:
exporter_traces.go: return error on span insert failure instead of just loggingexporter_logs.go: return error on log insert failure instead of just loggingLink to tracking issue
N/A - found during code review
Testing
ran the existing test suite:
all tests pass. verified with go fmt and go vet.
note: the component doesn't currently have integration tests that mock cassandra insert failures, so I verified the fix by reviewing the control flow and error paths manually.
Documentation
no documentation changes needed - this is a bug fix that makes the actual behavior match expected behavior (exporters should return errors on failure, not silently succeed).