Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ enum WebfluxTextMapGetter implements TextMapGetter<ServerWebExchange> {

@Override
public Iterable<String> keys(ServerWebExchange exchange) {
return exchange.getRequest().getHeaders().keySet();
return HeaderUtil.getKeys(exchange.getRequest().getHeaders());
}

@Nullable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,16 @@
package io.opentelemetry.instrumentation.spring.webflux.v5_3.internal;

import static java.util.Collections.emptyList;
import static java.util.Collections.emptySet;

import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.BiConsumer;
import java.util.function.Supplier;
import javax.annotation.Nullable;
import org.springframework.http.HttpHeaders;
Expand All @@ -23,6 +28,8 @@
public final class HeaderUtil {

@Nullable private static final MethodHandle GET_HEADERS;
@Nullable private static final MethodHandle FOR_EACH;
@Nullable private static final MethodHandle KEY_SET;

static {
GET_HEADERS =
Expand All @@ -32,6 +39,33 @@ public final class HeaderUtil {
() ->
findGetHeadersMethod(
MethodType.methodType(List.class, Object.class))); // before spring web 7.0

// Spring Web 7+
MethodHandle forEach = null;
try {
forEach =
MethodHandles.lookup()
.findVirtual(
HttpHeaders.class,
"forEach",
MethodType.methodType(void.class, BiConsumer.class));
} catch (Throwable t) {
// ignore - will fall back to keySet
}
FOR_EACH = forEach;

// Spring Web 6 and earlier
MethodHandle keySet = null;
if (FOR_EACH == null) {
try {
keySet =
MethodHandles.lookup()
.findVirtual(Map.class, "keySet", MethodType.methodType(Set.class));
} catch (Throwable t) {
// ignore
}
}
KEY_SET = keySet;
}

@Nullable
Expand Down Expand Up @@ -64,5 +98,27 @@ public static List<String> getHeader(HttpHeaders headers, String name) {
return emptyList();
}

@SuppressWarnings("unchecked") // HttpHeaders is a Map in Spring Web 6 and earlier
public static Set<String> getKeys(HttpHeaders headers) {
if (FOR_EACH != null) {
// Spring Web 7: HttpHeaders has forEach(BiConsumer) method
try {
Set<String> keys = new HashSet<>();
FOR_EACH.invoke(headers, (BiConsumer<String, ?>) (key, value) -> keys.add(key));
return keys;
} catch (Throwable t) {
// ignore
}
} else if (KEY_SET != null) {
// Spring Web 6 and earlier: HttpHeaders extends Map
try {
return (Set<String>) KEY_SET.invoke(headers);
} catch (Throwable t) {
// ignore
}
}
return emptySet();
}

private HeaderUtil() {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.spring.webflux.v5_3;

import static org.assertj.core.api.Assertions.assertThat;

import java.util.ArrayList;
import java.util.List;
import org.junit.jupiter.api.Test;
import org.springframework.http.HttpHeaders;
import org.springframework.mock.http.server.reactive.MockServerHttpRequest;
import org.springframework.mock.web.server.MockServerWebExchange;
import org.springframework.web.server.ServerWebExchange;

class WebfluxTextMapGetterTest {

@Test
void testKeysWithMultipleHeaders() {
MockServerHttpRequest request =
MockServerHttpRequest.get("/test")
.header("traceparent", "00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01")
.header("tracestate", "congo=t61rcWkgMzE")
.header("custom-header", "custom-value")
.header("x-forwarded-for", "192.168.1.1")
.build();

MockServerWebExchange exchange = MockServerWebExchange.from(request);

Iterable<String> keys = WebfluxTextMapGetter.INSTANCE.keys(exchange);
assertThat(keys).contains("traceparent", "tracestate", "custom-header", "x-forwarded-for");
}

@Test
void testGet() {
MockServerHttpRequest request =
MockServerHttpRequest.get("/test")
.header("traceparent", "00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01")
.header("custom-header", "custom-value")
.build();

MockServerWebExchange exchange = MockServerWebExchange.from(request);

String traceparent = WebfluxTextMapGetter.INSTANCE.get(exchange, "traceparent");
assertThat(traceparent).isEqualTo("00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01");

String customHeader = WebfluxTextMapGetter.INSTANCE.get(exchange, "custom-header");
assertThat(customHeader).isEqualTo("custom-value");
}

@Test
void testGetAll() {
MockServerHttpRequest request =
MockServerHttpRequest.get("/test")
.header("accept", "application/json")
.header("accept", "text/html")
.build();

MockServerWebExchange exchange = MockServerWebExchange.from(request);

List<String> acceptHeaders = new ArrayList<>();
WebfluxTextMapGetter.INSTANCE.getAll(exchange, "accept").forEachRemaining(acceptHeaders::add);

assertThat(acceptHeaders).containsExactly("application/json", "text/html");
}

@Test
void testGetAllSingleValue() {
MockServerHttpRequest request =
MockServerHttpRequest.get("/test").header("content-type", "application/json").build();

MockServerWebExchange exchange = MockServerWebExchange.from(request);

List<String> contentTypes = new ArrayList<>();
WebfluxTextMapGetter.INSTANCE
.getAll(exchange, "content-type")
.forEachRemaining(contentTypes::add);

assertThat(contentTypes).containsExactly("application/json");
}

@Test
void testGetNullExchange() {
String result = WebfluxTextMapGetter.INSTANCE.get(null, "any-header");
assertThat(result).isNull();
}

@Test
void testKeysDirectlyOnHttpHeaders() {
HttpHeaders headers = new HttpHeaders();
headers.add("traceparent", "00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01");
headers.add("tracestate", "congo=t61rcWkgMzE");
headers.add("custom-header", "custom-value");

MockServerHttpRequest request = MockServerHttpRequest.get("/test").headers(headers).build();
ServerWebExchange exchange = MockServerWebExchange.from(request);

// The keys() method internally calls HttpHeaders.keySet()
// This will throw NoSuchMethodError with Spring Web 7 if not properly handled
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

somewhat contradictory, NoSuchMethodError is avoided by not calling HttpHeaders.keySet() for spring web 7

Iterable<String> keys = WebfluxTextMapGetter.INSTANCE.keys(exchange);
assertThat(keys).hasSize(3).contains("traceparent", "tracestate", "custom-header");
}

@Test
void testGetAllNullExchange() {
assertThat(WebfluxTextMapGetter.INSTANCE.getAll(null, "any-header")).isExhausted();
}

@Test
void testKeysWithBaggageHeader() {
// Test that baggage headers are properly returned by keys()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't have added that many tests since most of the functionality is already tested elsewhere. Guess having more tests can't hurt.

MockServerHttpRequest request =
MockServerHttpRequest.get("/test")
.header("traceparent", "00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01")
.header("baggage", "test-baggage-key-1=test-baggage-value-1")
.build();

MockServerWebExchange exchange = MockServerWebExchange.from(request);

Iterable<String> keys = WebfluxTextMapGetter.INSTANCE.keys(exchange);
assertThat(keys).contains("traceparent", "baggage");

String baggageValue = WebfluxTextMapGetter.INSTANCE.get(exchange, "baggage");
assertThat(baggageValue).isEqualTo("test-baggage-key-1=test-baggage-value-1");
}

@Test
void testKeysWithMultipleBaggageHeaders() {
// Test that multiple baggage headers are properly returned by keys()
// The W3C Baggage propagator needs to iterate through all headers to find baggage entries
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

W3C Baggage propagator doesn't really use the keys() method

MockServerHttpRequest request =
MockServerHttpRequest.get("/test")
.header("traceparent", "00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01")
.header("baggage", "test-baggage-key-1=test-baggage-value-1")
.header("baggage", "test-baggage-key-2=test-baggage-value-2")
.header("x-custom", "custom-value")
.build();

MockServerWebExchange exchange = MockServerWebExchange.from(request);

Iterable<String> keys = WebfluxTextMapGetter.INSTANCE.keys(exchange);
assertThat(keys).contains("traceparent", "baggage", "x-custom");

List<String> baggageValues = new ArrayList<>();
WebfluxTextMapGetter.INSTANCE.getAll(exchange, "baggage").forEachRemaining(baggageValues::add);

assertThat(baggageValues)
.containsExactly(
"test-baggage-key-1=test-baggage-value-1", "test-baggage-key-2=test-baggage-value-2");
}
}
Loading