Skip to content

Nr 370484 fb metrics (#2033) #2034

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Nr 370484 fb metrics (#2033) #2034

wants to merge 7 commits into from

Conversation

voorepreethi
Copy link
Collaborator

This PR includes Fluentbit metrics forwarder in the default IA based on feature flag : "fluent_bit_metrics"
For more details please refer jira: https://new-relic.atlassian.net/browse/NR-370484

@voorepreethi
Nr 370484 fb metrics (#2033)
99244a6 
* feat: Updated Fb metrics by default

* feat: Updated test cases

* feat:Updated host details

* feat: Updated test cases and scrape interval
@voorepreethi voorepreethi requested a review from a team as a code owner April 14, 2025 05:09
@rajrohanyadav rajrohanyadav mentioned this pull request Apr 14, 2025
Closed
@coveralls
Copy link

Pull Request Test Coverage Report for Build 14462832288

Details

  • 95 of 110 (86.36%) changed or added relevant lines in 4 files are covered.
  • 3 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.1%) to 57.491%
Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/integrations/v4/logs/loader.go 2 4 50.0%
pkg/integrations/v4/logs/cfg.go 90 103 87.38%
Files with Coverage Reduction New Missed Lines %
internal/agent/event_sender.go 1 78.11%
internal/integrations/v4/testhelp/testemit/testemit.go 2 90.63%
Totals Coverage Status
Change from base Build 14325595165: 0.1%
Covered Lines: 15476
Relevant Lines: 26919
💛 - Coveralls

maya-jha
maya-jha reviewed Apr 15, 2025
View reviewed changes
internal/agent/cmdchannel/fflag/ffhandler.go
@@ -185,7 +186,8 @@ func isBasicFeatureFlag(flag string) bool {
return flag == FlagProtocolV4 ||
flag == FlagFullProcess ||
flag == FlagDmRegisterDeprecated ||
flag == FlagFullInventoryDeletion
flag == FlagFullInventoryDeletion ||
flag == FlagFluentBitMetrics
Copy link

Choose a reason for hiding this comment

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

How does this feature flag work? At what stage its being used? During installation?

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 now this works on start of agent , so on every FF change we need to restart the Agent. Let me check if there is a way to not restart

pkg/integrations/v4/logs/cfg.go Outdated
Comment on lines 421 to 422
MetricsPath: "/api/v2/metrics/prometheus",
ScrapeInterval: "60s",
Copy link

Choose a reason for hiding this comment

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

make them constant.

maya-jha
maya-jha suggested changes Apr 16, 2025
View reviewed changes
Copy link

@maya-jha maya-jha left a comment

Choose a reason for hiding this comment

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

  1. Please provide details about how changes were tested.
  2. Any changes in default logging.yml?
  3. What changes will be required to get it working with super agent?
  4. Here option to turn off metrics would be more important as there is no way to update this.
  5. Have you looked at health check options. Can this be useful in future ?https://docs.fluentbit.io/manual/administration/monitoring#health-check-for-fluent-bit

pkg/integrations/v4/logs/cfg.go Outdated
@@ -282,6 +317,8 @@ func NewFBConf(loggingCfgs LogsCfg, logFwdCfg *config.LogForward, entityGUID, ho
var fbOSConfig FBOSConfig
addOSDependantConfig(&fbOSConfig)

enableMetrics := isMetricsEnabled(ff)
Copy link

Choose a reason for hiding this comment

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

Will user have option to turn it off? Do we have PM alignment on this if we are not offering the ability to turn it off?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. we has a alignment with PM
cc: @nr-rkallempudi

pkg/integrations/v4/logs/cfg_template.go
Comment on lines +129 to +145
Log_Level {{ .LogLevel }}
{{- end }}
{{- if .Daemon }}
Daemon {{ .Daemon }}
{{- end }}
{{- if .ParsersFile }}
Parsers_File {{ .ParsersFile }}
{{- end }}
{{- if .HTTPServer }}
HTTP_Server {{ .HTTPServer }}
{{- end }}
{{- if .HTTPListen }}
HTTP_Listen {{ .HTTPListen }}
{{- end }}
{{- if .HTTPPort }}
HTTP_Port {{ .HTTPPort }}
{{- end }}
Copy link

Choose a reason for hiding this comment

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

from where we are getting these values?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sorry?

pkg/integrations/v4/logs/cfg.go
// Newrelic OUTPUT plugin will send all the collected logs to Vortex
fb.Output = newNROutput(logFwdCfg)
// Including Prometheus scrapper input plugin by default to pull Fluent bit metrics based on ff
includePrometheusScrapperInputPlugin(&fb, enableMetrics)
Copy link

Choose a reason for hiding this comment

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

Are we planning to send all metrics? What are the cost implications here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. We are planning to send all the metrics. Will share the Cost analysis doc

@voorepreethi
Copy link
Collaborator Author

voorepreethi commented Apr 17, 2025 •
edited
Loading

@maya-jha 1. Please provide details about how changes were tested.
--> https://github.com/newrelic/infrastructure-agent/blob/master/README.md#compile-and-build-the-agent. Build the code locally and update the IA bundles with the locally build files and restarting the Agent .
2. Any changes in default logging.yml?
--> No
3. What changes will be required to get it working with super agent?
--> IA is already having Super Agent support , so No Changes required for this changes to be included.
4. Here option to turn off metrics would be more important as there is no way to update this.
--> I have raised this point , with PM and Rajeev, they asked for a similar experience like k8s.
5. Have you looked at health check options. Can this be useful in future ?https://docs.fluentbit.io/manual/administration/monitoring#health-check-for-fluent-bit
Sure , we can check on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maya-jha maya-jha

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@voorepreethi @coveralls @maya-jha