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

Add IIS Filebeat module #6127

Merged
merged 15 commits into from
Feb 13, 2018
Merged

Add IIS Filebeat module #6127

merged 15 commits into from
Feb 13, 2018

Conversation

cgwrench
Copy link
Contributor

This pull request adds an IIS Filebeat module to parse access and error logs from the IIS web server. This contribution includes:

  • A fileset for parsing IIS access logs;
  • A fileset for parsing IIS error (HTTPERR) logs;
  • Test files for both filesets to verify the ingest node pipelines;
  • A sample Kibana dashboard.

For the IIS module access fileset, three grok patterns have been implemented for the ingest pipeline definition:

  • A minimal pattern that represents the default fields that are logged by a fresh install of IIS;
  • A pattern that matches a more verbose logging configuration that is used by IIS running in Azure;
  • A pattern that matches the case when IIS is configured to log all available fields.

I think this represents a complete first implementation of an IIS module. Please let me know if there is anything that I've missed or that should be changed.

There are a couple of areas that I'd love feedback on:

  1. Since IIS only runs on Windows, what is the correct way to specify that a Filebeat module should only run on Windows? For now I've only specified paths for Windows in the module manifest.yml.

  2. One issue that I observed is that the IIS fields don't appear to be in the index template populated when Filebeat starts up. Have I missed something in the setup/build of this new module? For configuring the Kibana dashboards I manually edited the index template.

@karmi
Copy link

karmi commented Jan 21, 2018

Hi @cgwrench, we have found your signature in our records, but it seems like you have signed with a different e-mail than the one used in yout Git commit. Can you please add both of these e-mails into your Github profile (they can be hidden), so we can match your e-mails to your Github profile?

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@cgwrench
Copy link
Contributor Author

Hi @karmi. Apologies for that. I've added this email address to my GitHub profile now. Let me know if there is still an issue.

@ruflin
Copy link
Contributor

ruflin commented Jan 21, 2018

@cgwrench @ph I wonder how much this PR overlaps with #5685 as I see @cgwrench also commenting there. In any case, thanks for pushing this forward.

@cgwrench
Copy link
Contributor Author

Hi @ruflin. There is some overlap between the two pull requests. I had started on my own implementation when I saw that #5685 added a basic IIS access logs fileset. My comments/review on that pull request suggested some improvements that could be made (as summarised here) that were based on my own experience working on this feature. In the absence of any response or progress on these suggestions I've carried working on and testing my own fork, which now address all of the feedback noted in #5685 (comment) and addresses a couple of bugs I found in in the changes proposed in #5685. Given that I think I've got a complete implementation of an IIS Filebeat module, I thought I'd submit this as a new pull request. I'm more than happy to handle this different though: just let me know how you'd like me to approach this.

Copy link
Contributor

@ph ph left a comment

Choose a reason for hiding this comment

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

@cgwrench Thanks for carrying this over, I've made a few small comments on the PR.

One issue that I observed is that the IIS fields don't appear to be in the index template populated when Filebeat starts up. Have I missed something in the setup/build of this new module? For configuring the Kibana dashboards I manually edited the index template.

If you run make update at the root of the project it will take care of the templates and the docs, your will have to add the modified file on this PR. The missing files are making this PR fails CI.

Concerning moving forward, I think @ruflin and myself agree that we can take this PR and give part of attribution in #5685

We have some responsability to take, we have been a bit too slow to review it.


The IIS module was tested with logs from version 10.

Note: Running IIS on anything other than Windows is not supported.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need that mention? We could always still parse IIS formatted log on a Linux machine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. I've removed this comment.

type: long
description: >
The HTTP response code.
- name: sub_status
Copy link
Contributor

Choose a reason for hiding this comment

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

I am looking at the https://msdn.microsoft.com/en-us/library/windows/desktop/aa814385(v=vs.85).aspx, I understand all the snake case reference now. :)

type: long
description: >
The Windows status code.
- name: time_taken_ms
Copy link
Contributor

Choose a reason for hiding this comment

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

resquest_time_ms maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, I've changed this to request_time_ms.

#Version: 1.0
#Date: 2018-01-01 10:11:12
#Fields: date time s-sitename s-computername s-ip cs-method cs-uri-stem cs-uri-query s-port cs-username c-ip cs-version cs(User-Agent) cs(Cookie) cs(Referer) cs-host sc-status sc-substatus sc-win32-status sc-bytes cs-bytes time-taken
2018-01-01 10:11:12 W3SVC1 MACHINE-NAME 127.0.0.1 GET / - 80 - 85.181.35.98 HTTP/1.1 Mozilla/5.0+(Windows+NT+6.1;+Win64;+x64;+rv:57.0)+Gecko/20100101+Firefox/57.0 - - example.com 200 0 0 123 456 789
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe one record could have a query string?

