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

Fix issues found in integrations after testing with logsdb #10919

Closed
wants to merge 45 commits into from

Conversation

jsoriano
Copy link
Member

@jsoriano jsoriano commented Aug 28, 2024

Proposed commit message

After testing with logsdb we identified some issues in packages, some of these issues seem legit.
We address them here. They were not detected before because for testing with logsdb we are:

  • Testing with higher versions of the stack, what can include more fields from the agent, and ecs@mappings.
  • Testing with synthetic source mode enabled in many more data streams.

Issues solved:

  • Addressed fields in transforms that were not defined.
  • Add definition for new fields added by the agent or other components of the stack.
  • Skipped some tests that cannot be addressed now.

A new daily job is added to test with logsdb.

elastic-package changes checklist

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.

Author's Checklist

@jsoriano jsoriano self-assigned this Aug 28, 2024
@andrewkroh
Copy link
Member

A new daily job is added to test with logsdb.

Consider making it possible to distinguish "logsdb" test failures in the GH issues reported by the testreporter. Like perhaps something in the title or body.

@elasticmachine
Copy link

elasticmachine commented Aug 28, 2024

🚀 Benchmarks report

Package proofpoint_on_demand 👍(1) 💚(1) 💔(1)

Expand to view
Data stream Previous EPS New EPS Diff (%) Result
mail 3115.26 2364.07 -751.19 (-24.11%) 💔

Package windows 👍(2) 💚(3) 💔(4)

Expand to view
Data stream Previous EPS New EPS Diff (%) Result
applocker_msi_and_script 9174.31 6060.61 -3113.7 (-33.94%) 💔
applocker_packaged_app_deployment 9009.01 7092.2 -1916.81 (-21.28%) 💔
powershell 2074.69 1631.32 -443.37 (-21.37%) 💔
sysmon_operational 2890.17 2267.57 -622.6 (-21.54%) 💔

To see the full report comment with /test benchmark fullreport

- name: answers.signer_name
type: keyword
- name: answers.type_covered
type: keyword
Copy link
Member Author

Choose a reason for hiding this comment

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

@elastic/sec-linux-platform I have found these many new fields in the dns data stream when enabling logsdb. It seems weird because I don't see them in the 8.16 daily builds https://buildkite.com/elastic/integrations/builds/15161.

Do you know where they can come from?

This can be tested with a stack started like this:

elastic-package stack up -v --version 8.16.0-SNAPSHOT -d -U stack.logsdb_enabled=true

Copy link
Member Author

Choose a reason for hiding this comment

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

There is a similar issue with ti_mandiant_advantage package, it is failing for some undefined fields, but this issue doesn't happen in daily builds 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

For the case of ti_mandiant_advantage, the failures happen when the format of the documents look like this:

    "mandiant.threat_intelligence.ioc.attributed_associations.id": [
      "malware--6c9e3c50-490d-5a8f-8ed6-56510a62055b",
      "threat-actor--6ca32cd4-0c60-5f0b-91fb-e6e590f1f10b"
    ],
    "mandiant.threat_intelligence.ioc.attributed_associations.name": [
      "IHSBACKCONNECT",
      "UNC961"
    ],
    "mandiant.threat_intelligence.ioc.attributed_associations.type": [
      "malware",
      "threat-actor"
    ],

They pass the tests if they are formatted like this:

    "mandiant": {
      "threat_intelligence": {
        "ioc": {
          "attributed_associations": [
            {
              "id": "threat-actor--6ca32cd4-0c60-5f0b-91fb-e6e590f1f10b",
              "name": "UNC961",
              "type": "threat-actor"
            },
            {
              "id": "malware--6c9e3c50-490d-5a8f-8ed6-56510a62055b",
              "name": "IHSBACKCONNECT",
              "type": "malware"
            }
          ],

Looks like an issue with validation, because the field names are the same at the end.

Copy link
Member Author

Choose a reason for hiding this comment

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

elastic/elastic-package#2054 may be needed, but then I wonder why tests are passing for the second format (without synthetic source).

Copy link
Member Author

Choose a reason for hiding this comment

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

Confirmed, it is the same issue in network_traffic, ti_mandiant_advantage and possibly others like mongodb_atlas.

Something like elastic/elastic-package#2054 is required, testing this patch now.

Copy link
Member Author

Choose a reason for hiding this comment

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

An additional change is needed to avoid the different validation results with and without logsdb for these cases elastic/elastic-package#2090

@jsoriano jsoriano force-pushed the fix-integrations-logsdb branch 2 times, most recently from f9c5293 to 25eb89b Compare August 29, 2024 13:53
jsoriano and others added 9 commits September 4, 2024 16:09
Bumps [github.com/elastic/elastic-package](https:/elastic/elastic-package) from 0.102.0 to 0.103.0.
- [Release notes](https:/elastic/elastic-package/releases)
- [Changelog](https:/elastic/elastic-package/blob/main/.goreleaser.yml)
- [Commits](elastic/elastic-package@v0.102.0...v0.103.0)

---
updated-dependencies:
- dependency-name: github.com/elastic/elastic-package
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
…/elastic/elastic-package-0.103.0' into fix-integrations-logsdb
@jsoriano
Copy link
Member Author

jsoriano commented Sep 4, 2024

Changes in ti packages moved to #11008.

jsoriano added a commit that referenced this pull request Sep 5, 2024
Issue detected in #10919, there is a field in transforms that is not documented.

This was only detected when enabling synthetic source (through logsdb), because
only in this case we check fields that are not in the source, but are in the index,
such as constant_keywords.

Not having these fields defined will be a problem when we enable testing with
logsdb, and/or when we fix validation of these fields.
@jsoriano
Copy link
Member Author

jsoriano commented Sep 5, 2024

Changes for nested fields moved to #11016.

@@ -737,6 +740,8 @@
type: keyword
- name: auditd.data.result
type: keyword
- name: auditd.data.a*
type: keyword
Copy link
Member Author

Choose a reason for hiding this comment

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

These changes should not be needed as these are subfields of a flattened object, fixing this in elastic/elastic-package#2088.

@jsoriano
Copy link
Member Author

All changes attempting to solve discrepancies in tests have been merged. The pending issues are already present when logsdb is not used. Closing this.

@jsoriano jsoriano closed this Sep 10, 2024
@jsoriano jsoriano deleted the fix-integrations-logsdb branch September 10, 2024 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment