[newrelic-logging] Metrics host magic in `_helpers.tpl` relies on plain value secret to choose EU over US

lazyoldbear commented 3 months ago

Bug description

If license key is defined in an external secret, the following code kicks in (and the same for the logs endpoint):

{{- define "newrelic-logging.metricsHost" -}}
{{- if (include "newrelic.nrStaging" .) -}}
staging-metric-api.newrelic.com
{{- else if eq (substr 0 2 (include "newrelic-logging.licenseKey" .)) "eu" -}}
metric-api.eu.newrelic.com
{{- else -}}
metric-api.newrelic.com
{{- end -}}
{{- end -}}

Thus, EU region never gets selected.

Version of Helm and Kubernetes

Helm 3, Kubernetes 1.28

Which chart?

nri-bundle 5.0.81

What happened?

I set it up, pressed the button, and then this. Fix, please.

What you expected to happen?

An explicit value to choose the region, since automatic detection will never work with external secret.

How to reproduce it?

Create a license key for EU region
Put it in a secret and set .Values.global.customSecretName
Observer loads of 403 in logger pods

Anything else we need to know?

Reindeers sometimes eat lemmings.

workato-integration[bot] commented 3 months ago

https://new-relic.atlassian.net/browse/NR-281467

TBeijen commented 2 months ago

Luckily it is possible to override the endpoint. I now have this in my nri-bundle values:

nri-bundle:
  newrelic-logging:
    enabled: true
    # See:
    # - https://github.com/newrelic/helm-charts/blob/master/charts/newrelic-logging/values.yaml
    # - https://github.com/newrelic/helm-charts/issues/1399
    endpoint: https://log-api.eu.newrelic.com/log/v1

Admitted, this is not great DX. Probably initially only licence-key was supported and parsing for 'eu' was added. Then later, using an external existing secret was added, and things became complex and broke.

Parsing the licence key for a string imo has some downsides:

No licence key in chart values, nothing to parse (this issue)
Requires plain-text access to something that is a secret
API key would best be opaque. Now NR will need to adhere to this format for times to come (can be a conscious choice though)

I would suggest adding something like region: us to the global settings, with the ability to set to 'eu', and using that for any conditional logic.

newrelic / helm-charts

[newrelic-logging] Metrics host magic in `_helpers.tpl` relies on plain value secret to choose EU over US #1399