Copy link
Contributor Author

@cgwrench cgwrench Jan 23, 2018

Choose a reason for hiding this comment

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

Agreed, I've added a query string example to the first log entry in this file. The query string is quite simple (I've just lifted this from the Nginx module test file). Let me know if you want a fancier query string here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your feedback @ph. All review items should be addressed now. Let me know if there is anything else you'd like me to pick up.

@@ -32,6 +32,8 @@ https://github.com/elastic/beats/compare/v6.0.0-beta2...master[Check the HEAD di
*Filebeat*
- Switch to docker prospector in sample manifests for Kubernetes deployment {pull}5963[5963]

- Add IIS module. {pull}6127[6127]
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Add IIS module to parse access log and error log.

@BongoEADGC6
Copy link

@cgwrench Thanks for taking this on. I'm willing to help however I can to get this moved into the beats product as soon as possible.

@ph
Copy link
Contributor

ph commented Jan 31, 2018

jenkins test this please

@ph
Copy link
Contributor

ph commented Jan 31, 2018

@BongoEADGC6 I think what we can do is, get this one merged and rebase changes on a new PR?

cc @ruflin

Copy link
Contributor

@ph ph left a comment

Choose a reason for hiding this comment

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

LGTM but WFSG

Waiting on CI to run everything, I've also restarted the travis filebeat job, It appears to fails because of some network error and weirdness of old pip version.

@BongoEADGC6
Copy link

BongoEADGC6 commented Jan 31, 2018

@ph No problem. I merged @cgwrench repo into mine and updated my pull request #5685 if you want to do that as well.

@ruflin
Copy link
Contributor

ruflin commented Feb 3, 2018

I think there are some fixes needed in the tests: https://beats-ci.elastic.co/job/elastic+beats+pull-request+multijob-linux/3041/beat=filebeat,label=ubuntu/testReport/junit/test_modules/Test/test_modules/

@ph I added you also as reviewer on to move this forward: #5685

@ph
Copy link
Contributor

ph commented Feb 5, 2018

@ruflin 👍 I will take a look at #5685, thanks @cgwrench for updating this PR :)

@cgwrench
Copy link
Contributor Author

cgwrench commented Feb 5, 2018

@ph No problem. @ruflin Thanks for pointing this out that failing test, I think I've addressed this now. Let me know if there is anything else I can pick up.

@ph
Copy link
Contributor

ph commented Feb 5, 2018

@cgwrench I've restarted the test, there seems to be some network/artifact issues unrelated to your changes, I am keeping an eye open.

@ph
Copy link
Contributor

ph commented Feb 8, 2018

jenkins test this please

@ph
Copy link
Contributor

ph commented Feb 12, 2018

@cgwrench We have one error with the test, I believe its because we now send input.type in the document see:

https://github.com/elastic/beats/blob/master/filebeat/module/logstash/slowlog/test/slowlog-plain.log-expected.json#L34-L36

AssertionError: The following expected object was not found:
 {u'error': {u'reason_phrase': u'Hostname', u'server_port': u'80', u'remote_port': u'2780', u'url': u'/ThisIsMyUrl.htm', u'geoip': {u'continent_name': u'Europe', u'country_iso_code': u'DE', u'location': {u'lat': 51, u'lon': 9}}, u'response_code': u'400', u'queue_name': u'-', u'server_ip': u'127.0.0.1', u'http_version': u'1.1', u'method': u'GET', u'remote_ip': u'85.181.35.98'}}
