[microsoft_dhcp] Fix incorrect log parsing for DHCP host.domain -> host.name by nicpenning · Pull Request #4280 · elastic/integrations

nicpenning · 2022-09-22T19:53:26Z

Closes #4279

What does this PR do?

Enhancement

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.

Fix improper field name

elasticmachine · 2022-09-22T19:57:03Z

❕ Build Aborted

The PR is not allowed to run in the CI yet

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2022-09-26T11:20:43.926+0000
Duration: 3 min 10 sec

Steps errors

Expand to view the steps failures

`Load a resource file from a library`

Took 0 min 0 sec . View more details here
Description: approval-list/elastic/integrations.yml

`Google Storage Download`

Took 0 min 0 sec . View more details here

`Error signal`

Took 0 min 0 sec . View more details here
Description: githubApiCall: The REST API call https://api.github.com/orgs/elastic/members/nicpenning return the message : java.lang.Exception: httpRequest: Failure connecting to the service https://api.github.com/orgs/elastic/members/nicpenning : httpRequest: Failure connecting to the service https://api.github.com/orgs/elastic/members/nicpenning : Code: 404Error: {"message":"User does not exist or is not a member of the organization","documentation_url":"https://docs.github.com/rest/reference/orgs#check-organization-membership-for-a-user"}

🤖 GitHub comments

Expand to view the GitHub comments

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

/test : Re-trigger the build.

efd6 · 2022-09-22T22:58:20Z

packages/microsoft_dhcp/changelog.yml

+- version: "1.6.1"
+  changes:
+    - description: Change host.domain to host.name to reflect the event data
+      type: enhancement


As this is an enhancement, please bump the version to 1.7.0.

Okay, I can do that.

packages/microsoft_dhcp/data_stream/log/elasticsearch/ingest_pipeline/dhcp.yml

nicpenning

Okay, I have made the appropriate changes. I don't have a way to test using .yml files for ingest pipelines, but the logic checks out when I updated my current pipeline in my test environment.

efd6 · 2022-09-23T00:13:34Z

/test

efd6 · 2022-09-23T06:30:31Z

Please run elastic-package build.

nicpenning · 2022-09-24T12:56:21Z

Is that something I need to execute locally? I made all of these changes in the browser :D

nicpenning · 2022-09-24T14:23:23Z

I went ahead and built the package and at first had an issue with the manifest.yml because it had the old version. I updated that and was then able to build the microsoft_dhcp-1.7.0.zip package.

nicpenning · 2022-09-24T14:34:39Z

/test

nicpenning

Updated manifest.yml looks good.

efd6 · 2022-09-24T21:41:21Z

/test

nicpenning

LGTM

nicpenning · 2022-09-24T23:24:18Z

I don't know why it failed on the test for the README.md. Please let me know how to proceed.

efd6 · 2022-09-25T00:55:34Z

When I build the package locally, I see this diff.

