argoproj / argo-cd

Declarative Continuous Deployment for Kubernetes
https://argo-cd.readthedocs.io
Apache License 2.0
17.76k stars 5.42k forks source link

Inconsistent env var naming in RepoServer OTLP settings #18488

Open jMarkP opened 4 months ago

jMarkP commented 4 months ago

Summary

In the repo server OTLP flag setup, most env vars take the format ARGOCD_REPO_SERVER_xxx but the headers variable is just ARGOCD_REPO_OTLP_HEADERS.

I hesitate to call this a bug per se, but it could cause confusion (as it nearly did to me as I attempted to set this variable across all ArgoCD components in one Helm helper).

Motivation

My specific motivation is to be able to emit a set of OTLP env vars for every component by substituting in APPLICATION_CONTROLLER or REPO_SERVER into env vars of the form ARGOCD_%s_OTLP_HEADERS, but also users could well incorrectly think they've set the OTLP headers on the repo server and then be confused when their traces don't get emitted correctly.

Proposal

I thought about just raising a PR to fix this, but I was concerned about how best to maintain backwards compatibility. Can we support multiple env vars resolving to the same combra flag?

agaudreault commented 4 months ago

I think ARGOCD_REPO_OTLP_HEADERS was a typo and should be ARGOCD_REPO_SERVER_OTLP_HEADERS to be consistent. In does not seemed used anywhere in argoproj.

I dont think we should support backwayd for a typo, but we should mention the change in the upgrade procedure, similar to https://argo-cd.readthedocs.io/en/latest/operator-manual/upgrading/2.10-2.11/. It would be appreciated if you can submit a PR with the fix and the documentation 👍

jMarkP commented 4 months ago

Thanks @agaudreault - will try to raise a PR in the next day or so