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

[IIS] Add regex and tests for Exchange logs #7559

Merged
merged 16 commits into from Oct 30, 2023

Conversation

LaZyDK
Copy link
Contributor

@LaZyDK LaZyDK commented Aug 28, 2023

What does this PR do?

Providing regex and tests for Exchange logs running on IIS 10.0.

If servername if present, set fields host.name and related.hosts to conform to ECS.

Remove duplicates in related fields.

Rearrange script needed for #6610 to function.

Extract user.domain.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

@LaZyDK LaZyDK marked this pull request as ready for review August 28, 2023 09:09
@LaZyDK LaZyDK requested a review from a team as a code owner August 28, 2023 09:09
@elasticmachine
Copy link

elasticmachine commented Aug 28, 2023

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-10-27T10:44:13.653+0000

  • Duration: 15 min 23 sec

Test stats 🧪

Test Results
Failed 0
Passed 29
Skipped 0
Total 29

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@jamiehynds
Copy link

Thanks for the PR @LaZyDK. A member of @SubhrataK team will review, as the integration is owned by them.

@aliabbas-elastic
Copy link
Contributor

/test

@elasticmachine
Copy link

elasticmachine commented Sep 4, 2023

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (2/2) 💚
Files 100.0% (2/2) 💚 3.837
Classes 100.0% (2/2) 💚 3.837
Methods 86.111% (31/36) 👎 -6.001
Lines 96.143% (349/363) 👍 7.55
Conditionals 100.0% (0/0) 💚

@LaZyDK
Copy link
Contributor Author

LaZyDK commented Sep 5, 2023

Ready for another test @aliabbas-elastic

@aliabbas-elastic
Copy link
Contributor

/test

@LaZyDK
Copy link
Contributor Author

LaZyDK commented Sep 5, 2023

Please run another test now.

@aliabbas-elastic
Copy link
Contributor

/test

@LaZyDK
Copy link
Contributor Author

LaZyDK commented Sep 5, 2023

This was better. Thanks :)

@LaZyDK
Copy link
Contributor Author

LaZyDK commented Sep 6, 2023

Ready for review.

@muthu-mps
Copy link
Contributor

@LaZyDK - Is this something that comes as a default log format from the exchange server logs? If we are adding the hostname field as a custom field in the access logs, Then we might have different custom field combinations for which we need to create newer GROK patterns. My suggestion is any custom field combination can have a custom ingest pipeline processor to ingest the data.

@LaZyDK
Copy link
Contributor Author

LaZyDK commented Sep 6, 2023

@muthu-mps I do believe that this is a default log, as I am seeing this at multiple customers.

@LaZyDK
Copy link
Contributor Author

LaZyDK commented Sep 13, 2023

Any updates on this one?

@LaZyDK
Copy link
Contributor Author

LaZyDK commented Sep 22, 2023

Fixed conflicts.

@muthu-mps
Copy link
Contributor

/test

@LaZyDK
Copy link
Contributor Author

LaZyDK commented Oct 6, 2023

Merge?

@LaZyDK
Copy link
Contributor Author

LaZyDK commented Oct 11, 2023

Is this alright?

@muthu-mps
Copy link
Contributor

Is this alright?

The field change you have made is s-computename as per this commit. When looking into the IIS server the computername is the server_name for access logs. The server_name is already included in the GROK pattern by enabling all the available logging fields.

Please look at the pattern with server_name below,

'%{TIMESTAMP_ISO8601:iis.access.time} (?:-|%{NOTSPACE:iis.access.site_name}) 
(?:-|%{NOTSPACE:iis.access.server_name})
(?:-|%{IPORHOST:destination.address}) (?:-|%{WORD:http.request.method}) (?:-|%{NOTSPACE:_temp_.url_path})
(?:-|%{NOTSPACE:_temp_.url_query}) (?:-|%{NUMBER:destination.port:long}) (?:-|%{NOTSPACE:user.name})
(?:-|%{IPORHOST:source.address}) (?:-|HTTP/%{NUMBER:http.version}) (?:-|%{NOTSPACE:user_agent.original})
(?:-|%{NOTSPACE:iis.access.cookie}) (?:-|%{NOTSPACE:http.request.referrer}) (?:-|%{NOTSPACE:destination.domain})
(?:-|%{NUMBER:http.response.status_code:long}) (?:-|%{NUMBER:iis.access.sub_status:long})
(?:-|%{NUMBER:iis.access.win32_status:long}) (?:-|%{NUMBER:http.response.body.bytes:long})
(?:-|%{NUMBER:http.request.body.bytes:long}) (?:-|%{NUMBER:_temp_.duration:long})( (?:-|%{IPORHOST:network.forwarded_ip}))?'

and the sample-event.json with server_name is,
Screenshot 2023-10-12 at 10 53 36 AM

Considering the server_name (s-computername) is already processed using the ingest pipeline script. Do you think that we can ignore this GROK pattern ?

@LaZyDK
Copy link
Contributor Author

LaZyDK commented Oct 12, 2023

In my opinion we cannot ignore this GROK pattern, as the one you mention is only used if site_name is also present. Keep in mind that my GROK statement covers the default field order for the Exchange server.

Also, the server_name in that particular GROK pattern should be mapped to host.name.

@muthu-mps
Copy link
Contributor

In my opinion we cannot ignore this GROK pattern, as the one you mention is only used if site_name is also present. Keep in mind that my GROK statement covers the default field order for the Exchange server.

Also, the server_name in that particular GROK pattern should be mapped to host.name.

okay, Let me review and add my comments.

LaZyDK and others added 2 commits October 12, 2023 08:54
Co-authored-by: muthu-mps <101238137+muthu-mps@users.noreply.github.com>
…default.yml

Co-authored-by: muthu-mps <101238137+muthu-mps@users.noreply.github.com>
@LaZyDK
Copy link
Contributor Author

LaZyDK commented Oct 12, 2023

Ready for test

@LaZyDK
Copy link
Contributor Author

LaZyDK commented Oct 19, 2023

Test?

@LaZyDK
Copy link
Contributor Author

LaZyDK commented Oct 20, 2023

Resolved conflicts. Fixed errors relating to 8.7+.

@LaZyDK
Copy link
Contributor Author

LaZyDK commented Oct 23, 2023

Test?

@LaZyDK
Copy link
Contributor Author

LaZyDK commented Oct 27, 2023

Ready for test

@muthu-mps muthu-mps self-requested a review October 27, 2023 10:43
@muthu-mps
Copy link
Contributor

/test

Copy link
Contributor

@muthu-mps muthu-mps left a comment

Choose a reason for hiding this comment

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

LGTM!

@muthu-mps muthu-mps merged commit 359415a into elastic:main Oct 30, 2023
4 checks passed
@elasticmachine
Copy link

Package iis - 1.17.0 containing this change is available at https://epr.elastic.co/search?package=iis

@LaZyDK LaZyDK deleted the iis10_exchange branch October 30, 2023 14:19
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.

None yet

6 participants