diff --git a/packages/microsoft_dhcp/docs/README.md b/packages/microsoft_dhcp/docs/README.md
index dfdc14308..efd91c390 100644
--- a/packages/microsoft_dhcp/docs/README.md
+++ b/packages/microsoft_dhcp/docs/README.md
@@ -75,7 +75,7 @@ An example event for `log` looks as following:
     },
     "host": {
         "ip": "2a02:cf40:add:4002:91f2:a9b2:e09a:6fc6",
-        "name": "test-host"
+        "domain": "test-host"
     },
     "event": {
         "agent_id_status": "verified",
@@ -119,9 +119,9 @@ An example event for `log` looks as following:
 | event.timezone | This field should be populated when the event's timestamp does not include timezone information already (e.g. default Syslog timestamps). It's optional otherwise. Acceptable timezone formats are: a canonical ID (e.g. "Europe/Amsterdam"), abbreviated (e.g. "EST") or an HH:mm differential (e.g. "-05:00"). | keyword |
 | event.type | This is one of four ECS Categorization Fields, and indicates the third level in the ECS category hierarchy. `event.type` represents a categorization "sub-bucket" that, when used along with the `event.category` field values, enables filtering events down to a level appropriate for single visualization. This field is an array. This will allow proper categorization of some events that fall in multiple event types. | keyword |
 | host.domain | Name of the domain of which the host is a member. For example, on Windows this could be the host's Active Directory domain or NetBIOS domain name. For Linux this could be the domain of the host's LDAP provider. | keyword |
-| host.name | Name of the host. It can contain what hostname returns on Unix systems, the fully qualified domain name, or a name specified by the user. The sender decides which value to use. | keyword |
 | host.ip | Host ip addresses. | ip |
 | host.mac | Host MAC addresses. The notation format from RFC 7042 is suggested: Each octet (that is, 8-bit byte) is represented by two [uppercase] hexadecimal digits giving the value of the octet as an unsigned integer. Successive octets are separated by a hyphen. | keyword |
+| host.name | Name of the host. It can contain what `hostname` returns on Unix systems, the fully qualified domain name, or a name specified by the user. The sender decides which value to use. | keyword |
 | input.type |  | keyword |
 | log.file.path | Full path to the log file this event came from, including the file name. It should include the drive letter, when appropriate. If the event wasn't read from a log file, do not populate this field. | keyword |
 | log.offset |  | long |

I don't want to push this to your branch because you are sending the change from your master branch, and this complicates my repo, so can you make that change — this should be the result when you run elastic-package build.

It looks like you should also run elastic-package test system -g first as well to update the sample events, but that is not necessary to get the tests to pass, just to get the docs up to date.

nicpenning · 2022-09-25T01:37:23Z

Okay. Yeah, locally I don't get much out of the build:

$ elastic-package build
Build the package
README.md file rendered: /Users/.../integrations/packages/microsoft_dhcp/docs/README.md
2022/09/24 20:33:21  INFO License text found in "/Users/.../integrations/LICENSE.txt" will be included in package
Package built: /Users/.../integrations/build/packages/microsoft_dhcp-1.7.0.zip
Done

I can make those changes. That is a bad example anyways :/ test-host doesn't make sense for domain but rather host.name. In this example, host.domain wouldn't event exist as it would expect something like test-host.local to produce host.domain:local and host.name:test-host.local

Docs are important! I will see what I can do.

nicpenning · 2022-09-25T01:49:24Z

I tried your other command, but I am not running a stack so that could be part of my problem.

I updated the README.md to include the host.domain but updated the host.name example.

How does the test know what example to use? When I look at the test docs here:
https://github.com/nicpenning/integrations/blob/master/packages/microsoft_dhcp/data_stream/log/_dev/test/pipeline/test-log.log-expected.json

None of what is in the README.md aligns, so it must not be doing actually test with the data in conjunction with the README.md as I think you telling me.

Does my README.md update help?

nicpenning · 2022-09-25T02:56:23Z

/test

efd6 · 2022-09-26T04:51:22Z

/test

efd6 · 2022-09-26T06:16:28Z

Please apply this patch to your branch at 33f65b0. You will need to gunzip it first (GitHub does not allow patch files to be uploaded), and then git apply 0001-microsoft_dhcp-bring-sample-expects-and-docs-up-to-d.patch and then add and commit the changes.

0001-microsoft_dhcp-bring-sample-expects-and-docs-up-to-d.patch.gz

cla-checker-service · 2022-09-26T11:20:30Z

❌ Author of the following commits did not sign a Contributor Agreement:
, , , , , , , , , , , , , , , , , , , , 814c308

Please, read and sign the above mentioned agreement if you want to contribute to this project

nicpenning · 2022-09-26T11:30:44Z

By applying the patch locally on my system and pushing up, I apparently and inadvertently used the wrong credentials in my browser. I have an idea how that happened, but unsure how to resolve, yet.

Update: This used the right credentials, however, the Git Credential Manager just passes first and last name with the commit, which confuses the Elastic cla-checker-service. I will see if I can correct.

nicpenning · 2022-09-26T11:56:04Z

Since I can't revert here on the web, we might have to figure out a way to remove these changes and re-apply unless you can override that message.

I think this is a bug in the CLA since it was me that pushed, but since it doesn't include my email it fails to recognize me. Do you know if there is a workaround to allow First/Last name to show as having signed the CA?

nicpenning

These all look good.

efd6 · 2022-09-26T22:09:42Z

The CLA checker is not recognising the email address in the change. Can you git reset --hard HEAD~1 && git push -f <your remote> master. This will remove the commit and may make the CLA checker happy. Then redo the push as before but with the correct author details.

efd6 · 2022-09-26T22:11:15Z

The alternative approach is to close this PR and make a new one using a named non-master branch. I can then help out with changes if needed.

nicpenning · 2022-09-26T22:38:26Z

I will take a run at your first suggestion!

nicpenning · 2022-09-27T01:38:25Z

Manually created new branch and added patch. :/ I have no idea how to use GitHub and publish with my email, it seems to always just generically send my First and Last name. Maybe it is a Git on Windows thing. Someday I will figure that out. Closing this PR in favor of the other branch I used. Thanks for all of your help. See you over on the new PR: #4311

nicpenning added 10 commits September 22, 2022 14:27

Change host.domain to host.name

5625d41

Fix improper field name

Fix improper field name

24d9805

Update field name in sample

4b6a2dc

Update documentation and ECS field

e7c13a9

Update changelog.yml

c22fbc0

Change host.domain to host.name

eb17199

Update host.domain to host.name

05b68f5

Change host.domain and to host.name

3eb8ef6

Change host.domain to host.name

4fa634f

Update changelog.yml

ffbf587

nicpenning requested a review from a team as a code owner September 22, 2022 19:53

nicpenning changed the title ~~Fix incorrect log parsing for DHCP host.domain -> host.name~~ [microsoft_dhcp] Fix incorrect log parsing for DHCP host.domain -> host.name Sep 22, 2022

Update changelog.yml

4384ed9

efd6 reviewed Sep 22, 2022

View reviewed changes

nicpenning added 7 commits September 22, 2022 18:36

Update changelog.yml

e128baf

Add host.domain grok extraction from host.name

f242e24

Add host.domain grok extraction from host.name

4c2c8a6

Examples of host.domain extracted from host.name

2e09bb5

Add host.domain example

d8942bb

Add host.domain back

2676208

Add host.domain back in

cc74f52

nicpenning commented Sep 23, 2022

View reviewed changes

Update version info to latest

b501ddc

nicpenning commented Sep 24, 2022

View reviewed changes

nicpenning closed this Sep 24, 2022

nicpenning reopened this Sep 24, 2022

nicpenning commented Sep 24, 2022

View reviewed changes

Add host.domain example

33f65b0

Used purposed patch to update the PR.

814c308

nicpenning commented Sep 26, 2022

View reviewed changes

This was referenced Sep 27, 2022

Tests for dhcp #4309

Closed

[microsoft_dhcp] Fix incorrect log parsing for DHCP host.domain -> host.name (redo) #4311

Merged

nicpenning closed this Sep 27, 2022

andrewkroh mentioned this pull request Sep 29, 2022

[production] Promote many packages from snapshot elastic/package-storage#5760

Merged

Conversation

nicpenning commented Sep 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Checklist

Uh oh!

elasticmachine commented Sep 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❕ Build Aborted

Build stats

Steps errors

Load a resource file from a library

Google Storage Download

Error signal

🤖 GitHub comments

Uh oh!

efd6 Sep 22, 2022

Choose a reason for hiding this comment

Uh oh!

nicpenning Sep 22, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nicpenning left a comment

Choose a reason for hiding this comment

Uh oh!

efd6 commented Sep 23, 2022

Uh oh!

efd6 commented Sep 23, 2022

Uh oh!

nicpenning commented Sep 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicpenning commented Sep 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicpenning commented Sep 24, 2022

Uh oh!

nicpenning left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

efd6 commented Sep 24, 2022

Uh oh!

nicpenning left a comment

Choose a reason for hiding this comment

Uh oh!

nicpenning commented Sep 24, 2022

Uh oh!

efd6 commented Sep 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicpenning commented Sep 25, 2022

Uh oh!

nicpenning commented Sep 25, 2022

Uh oh!

nicpenning commented Sep 25, 2022

Uh oh!

efd6 commented Sep 26, 2022

Uh oh!

efd6 commented Sep 26, 2022

Uh oh!

cla-checker-service bot commented Sep 26, 2022

Uh oh!

nicpenning commented Sep 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicpenning commented Sep 26, 2022

Uh oh!

nicpenning left a comment

Choose a reason for hiding this comment

Uh oh!

efd6 commented Sep 26, 2022

Uh oh!

efd6 commented Sep 26, 2022

Uh oh!

nicpenning commented Sep 26, 2022

Uh oh!

nicpenning commented Sep 27, 2022

Uh oh!

nicpenning commented Sep 22, 2022 •

edited

Loading

elasticmachine commented Sep 22, 2022 •

edited

Loading

`Load a resource file from a library`

`Google Storage Download`

`Error signal`

nicpenning commented Sep 24, 2022 •

edited

Loading

nicpenning commented Sep 24, 2022 •

edited

Loading

nicpenning left a comment •

edited

Loading

efd6 commented Sep 25, 2022 •

edited

Loading

nicpenning commented Sep 26, 2022 •

edited

Loading