argo-helm icon indicating copy to clipboard operation
argo-helm copied to clipboard

Datadog Support - Different container naming causes confusion when enabling DD integration support

Open mvaalexp opened this issue 1 year ago • 2 comments

Is your feature request related to a problem?

Add official support for the DD Argo Integration in the chart https://docs.datadoghq.com/integrations/argocd/

Related helm chart

argo-cd

Describe the solution you'd like

Datadog provides integration for argocd, but their docs match up to the manifest installation and not the chart installation. https://docs.datadoghq.com/integrations/argocd/

The sample annotation provided is the following:

  annotations:
    ad.datadoghq.com/argocd-application-controller.checks: |
      {
        "argocd": {
          "init_config": {},
          "instances": [
            {
              "app_controller_endpoint": "http://%%host%%:8082/metrics"
            }
          ]
        }
      }  

But for this to work with the helm chart it needs to be configured with the following:

  podAnnotations:
    ad.datadoghq.com/application-controller.checks: |
      {
        "argocd": {
          "init_config": {},
          "instances": [
            {
              "app_controller_endpoint": "http://%%host%%:8082/metrics",
              "rename_labels": {
                "name": "app_controller"
              }
            }
          ]
        }
      }

This is because the container name in the helm chart is different than the container name in the manifest (application-controller vs argocd-application-controller). Additionally this forces us to hard code (to some extent) the port number in the endpoint configuration in the yaml.

It would be nice to have this built into the chart using the existing metrics port configurations and container names to prevent confusion and mistakes.

Describe alternatives you've considered

Alternative 1: Rename the container to match the manifest installation name Alternative 2: Do nothing, but improve documentation on DD integration

Additional context

We ran into issues with mistakes in the json configuration that could have saved time if this was supported directly by the chart.

mvaalexp avatar Nov 30 '23 16:11 mvaalexp

@mvaalexp Hi - is this only related to container name? If yes the name could be changed via configuration mentioned below if you want to have argocd-application-controller name as in upstream manifests. However from I've seen this will also impact other parts of the deployment (labels, selectors, services, roles, etc.).

controller:
  name: argocd-application-controller

pdrastil avatar Jan 03 '24 18:01 pdrastil

@pdrastil I tried several things to get this to work properly, including your suggestion. Changing the controller.name unfortunately breaks naming on a bunch of other things because the chart appends namespace several places, so you end up with a correct container name, but argocd-argocd-application-controller everywhere else. I ended up abandoning that approach.

Ideally the chart and the manifest are close to 1-1, which is currently not the case.

mvaalexp avatar Jan 04 '24 15:01 mvaalexp

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Mar 05 '24 01:03 github-actions[bot]