Metricbeat: Support multiple apache status page versions #6450
Conversation
jsoriano
added
enhancement
discuss
jsoriano
force-pushed
the
server-status-apache-versions
branch
from
3541aaa to
d198b42
Compare
There was a problem hiding this comment.
Thanks for taking a stab on this. So far there is not much precedence on how to deal with this. We test across multiple versions in the Elasticsearch module but there we just start an http and endpoint to serve a predefined json object. I think that is part of the "static" solution to support multiple versions.
The other part about actually running the services is also important as it helps us to detect the edge cases. I'm good with moving forward as you have specified it in this PR to have multiple images per module for now.
It does not seem to work on CI at the moment:
https://beats-ci.elastic.co/job/elastic+beats+pull-request+multijob-linux/3335/beat=metricbeat,label=ubuntu/console
@exekias WDYT? This should probably work with your compose magic?
| @@ -141,6 +163,9 @@ func eventMapping(scanner *bufio.Scanner, hostname string) (common.MapStr, *s.Er | |||
| }, | |||
| } | |||
| _, err := schema.ApplyTo(event, fullEvent) | |||
| if err != nil { | |||
There was a problem hiding this comment.
Is there a way to identify on the first request which Apache version it is to then pick the schema? Otherwise we decode every event twice.
There was a problem hiding this comment.
Umm, indeed I can do it depend on the different fields seen in fullEvent instead of completelly decoding it twice.
| @@ -51,6 +50,29 @@ var ( | |||
| "15": c.Float("Load15", s.Optional), | |||
| }, | |||
| } | |||
|
|
|||
| // Schema used till apache 2.4.12 | |||
| schemaOld = s.Schema{ | |||
There was a problem hiding this comment.
We must make sure all these fields are also in the fields.yml documented.
There was a problem hiding this comment.
schemaOld contains a subset of the fields of the current schema.
There was a problem hiding this comment.
Well, I have removed the server_uptime field, it looks redundant to me with the uptime field. Also I think uptime shouldn't be a group, it should be enough with a long for this value, but changing from group to long looks fearful for backwards compatibility.
There was a problem hiding this comment.
Let's not introduce a breaking change here as this potentially goes out in a minor.
We should start a list of fields we want to change in the next master where we can break things.
For server_uptime vs uptime I'm not sure if one is from the actual host itself and the other one apache?
There was a problem hiding this comment.
Right, I revert these changes on server_update.
jsoriano
force-pushed
the
server-status-apache-versions
branch
3 times, most recently
from
a917a29 to
548d858
Compare
jsoriano
force-pushed
the
server-status-apache-versions
branch
from
548d858 to
c5813c0
Compare
|
I'm ok with this approach while we work on a permanent solution, please have a look to failing tests, they look legit |
jsoriano
force-pushed
the
server-status-apache-versions
branch
2 times, most recently
from
ecb2cb7 to
048e5f3
Compare
|
jenkins, test it |
There was a problem hiding this comment.
LGTM. Can you add a CHANGELOG entry?
Can you also check if this needs some update? https://github.com/elastic/beats/blob/master/metricbeat/module/apache/_meta/docs.asciidoc If you change it, run make update.
Apache before version 2.4.16 had less information in its status page. Add support to both, old and new status pages.
jsoriano
force-pushed
the
server-status-apache-versions
branch
from
048e5f3 to
db217fa
Compare
|
Changelog and info about tested versions in doc updated |
) Removed by mistake on elastic#6450 (cherry picked from commit a365e15)
) Removed by mistake on elastic#6450 (cherry picked from commit a365e15)
) Removed by mistake on elastic#6450 (cherry picked from commit a365e15)
) (elastic#8554) Removed by mistake on elastic#6450 (cherry picked from commit 9a4bcd2)
In Apache 2.4.16 server status page changed to provide more CPU information and uptime in human format (#5637). These changes make apache module support old and new schemas.
To test it, I add a second apache with the latest version using the old format, but it doesn't look very orthodox as we are not doing it with any other module. @exekias @ruflin wdyt? do we have any other approach when needing to support multiple formats?