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.

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

Muzzle should recognise mismatch on unimplemented abstract methods at runtime #1337

Closed
pavolloffay opened this issue Oct 7, 2020 · 7 comments · Fixed by #1357
Closed

Muzzle should recognise mismatch on unimplemented abstract methods at runtime #1337

pavolloffay opened this issue Oct 7, 2020 · 7 comments · Fixed by #1357
Assignees
Labels
bug Something isn't working

Comments

@pavolloffay
Copy link
Member

pavolloffay commented Oct 7, 2020

This is related to: #1123

I have a custom Servlet instrumentation that wraps request/response to read bodies (similar to #1167). The instrumentation works well with servlet 3.0 however the instrumentation does not implement 'abstract boolean isReady()' of abstract class javax.servlet.ServletInputStream and hence it fails at runtime for servlet 3.1.

The buil
d with muzzle gradle plugin correctly fails.

> Task :instrumentation:servlet:servlet-3.0:muzzle-AssertPass-javax.servlet-javax.servlet-api-3.1.0 FAILED
FilterChain advice
FAILED MUZZLE VALIDATION: io.opentelemetry.instrumentation.auto.servlet.v3_0.bodycapture.Servlet3BodyInstrumentation mismatches:
-- io.opentelemetry.instrumentation.auto.servlet.v3_0.bodycapture.BufferingHttpServletRequest$ServletInputStreamWrapper:263 Missing method javax.servlet.ServletInputStream#isFinished()Z
-- io.opentelemetry.instrumentation.auto.servlet.v3_0.bodycapture.BufferingHttpServletRequest$ServletInputStreamWrapper:263 Missing method javax.servlet.ServletInputStream#isReady()Z
-- io.opentelemetry.instrumentation.auto.servlet.v3_0.bodycapture.BufferingHttpServletRequest$ServletInputStreamWrapper:263 Missing method javax.servlet.ServletInputStream#setReadListener(Ljavax/servlet/ReadListener;)V
-- io.opentelemetry.instrumentation.auto.servlet.v3_0.bodycapture.BufferingHttpServletResponse:68 Missing method javax.servlet.ServletOutputStream#isReady()Z
-- io.opentelemetry.instrumentation.auto.servlet.v3_0.bodycapture.BufferingHttpServletResponse:68 Missing method javax.servlet.ServletOutputStream#setWriteListener(Ljavax/servlet/WriteListener;)V

> Task :instrumentation:servlet:servlet-3.0:muzzle-AssertPass-javax.servlet-javax.servlet-api-4.0.1 FAILED
FilterChain advice
FAILED MUZZLE VALIDATION: io.opentelemetry.instrumentation.auto.servlet.v3_0.bodycapture.Servlet3BodyInstrumentation mismatches:
-- io.opentelemetry.instrumentation.auto.servlet.v3_0.bodycapture.BufferingHttpServletRequest$ServletInputStreamWrapper:263 Missing method javax.servlet.ServletInputStream#isFinished()Z
-- io.opentelemetry.instrumentation.auto.servlet.v3_0.bodycapture.BufferingHttpServletRequest$ServletInputStreamWrapper:263 Missing method javax.servlet.ServletInputStream#isReady()Z
-- io.opentelemetry.instrumentation.auto.servlet.v3_0.bodycapture.BufferingHttpServletRequest$ServletInputStreamWrapper:263 Missing method javax.servlet.ServletInputStream#setReadListener(Ljavax/servlet/ReadListener;)V
-- io.opentelemetry.instrumentation.auto.servlet.v3_0.bodycapture.BufferingHttpServletResponse:68 Missing method javax.servlet.ServletOutputStream#isReady()Z
-- io.opentelemetry.instrumentation.auto.servlet.v3_0.bodycapture.BufferingHttpServletResponse:68 Missing method javax.servlet.ServletOutputStream#setWriteListener(Ljavax/servlet/WriteListener

Ideally, the muzzle matcher should also recognize mismatch at runtime and prevent instrumentation from being loaded.

@mateuszrzeszutek
Copy link
Member

Hmm, so at runtime muzzle allows this instrumentation to happen? That's pretty strange - muzzle compile task and MuzzleMatcher call pretty much the same method. Have you tried turning the debug logging on? Does MuzzleMatcher log anything?
Also, if it's possible please provide an example -- a draft PR, unit test, whatever that allows reproducing this issue.

@devinsba
Copy link
Contributor

devinsba commented Oct 8, 2020

Unrelated to your issue, but I would warn against wrapping servlet types. I have run into issues where some application servers will cast to a concrete type so wrapping will break these servers

@pavolloffay
Copy link
Member Author

Agree, it's not the easiest feature to implement. @devinsba do you remember which app servlet/servlet container is casting to specific types?

@devinsba
Copy link
Contributor

devinsba commented Oct 8, 2020

It was Oracle WebLogic Server, the version in my downloads folder is 10.3.6.0

@iNikem
Copy link
Contributor

iNikem commented Oct 8, 2020

Agree, it's not the easiest feature to implement. @devinsba do you remember which app servlet/servlet container is casting to specific types?

Even Jetty does that

@pavolloffay
Copy link
Member Author

@mateuszrzeszutek here is the reproducer #1354

@mateuszrzeszutek
Copy link
Member

Thanks! I'll take a look at it right away

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
4 participants