Skip to content

Add custom configuration to nginx/metrics#12865

Closed
toms-place wants to merge 14 commits into
elastic:mainfrom
toms-place:nginx-custom-configuration
Closed

Add custom configuration to nginx/metrics#12865
toms-place wants to merge 14 commits into
elastic:mainfrom
toms-place:nginx-custom-configuration

Conversation

@toms-place
Copy link
Copy Markdown

@toms-place toms-place commented Feb 21, 2025

Proposed commit message

This PR adds the possibility to add custom configuration to the nginx/metrics input.
This allows to add custom conditions to nginx metrics, in order to use http://${kubernetes.pod.ip} in the hosts field on fleet managed agents.

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.
  • I have verified that any added dashboard complies with Kibana's Dashboard good practices

Author's Checklist

  • Discuss if this is also needed for logs

How to test this PR locally

Add ingress nginx to a k8s cluster, deploy a fleet agent on an ingress nginx node and scrape the metrics directly from the pod.

Related issues

With this PR the headless service is not needed anymore.

Screenshots

@toms-place toms-place requested a review from a team as a code owner February 21, 2025 16:50

Example:

condition: ${kubernetes.labels.app} == 'ingress-nginx'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Author

@toms-place toms-place Feb 27, 2025

Choose a reason for hiding this comment

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

Hi @muthu-mps :)
Thanks for reviewing!

The nginx_ingress_contoller package only supports logs. It currently does not support metrics/stubstatus.
That's why I adapted the nginx package.

Adding the condition only is also an option, but I thought having more control by just using a custom field is appreciated.

I also have an open bullet point to discuss, if we want to have this for acces/error logs as well in the nginx package. What do you think?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@toms-place - As the logs can be fetched using the nginx_ingress_controller integration, we can adopt the condition for the metrics.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

So do you want me to change it to condition only with an empty string as default , instead of using a custom field?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As we try to fetch data by applying condition, we could probably change this to condition instead of custom input without applying default variable.

@toms-place - Can we try applying the config and process the data using stubstatus datastream to make sure that it process the expected metrics.

Copy link
Copy Markdown
Author

@toms-place toms-place Mar 4, 2025

Choose a reason for hiding this comment

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

I changed it to condition now ;)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@muthu-mps can this be merged now? :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@toms-place - Sorry for the delay here, I would like to understand if this is going to be a redundant feature when we enable this data stream to nginx-ingress-controller integration. Will it be beneficial either way when used in both the integrations? Can we add the metrics data stream to the ingress-controller and adopt this change? WDYT?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@muthu-mps - Ah I think the confusion here is that I am talking about metrics. the nginx-ingress-controller integration only handles logs atm.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

But it could be beneficial there as well I guess.

@andrewkroh andrewkroh added Integration:nginx Nginx Team:Obs-InfraObs Observability Infrastructure Monitoring team [elastic/obs-infraobs-integrations] labels Mar 13, 2025
@botelastic
Copy link
Copy Markdown

botelastic Bot commented May 29, 2025

Hi! We just realized that we haven't looked into this PR in a while. We're sorry! We're labeling this issue as Stale to make it hit our filters and make sure we get back to it as soon as possible. In the meantime, it'd be extremely helpful if you could take a look at it as well and confirm its relevance. A simple comment with a nice emoji will be enough :+1. Thank you for your contribution!

@botelastic botelastic Bot added the Stalled label May 29, 2025
@botelastic
Copy link
Copy Markdown

botelastic Bot commented Jun 28, 2025

Hi! This PR has been stale for a while and we're going to close it as part of our cleanup procedure. We appreciate your contribution and would like to apologize if we have not been able to review it, due to the current heavy load of the team. Feel free to re-open this PR if you think it should stay open and is worth rebasing. Thank you for your contribution!

@botelastic botelastic Bot closed this Jun 28, 2025
@toms-place
Copy link
Copy Markdown
Author

Please reopen it.

@muthu-mps muthu-mps reopened this Jun 30, 2025
@botelastic botelastic Bot removed the Stalled label Jun 30, 2025
@botelastic
Copy link
Copy Markdown

botelastic Bot commented Jul 30, 2025

