-
Notifications
You must be signed in to change notification settings - Fork 119
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
Adding apache test package #111
Conversation
I'm going to investigate it today. |
@ycombinator I think you can move forward with this PR as #112 is merged. |
745a3d0
to
0bb06db
Compare
If you prefer to merge it partially (2/3 datasets), I'm fine with it. Feel free to convert it a standard PR once is ready for review. |
Nah, I think I'm pretty close to getting the metrics dataset working too. If I can't get that by tomorrow, I'll just go with the two I have and put it into review. |
Hmmm, after I rebased on the latest I checked the Agent container and the Apache access and error logs are making it to the expected folder and have data in them:
However, if I check the
If I check the policy in Kibana, it appears to be correct:
@mtojek do you spot anything wrong in the above? Can you try to reproduce on your end with this PR? Thanks. I'll keep investigating on my end too, of course. |
Not sure if this is related to the problem mentioned in the previous comment, but I inspected the Agent logs. I see messages like these repeating frequently:
Sure enough, if you try to visit https://artifacts.elastic.co/downloads/beats/filebeat/filebeat-8.0.0-SNAPSHOT-linux-x86_64.tar.gz.asc, it 404s. I think the domain in that URL needs to be |
FWIW, the same error as shown in the previous comment is seen with the
|
Related issue: elastic/beats#21320. |
0bb06db
to
76f437f
Compare
02ddb48
to
26a9e7e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@@ -238,10 +239,22 @@ func (r *runner) run() error { | |||
} | |||
|
|||
func (r *runner) tearDown() { | |||
if logger.IsDebugMode() { | |||
// Sleep to give some time for humans to scrutinize the current |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disabled it on purpose in one of recent PR not to slow down testing. Especially if we add next tests in the future. If you prefer we can leave it here or add a command argument not to wipe out the cluster state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it is only adding the sleep in debug mode. I wonder if we can add a check here to only do this if the test fails? If the test passes it's unlikely that we want to inspect the state of the system, right?
[EDIT] Let me know what you think of this idea. If you like it I'll make an issue/PR for it.
@@ -43,6 +43,32 @@ type portMapping struct { | |||
// UnmarshalYAML unmarshals a Docker Compose port mapping in YAML to | |||
// a portMapping. | |||
func (p *portMapping) UnmarshalYAML(node *yaml.Node) error { | |||
// Depending on how the port mapping is specified in the Docker Compose |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know there are kinds of responses present. Good to know.
Let met merge this PR as it's already decent improvement. |
This PR adds an
apache
test package.Included in this test package are files needed for system testing this package. Consequently, we should see CI running system tests for the
apache
package (and hopefully passing).Once this PR is merged the files related to system testing the
apache
test package in this PR should be incorporated into elastic/integrations#263.