search

open-telemetry / otel-arrow

Protocol and libraries for sending and receiving OpenTelemetry data using Apache Arrow
Apache License 2.0
68 stars 13 forks source link

Integration testing for memory limits #210

Closed jmacd closed 3 months ago

jmacd commented 3 months ago

This contains 3 PRs worth of work and will be refactored. The sequence is as follows:

  1. While race-testing the new integration test, the Arrow receiver turned up a bug identified by the race detector. the inFlightWG was shared by all streams in the receiver, so it would work in testing (almost always) but was incorrect. This may explain unusually long stream shutdowns.
  2. The Arrow exporter was not consistently using gRPC status errors, which the Reciever had fixed in https://github.com/open-telemetry/otel-arrow/pull/205; the code was out-of-line with the design of the top-level directory. Whereas the OTel-Arrow exporter had been inserting consumererror.NewPermanent() wrappers, it is the Exporter module which supports standard OTLP and Arrow one layer up that is responsible for permanent error labeling. Returning gRPC status errors is always preferred to fmt.Errorf in gRPC components.
  3. A new memory-limited integration test is added, which exposed the need for correct status error handling, also exposed an Arrow Consumer bug. The underlying Arrow IPC Reader object has an Err() method that was not being checked--instead we had (3a) a fallback mechanism, checking expected record count, and (3b) write to os.Stderr because of the (known) missing error.
jmacd commented 3 months ago

Replaced by all the above mentions.