Skip to content

convert netty4.1 to java#12108

Merged
trask merged 16 commits into
open-telemetry:mainfrom
shalk:test-netty-4.1
Sep 13, 2024
Merged

convert netty4.1 to java#12108
trask merged 16 commits into
open-telemetry:mainfrom
shalk:test-netty-4.1

Conversation

@shalk

@shalk shalk commented Aug 26, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

relate to #7195

@shalk shalk requested a review from a team August 26, 2024 07:30
@shalk shalk changed the title convert netty4.1 to java [WIP] convert netty4.1 to java Aug 26, 2024
@zeitlinger zeitlinger mentioned this pull request Aug 26, 2024
Closed
@zeitlinger

zeitlinger commented Aug 26, 2024

Copy link
Copy Markdown
Member

no groovy files are deleted

@shalk

shalk commented Aug 26, 2024 •
edited
Loading

Copy link
Copy Markdown
Member Author

no groovy files are deleted

done

@shalk shalk changed the title [WIP] convert netty4.1 to java [DRAFT] convert netty4.1 to java Aug 26, 2024
@shalk shalk changed the title [DRAFT] convert netty4.1 to java convert netty4.1 to java Aug 27, 2024
jaydeluca
jaydeluca reviewed Sep 2, 2024
View reviewed changes
Comment thread ...src/test/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/ChannelPipelineTest.java Outdated
Comment thread ...src/test/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/ChannelPipelineTest.java Outdated
Comment thread ...src/test/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/ChannelPipelineTest.java Outdated
Comment thread ...src/test/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/ChannelPipelineTest.java Outdated
Comment thread ...src/test/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/ChannelPipelineTest.java Outdated
Comment thread ...src/test/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/ChannelPipelineTest.java
Comment thread ...src/test/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/ChannelPipelineTest.java
Comment thread ...src/test/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/ChannelPipelineTest.java
Comment thread ...src/test/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/ChannelPipelineTest.java
channelPipeline.removeLast();

// http and instrumentation handlers will be remained;
assertThat(channelPipeline.first()).isEqualTo(handler);

@jaydeluca jaydeluca Sep 2, 2024

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.

same as above, we're missing the assertions for the channelPipeline.toMap().size() in most of these

@shalk shalk Sep 6, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

done, add more assertions

Comment thread ...st/java/io/opentelemetry/javaagent/instrumentation/netty/v4_1/Netty41ConnectionSpanTest.java Outdated
@trask trask merged commit c477eca into open-telemetry:main Sep 13, 2024
@shalk shalk mentioned this pull request Oct 11, 2024
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@steverao steverao steverao left review comments

@laurit laurit laurit approved these changes

@jaydeluca jaydeluca jaydeluca approved these changes

Assignees

No one assigned

Labels

area:tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@shalk @zeitlinger @laurit @jaydeluca @steverao @trask @SylvainJuge