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 the virtual thread executor plugin #751

Merged
merged 6 commits into from
Mar 9, 2025
Merged

Conversation

peachisai
Copy link
Contributor

  • If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #.
  • Update the CHANGES log.

@@ -260,4 +260,6 @@ public class ComponentsDefine {
public static final OfficialComponent SOLON_MVC = new OfficialComponent(158, "SolonMVC");

public static final OfficialComponent CAFFEINE = new OfficialComponent(160, "Caffeine");

public static final OfficialComponent THREAD_PER_TASK_EXECUTOR = new OfficialComponent(161, "ThreadPerTask-executor");
Copy link
Member

Choose a reason for hiding this comment

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

ID submitted through apache/skywalking#13090

@wu-sheng wu-sheng added this to the 9.5.0 milestone Mar 8, 2025
Comment on lines 159 to 161
* CARRIER_THREAD represents the actual operating system thread that carries out the execution of the virtual thread.
*/
public static final StringTag CARRIER_THREAD = new StringTag(24, "carrier.thread");
Copy link
Member

Choose a reason for hiding this comment

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

To follow the naming style, I think the thread.carrier is better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To follow the naming style, I think the thread.carrier is better?

Yes, better, done.

@wu-sheng
Copy link
Member

wu-sheng commented Mar 8, 2025

Could you try to fix the deadlink? I think the previous link is pointing to very old repo, which should be changed to skywalking asf main repo.

@peachisai
Copy link
Contributor Author

Could you try to fix the deadlink? I think the previous link is pointing to very old repo, which should be changed to skywalking asf main repo.

Done.

Comment on lines +38 to +44
<build>
<plugins>
<plugin>
<artifactId>maven-deploy-plugin</artifactId>
</plugin>
</plugins>
</build>
Copy link
Member

Choose a reason for hiding this comment

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

Could you check whether your new module could generate javadoc jar? Another plugin pom.xml has relative configurations to generate that file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you check whether your new module could generate javadoc jar? Another plugin pom.xml has relative configurations to generate that file.

When I added the maven-javadoc-plugin, I could generate the javadoc.jar

Copy link
Member

Choose a reason for hiding this comment

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

Please add that plugin, otherwise, this will fail the release process.

Copy link
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

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

LGTM

@wu-sheng wu-sheng merged commit 35856b4 into apache:main Mar 9, 2025
195 checks passed
@peachisai peachisai deleted the uat branch March 9, 2025 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants