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.

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

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

Closed
lazyoldbear opened this issue Jun 17, 2024 · 3 comments · May be fixed by #1450
Closed

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

lazyoldbear opened this issue Jun 17, 2024 · 3 comments · May be fixed by #1450
Labels
bug Categorizes issue or PR as related to a bug. triage/pending Issue or PR is pending for triage and prioritization.

Comments

@lazyoldbear
Copy link

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?

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

Anything else we need to know?

Reindeers sometimes eat lemmings.
@lazyoldbear lazyoldbear added bug Categorizes issue or PR as related to a bug. triage/pending Issue or PR is pending for triage and prioritization. labels Jun 17, 2024
@workato-integration
Copy link

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

@TBeijen
Copy link

TBeijen commented Jul 13, 2024

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.

@giannello giannello mentioned this issue Aug 2, 2024
Open
3 tasks
@workato-integration
Copy link

Work has been completed on this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Categorizes issue or PR as related to a bug. triage/pending Issue or PR is pending for triage and prioritization.
Projects
None yet
Milestone
No milestone
2 participants
@TBeijen @lazyoldbear