[newrelic-logging] enable sendMetrics and fluent bit metrics config by default #1547
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
voorepreethi
previously approved these changes
Dec 12, 2024
rohit-bandlamudi-nr
force-pushed
the
add-labels-for-entity-synthesis
branch
from
a38dea4 to
709e548
Compare
voorepreethi
previously approved these changes
Dec 18, 2024
voorepreethi
previously approved these changes
Dec 18, 2024
voorepreethi
previously approved these changes
Jan 7, 2025
voorepreethi
reviewed
Jan 7, 2025
rohit-bandlamudi-nr
force-pushed
the
add-labels-for-entity-synthesis
branch
from
751987d to
d108d79
Compare
rohit-bandlamudi-nr
changed the title
rohit-bandlamudi-nr
force-pushed
the
add-labels-for-entity-synthesis
branch
from
d108d79 to
c0b69fa
Compare
maya-jha
requested changes
Feb 3, 2025
charts/newrelic-logging/Chart.yaml
Outdated
| @@ -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 | |||
There was a problem hiding this comment.
Why are we using beta here?
There was a problem hiding this comment.
We want to release this as a beta version and test the entity synthesis and forwarding of metrics (if possible, we want to test this with fleet control and agent control as well)
| Returns fluentbit config to collect and forward its metrics to New Relic | ||
| */}} | ||
| {{- define "newrelic-logging.fluentBit.monitoring.config" -}} | ||
| [INPUT] |
There was a problem hiding this comment.
Why don't we just keep this in values.yaml for better visibility.
There was a problem hiding this comment.
We are enabling sending of metrics by default. We don't want this to be configurable by the user using the values file. Basically, disabling metrics should not be an option
There was a problem hiding this comment.
I think visibility is important. If a customer really wants to remove it, can't they do it anyway?
maya-jha
reviewed
Feb 3, 2025
| @@ -174,7 +175,7 @@ fluentBit: | |||
| licenseKey ${LICENSE_KEY} | |||
| endpoint ${ENDPOINT} | |||
| lowDataMode ${LOW_DATA_MODE} | |||
| sendMetrics ${SEND_OUTPUT_PLUGIN_METRICS} | |||
| sendMetrics true | |||
There was a problem hiding this comment.
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
There was a problem hiding this comment.
yes, sendMetrics are defaulted to false in the newrelic-fluentbit-output repo. We can change that to true. Until then, we have set this to true for testing
There was a problem hiding this comment.
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
Yeah, note that , these metrics are not required for creation of fluentbit entity. But we will be missing on some key metrics forwarded by the newrelic fluentbit output plugin that are used to power few charts on the fluentbit entity view
There was a problem hiding this comment.
Discussed offline with @nr-rkallempudi and Joshua and we aligned on not changing anything related fluent bit output plugin metrics. The metrics added by fluent bit output plugin are not being used for fluent bit dashboard AFAIK.
nr-rkallempudi
previously approved these changes
Feb 5, 2025
rohit-bandlamudi-nr
dismissed stale reviews from nr-rkallempudi and jsubirat
via
667213d
<!-- Thank you for contributing to New Relic's Helm charts. Before you submit this PR we'd like to make sure you are aware of our technical requirements: * https://github.com/newrelic-experimental/helm-charts/blob/master/CONTRIBUTING.md#technical-requirements For a quick overview across what we will look at reviewing your PR, please read our review guidelines: * https://github.com/newrelic-experimental/helm-charts/blob/master/REVIEW_GUIDELINES.md Following our best practices right from the start will accelerate the review process and help get your PR merged quicker. When updates to your PR are requested, please add new commits and do not squash the history. This will make it easier to identify new changes. The PR will be squashed anyways when it is merged. Thanks. For fast feedback, please @-mention maintainers that are listed in the Chart.yaml file. Please make sure you test your changes before you push them. Once pushed, a Github Action will run across your changes and do some initial checks and linting. These checks run very quickly. Please check the results. We would like these checks to pass before we even continue reviewing your changes. --> #### Is this a new chart No #### What this PR does / why we need it: Adds the changes from #1520 #### Which issue this PR fixes *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)* - fixes # #### Special notes for your reviewer: #### Checklist [Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.] - [x] Chart Version bumped - [x] Variables are documented in the README.md - [x] Title of the PR starts with chart name (e.g. `[mychartname]`) --------- Signed-off-by: kpattaswamy <[email protected]>
#1592) #### What this PR does / why we need it: Uses a dedicated image for the system identity registration step for Agent Control deployments. #### Which issue this PR fixes https://new-relic.atlassian.net/browse/NR-362687 #### Checklist [Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.] - [x] Chart Version bumped - [x] Variables are documented in the README.md - [x] Title of the PR starts with chart name (e.g. `[mychartname]`)
<!-- Thank you for contributing to New Relic's Helm charts. Before you submit this PR we'd like to make sure you are aware of our technical requirements: * https://github.com/newrelic-experimental/helm-charts/blob/master/CONTRIBUTING.md#technical-requirements For a quick overview across what we will look at reviewing your PR, please read our review guidelines: * https://github.com/newrelic-experimental/helm-charts/blob/master/REVIEW_GUIDELINES.md Following our best practices right from the start will accelerate the review process and help get your PR merged quicker. When updates to your PR are requested, please add new commits and do not squash the history. This will make it easier to identify new changes. The PR will be squashed anyways when it is merged. Thanks. For fast feedback, please @-mention maintainers that are listed in the Chart.yaml file. Please make sure you test your changes before you push them. Once pushed, a Github Action will run across your changes and do some initial checks and linting. These checks run very quickly. Please check the results. We would like these checks to pass before we even continue reviewing your changes. --> #### Is this a new chart No. #### What this PR does / why we need it: Releases new version of the ebpf agent. Docker hub to verify the images are deployed. https://hub.docker.com/r/newrelic/newrelic-ebpf-agent #### Which issue this PR fixes *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)* - fixes # #### Special notes for your reviewer: #### Checklist [Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.] - [x] Chart Version bumped - [x] Variables are documented in the README.md - [x] Title of the PR starts with chart name (e.g. `[mychartname]`)
<!-- Thank you for contributing to New Relic's Helm charts. Before you submit this PR we'd like to make sure you are aware of our technical requirements: * https://github.com/newrelic-experimental/helm-charts/blob/master/CONTRIBUTING.md#technical-requirements For a quick overview across what we will look at reviewing your PR, please read our review guidelines: * https://github.com/newrelic-experimental/helm-charts/blob/master/REVIEW_GUIDELINES.md Following our best practices right from the start will accelerate the review process and help get your PR merged quicker. When updates to your PR are requested, please add new commits and do not squash the history. This will make it easier to identify new changes. The PR will be squashed anyways when it is merged. Thanks. For fast feedback, please @-mention maintainers that are listed in the Chart.yaml file. Please make sure you test your changes before you push them. Once pushed, a Github Action will run across your changes and do some initial checks and linting. These checks run very quickly. Please check the results. We would like these checks to pass before we even continue reviewing your changes. --> #### Is this a new chart No. #### What this PR does / why we need it: Bumps the version of helm from 3.0.0 to 3.2.0 for the lint workflow. This enables us to use the lookup function. #### Which issue this PR fixes *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)* - fixes # #### Special notes for your reviewer: #### 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]`)
<!-- Thank you for contributing to New Relic's Helm charts. Before you submit this PR we'd like to make sure you are aware of our technical requirements: * https://github.com/newrelic-experimental/helm-charts/blob/master/CONTRIBUTING.md#technical-requirements For a quick overview across what we will look at reviewing your PR, please read our review guidelines: * https://github.com/newrelic-experimental/helm-charts/blob/master/REVIEW_GUIDELINES.md Following our best practices right from the start will accelerate the review process and help get your PR merged quicker. When updates to your PR are requested, please add new commits and do not squash the history. This will make it easier to identify new changes. The PR will be squashed anyways when it is merged. Thanks. For fast feedback, please @-mention maintainers that are listed in the Chart.yaml file. Please make sure you test your changes before you push them. Once pushed, a Github Action will run across your changes and do some initial checks and linting. These checks run very quickly. Please check the results. We would like these checks to pass before we even continue reviewing your changes. --> #### Is this a new chart No #### What this PR does / why we need it: Un-commenting some code that checks for existing certs before creating new ones. We can do this now that the version of helm has been bumped. #### Which issue this PR fixes *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)* - fixes # #### Special notes for your reviewer: #### Checklist [Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.] - [x] Chart Version bumped - [x] Variables are documented in the README.md - [x] Title of the PR starts with chart name (e.g. `[mychartname]`)
<!-- Thank you for contributing to New Relic's Helm charts. Before you submit this PR we'd like to make sure you are aware of our technical requirements: * https://github.com/newrelic-experimental/helm-charts/blob/master/CONTRIBUTING.md#technical-requirements For a quick overview across what we will look at reviewing your PR, please read our review guidelines: * https://github.com/newrelic-experimental/helm-charts/blob/master/REVIEW_GUIDELINES.md Following our best practices right from the start will accelerate the review process and help get your PR merged quicker. When updates to your PR are requested, please add new commits and do not squash the history. This will make it easier to identify new changes. The PR will be squashed anyways when it is merged. Thanks. For fast feedback, please @-mention maintainers that are listed in the Chart.yaml file. Please make sure you test your changes before you push them. Once pushed, a Github Action will run across your changes and do some initial checks and linting. These checks run very quickly. Please check the results. We would like these checks to pass before we even continue reviewing your changes. --> #### Is this a new chart No. #### What this PR does / why we need it: It bumps the version of the eBPF agent. Fixes a bug where helm installs that are not named "nr-ebpf-agent" brick the eBPF Agent. #### Which issue this PR fixes *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)* - fixes # #### Special notes for your reviewer: #### Checklist [Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.] - [x] Chart Version bumped - [x] Variables are documented in the README.md - [x] Title of the PR starts with chart name (e.g. `[mychartname]`)
<!-- Thank you for contributing to New Relic's Helm charts. Before you submit this PR we'd like to make sure you are aware of our technical requirements: * https://github.com/newrelic-experimental/helm-charts/blob/master/CONTRIBUTING.md#technical-requirements For a quick overview across what we will look at reviewing your PR, please read our review guidelines: * https://github.com/newrelic-experimental/helm-charts/blob/master/REVIEW_GUIDELINES.md Following our best practices right from the start will accelerate the review process and help get your PR merged quicker. When updates to your PR are requested, please add new commits and do not squash the history. This will make it easier to identify new changes. The PR will be squashed anyways when it is merged. Thanks. For fast feedback, please @-mention maintainers that are listed in the Chart.yaml file. Please make sure you test your changes before you push them. Once pushed, a Github Action will run across your changes and do some initial checks and linting. These checks run very quickly. Please check the results. We would like these checks to pass before we even continue reviewing your changes. --> #### Is this a new chart #### What this PR does / why we need it: #### Which issue this PR fixes *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)* - fixes # #### Special notes for your reviewer: #### 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]`)
<!-- Thank you for contributing to New Relic's Helm charts. Before you submit this PR we'd like to make sure you are aware of our technical requirements: * https://github.com/newrelic-experimental/helm-charts/blob/master/CONTRIBUTING.md#technical-requirements For a quick overview across what we will look at reviewing your PR, please read our review guidelines: * https://github.com/newrelic-experimental/helm-charts/blob/master/REVIEW_GUIDELINES.md Following our best practices right from the start will accelerate the review process and help get your PR merged quicker. When updates to your PR are requested, please add new commits and do not squash the history. This will make it easier to identify new changes. The PR will be squashed anyways when it is merged. Thanks. For fast feedback, please @-mention maintainers that are listed in the Chart.yaml file. Please make sure you test your changes before you push them. Once pushed, a Github Action will run across your changes and do some initial checks and linting. These checks run very quickly. Please check the results. We would like these checks to pass before we even continue reviewing your changes. --> *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)* - fixes # [Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.] - [X] Chart Version bumped - [X] Title of the PR starts with chart name (e.g. `[mychartname]`)
<!-- Thank you for contributing to New Relic's Helm charts. Before you submit this PR we'd like to make sure you are aware of our technical requirements: * https://github.com/newrelic-experimental/helm-charts/blob/master/CONTRIBUTING.md#technical-requirements For a quick overview across what we will look at reviewing your PR, please read our review guidelines: * https://github.com/newrelic-experimental/helm-charts/blob/master/REVIEW_GUIDELINES.md Following our best practices right from the start will accelerate the review process and help get your PR merged quicker. When updates to your PR are requested, please add new commits and do not squash the history. This will make it easier to identify new changes. The PR will be squashed anyways when it is merged. Thanks. For fast feedback, please @-mention maintainers that are listed in the Chart.yaml file. Please make sure you test your changes before you push them. Once pushed, a Github Action will run across your changes and do some initial checks and linting. These checks run very quickly. Please check the results. We would like these checks to pass before we even continue reviewing your changes. --> #### Is this a new chart #### What this PR does / why we need it: #### Which issue this PR fixes *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)* - fixes # #### Special notes for your reviewer: #### 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]`)
<!-- Thank you for contributing to New Relic's Helm charts. Before you submit this PR we'd like to make sure you are aware of our technical requirements: * https://github.com/newrelic-experimental/helm-charts/blob/master/CONTRIBUTING.md#technical-requirements For a quick overview across what we will look at reviewing your PR, please read our review guidelines: * https://github.com/newrelic-experimental/helm-charts/blob/master/REVIEW_GUIDELINES.md Following our best practices right from the start will accelerate the review process and help get your PR merged quicker. When updates to your PR are requested, please add new commits and do not squash the history. This will make it easier to identify new changes. The PR will be squashed anyways when it is merged. Thanks. For fast feedback, please @-mention maintainers that are listed in the Chart.yaml file. Please make sure you test your changes before you push them. Once pushed, a Github Action will run across your changes and do some initial checks and linting. These checks run very quickly. Please check the results. We would like these checks to pass before we even continue reviewing your changes. --> #### Is this a new chart #### What this PR does / why we need it: #### Which issue this PR fixes *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)* - fixes # #### Special notes for your reviewer: #### 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]`)
<!-- Thank you for contributing to New Relic's Helm charts. Before you submit this PR we'd like to make sure you are aware of our technical requirements: * https://github.com/newrelic-experimental/helm-charts/blob/master/CONTRIBUTING.md#technical-requirements For a quick overview across what we will look at reviewing your PR, please read our review guidelines: * https://github.com/newrelic-experimental/helm-charts/blob/master/REVIEW_GUIDELINES.md Following our best practices right from the start will accelerate the review process and help get your PR merged quicker. When updates to your PR are requested, please add new commits and do not squash the history. This will make it easier to identify new changes. The PR will be squashed anyways when it is merged. Thanks. For fast feedback, please @-mention maintainers that are listed in the Chart.yaml file. Please make sure you test your changes before you push them. Once pushed, a Github Action will run across your changes and do some initial checks and linting. These checks run very quickly. Please check the results. We would like these checks to pass before we even continue reviewing your changes. --> #### Is this a new chart No. #### What this PR does / why we need it: Stops us from using the config option `hostNetwork: true` for the otel collector. This config binds the otel collector's network to the network of the host. One byproduct of this is that the port's uses are the same ports of the host. By default this is not the case. Ports on a container can bind to any open port on the host. However with this option it binds strictly to the same host port. This causes an issue if other containers on the node are trying to bind the the same port. This is known as a "port conflict" and will only allow one container to bind to the port, leaving the other container in a bad state. By decoupling the collector's networking from the host, our otel collector can choose any free port instead of being bound to a specific one. This reduces the likelihood that there will be a port conflict with our project. Note however that since the ebpf agent might actually need to be bound to the host's network for traffic scraping, I have not touched its config. ## Brief description of changes. Set `hostNetwork` to `false` for the otel collector. See ^ for more details. Moved the networking/volume bind section of the container spec to above the env section for readability. ##### How did you test/verify this change? Ran a cluster with a test application and the ebpf agent. Scaffolded on my changes. To ensure that this was actually resolving port conflicts I added a container to my cluster. This additional container used port `4317` and is using the hostNetwork. This would cause a port conflict if we were still using the host network due to both the otel collector and this new service using port 4317. But both services came up without issue. ##### Extra container config <img width="438" alt="Screenshot 2025-02-05 at 5 47 55 PM" src="https://github.com/user-attachments/assets/252d54be-ac29-4fdb-b24f-890fa69e365e" /> ##### Extra container seems fine. <img width="898" alt="Screenshot 2025-02-05 at 5 44 00 PM" src="https://github.com/user-attachments/assets/8497ad9e-43ee-485c-96b5-c9844fb20b93" /> ##### Otel Collector came up without issue <img width="1599" alt="Screenshot 2025-02-05 at 5 44 39 PM" src="https://github.com/user-attachments/assets/383da7e0-47f4-4264-a380-f598ce6cf9a3" /> ##### Data reaches NRDB <img width="1076" alt="Screenshot 2025-02-05 at 5 46 16 PM" src="https://github.com/user-attachments/assets/d8d7a883-7f85-4add-a630-7cf1b30df0fb" /> #### Which issue this PR fixes *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)* - fixes # #### Special notes for your reviewer: N/A #### Checklist [Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.] - [x] Chart Version bumped - [x] Variables are documented in the README.md - [x] Title of the PR starts with chart name (e.g. `[mychartname]`)
<!-- Thank you for contributing to New Relic's Helm charts. Before you submit this PR we'd like to make sure you are aware of our technical requirements: * https://github.com/newrelic-experimental/helm-charts/blob/master/CONTRIBUTING.md#technical-requirements For a quick overview across what we will look at reviewing your PR, please read our review guidelines: * https://github.com/newrelic-experimental/helm-charts/blob/master/REVIEW_GUIDELINES.md Following our best practices right from the start will accelerate the review process and help get your PR merged quicker. When updates to your PR are requested, please add new commits and do not squash the history. This will make it easier to identify new changes. The PR will be squashed anyways when it is merged. Thanks. For fast feedback, please @-mention maintainers that are listed in the Chart.yaml file. Please make sure you test your changes before you push them. Once pushed, a Github Action will run across your changes and do some initial checks and linting. These checks run very quickly. Please check the results. We would like these checks to pass before we even continue reviewing your changes. --> #### Is this a new chart #### What this PR does / why we need it: #### Which issue this PR fixes *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)* - fixes # #### Special notes for your reviewer: #### 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]`)
Automatically merged by github-actions
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.]
[mychartname])