Enforce OTLP request size limits#8446
Conversation
|
280870d to
6720aae
Compare
|
@jkwatson Thanks — I pushed a follow-up that fixes the NullAway compile failure in the OTLP internal exporters and applies the naming nits to use constants and public API parameter names. |
jack-berg
left a comment
There was a problem hiding this comment.
A few small comments, but looks pretty good! Thanks for working on this!
| int contentLength = messageWriter.getContentLength(); | ||
| if (contentLength >= 0) { | ||
| return contentLength; | ||
| } |
There was a problem hiding this comment.
Under what conditions do we pass through this?
My read of the code shows that only Marshaler.toJsonMessageWriter produces a MessageWriter.getContentLength which can return a value <= 0.
I don't know that we can cheaply compute the / enforce the body size for OTLP/json. One more reason not to use it. Officially, we don't support it today. See #5833
afad5b0 to
3cf2f56
Compare
|
Fixed the remaining Spotless failure in |
jack-berg
left a comment
There was a problem hiding this comment.
Looks like there are still some build failures from static analysis.
Couple more nits to fix along with the build, but looks good to me
|
|
||
| TelemetryExporterBuilder<T> setMaxRequestBodySize(long maxRequestBodySize); | ||
|
|
||
| TelemetryExporterBuilder<T> setMaxRequestMessageSize(long maxRequestMessageSize); |
| CompletableResultCode result = new CompletableResultCode(); | ||
| result.failExceptionally(FailedExportException.grpcFailedExceptionally(exception)); | ||
| return result; |
There was a problem hiding this comment.
#nit: can initialize CompletableResultCode as failed with the exception:
| CompletableResultCode result = new CompletableResultCode(); | |
| result.failExceptionally(FailedExportException.grpcFailedExceptionally(exception)); | |
| return result; | |
| return CompletableResultCode.ofExceptionalFailure(FailedExportException.grpcFailedExceptionally(exception)); |
Fixes #8444.
Why
opentelemetry-proto#782now recommends that OTLP exporters enforce configurable outbound request/message size limits.What
docs/apidiffs/current_vs_latest/opentelemetry-exporter-otlp.txtfor the new public builder methodsTesting
I was not able to complete local Gradle verification on this Windows machine because Gradle / the Kotlin compiler daemon repeatedly failed to start due to paging-file / metaspace limits before compilation finished. The changes are pushed for CI verification.