Hi! We just realized that we haven't looked into this PR in a while. We're sorry! We're labeling this issue as Stale to make it hit our filters and make sure we get back to it as soon as possible. In the meantime, it'd be extremely helpful if you could take a look at it as well and confirm its relevance. A simple comment with a nice emoji will be enough :+1. Thank you for your contribution!

@botelastic botelastic Bot added Stalled and removed Stalled labels Jul 30, 2025
@toms-place toms-place force-pushed the nginx-custom-configuration branch from 2e439cc to 0b3c314 Compare July 31, 2025 09:57
@andrewkroh andrewkroh added the enhancement New feature or request label Aug 7, 2025
@botelastic
Copy link
Copy Markdown

botelastic Bot commented Sep 6, 2025

Hi! We just realized that we haven't looked into this PR in a while. We're sorry! We're labeling this issue as Stale to make it hit our filters and make sure we get back to it as soon as possible. In the meantime, it'd be extremely helpful if you could take a look at it as well and confirm its relevance. A simple comment with a nice emoji will be enough :+1. Thank you for your contribution!

@botelastic botelastic Bot added the Stalled label Sep 6, 2025
@ishleenk17
Copy link
Copy Markdown
Member

/test

@botelastic botelastic Bot removed the Stalled label Sep 7, 2025
@elasticmachine
Copy link
Copy Markdown

💔 Build Failed

Failed CI Steps

@ishleenk17
Copy link
Copy Markdown
Member

@toms-place : Would you like to resume it ? I would take it to completion.

@toms-place
Copy link
Copy Markdown
Author

@toms-place : Would you like to resume it ? I would take it to completion.

@ishleenk17 - Yes, what is still left here?

@ishleenk17
Copy link
Copy Markdown
Member

@toms-place : Would you like to resume it ? I would take it to completion.

@ishleenk17 - Yes, what is still left here?

Some merge conflict resolutions. Then we should be GTG.

@botelastic
Copy link
Copy Markdown

botelastic Bot commented Oct 7, 2025

Hi! We just realized that we haven't looked into this PR in a while. We're sorry! We're labeling this issue as Stale to make it hit our filters and make sure we get back to it as soon as possible. In the meantime, it'd be extremely helpful if you could take a look at it as well and confirm its relevance. A simple comment with a nice emoji will be enough :+1. Thank you for your contribution!

@botelastic botelastic Bot added the Stalled label Oct 7, 2025
@botelastic botelastic Bot removed the Stalled label Oct 8, 2025
@toms-place
Copy link
Copy Markdown
Author

@toms-place : Would you like to resume it ? I would take it to completion.

@ishleenk17 - Yes, what is still left here?

Some merge conflict resolutions. Then we should be GTG.

All resolved, please review again :)

@botelastic
Copy link
Copy Markdown

botelastic Bot commented Dec 31, 2025

Hi! We just realized that we haven't looked into this PR in a while. We're sorry! We're labeling this issue as Stale to make it hit our filters and make sure we get back to it as soon as possible. In the meantime, it'd be extremely helpful if you could take a look at it as well and confirm its relevance. A simple comment with a nice emoji will be enough :+1. Thank you for your contribution!

@botelastic botelastic Bot added the Stalled label Dec 31, 2025
@botelastic botelastic Bot removed the Stalled label Jan 14, 2026
@botelastic
Copy link
Copy Markdown

botelastic Bot commented Mar 1, 2026

Hi! We just realized that we haven't looked into this PR in a while. We're sorry! We're labeling this issue as Stale to make it hit our filters and make sure we get back to it as soon as possible. In the meantime, it'd be extremely helpful if you could take a look at it as well and confirm its relevance. A simple comment with a nice emoji will be enough :+1. Thank you for your contribution!

@botelastic botelastic Bot added the Stalled label Mar 1, 2026
@botelastic
Copy link
Copy Markdown

botelastic Bot commented Mar 31, 2026

Hi! This PR has been stale for a while and we're going to close it as part of our cleanup procedure. We appreciate your contribution and would like to apologize if we have not been able to review it, due to the current heavy load of the team. Feel free to re-open this PR if you think it should stay open and is worth rebasing. Thank you for your contribution!

@botelastic botelastic Bot closed this Mar 31, 2026
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:nginx Nginx Stalled Team:Obs-InfraObs Observability Infrastructure Monitoring team [elastic/obs-infraobs-integrations]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[NGINX]: DNS Lookup of multiple ingress pods via headless service

5 participants