Skip to content

add SAMPLE_AVG_CPU_LOAD_LIMIT config#135

Merged
wu-sheng merged 8 commits intoapache:mainfrom
zhyyu:feature-load-limit
Apr 25, 2022
Merged

add SAMPLE_AVG_CPU_LOAD_LIMIT config#135
wu-sheng merged 8 commits intoapache:mainfrom
zhyyu:feature-load-limit

Conversation

@zhyyu
Copy link
Copy Markdown
Contributor

@zhyyu zhyyu commented Apr 1, 2022

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

@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented Apr 1, 2022

Take a look at how TraceIgnoreExtendService extends from SamplingService. I think it would be more elegant.
This doesn't have to change from the core level.

Another thing is, you add another thread just for CPU usage, which concerns me much. The agent is very sensitive about a new thread. It should be more reasonable to ask JVMService holds the latest CPU value, and provide a method for this new plugin.

@zhyyu
Copy link
Copy Markdown
Contributor Author

zhyyu commented Apr 1, 2022

Take a look at how TraceIgnoreExtendService extends from SamplingService. I think it would be more elegant. This doesn't have to change from the core level.

Another thing is, you add another thread just for CPU usage, which concerns me much. The agent is very sensitive about a new thread. It should be more reasonable to ask JVMService holds the latest CPU value, and provide a method for this new plugin.

Ok, I will try it later using JVMService, add a new plugin like trace-ignore-plugin.

@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented Apr 2, 2022

I have commented on the codes, and I think you haven't tried this in the real case, please do so before submitting codes.

@zhyyu
Copy link
Copy Markdown
Contributor Author

zhyyu commented Apr 2, 2022

I have commented on the codes, and I think you haven't tried this in the real case, please do so before submitting codes.

I just used idea to debug the agent code, maybe not production scenario... But it do sent the segment data to OAP. I will change the code according to the comment above.

@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented Apr 2, 2022

As you are adding a CPU based sampler, so, you should verify whether this is really working.
The code review only could find out some issues, but can't approve this could work as expected. Also, this plugin doesn't like other plugins, we can't test it automatically.

So, please try to establish a runtime env to verify this plugin.

@wu-sheng
Copy link
Copy Markdown
Member

wu-sheng commented Apr 7, 2022

Any update?

@zhyyu
Copy link
Copy Markdown
Contributor Author

zhyyu commented Apr 14, 2022

Any update?

Recently my work is busy, I will update it when I am free

yuzhongyu added 2 commits April 24, 2022 17:59
# Conflicts:
#	apm-sniffer/optional-plugins/pom.xml
@zhyyu zhyyu force-pushed the feature-load-limit branch from 64e55c9 to c474485 Compare April 24, 2022 10:00
@zhyyu
Copy link
Copy Markdown
Contributor Author

zhyyu commented Apr 24, 2022

image

I create a thread run while(true) code, to increase the cpu usage to 15% (124/ 8核)

and then debug the trySampling code

image

my config is 10% cpu usage limit, 15% cpu usage is above the limit, trySampling return false which meet the expectation.

@wu-sheng
Copy link
Copy Markdown
Member

Please update changes.md and optional plugin doc with agent.config accordingly.

@wu-sheng wu-sheng merged commit 88c4c71 into apache:main Apr 25, 2022
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