Skip to content

Commit d706c80

Browse files
authored
Fix OTLP handling of identity content-encoding (#8277)
1 parent 2e385f8 commit d706c80

File tree

4 files changed

+61
-4
lines changed

4 files changed

+61
-4
lines changed

exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractGrpcTelemetryExporterTest.java

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,8 @@ public abstract class AbstractGrpcTelemetryExporterTest<T, U extends Message> {
131131

132132
private static final AtomicInteger grpcEncodingServerAttempts = new AtomicInteger();
133133

134+
private static volatile String grpcEncodingServerEncoding = "brotli";
135+
134136
@RegisterExtension
135137
@Order(1)
136138
static final SelfSignedCertificateExtension certificate = new SelfSignedCertificateExtension();
@@ -243,7 +245,8 @@ private static <R extends Message> void handleExport(
243245
}
244246
}
245247

246-
// A minimal server that returns grpc-encoding: brotli to test unsupported encoding handling.
248+
// A minimal server that returns a configurable grpc-encoding to test encoding handling.
249+
// The encoding is controlled via grpcEncodingServerEncoding (defaults to "brotli").
247250
// Both OkHttpGrpcSender (our code) and UpstreamGrpcSender (grpc-java framework) reject unknown
248251
// encodings and fail the export with INTERNAL status.
249252
@RegisterExtension
@@ -261,7 +264,7 @@ protected void configure(ServerBuilder sb) {
261264
return HttpResponse.of(
262265
ResponseHeaders.builder(HttpStatus.OK)
263266
.contentType(MediaType.parse("application/grpc+proto"))
264-
.add("grpc-encoding", "brotli")
267+
.add("grpc-encoding", grpcEncodingServerEncoding)
265268
.add("grpc-status", "0")
266269
.build(),
267270
HttpData.wrap(frame));
@@ -329,6 +332,7 @@ void reset() {
329332
attempts.set(0);
330333
httpRequests.clear();
331334
grpcEncodingServerAttempts.set(0);
335+
grpcEncodingServerEncoding = "brotli";
332336
}
333337

334338
@Test
@@ -461,6 +465,36 @@ void unsupportedGrpcMessageEncoding() {
461465
}
462466
}
463467

468+
// Verifies that a response with flag=1 and grpc-encoding: identity is rejected with INTERNAL.
469+
// Although identity is advertised in grpc-accept-encoding (meaning "no encoding applied"),
470+
// a server setting the compressed flag to 1 is contradictory and should not be accepted.
471+
@Test
472+
@SuppressLogger(GrpcExporter.class)
473+
void identityGrpcMessageEncodingWithCompressedFlagRejected() {
474+
grpcEncodingServerEncoding = "identity";
475+
try (TelemetryExporter<T> testExporter =
476+
exporterBuilder()
477+
.setEndpoint(grpcEncodingServer.httpUri().toString())
478+
.setRetryPolicy(null)
479+
.build()) {
480+
CompletableResultCode result =
481+
testExporter
482+
.export(Collections.singletonList(generateFakeTelemetry()))
483+
.join(10, TimeUnit.SECONDS);
484+
assertThat(result.isSuccess()).isFalse();
485+
assertThat(grpcEncodingServerAttempts).hasValue(1);
486+
assertThat(result.getFailureThrowable())
487+
.isInstanceOfSatisfying(
488+
FailedExportException.GrpcExportException.class,
489+
ex ->
490+
assertThat(requireNonNull(ex.getResponse()))
491+
.satisfies(
492+
grpcResponse ->
493+
assertThat(grpcResponse.getStatusCode())
494+
.isEqualTo(GrpcStatusCode.INTERNAL)));
495+
}
496+
}
497+
464498
@Test
465499
void multipleItems() {
466500
List<T> telemetry = new ArrayList<>();

exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractHttpTelemetryExporterTest.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -651,6 +651,25 @@ void responseBodyBoundsGzipCompressed() throws IOException {
651651
.hasMessageContaining("HTTP response body exceeded limit of"));
652652
}
653653

654+
@Test
655+
void identityResponseContentEncoding() {
656+
// Server responds with Content-Encoding: identity, which means no encoding. The export should
657+
// succeed just like a response with no Content-Encoding header.
658+
AbstractMessageLite<?, ?> responseBody = exporter.exportResponse(0);
659+
httpErrors.add(
660+
HttpResponse.of(
661+
ResponseHeaders.builder(HttpStatus.OK)
662+
.contentType(MediaType.PLAIN_TEXT_UTF_8)
663+
.add("Content-Encoding", "identity")
664+
.build(),
665+
HttpData.wrap(responseBody.toByteArray())));
666+
CompletableResultCode result =
667+
exporter
668+
.export(Collections.singletonList(generateFakeTelemetry()))
669+
.join(10, TimeUnit.SECONDS);
670+
assertThat(result.isSuccess()).isTrue();
671+
}
672+
654673
@Test
655674
@SuppressLogger(HttpExporter.class)
656675
void unsupportedResponseContentEncoding() {

exporters/sender/jdk/src/main/java/io/opentelemetry/exporter/sender/jdk/internal/JdkHttpSender.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,9 @@ private HttpResponse toHttpResponse(java.net.http.HttpResponse<InputStream> resp
350350
: (int) (maxResponseBodySize + 1);
351351

352352
String contentEncoding = response.headers().firstValue("Content-Encoding").orElse(null);
353-
if (contentEncoding != null && !"gzip".equalsIgnoreCase(contentEncoding)) {
353+
if (contentEncoding != null
354+
&& !"gzip".equalsIgnoreCase(contentEncoding)
355+
&& !"identity".equalsIgnoreCase(contentEncoding)) {
354356
throw new UnsupportedContentEncodingException(
355357
"Unsupported Content-Encoding: " + contentEncoding);
356358
}

exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSender.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,9 @@ private void handleResponse(
160160
Response response, Consumer<HttpResponse> onResponse, Consumer<Throwable> onError) {
161161
try (ResponseBody body = response.body()) {
162162
String contentEncoding = response.header("Content-Encoding");
163-
if (contentEncoding != null && !"gzip".equalsIgnoreCase(contentEncoding)) {
163+
if (contentEncoding != null
164+
&& !"gzip".equalsIgnoreCase(contentEncoding)
165+
&& !"identity".equalsIgnoreCase(contentEncoding)) {
164166
onError.accept(new IOException("Unsupported Content-Encoding: " + contentEncoding));
165167
return;
166168
}

0 commit comments

Comments
 (0)