Skip to content

Test undertow and armeria http2 server#11361

Merged
trask merged 2 commits into
open-telemetry:mainfrom
laurit:test-http2-server
May 15, 2024
Merged

Test undertow and armeria http2 server#11361
trask merged 2 commits into
open-telemetry:mainfrom
laurit:test-http2-server

Conversation

@laurit

@laurit laurit commented May 15, 2024

Copy link
Copy Markdown
Contributor

This PR adds http2 server tests for armeria and undertow. It also fixes a bug in undertow instrumentation where HttpServerResponseMutator didn't work in undertow with http2.

@laurit laurit requested a review from a team May 15, 2024 14:02

public static String normalizeHttpVersion(String version) {
if ("2.0".equals(version)) {
return "2";

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

}

public static String getVersion(@Nullable String protocol) {
if (protocol != null && protocol.startsWith("HTTP/")) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I extracted an util class because this same pattern is used in many instrumentations and all of them probably report http2 version as 2.0 instead of 2.

Comment on lines -754 to +766
.hasEntrySatisfying(
NetworkAttributes.NETWORK_PROTOCOL_VERSION,
entry -> assertThat(entry).isIn("1.1", "2.0"));
.containsEntry(
NetworkAttributes.NETWORK_PROTOCOL_VERSION, options.useHttp2 ? "2" : "1.1");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

@trask trask left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nice ❤️

@trask trask merged commit defd7cb into open-telemetry:main May 15, 2024
@laurit laurit deleted the test-http2-server branch May 17, 2024 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants