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

[Filebeat] Support the custom nginx-ingress-controller Nginx log format #8451

Closed
nikolay opened this issue Sep 26, 2018 · 15 comments · Fixed by #16197
Closed

[Filebeat] Support the custom nginx-ingress-controller Nginx log format #8451

nikolay opened this issue Sep 26, 2018 · 15 comments · Fixed by #16197
Assignees
Labels
containers Related to containers use case enhancement Filebeat Filebeat in progress Pull request is currently in progress. module Team:Integrations Label for the Integrations team Team:Platforms Label for the Integrations - Platforms team v7.7.0

Comments

@nikolay
Copy link

nikolay commented Sep 26, 2018

In the Kubernetes world, one of the most popular choices for an ingress controller is the nginx-ingress-controller, which doesn't use a standard Nginx log format though. Instead, it uses a custom format, documented here: https://github.com/kubernetes/ingress-nginx/blob/master/docs/user-guide/nginx-configuration/log-format.md

Given how many projects use this ingress controller, it would be nice to add a module called nginx-ingress-controller and make things work for people out of the box instead of every projects having to clone the nginx module. Or maybe add some kind of option to the nginx module to support this de facto standard in the Kubernetes world format instead.

I think this will greatly improve the adoption of Filebeats within the Kubernetes ecosystem!

@nikolay nikolay changed the title Support the custom nginx-ingress-controller Nginx log format [Filebeat] Support the custom nginx-ingress-controller Nginx log format Sep 26, 2018
@ruflin ruflin added enhancement module Filebeat Filebeat containers Related to containers use case labels Oct 1, 2018
@ruflin
Copy link
Member

ruflin commented Oct 1, 2018

Interesting suggestion. @exekias Thoughts?

@exekias
Copy link
Contributor

exekias commented Oct 1, 2018

Thanks for reporting @nikolay, we definitely want to support this use case. I'm wondering if it would be possible to add the kubernetes pattern to the existing pipeline definition and keep compatibility with both the standard format and this one.

Is that possible @ruflin? maybe @kvch knows this one

@kvch
Copy link
Contributor

kvch commented Oct 1, 2018

It's possible to add new patterns to the existing pipeline. I am thinking about if we should add it to the existing pipelines or add a new separate fileset.
I might go for introducing it as a new fileset, because if someone uses nginx-ingress-controller, there are always going to be one pattern (the default one) which does not match. We could save a few CPU ticks and reduce complexity by providing a new specialized pipeline for this format.

@ruflin
Copy link
Member

ruflin commented Oct 1, 2018

It's a more general problem we have to solve: Having 2 different types of log files for 1 dataset. Elasticsearch logs will change in 7.0. How do we cope with the old and new format (@ycombinator )? Can we have a config option to define which pipeline is used in fileset and can this config option be set during enabling a module or for example through autodiscovery?

@nikolay
Copy link
Author

nikolay commented Oct 1, 2018

@ruflin I think for simplicity, this should be a different module as the goals of an ingress controller is more narrow than that of a general-purpose web server.

@kvch
Copy link
Contributor

kvch commented Oct 2, 2018

Path to the selected pipeline of modules are already configurable in manifest.yml files. This option could be set by users based on their input application version. Or maybe each module can have an option named pipeline_version or version which is the version of the input. E.g in case of ES version could be set to 7.0 to tell Filebeat to upload the pipeline of ES 7.0. If the version is not set, we default to the pipeline of the latest release.

Example configuration:

- module: elasticsearch
  version: 7.0
  server:
    enabled: true

Versions could be corresponding to the input application versions in order to minimize confusion.

However, nginx-ingest-controller does not have anything to do with this problem. It's a separate application which configures the logging format of nginx differently. What happens if we need to support multiple versions of this? By adding it as a separate fileset and introducing the new option version, it can be set properly. I think this leads to a cleaner configuration.

- module: nginx
  ingress-controller:
    enabled: true

instead of

- module: nginx
  access:
    enabled: true
    version: ingress-controller

@nikolay
Copy link
Author

nikolay commented Oct 2, 2018

@kvch Even without Nginx controller in the picture, Nginx already has at least two built-in formats, and allows for custom. I think using version is abusing the purpose of it. Using filesets is also a hack. I just think a fileset should have a setting for format, plus, ideally, it should allow to easily add a custom format to an existing module.

@ruflin
Copy link
Member

ruflin commented Oct 3, 2018

How does the data from ingest-controller look compared to other ingest datasets (I used dataset instead of fileset here). In general I fileset/dataset are to group together very similar data which is normally the case inside 1 log file. If the data is compeletly different or have different purpose I also think it should be it's own fileset.

Nginx is a bit special here as it allows all this custom config for the logs.

@kvch
Copy link
Contributor

kvch commented Oct 3, 2018

@nikolay I think we should introduce a new Ingest pipeline for ingress-controller to decrease the time to find the matching pattern. So letting users select the pipeline they would like to use during processing is not abusing anything. Also, as @ruflin pointed it out, a fileset processes data from a single log file. Is the logs of ingest-controller are written to the same file as other nginx logs?
If yes, it makes even more sense to make pipelines configurable on fileset level.

Regarding custom config for patterns I think those are for users who have custom log format configured to fit their needs. This is an application default you have linked to, so we should provide something for users who do not want to come up with a matching pattern.

@exekias
Copy link
Contributor

exekias commented Oct 3, 2018

Something to take into account here is also easiness of the configuration. Using module parameters is ok, but things like that are not available in autodiscover (at least not as of today). We may want to consider the specific module because of that.

I think this ties to how we handle pipelines & modules, I think it worth a discussion for the longer term.

@nikolay
Copy link
Author

nikolay commented Oct 3, 2018

@kvch It's abusing it as in the case of Nginx, an instance have all filesets, they are just in different formats. Like MySQL slow log, etc. In the Nginx example, the error log and the access log are always present. So, you want to add a third fileset, which actually is a variant of the access log - it will be confusing and an abuse of your original design. Well, if you add a "variant" or "flavor" to a fileset with marking one as the default, then that's a different story! Then in the hints you can provide something like access/ingress-controller and if no "flavor" is specified after the slash, the default is assumed.

@kvch
Copy link
Contributor

kvch commented Oct 4, 2018

I agree with you. I think we are thinking about the same thing when you speak about "flavor" and I do about pipeline versioning. We need something which takes care of different variants of the same logs under the same path.
However, until we don't have versioned pipelines or flavors, I would separate this pattern from the existing pipeline.

@ruflin
Copy link
Member

ruflin commented Oct 5, 2018

I quite like the syntax that @nikolay used do describe a "flavor/version". An alternative would be to use access:ingress-controller. Both separator would be unique and should not show up in a fileset name itself.

@ruflin ruflin added the Team:Integrations Label for the Integrations team label Nov 27, 2018
@mmisztal1980
Copy link

can anyone comment on the current state of affairs ? is this solved? or are there any known workarounds?

@ChrsMark
Copy link
Member

ChrsMark commented Feb 7, 2020

Hey @kvch @exekias any final suggestions about this in order to push it forward?

Are we leaning towards to sth like

- module: nginx
  access:
    enabled: true
    version: ingress-controller

and access:ingress-controller for autodiscover hints?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
containers Related to containers use case enhancement Filebeat Filebeat in progress Pull request is currently in progress. module Team:Integrations Label for the Integrations team Team:Platforms Label for the Integrations - Platforms team v7.7.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants