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

ZabbixSender: match zabbix_sender command line tool behaviour regarding metric 'host' attribute #131

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

Conversation

m-khvoinitsky
Copy link

  • ZabbixSender: add agent_hostname option, read it from config if enabled
  • if ZabbixMetric host attribute is None or '-', use aforementioned value from ZabbixSender

Related zabbix_sender documentation (see -i, --input-file input-file paragraph).

@coveralls
Copy link

coveralls commented May 19, 2020

Pull Request Test Coverage Report for Build 343

  • 8 of 8 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 90.062%

Totals Coverage Status
Change from base Build 331: 0.2%
Covered Lines: 290
Relevant Lines: 322

💛 - Coveralls

@m-khvoinitsky m-khvoinitsky force-pushed the master branch 6 times, most recently from 8d47c22 to 5173b7e Compare May 21, 2020 17:26
…ed; if ZabbixMetric host attribute is None or '-', use aforementioned value from ZabbixSender which matches the behaviour of zabbix_sender tool
@m-khvoinitsky
Copy link
Author

Anyone?

Copy link
Owner

@adubkov adubkov left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution. I reviewed your pull request and I don't think we should add agent_hostname as separate class attribute. It should be set up as part of use_config. Otherwise this may break code for many people.

What we should do is following:

  1. Modify _load_from_config function to set properties directly via self and return not None value on error.
  2. Update __init__ to:
self.agent_hostname = None
self.zabbix_uri = [(zabbix_server, zabbix_port)]

if use_config:
   assert self._load_from_config(use_config) == None, "error occurred during loading zabbix agent config"
  1. When formatting metrics use agent_hostname only if metric hostname is '' and if it was loaded from config:
for m in metrics:
    if m.host == '' and self.agent_hostname:
        m.host = self.agent_hostname
 ....

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