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

fix: shade async-profiler packages #727

Merged
merged 3 commits into from
Nov 5, 2024
Merged

Conversation

zhengziyi0117
Copy link
Contributor

@wu-sheng wu-sheng added the bug Something isn't working label Nov 5, 2024
@wu-sheng wu-sheng added this to the 9.4.0 milestone Nov 5, 2024
@wu-sheng
Copy link
Member

wu-sheng commented Nov 5, 2024

Could you post the screenshot of new binary package here?

And, have you tested the feature again? Shading sometimes breaks the function.

@zhengziyi0117
Copy link
Contributor Author

Could you post the screenshot of new binary package here?你能在这里发布新的二进制包的屏幕截图吗?

And, have you tested the feature again? Shading sometimes breaks the function.而且,您再次测试过该功能吗?阴影有时会破坏功能。

I have tested it locally and the original function is normal.

The following is the package structure diagram:
image

@wu-sheng
Copy link
Member

wu-sheng commented Nov 5, 2024

About the so files, are they able to move as well?

@zhengziyi0117
Copy link
Contributor Author

About the so files, are they able to move as well?

The .so file has not been moved yet.

I think it should not be possible to move the .so file because the async-profiler code gets the dynamic library path based on the following code
String resourceName = "/" + getPlatformTag() + "/libasyncProfiler.so";

wu-sheng
wu-sheng previously approved these changes Nov 5, 2024
Copy link
Contributor

@lujiajing1126 lujiajing1126 left a comment

Choose a reason for hiding this comment

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

It is a solution as long as you understand that shading in the agent package instead of agent-core will make it impossible to be used from other packages like plugins

@wu-sheng
Copy link
Member

wu-sheng commented Nov 5, 2024

It is a solution as long as you understand that shading in the agent package instead of agent-core will make it impossible to be used from other packages like plugins

Do you mean you prefer to move this(shade) into core module? I think plugins would not access this part of codes.

@lujiajing1126
Copy link
Contributor

It is a solution as long as you understand that shading in the agent package instead of agent-core will make it impossible to be used from other packages like plugins

Do you mean you prefer to move this(shade) into core module? I think plugins would not access this part of codes.

I am OK with this one.

lujiajing1126
lujiajing1126 previously approved these changes Nov 5, 2024
@wu-sheng
Copy link
Member

wu-sheng commented Nov 5, 2024

@zhengziyi0117 Could you try that?
I think we don't need async profilor accessible from.plugins.

@zhengziyi0117 zhengziyi0117 dismissed stale reviews from lujiajing1126 and wu-sheng via 9806c91 November 5, 2024 11:16
@@ -226,6 +228,7 @@
<exclude>org.checkerframework:checker-compat-qual:jar:</exclude>
<exclude>org.checkerframework:checker-qual:jar:</exclude>
<exclude>org.codehaus.mojo:animal-sniffer-annotations:jar:</exclude>
<!-- <exclude>tools.profiler:async-profiler:jar:</exclude>-->
Copy link
Member

Choose a reason for hiding this comment

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

I think you should have this?

@wu-sheng wu-sheng merged commit f024586 into apache:main Nov 5, 2024
190 checks passed
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
Development

Successfully merging this pull request may close these issues.

3 participants