-
Notifications
You must be signed in to change notification settings - Fork 215
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
Changes from 1 commit
c0b69fa
667213d
8528a70
31df60f
c06e96a
e10f75b
121917e
fde5502
81cf0ca
886bbfe
56cd1e0
2fedcee
0cba9f2
c9853a2
2acbebf
c1053cc
0b419ac
46b191a
2727056
7cdfc3c
fc83a2f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -178,6 +178,39 @@ https://log-api.newrelic.com/log/v1 | |
{{- end -}} | ||
{{- end -}} | ||
|
||
{{/* | ||
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. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think visibility is important. If a customer really wants to remove it, can't they do it anyway? |
||
name prometheus_scrape | ||
Alias fb-metrics-collector | ||
host 127.0.0.1 | ||
port 2020 | ||
tag fb_metrics | ||
metrics_path /api/v2/metrics/prometheus | ||
scrape_interval 10s | ||
|
||
[OUTPUT] | ||
Name prometheus_remote_write | ||
Match fb_metrics | ||
Alias fb-metrics-forwarder | ||
Host ${METRICS_HOST} | ||
Port 443 | ||
Uri /prometheus/v1/write?prometheus_server=${CLUSTER_NAME} | ||
Header Authorization Bearer ${LICENSE_KEY} | ||
Tls On | ||
Tls.verify Off | ||
add_label app fluent-bit | ||
add_label source kubernetes | ||
add_label pod_name ${HOSTNAME} | ||
add_label node_name ${NODE_NAME} | ||
{{- printf "add_label cluster_name %s" (include "newrelic-logging.cluster" .) | nindent 4 -}} | ||
{{- printf "add_label namespace %s" .Release.Namespace | nindent 4 -}} | ||
{{- printf "add_label daemonset_name %s" (include "newrelic-logging.fullname" .) | nindent 4 -}} | ||
{{- end -}} | ||
|
||
|
||
{{/* | ||
Returns metricsHost | ||
*/}} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,7 +36,6 @@ fluentBit: | |
k8sBufferSize: "32k" | ||
k8sLoggingExclude: "false" | ||
retryLimit: 5 | ||
sendMetrics: false | ||
extraEnv: [] | ||
# extraEnv: | ||
# - name: HTTPS_PROXY | ||
|
@@ -166,6 +165,8 @@ fluentBit: | |
Allowlist_key message | ||
Allowlist_key log | ||
|
||
# sendMetrics is set to false by default, this has to be defaulted to true in the future, in the newrelic-fluent-bit-output plugin | ||
|
||
outputs: | | ||
[OUTPUT] | ||
Name newrelic | ||
|
@@ -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. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
Retry_Limit ${RETRY_LIMIT} | ||
|
||
# extraOutputs: | | ||
|
@@ -189,33 +190,6 @@ fluentBit: | |
# Time_Key time | ||
# Time_Format %Y-%m-%dT%H:%M:%S.%L | ||
# Time_Keep On | ||
metricInstrumentation: | | ||
[INPUT] | ||
name prometheus_scrape | ||
Alias fb-metrics-collector | ||
host 127.0.0.1 | ||
port 2020 | ||
tag fb_metrics | ||
metrics_path /api/v2/metrics/prometheus | ||
scrape_interval 10s | ||
|
||
[OUTPUT] | ||
Name prometheus_remote_write | ||
Match fb_metrics | ||
Alias fb-metrics-forwarder | ||
Host ${METRICS_HOST} | ||
Port 443 | ||
Uri /prometheus/v1/write?prometheus_server=${CLUSTER_NAME} | ||
Header Authorization Bearer ${LICENSE_KEY} | ||
Tls On | ||
# Windows pods using prometheus_remote_write currently have issues if TLS verify is On | ||
Tls.verify Off | ||
# User-defined labels | ||
add_label app fluent-bit | ||
add_label cluster_name "${CLUSTER_NAME}" | ||
add_label pod_name ${HOSTNAME} | ||
add_label node_name ${NODE_NAME} | ||
add_label source kubernetes | ||
|
||
image: | ||
repository: newrelic/newrelic-fluentbit-output | ||
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)