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

[nagios_xi] Add Nagios package with Service data stream #2944

Merged
merged 9 commits into from
May 9, 2022

Conversation

kush-elastic
Copy link
Collaborator

  • Enhancement

What does this PR do?

  • Generated the skeleton of Nagios integration package.
  • Added 1 data stream (Service).
  • Added data collection logic for the data streams.
  • Added the ingest pipeline for the data streams.
  • Mapped fields according to the ECS schema and added Fields metadata in the appropriate yaml files.
  • Added dashboards and visualizations.
  • Added system test cases for the data stream.

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.

How to test this PR locally

  • Clone integrations repo.
  • Install elastic-package locally.
  • Start elastic stack using elastic-package.
  • Move to integrations/packages/nagios_xi directory.
  • Run the following command to run tests.
    elastic-package test

Related issues

Screenshots

@kush-elastic kush-elastic requested a review from a team as a code owner March 30, 2022 18:04
@elasticmachine
Copy link

elasticmachine commented Mar 30, 2022

💚 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: 2022-05-03T02:18:41.805+0000

  • Duration: 16 min 20 sec

Test stats 🧪

Test Results
Failed 0
Passed 15
Skipped 0
Total 15

🤖 GitHub comments

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

  • /test : Re-trigger the build.

- monitoring
release: beta
conditions:
kibana.version: ^8.0.0
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This will be updated with 8.2.0 after testing this integration on 8.2.0.

@kush-elastic kush-elastic self-assigned this Mar 30, 2022
@kush-elastic kush-elastic added enhancement New feature or request Team:Integrations Label for the Integrations team New Integration labels Mar 30, 2022
@elasticmachine
Copy link

Pinging @elastic/integrations (Team:Integrations)

@kush-elastic kush-elastic mentioned this pull request Mar 31, 2022
4 tasks
@kush-elastic kush-elastic marked this pull request as draft March 31, 2022 09:06
changes:
- description: Nagios integration package with Logs, Host and Service metrics
type: enhancement
link: https://github.com/elastic/integrations/pull/2824
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: you have to update the PR link reference to point to this review.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right.

packages/nagios_xi/docs/README.md Show resolved Hide resolved
- name: nagios_xi
type: group
fields:
- name: service
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it contain the same fields as for the host data stream?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not all the fields are same, but there are some common fields.

Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't check precisely the percentage of different fields, but maybe it's better to keep it as a single data stream. WDYT @ruflin ?

Copy link
Member

Choose a reason for hiding this comment

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

The decision if one or multiple data streams should be less driven by how many fields are different then how users are going to consume it. Will some users only want to collect service or host? Will there be different retention periods?

Copy link
Collaborator Author

@kush-elastic kush-elastic Apr 1, 2022

Choose a reason for hiding this comment

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

There are groups based on hosts and services.
If we consider merging host and service data-streams in one, user will get confused.
Services depends on hosts so same type of service can be collected from different hosts.

Another reason is, we have different endpoints for hosts and services. means we need two httpjson input.
And as we know we don't have a way to use multiple input in single data_stream.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the details @kush-elastic Based on this information I would go for 2 data streams for now. It is easier to merge 2 into one later on then the other way around I think.

@kush-elastic kush-elastic requested a review from mtojek April 1, 2022 04:55
@kush-elastic kush-elastic marked this pull request as ready for review April 5, 2022 11:58
@kush-elastic kush-elastic requested a review from a team as a code owner April 5, 2022 11:58
@@ -334,7 +334,7 @@
"check_type": "0",
"current_check_attempt": "1",
"current_notification_number": "0",
"current_state": "0",
"current_state": "Up/Pending",
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@kush-elastic kush-elastic changed the title [nagios] Add Nagios package with Service data stream [nagios_xi] Add Nagios package with Service data stream Apr 7, 2022
"failure_prediction_enabled": "0",
"flap_detection_enabled": "0",
"has_been_checked": "1",
"host_address": "www.nagios.org",
Copy link
Collaborator

Choose a reason for hiding this comment

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

How these host* fields under the "service" metrics. Should these be part of core ECS host* fields. I am not sure, Checking with @mtojek

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hey @lalit-satapathy,
In Nagios XI, There are groups based on hosts and services. and each services have again on specific hosts. so user will get confused if we show host name in both host and service data-stream. if we have different fields here user will be able to identify it much better.
Here services runs on specific hosts.
What do you think?

@elasticmachine
Copy link

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (3/3) 💚
Files 100.0% (3/3) 💚 3.21
Classes 100.0% (3/3) 💚 3.21
Methods 97.059% (33/34) 👍 8.779
Lines 96.684% (1137/1176) 👍 7.784
Conditionals 100.0% (0/0) 💚

@mtojek mtojek merged commit f3e5415 into elastic:main May 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Integration:NagiosXI Nagios XI New Integration Team:Integrations Label for the Integrations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants