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

[newrelic-logging] enable sendMetrics and fluent bit metrics config by default #1547

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rohit-bandlamudi-nr
Copy link

@rohit-bandlamudi-nr rohit-bandlamudi-nr commented Dec 12, 2024

Is this a new chart

No

What this PR does / why we need it:

This PR enabled metrics forwarding by default and removes the option to enable / disable this behaviour. The labels that are required for fluent bit entity synthesis are also added as part of the prometheus remote-write configuration.

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • Chart Version bumped
  • Variables are documented in the README.md
  • Title of the PR starts with chart name (e.g. [mychartname])

voorepreethi
voorepreethi previously approved these changes Dec 12, 2024
voorepreethi
voorepreethi previously approved these changes Dec 18, 2024
voorepreethi
voorepreethi previously approved these changes Dec 18, 2024
voorepreethi
voorepreethi previously approved these changes Jan 7, 2025
Copy link
Contributor

@voorepreethi voorepreethi left a comment

Choose a reason for hiding this comment

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

Workflows are failing. Please check

@rohit-bandlamudi-nr rohit-bandlamudi-nr force-pushed the add-labels-for-entity-synthesis branch from 751987d to d108d79 Compare January 30, 2025 11:56
@rohit-bandlamudi-nr rohit-bandlamudi-nr changed the title [newrelic-logging] add labels to metrics output to enable entity synthesis [newrelic-logging] enable sendMetrics and fluent bit metrics config by default Jan 30, 2025
Copy link
Contributor

@jsubirat jsubirat left a comment

Choose a reason for hiding this comment

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

Nice stuff 👍

@@ -1,7 +1,7 @@
apiVersion: v2
description: A Helm chart to deploy New Relic Kubernetes Logging as a DaemonSet, supporting both Linux and Windows nodes and containers
name: newrelic-logging
version: 1.23.5
version: 1.23.6-beta
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we using beta here?

Returns fluentbit config to collect and forward its metrics to New Relic
*/}}
{{- define "newrelic-logging.fluentBit.monitoring.config" -}}
[INPUT]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't we just keep this in values.yaml for better visibility.

@@ -174,7 +175,7 @@ fluentBit:
licenseKey ${LICENSE_KEY}
endpoint ${ENDPOINT}
lowDataMode ${LOW_DATA_MODE}
sendMetrics ${SEND_OUTPUT_PLUGIN_METRICS}
sendMetrics true
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to enable this for fluent bit? It seems to me that its specific for sending output plugin related metrics. : https://github.com/newrelic/newrelic-fluent-bit-output/pull/142/files @jsubirat

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants