Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Netty ChannelPipeline#removeLast() removes instrumentation handler #4056

Closed
mateuszrzeszutek opened this issue Sep 7, 2021 · 1 comment · Fixed by #4123
Closed

Netty ChannelPipeline#removeLast() removes instrumentation handler #4056

mateuszrzeszutek opened this issue Sep 7, 2021 · 1 comment · Fixed by #4123
Labels
bug Something isn't working

Comments

@mateuszrzeszutek
Copy link
Member

Describe the bug
Our instrumentation ChannelHandlers are always inserted after the original one. That means if someone calls ChannelPipeline#removeLast() it'll remove our handler, but leave the original HTTP client/server one in the pipeline.

What did you expect to see?
Probably we should always remove both handlers (the original HTTP one & the instrumentation one) at a time?

Additional context
#4053 (comment)
@mateuszrzeszutek mateuszrzeszutek added the bug Something isn't working label Sep 7, 2021
@tydhot
Copy link
Member

tydhot commented Sep 7, 2021

maybe we should check the handler type when removeLast is called. If the handler type is ourhandler, we should call removeLast again to remove the handler user actually want.

@mateuszrzeszutek mateuszrzeszutek linked a pull request Sep 16, 2021 that will close this issue
#4056 remove oringin handler when removelast in netty #4123
Merged
trask pushed a commit that referenced this issue Sep 17, 2021
@tydhot
#4056 remove oringin handler when removelast in netty (#4123)
3bc0e07 
* removelast

* removelast

* addAfter and removeLast

* addAfter and removeLast

* addAfter and removeLast
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

#4056 remove oringin handler when removelast in netty tydhot/opentelemetry-java-instrumentation
2 participants
@tydhot @mateuszrzeszutek