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

zabbix agent: set panic on not existed provider optionally #15

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ipslot
Copy link

@ipslot ipslot commented Jan 22, 2019

Add use case support:
On same host (zabbix node) running more than one agent applications and sending different data to zabbix (I mean different metric providers).

@michaelquigley
Copy link
Owner

Thanks for the contribution.

I'm not sure I understand the intent behind it. The Zabbix protocol is designed to either return a metric value or return ZBX_NOTSUPPORTED. What is the reason for throwing an exception here?

@ipslot
Copy link
Author

ipslot commented Jan 23, 2019

In reality, I ran into a problem. On one zabbix node there are two applications with built-in agents. The first application sends the key "application.A.uptime" the second sends the values of key "application.В.uptime". Before the fixes, both applications send both keys, although in each of them only one metric provider is implemented.
This fix allows the agent not to send a ZBX_NOTSUPPORTED error if a profiler is not specified for this metric. This behavior is optional and the default behavior is as before.

@michaelquigley
Copy link
Owner

So rather than trying to change the behavior of the agent, is there a way that we can alter your usage scenario so that the change is not required?

It concerns me to make the agent work in a way that isn't "correct", even if that functionality is made optional.

I'd like to better understand your scenario, so that maybe we can find a solution that will work for everyone?

Copy link

@Bobiology Bobiology left a comment

Choose a reason for hiding this comment

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

LGTM

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.

3 participants