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 version collection #5252

Merged
merged 2 commits into from
Dec 20, 2019
Merged

Fix version collection #5252

merged 2 commits into from
Dec 20, 2019

Conversation

FlorianVeaux
Copy link
Member

@FlorianVeaux FlorianVeaux commented Dec 19, 2019

Apache version can be collected from three places:

  • from the mod_status html output
  • from the mod_status machine-readable output
  • or from the 'Status' header in any request to Apache.

A few notes:

  • The check does not expect an html output.
  • The machine-readable output only contains the version since version 2.4.20
  • The status header does not always contain the full version (it can be hidden using server configuration parameters).

This PR first fixes the behavior of the _collect_metadata method to prevent failing.
It also tries to extract the version from the machine-readable output before falling back to the 'Status' header

@codecov
Copy link

codecov bot commented Dec 19, 2019

Copy link
Contributor

@therve therve left a comment

Choose a reason for hiding this comment

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

Some minor comments, looks good otherwise.

apache/datadog_checks/apache/apache.py Show resolved Hide resolved
apache/datadog_checks/apache/apache.py Show resolved Hide resolved
apache/datadog_checks/apache/apache.py Outdated Show resolved Hide resolved
@FlorianVeaux FlorianVeaux merged commit 4912963 into master Dec 20, 2019
@FlorianVeaux FlorianVeaux deleted the florian/fix_apache_version branch December 20, 2019 12:36
@mattyo161
Copy link

@FlorianVeaux is it possible to patch this fix into the 6.16.x branch as well?

@FlorianVeaux
Copy link
Member Author

@mattyo161 Yes working on it 👍

FlorianVeaux added a commit that referenced this pull request Dec 23, 2019
* Fix version collection

* Address review
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.

3 participants