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

Extract "Ingress Controller" to a separate integration #451

Merged
merged 14 commits into from
Dec 9, 2020

Conversation

mtojek
Copy link
Contributor

@mtojek mtojek commented Dec 8, 2020

What does this PR do?

This PR extracts ingress_controller from the nginx to a separate integration.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all datasets collect metrics or logs.

How to test this PR locally

Run system tests with defer cleanup option: elastic-package test system -v --defer-cleanup=120s,
navigate to dashboards to see access logs.

Related issues

Screenshots

nginx-ingress-controller-access-logs
nginx-ingress-controller-overview

@mtojek mtojek added enhancement New feature or request Team:Integrations Label for the Integrations team labels Dec 8, 2020
@mtojek mtojek requested a review from a team December 8, 2020 15:16
@mtojek mtojek self-assigned this Dec 8, 2020
@elasticmachine
Copy link

Pinging @elastic/integrations (Team:Integrations)

@mtojek mtojek mentioned this pull request Dec 8, 2020
16 tasks
@elasticmachine
Copy link

elasticmachine commented Dec 8, 2020

💚 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

Expand to view the summary

Build stats

  • Build Cause: mtojek commented: jenkins run the tests please

  • Start Time: 2020-12-09T18:33:22.565+0000

  • Duration: 14 min 25 sec

Test stats 🧪

Test Results
Failed 0
Passed 90
Skipped 0
Total 90

Copy link
Member

@ChrsMark ChrsMark left a comment

Choose a reason for hiding this comment

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

Looks good, thank you for taking care of this!
Just left some minors, let me know what you think.


The `access` data stream collects the Nginx Ingress Controller access logs.

{{fields "access"}}
Copy link
Member

Choose a reason for hiding this comment

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

Sample events will not be added?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right I forgot about it!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

title: Nginx ingress controller logs
description: Collect Nginx ingress controller logs
title: Nginx Ingress Controller access logs
description: Collect Nginx Ingress Controller access logs
Copy link
Member

Choose a reason for hiding this comment

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

Just to note that error logs will not be parsed (these error logs are actually typical golang error logs). We need to track it down somehow since it was part of the migration assessment for this package.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm.. not sure if I understand the part about Golang errors. Could you please elaborate more? Are there any specific nginx ingress controllers Go errors dumped in Beats?

Copy link
Member

@ChrsMark ChrsMark Dec 9, 2020

Choose a reason for hiding this comment

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

Ingress Nginx will log errors like any other golang application. There have been requests from users to handle them as well. Unfortunately it seems it was skipped from the migration assessment. or I cannot find it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added an action item to #373 (comment) .

I will add support for error logs in the follow up issue.

@mtojek mtojek requested a review from ChrsMark December 9, 2020 14:12
@mtojek
Copy link
Contributor Author

mtojek commented Dec 9, 2020

/test

@mtojek
Copy link
Contributor Author

mtojek commented Dec 9, 2020

jenkins run the tests please

1 similar comment
@mtojek
Copy link
Contributor Author

mtojek commented Dec 9, 2020

jenkins run the tests please

@mtojek mtojek merged commit 4d1ef9c into elastic:master Dec 9, 2020
eyalkraft pushed a commit to build-security/integrations that referenced this pull request Mar 30, 2022
* Copy nginx_ingress_controller from nginx

* Adjust ingress controller

* Extract Nginx Ingress Controller from nginx

* Fix: typo

* Dump ingress controller logs

* Remove Ingress Controller from Nginx

* Adjust tests

* Docker changes

* Docker for system tests

* Fix: missing fields

* Adjust dashboards

* Fix: missing IP address

* Adjust screenshots

* Add missing sample event
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Team:Integrations Label for the Integrations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants