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

Add agent option for adjust ByteBuddy redefinition strategy #7835

Closed

Conversation

KyongSik-Yoon
Copy link

I'm doing maintanance javaagent of APM product.
Recently, We have scheduled accpet OpenTelemetry data from various agent of them.
So I tried install our javaagent with otel javaagent.
It's purpose that fill not enough funtionality of each of them.

My javaagent add interface to HttpServletRequest and HttpServletResponse loading them to free class reference when use their method as like getParameter()
But When otel javaagent installed sametimes interface added by my agent was gone.
That's look like return to origin shape.
I'm tried to found root cause and workaround then I found below issue.
#7594

A reason is otel javaagent using ByteBuddy redeifne strategy is RETRANSFORM
It's works that instrument byte code after finish class loading.
So redefined class ignored after doing retransform.

Surely, ByteBuddy has three redefinition strategy. (NONE, RETRANSFORM, REDEFINITION)
When I set REDEFINITION strategy to AgentInstaller, otel agent and my agent working well together.
But I'm not sure how affect to opentelemetry agent entire range soI added agent option can adjust redefinition strategy.
We can expect user who want to use multiple agent setting this option.

Signed-off-by: Yoon Kyong Sik <sam1287@gmail.com>
@KyongSik-Yoon KyongSik-Yoon requested a review from a team February 16, 2023 10:14
@trask
Copy link
Member

trask commented Feb 16, 2023

hi @KyongSik-Yoon! do all of the opentelemetry-javaagent tests still pass when using this option?

@KyongSik-Yoon
Copy link
Author

I will comment after do that.

@KyongSik-Yoon
Copy link
Author

@trask I was checked all test passing with this option.

@laurit
Copy link
Contributor

laurit commented Feb 22, 2023

With this it would be possible to pass DISABLED as the redefinition strategy that would break the agent, not sure if we should allow passing options that will not result in functioning agent.
I find it strange that REDEFINITION works while RETRANSFORM doesn't. These should affect only the initial transform of already loaded classes. You mention adding interface to HttpServletRequest/HttpServletResponse which means that these classes are not loaded during the agent start, so they shouldn't really be affected by the choice of redefinition strategy.
From your description it seems that when using RETRANSFORM the missing interface issue happens only sometimes, is this true? Any chance of building a reproducer for this?

@KyongSik-Yoon
Copy link
Author

@laurit
With this it would be possible to pass as the redefinition strategy that would break the agent, not sure if we should allow passing options that will not result in functioning agent.
-> I will add assertion check for passing strategy should be redefinition or retransform.

I find it strange that works while DISABLEDREDEFINITIONRETRANSFORM doesn't. These should affect only the initial transform of already loaded classes
-> I don't have experience working with ByteBuddy. When I use bytecode instrumentation lib as like BCEL, ASM, Javassit I didn't saw like this situation. As I know bytecode handling should not affect metadata of already loaded classes.

From your description it seems that when using the missing interface issue happens only sometimes, is this true?
-> In my case, missing interface issue always happend when use otel agent and own.
Three is one strange things that although configure REDEFINITION missing interface possible happended by javaagent configuration order. Everything is ok just when otel javaagent setting position before than other javaagent.
ex) -javaagent:otel.javaagent.jar -javaagent:other.javaagent.jar
In other case, missing interface happended again. I think it's caused by ByteBuddy's RETRANSFORM

Any chance of building a reproducer for this?
-> I'wll try make this case using unit test. If it's not possible I will attach a part of using code for adding interface.

Only allowed retransformation or redefinition
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Feb 23, 2023

CLA Missing ID CLA Not Signed

@KyongSik-Yoon
Copy link
Author

@laurit Sorry. I make PR again due to my missing cla commits.
#7887

If you can check close this PR and continue on #7887

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.

4 participants