Searched in: 
[{u'iis': {u'error': {u'reason_phrase': u'Hostname', u'server_port': u'80', u'remote_port': u'2780', u'url': u'/ThisIsMyUrl.htm', u'geoip': {u'continent_name': u'Europe', u'city_name': u'Berlin', u'location': {u'lat': 52.5167, u'lon': 13.4}, u'region_name': u'Land Berlin', u'country_iso_code': u'DE'}, u'response_code': u'400', u'queue_name': u'-', u'server_ip': u'127.0.0.1', u'http_version': u'1.1', u'method': u'GET', u'remote_ip': u'85.181.35.98'}}, u'beat': {u'hostname': u'ba2261c759f9', u'name': u'ba2261c759f9', u'version': u'7.0.0-alpha1'}, u'@timestamp': u'2018-01-01T09:10:11.000Z', u'read_timestamp': u'2018-02-08T14:40:04.429Z', u'fileset': {u'name': u'error', u'module': u'iis'}, u'source': u'/go/src/github.com/elastic/beats/filebeat/module/iis/error/test/test.log', u'offset': 384, u'input': {u'type': u'log'}, u'prospector': {u'type': u'log'}}, {u'iis': {u'error': {u'reason_phrase': u'ConnLimit', u'server_port': u'80', u'remote_port': u'2094', u'url': u'/qos/1kbfile.txt', u'http_version': u'1.1', u'response_code': u'503', u'queue_name': u'-', u'server_ip': u'172.31.77.6', u'method': u'GET', u'remote_ip': u'172.31.77.6'}}, u'beat': {u'hostname': u'ba2261c759f9', u'name': u'ba2261c759f9', u'version': u'7.0.0-alpha1'}, u'@timestamp': u'2018-01-01T08:09:10.000Z', u'read_timestamp': u'2018-02-08T14:40:04.429Z', u'fileset': {u'name': u'error', u'module': u'iis'}, u'source': u'/go/src/github.com/elastic/beats/filebeat/module/iis/error/test/test.log', u'offset': 286, u'input': {u'type': u'log'}, u'prospector': {u'type': u'log'}}, {u'iis': {u'error': {u'reason_phrase': u'Version_N/S', u'server_port': u'80', u'remote_port': u'2894', u'url': u'/', u'geoip': {u'continent_name': u'Europe', u'city_name': u'Berlin', u'location': {u'lat': 52.5167, u'lon': 13.4}, u'region_name': u'Land Berlin', u'country_iso_code': u'DE'}, u'response_code': u'505', u'queue_name': u'-', u'server_ip': u'127.0.0.1', u'http_version': u'2.0', u'method': u'GET', u'remote_ip': u'85.181.35.98'}}, u'beat': {u'hostname': u'ba2261c759f9', u'name': u'ba2261c759f9', u'version': u'7.0.0-alpha1'}, u'@timestamp': u'2018-01-01T10:11:12.000Z', u'read_timestamp': u'2018-02-08T14:40:04.429Z', u'fileset': {u'name': u'error', u'module': u'iis'}, u'source': u'/go/src/github.com/elastic/beats/filebeat/module/iis/error/test/test.log', u'offset': 470, u'input': {u'type': u'log'}, u'prospector': {u'type': u'log'}}, {u'iis': {u'error': {u'reason_phrase': u'Timer_MinBytesPerSecond', u'server_ip': u'127.0.0.1', u'remote_port': u'64388', u'geoip': {u'continent_name': u'Europe', u'city_name': u'Berlin', u'location': {u'lat': 52.5167, u'lon': 13.4}, u'region_name': u'Land Berlin', u'country_iso_code': u'DE'}, u'queue_name': u'-', u'server_port': u'80', u'remote_ip': u'85.181.35.98'}}, u'beat': {u'hostname': u'ba2261c759f9', u'name': u'ba2261c759f9', u'version': u'7.0.0-alpha1'}, u'@timestamp': u'2018-01-01T11:12:13.000Z', u'read_timestamp': u'2018-02-08T14:40:04.429Z', u'fileset': {u'name': u'error', u'module': u'iis'}, u'source': u'/go/src/github.com/elastic/beats/filebeat/module/iis/error/test/test.log', u'offset': 558, u'input': {u'type': u'log'}, u'prospector': {u'type': u'log'}}]

@cgwrench
Copy link
Contributor Author

@ph Thanks for pointing this out. I couldn't see input.type in the array of documents searched. I think the failing test is because the expected geoip object isn't correct. I fixed this previously for the access fileset and obviously missed this same issue in the error fileset. Hopefully with this latest change all the test now pass. Please let me know if there's something else I've missed.

@ph
Copy link
Contributor

ph commented Feb 12, 2018

jenkins test this please

@ph ph merged commit 652b163 into elastic:master Feb 13, 2018
@ph
Copy link
Contributor

ph commented Feb 13, 2018

@cgwrench 👍 🤗 ❤️ Thanks for the all works :)

@cgwrench cgwrench deleted the iis-filebeat-module branch February 13, 2018 21:23
@cgwrench
Copy link
Contributor Author

@ph No problem, thanks for all your input!

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.

6 participants